Closed Bug 971923 Opened 6 years ago Closed 6 years ago

[Calendar] display current time on day and week views

Categories

(Firefox OS Graveyard :: Gaia::Calendar, defect)

x86
macOS
defect
Not set

Tracking

(feature-b2g:2.0)

VERIFIED FIXED
2.0 S3 (6june)
feature-b2g 2.0

People

(Reporter: mmedeiros, Assigned: mmedeiros)

References

Details

(Whiteboard: [p=5])

User Story

- it should be displayed on the Week and Day views if current date is being displayed.
 - it updates the displayed time and line position at each minute.
 - when reaching midnight if next day is not visible the current time should hide otherwise it should restart at 00:00 and mark the next day as current day (highlight date in blue if on Week View).
 - when navigating between dates, the current time should only be displayed on current date.
 - if calendar is displaying a date in the future and you leave it open enough time so that the displayed day becomes the current day, the current time should be displayed (eg. 23:59 and you wait until 00:01).
 - If user or system time changes, the calendar should reflect that change of time. 
 - hide the hour on left column if current time indicator is overlapping the hour.

Attachments

(3 files)

according to the 1.4 visual refresh we need to display a "blue line" with current time on the day and week views.

creating it as a separate issue since it is part of both views, and we will code it as a separate component.
Blocks: 950209
Depends on: 951071, 951075
calendar specs had a reference to another bug (https://bugzilla.mozilla.org/show_bug.cgi?id=896817) but that issue is related to Lightning and not Gaia::Calendar.
feature-b2g: --- → 2.0
Assignee: nobody → mmedeiros
Target Milestone: --- → 2.0 S2 (23may)
Flags: in-moztrap+
I still need to write integration tests and update the unit tests for DayView and WeekView to make sure they are calling `activate/deactivate/destroy` and passing proper values to constructor. But I believe this should be working as expected.

PS: it only displays the current time if you are viewing the current date.
Attachment #8427468 - Flags: ui-review?(hhsu)
Attachment #8427468 - Flags: review?(gaye)
Whiteboard: [p=5]
Attached image review.png
Hi Miller, some feedbacks from me & Peko:
1. The edge of the triangle indicator has zigzag and it is not smooth
2. The current time indicator should not have white background color, and should be transparent
3. If current time indicator overlapped with the time indicator, hide the time indicator and display only the current time indicator (see attachment for example, just display 3:05 and hide 3P)
Flags: needinfo?(mmedeiros)
Attachment #8427468 - Flags: ui-review?(hhsu) → ui-review-
Comment on attachment 8427468 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19573

Hi Harly, the white background was supposed to be just under the text, I forgot to remove it from the lines.. (now I removed the white background altogether)

I updated the patch to match your requirements, right now I'm checking if the hour intersects with the current time and toggling the display based on that.

I also added some CSS transforms to make sure the triangle looks smooth and also to center the blue line.

Another bug that I fixed together with this patch (since the logic is related) is that if you are on Week View at 23:59 and wait until midnight it will mark the next day as the current day and only display the current time marker if the current day is being displayed (before the current time marker was getting stuck at 23:59 if it was the last day of the week).

Let me know what you guys think and if you notice any errors.
Attachment #8427468 - Flags: ui-review- → ui-review?(hhsu)
Flags: needinfo?(mmedeiros)
Comment on attachment 8427468 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19573

Since Gareth is on PTO this week, I'm going to ask Evan to review it.

Evan, I improved the code a lot last Friday (fixed a few bugs and did some cleanup) but I still need to add tests for the Week and Day views to make sure they are calling the proper `CurrentTime` methods. I should probably finish the test till EOD and will do it as a separate commit (won't squash it for now). - So that means you can start reviewing but there will probably be a few more lines of code to review later (mostly unit and marionette tests). Thanks!
Attachment #8427468 - Flags: review?(gaye) → review?(evanxd)
I updated the patch to include the unit + marionette tests.. but for some weird reason some old marionette tests stopped working locally (unrelated to my changes), I still need to wait travis to finish running to see if it also fails there.. My bet is that it's a gecko regression (PS: I tried `make clean && make really-clean` but it did not help). Will investigate it tomorrow.
Comment on attachment 8427468 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19573

Awesome work! Miller, thanks!
Attachment #8427468 - Flags: ui-review?(hhsu) → ui-review+
Target Milestone: 2.0 S2 (23may) → 2.0 S3 (6june)
QA Contact: edchen
Harly, can you confirm if the User Story is correct and/or update it? Thanks!
User Story: (updated)
Flags: needinfo?(hhsu)
Yes, the user story looks correct, I was wondering that should we add something like:
- If user or system time changes, the calendar should reflect that change of time 
- If current time indicator overlapped with the time indicator, hide the time indicator and display only the current time indicator
Flags: needinfo?(hhsu)
User Story: (updated)
Hi Miller,

The patch looks good.

I added comments for it.
Flags: needinfo?(mmedeiros)
Evan, I updated the patch. Thanks.
Flags: needinfo?(mmedeiros) → needinfo?(evanxd)
Comment on attachment 8427468 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19573

Hi Miller,
Nice work!
Attachment #8427468 - Flags: review?(evanxd) → review+
Flags: needinfo?(evanxd)
master: 8b39729fcfcdf1cc901f0e16fad9c8e011175d8a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Is there enough space to display 24h format? The screenshot kind of scares me.
[Environment]
Gaia      8d865839d932bfbd5e157f376f74d8cb12bfdd51
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/1d4046a8cb6c
BuildID   20140610000223
Version   32.0a2
ro.build.version.incremental=94
ro.build.date=Tue May 20 09:29:20 CST 2014

[Result]
Pass
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.