Closes T8189, Calendar events should be green if viewer is invited, and grey if not.
Details
Details
- Reviewers
epriestley chad - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T8189: Only Calendar month view events that viewer is invited to should be displayed in color.
- Commits
- Restricted Diffusion Commit
rP4016107411c1: Calendar events should be green if viewer is invited, and grey if not.
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
Diff Detail
- Repository
- rP Phabricator
- Branch
- calendarcolorcoding
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 5997 Build 6017: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
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?
Comment Actions
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? | |
Comment Actions
@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.
Comment Actions
@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.
Comment Actions
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: