Page MenuHomePhabricator

Calendar events should be green if viewer is invited, and grey if not.
ClosedPublic

Authored by lpriestley on May 15 2015, 12:35 AM.
Tags
None
Referenced Files
F13220179: D12850.id30916.diff
Sat, May 18, 11:48 PM
F13202129: D12850.diff
Tue, May 14, 10:18 PM
F13190979: D12850.diff
Sat, May 11, 4:04 PM
Unknown Object (File)
Sat, May 4, 6:44 PM
Unknown Object (File)
Tue, Apr 30, 11:15 PM
Unknown Object (File)
Sat, Apr 27, 9:59 PM
Unknown Object (File)
Mon, Apr 22, 6:59 AM
Unknown Object (File)
Sun, Apr 21, 10:28 AM
Subscribers

Details

Summary

Closes T8189, Calendar events should be green if viewer is invited, and grey if not.

Test Plan

Check that month and day views display events that viewer is invited to as green and grey otherwise, including month day badges on mobile monthly view.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

lpriestley retitled this revision from to Calendar events should be green if viewer is invited, and grey if not..
lpriestley updated this object.
lpriestley edited the test plan for this revision. (Show Details)
lpriestley added reviewers: epriestley, chad.

I'm concerned the color here isn't obvious to what it means, and may not be a good choice if someone is color blind. I'd prefer seeing something perhaps more obvious if your invited/attending an event. Perhaps we could just bold anything you're attending/invited to, and add an icon to denote the state specifically?

Event
Invited Event
Attending Event

What do you think?

Some inlines, mostly around being more aggressive about removing setColor(). Particularly, this will let us change the color or style later without ending up with a bunch of .green-day-event CSS classes which actually make things blue.

src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php
415–416

This seems a little weird: it looks like we use "viewerIsInvited" on the Month view and ignore "color", but use "color" on the day view and ignore "viewerIsInvited".

Maybe consider removing setColor() entirely and always using "viewerIsInvited"?

505

Prefer PhabricatorCalendarEvent typehint.

509–517

Maybe implement a getIsUserInvited() method on PhabricatorCalendarEvent, similar to getIsUserAttending()?

webroot/rsrc/css/phui/calendar/phui-calendar-day.css
52

Consider invited-day-event or similar? This would be consistent with removing setColor().

webroot/rsrc/css/phui/calendar/phui-calendar.css
39

Are most of the rules in this file now unused?

lpriestley edited edge metadata.

Removing setColor from Month and Day views

@chad I think that currently those dots are being used for user handles all over the product, so I don't think I should combine that implementation with these changes. But, yeah, currently those dots are a bit ambiguous to me, for sure. I made the green events bold, as well, for now, so those should definitely be easier to distinguish.

epriestley edited edge metadata.

@chad, we talked about it in person and I want to figure out what's happening with icons first, and Lyuba wanted to change the dots in a different diff if we're going to change them since they get used in a lot of places. The updated diff does bold the attending/invited events.

On icons, I'm partial to them but this might look weird:

  • Event Name

..although maybe that's actually fine, or maybe I have a far stronger love of icons than anyone else.

This revision is now accepted and ready to land.May 15 2015, 2:20 AM
This revision was automatically updated to reflect the committed changes.

No problem, forgot about the other icons!

Maybe we could do icon if set, small circle if not set (ditch the li rendering circle outright). Then if we wanted to double icon it, we could background-color circle the event icon, and little inline icon for the status. Or skip the status and just be bold. Here's an example from Sunrise:

pasted_file (100×131 px, 18 KB)