971923 - [Calendar] display current time on day and week views

Status: VERIFIED FIXED

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

Description

[Miller Medeiros [:millermedeiros]]

Attachment: FFOS_VisualRefresh_1.4_calendar_dayview_spec.pdf

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.

Comment 1

[Miller Medeiros [:millermedeiros]]

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.

Comment 2

[Miller Medeiros [:millermedeiros]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19573

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.

Comment 3

[Harly Hsu[:harly]]

Attachment: 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)

Comment 4

[Miller Medeiros [:millermedeiros]]

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.

Comment 5

[Miller Medeiros [:millermedeiros]]

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!

Comment 6

[Miller Medeiros [:millermedeiros]]

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 7

[Harly Hsu[:harly]]

Comment on attachment 8427468 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19573

Awesome work! Miller, thanks!

Comment 8

[Miller Medeiros [:millermedeiros]]

Harly, can you confirm if the User Story is correct and/or update it? Thanks!

Comment 9

[Harly Hsu[:harly]]

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

Comment 10

[Evan Tseng [:evanxd]]

Hi Miller,

The patch looks good.

I added comments for it.

Comment 11

[Miller Medeiros [:millermedeiros]]

Evan, I updated the patch. Thanks.

Comment 12

[Evan Tseng [:evanxd]]

Comment on attachment 8427468 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19573

Hi Miller,
Nice work!

Comment 13

[Evan Tseng [:evanxd]]

master: 8b39729fcfcdf1cc901f0e16fad9c8e011175d8a

Comment 14

[Francesco Lodolo [:flod]]

Is there enough space to display 24h format? The screenshot kind of scares me.

Comment 15

[Edward Chen[:edchen]]

[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