Page MenuHomePhabricator

Calendar day view and corresponding sidebar should correctly display all all-day events returned from query
ClosedPublic

Authored by lpriestley on May 8 2015, 8:02 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 13, 9:19 PM
Unknown Object (File)
Thu, Nov 28, 12:45 AM
Unknown Object (File)
Fri, Nov 22, 8:44 AM
Unknown Object (File)
Nov 19 2024, 8:10 AM
Unknown Object (File)
Oct 27 2024, 1:49 PM
Unknown Object (File)
Oct 23 2024, 5:20 PM
Unknown Object (File)
Oct 23 2024, 10:31 AM
Unknown Object (File)
Oct 19 2024, 5:14 AM
Subscribers

Details

Summary

Closes T8085, Calendar day view and corresponding sidebar should correctly display all all-day events returned from query

Test Plan

Open day view with all-day and multi-day events, all events should correctly be drawn in day view in correct order, and sidebar preview should correctly mark future day boxes with all day events.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

lpriestley retitled this revision from to Calendar day view and corresponding sidebar should correctly display all all-day events returned from query.
lpriestley updated this object.
lpriestley edited the test plan for this revision. (Show Details)
lpriestley added a reviewer: epriestley.
epriestley edited edge metadata.
epriestley added inline comments.
src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php
395–397

At some point, we should probably do this as a part of the query parameters we show to the user instead, e.g. add a dropdown like:

Cancelled: [ Show Only Active Events           V ]
           [ Show Only Cancelled Events          ]
           [ Show Active and Cancelled Events    ]

...so by default it would only show active events, but you could show or search for cancelled events if you wanted.

src/view/phui/calendar/PHUICalendarDayView.php
56

Do we need this part (start == start)? I'd think the other part would be good enough on its own.

82

This may not work after constraining the day's start time to 8AM.

196–198

Does this get multi-day events which are not all-day events right? I'd expect to only need the last condition:

(start < end && end > start)
This revision is now accepted and ready to land.May 8 2015, 8:10 PM
lpriestley marked 4 inline comments as done.
lpriestley edited edge metadata.

Logic cleanup

This revision was automatically updated to reflect the committed changes.