951069 - [Calendar] Month View_1.4 Visual Refresh

Status: VERIFIED FIXED

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

Comment 1

[Gareth Aye [:gaye] (back from PTO)]

Hey Harly! Can you have a visual designer create the new assets for us?

Comment 2

[Gareth Aye [:gaye] (back from PTO)]

Also can we get a mockup/spec for what the month view would look like if there are two events (at the bottom) with the same time?

Comment 3

[Gareth Aye [:gaye] (back from PTO)]

Also are the events' alarm icons meant to go away?

Comment 4

[Harly Hsu[:harly]]

Hi Gareth, I don't quite understand what you mean by having visual designer to create the new assets? Do you mean adding sub-bugs related to month view visual refresh?

And about the month view, we are trying to change from current mini-day view to a list of events instead, and if two events with the same time, system will arrange it by alphabetical order of the event name. I know this might take you more time to work on it, but could you estimate that if it is possible to do this on 1.4? 

Oh, and about the alarm icons, it shouldn't go away, will talk to visual designer to put it back.

Comment 5

[Gareth Aye [:gaye] (back from PTO)]

(In reply to Harly Hsu from comment #4)
> Hi Gareth, I don't quite understand what you mean by having visual designer
> to create the new assets? Do you mean adding sub-bugs related to month view
> visual refresh?

I mean that there are some new icons we need like the three vertically-aligned dots, the calendar in the bottom left hand corner, the envelope, and the blue dot and the blue circle for active and inactive events, etc.

> And about the month view, we are trying to change from current mini-day view
> to a list of events instead, and if two events with the same time, system
> will arrange it by alphabetical order of the event name. I know this might
> take you more time to work on it, but could you estimate that if it is
> possible to do this on 1.4? 

It's definitely possible! I was just trying to clarify :)

Also, how should the new UI handle all-day events?

Thanks for your guidance Harly!

Comment 6

[Harly Hsu[:harly]]

Great to hear that it is possible to have a list of event, and about all-day event, it will always show at the top of the list, and it will also be arranged in alphabetical order if there are multiple all-day event. Needinfo Peko to add the new assets for visual stuff.

Comment 7

[Gareth Aye [:gaye] (back from PTO)]

Attachment: FFOS_VisualRefresh_1.4_calendar.pdf

Original, visual refresh spec from pchen

Comment 8

[Peko Chen [:pekochen]]

Attachment: FFOS_VisualRefresh_1.4_calendar_monthview_spec.pdf

Month view spec updated.

Comment 9

[Gareth Aye [:gaye] (back from PTO)]

Hi pchen!

I applaud making the squares in the month view shorter, but if we make them thinner (44px like in the attachment), then they won't fill the entire width. 44 * 7 = 308 and most of our devices are 320px width. What we have right now (which I think is correct) is to specify the width of each square as 14.286% of the width (1/7th).

Comment 10

[Peko Chen [:pekochen]]

Attachment: Calendar_v1.4_month_divider_Spec.png

Hi, Gareth Aye

please see attache image for detail.
the square should be keep 44px,otherwise it may looks blur.
thanks

Peko

Comment 11

[Gareth Aye [:gaye] (back from PTO)]

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

Comment 12

[Miller Medeiros [:millermedeiros]]

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

patch still isn't matching the comps:

 1) month name at the header should be abbreviated. (Feb 2014)
 2) week days (SMTWTFS) looks darker and not centered.
 3) dates also looks darker and not centered.
 4) dots (busy indicator) are too close to each other and feels bigger.
 5) items on the event list at the bottom also doesn't look to be aligned properly.
 6) "#event-list-date" copy should be aligned with event hours.
 7) blue circle (next to hour) should be moved to the left, and it looks "bolder" (and bigger) as well.
 8) event title looks bolder and should be moved to the left to match comps.
 9) need to remove shadow after calendar.

please fix the visuals before I can go ahead and do the full technical review.

Comment 13

[Gareth Aye [:gaye] (back from PTO)]

Thanks Miller. I will reflag you for review once I get to refactoring my patch and updating the styling to more closely match the specs.

Comment 14

[Harly Hsu[:harly]]

Attachment: device-2014-02-14-102652.png

Hi Gareth,
It is great to see that we are making progress. Good work!

To echo what Miller has said, Me & Peko have also find some visual issues:
1. Date title on event list should align with time
2. Event name & location in event list is not aligned properly, please follow the invitation spec on bug 932254
3. Event name when exceeds limit overlapped with reminder icon
4. Reminder icon is still the old one
5. No background color for event list, and when there is only 1 event, a black background will show up

Comment 15

[Miller Medeiros [:millermedeiros]]

I created a custom font with the busy time indicators, bullet points, alarm icon and all day events (Bug 972871) - one step closer to resolution independent designs!

Comment 16

[Gareth Aye [:gaye] (back from PTO)]

(In reply to Miller Medeiros [:millermedeiros] from comment #12)
> Comment on attachment 8375151 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/16231

Hey Miller! Thanks for the detailed visual review. A couple notes && calls for clarification...

> patch still isn't matching the comps:
> 
>  1) month name at the header should be abbreviated. (Feb 2014)

This lives outside the month view. Would it work best for your week view and day view changes if I made the header changes here?

>  2) week days (SMTWTFS) looks darker and not centered.

https://github.com/gaye/gaia/blob/429a75c6071d7095a64594bfb5244cc15415f640/apps/calendar/style/ui.css#L289 shows that they are centered since we have |text-align: center;| on them and also even horizontal margins and padding. The color is wrong though (good catch!). I had black specified whereas the spec calls for #657073.

>  3) dates also looks darker and not centered.

They should also be centered since https://github.com/gaye/gaia/blob/429a75c6071d7095a64594bfb5244cc15415f640/apps/calendar/style/ui.css#L352 specifies |text-align: center;| and there are no margins and paddings on the .day elements or their parent lis. The spec calls for using black (#000000) for the numbers so I don't think they could be too dark :).

>  4) dots (busy indicator) are too close to each other and feels bigger.

Good catches also. I agree on both points.

>  5) items on the event list at the bottom also doesn't look to be aligned
> properly.
>  6) "#event-list-date" copy should be aligned with event hours.
>  7) blue circle (next to hour) should be moved to the left, and it looks
> "bolder" (and bigger) as well.
>  8) event title looks bolder and should be moved to the left to match comps.

Yeah I agree with these also. I probably need to take another peek at the invitation spec.

>  9) need to remove shadow after calendar.

+1

Comment 17

[Gareth Aye [:gaye] (back from PTO)]

(In reply to Harly Hsu from comment #14)
> Created attachment 8375985 [details]
> device-2014-02-14-102652.png

Also thank you Harly for feedback! Comments inline.

> 1. Date title on event list should align with time
> 2. Event name & location in event list is not aligned properly, please
> follow the invitation spec on bug 932254

Yes yes :)

> 3. Event name when exceeds limit overlapped with reminder icon

We spoke about this in triage last week briefly as it related to overlapping events and Miller thought that it might be affected by his visual refresh changes. I can certainly fix this here but don't want to step on any changes Miller is making. Miller?

> 4. Reminder icon is still the old one

Can you and Peko send me assets for the calendar icon that's replacing the "Today" tab and also the new reminder icon?

> 5. No background color for event list, and when there is only 1 event, a
> black background will show up

Good catches. I will have to look into the black line issue...

Comment 18

[Gareth Aye [:gaye] (back from PTO)]

Sorry I realize that you (harly) asked for clarification about me wanting assets before. There are two new icons (one is the calendar that's replacing "Today" in the left-most tab and the other is the alternate reminder icon). I need those to be created for me.

Comment 19

[Miller Medeiros [:millermedeiros]]

Gareth, the new reminder icon is on the icon font (Bug 972871). Evan is currently working on the "today button" (Bug 951084).

Comment 20

[Peko Chen [:pekochen]]

Hi Gareth,

you can get these icons(png and svg format) from here.
https://bugzilla.mozilla.org/show_bug.cgi?id=951084

thanks

Comment 21

[Evan Tseng [:evanxd]]

Currently we have these 8 icons https://github.com/mozilla-b2g/gaia/blob/calendar-visual-refresh/apps/calendar/style/icons.css#L60-L83 in the calendar-visual-refresh branch.

Comment 22

[Harly Hsu[:harly]]

Hi Gareth,
I think you can find all your icons there, but surely tell us if there is something we've missed out.
Thanks

Comment 23

[Gareth Aye [:gaye] (back from PTO)]

Ok great thanks guys

Comment 24

[Gareth Aye [:gaye] (back from PTO)]

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

Hey Miller, Harly, and Peko,

Would you mind taking a look at my pull request to make sure it's up to spec now? I attached a screenshot of the new month view to the pull request (also here https://cloud.githubusercontent.com/assets/535859/2823783/d173e164-cf24-11e3-9700-24f57d87b44b.png). If it looks good, I will go ahead and ask Miller for a code review!

Thanks :)

Comment 25

[Jason Smith [:jsmith]]

This shouldn't block - this is a feature, which is something we won't block on unless we're past FL & we're planning to still land the feature.

Comment 26

[Harly Hsu[:harly]]

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

Hi Gareth, nice work, most of the previous issues are fixed, 
but there are some UX issues that I've found:
1. The mini-day view at the bottom of month view should be changed to a list of events, and if two events with the same time, system will arrange it by alphabetical order of the event name. 
2. Need to add end time for events in the event list. (Peko will add in the update spec)
2. Date displayed at the event list need to push with the list when scrolling up.
3. Display "No Events" in the event list when there are no events available. (Peko will add in the update spec)

Comment 27

[Miller Medeiros [:millermedeiros]]

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

we're getting close but there are a few things that still need to be solved.

 1. I still think the event title is bolder/darker than it should (we don't have any #000 text in the app & everything else is very light).
 2. it is not displaying events that lasts longer than 1h properly; specially if event starts at the end of the hour (eg. 3h30, 15h50).
 3. I think event list looks weird when event doesn't have a location (very common), I think it should center the event title vertically inside the area.
 4. The dates should be centered vertically and horizontally inside the container (increase `.day` line-height to 4.1rem)
 5. the `.busy-indicator` is a little bit too close to the bottom of the day.

Comment 28

[Gareth Aye [:gaye] (back from PTO)]

(In reply to Harly Hsu from comment #26)
> Comment on attachment 8414054 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18599
> 
> Hi Gareth, nice work, most of the previous issues are fixed, 
> but there are some UX issues that I've found:
> 1. The mini-day view at the bottom of month view should be changed to a list
> of events, and if two events with the same time, system will arrange it by
> alphabetical order of the event name. 

It is a list? I can work on the alphabetization bit though.

> 2. Need to add end time for events in the event list. (Peko will add in the
> update spec)
> 3. Display "No Events" in the event list when there are no events available.
> (Peko will add in the update spec)

Ok... will wait for Peko's updates.

> 2. Date displayed at the event list need to push with the list when
> scrolling up.

Can you clarify this point a bit more?

Comment 29

[Gareth Aye [:gaye] (back from PTO)]

(In reply to Miller Medeiros [:millermedeiros] from comment #27)
> Comment on attachment 8414054 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18599
> 
> we're getting close but there are a few things that still need to be solved.
> 
>  1. I still think the event title is bolder/darker than it should (we don't
> have any #000 text in the app & everything else is very light).

Yes you're about the font (it's supposed to be #252525 according to https://bug932254.bugzilla.mozilla.org/attachment.cgi?id=8372138). Will fix thanks!

>  2. it is not displaying events that lasts longer than 1h properly;
> specially if event starts at the end of the hour (eg. 3h30, 15h50).

I did not check this case... thanks for noticing.

>  3. I think event list looks weird when event doesn't have a location (very
> common), I think it should center the event title vertically inside the area.

Harly and Peko do you agree that the title should be centered when there's no location? If so, can we add to spec?

>  4. The dates should be centered vertically and horizontally inside the
> container (increase `.day` line-height to 4.1rem)

Ah this is a good catch. Thanks also.

>  5. the `.busy-indicator` is a little bit too close to the bottom of the day.

I agree with you aesthetically, but the spec calls for spacing the bullets 4px from the bottom which they are. Harly and Peko?

Comment 30

[Peko Chen [:pekochen]]

Attachment: FFOS_VisualRefresh_1.4_calendar_monthview_spec_20140429.pdf

Hi Gareth,

please see attached file for the latest month view spec.
and please notice red line and fonts in the spec.
thanks~

Peko

Comment 31

[Harly Hsu[:harly]]

(In reply to Gareth Aye [:gaye] from comment #28)
> (In reply to Harly Hsu from comment #26)
> > Comment on attachment 8414054 [details] [review]
> > Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18599
> > 
> > Hi Gareth, nice work, most of the previous issues are fixed, 
> > but there are some UX issues that I've found:
> > 1. The mini-day view at the bottom of month view should be changed to a list
> > of events, and if two events with the same time, system will arrange it by
> > alphabetical order of the event name. 
> 
> It is a list? I can work on the alphabetization bit though.
Yes, it is just a list of events instead of mini-day view.
> 
> > 2. Need to add end time for events in the event list. (Peko will add in the
> > update spec)
> > 3. Display "No Events" in the event list when there are no events available.
> > (Peko will add in the update spec)
> 
> Ok... will wait for Peko's updates.
> 
> > 2. Date displayed at the event list need to push with the list when
> > scrolling up.
> 
> Can you clarify this point a bit more?
What I meant was when you scroll up on the event list, the date(TUESDAY, APR 29) will be push along with the event list instead of staying fixed at the top.

Comment 32

[Miller Medeiros [:millermedeiros]]

another thing I just realized is that on months that needs 6 rows (eg. March 2014) the event list is not useable (too short, can barely scroll/see 1 event at a time). We need to fix that somehow, maybe reduce padding around the calendar days if it contains 6 rows.

Comment 33

[Gareth Aye [:gaye] (back from PTO)]

(In reply to Harly Hsu from comment #31)
> (In reply to Gareth Aye [:gaye] from comment #28)
> > (In reply to Harly Hsu from comment #26)
> > > Comment on attachment 8414054 [details] [review]
> > > Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18599
> > > 
> > > Hi Gareth, nice work, most of the previous issues are fixed, 
> > > but there are some UX issues that I've found:
> > > 1. The mini-day view at the bottom of month view should be changed to a list
> > > of events, and if two events with the same time, system will arrange it by
> > > alphabetical order of the event name. 
> > 
> > It is a list? I can work on the alphabetization bit though.
> Yes, it is just a list of events instead of mini-day view.

What I meant was to say that the current implementation has a list. Maybe it would be better phrased "how does my patch not produce a list currently?".

Comment 34

[Gareth Aye [:gaye] (back from PTO)]

(In reply to Harly Hsu from comment #31)
> What I meant was when you scroll up on the event list, the date(TUESDAY, APR
> 29) will be push along with the event list instead of staying fixed at the
> top.

I think Miller and I are already feeling like there's not much room to scroll through events. This can certainly be done, but I'm not sure it's what we want...

Comment 35

[Gareth Aye [:gaye] (back from PTO)]

(In reply to Gareth Aye [:gaye] from comment #34)
> (In reply to Harly Hsu from comment #31)
> > What I meant was when you scroll up on the event list, the date(TUESDAY, APR
> > 29) will be push along with the event list instead of staying fixed at the
> > top.
> 
> I think Miller and I are already feeling like there's not much room to
> scroll through events. This can certainly be done, but I'm not sure it's
> what we want...

Actually upon re-reading your comment I'm pretty sure I don't know which option you'd prefer. Do you want for the date to scroll off screen or for it to stay on screen while the user scrolls?

Comment 36

[Gareth Aye [:gaye] (back from PTO)]

(In reply to Jason Smith [:jsmith] from comment #25)
> This shouldn't block - this is a feature, which is something we won't block
> on unless we're past FL & we're planning to still land the feature.

The whole visual refresh is landing in 2.0 and we want to avoid the situation where some of it lands and other parts do not so that we have a consistent UI across the various core apps. Can you 2.0+ this again Jason?

Comment 37

[Andrew Sutherland [:asuth] (he/him)]

You can already land anything you want on trunk/master which is 2.0.  Wild wild west!

Comment 38

[Gareth Aye [:gaye] (back from PTO)]

https://github.com/mozilla-b2g/gaia/pull/18599 updates my work to include everyone's suggestions. The only pieces of work that I've left out are

1. miller's suggestion to change the vertical alignment of the event title when an event lacks a location and
2. harly's suggestion to alphabetize events that share the same start hour

I skipped out on (1) since Harly and Peko haven't responded about that yet and I skipped out on (2) since alphabetizing the events in the day view is actually going to be more hassle than it's worth given the state of the day* views. Unless it's critically important, I'd like to push back on that until we do some view refactoring later in the 2.0 release.

Thanks again for all of your UI/UX input. If you all wouldn't mind doing another review round I would very much appreciate it.

Comment 39

[Miller Medeiros [:millermedeiros]]

nice work! it's getting better at each iteration! a few more notes:

 1. when calendar first opens the "no events" message isn't displayed.
 2. all day events looks weird with start/end time being the same.
 3. if you have events that are part of different ".hour" (eg. 1:30-2:00, 8:00-9:00) it is not displaying the lines in between the items.
 4. the margin between the hours and the event title/location seems bigger than it should. (did not measure it tho)
 5. the circle next to the hours is supposed to be blue, not black.

Comment 40

[Harly Hsu[:harly]]

Hi Gareth, 
I must have install the wrong patch previously. Now I see the correct version, and the list and the date is just what we want, great work! Below are a few comments:
1. The circle next to the hours should represent the account color, and the notification icon should also be the same color as account color
2. Use a hollow circle if the event is tentative, and use a fully filled circle if the event is accepted
3. All day event should just display "All Day" and aligh vertically center to the event
4. If no location in a event, align the event title vertically center to the event
5. Please change the margin from left of the screen to event title/location to 105px instead of margin from time to title/location 20px, cause time's length will change
6. Change divider of event list to #e5e5e5
7. Change "No Event" text color to #b8b8b8

Comment 41

[Harly Hsu[:harly]]

About alphabetize events that share the same start hour, it isn't that critical, so I am fine with what we have right now. Another thing is that I just found a bug that when I uncheck the accounts in drawer, the event will not show in the list, but the dot on the month view still exist. This need to be fixed.

Comment 42

[Gareth Aye [:gaye] (back from PTO)]

(In reply to Harly Hsu from comment #40)
> Hi Gareth, 
> I must have install the wrong patch previously. Now I see the correct
> version, and the list and the date is just what we want, great work! Below
> are a few comments:
> 1. The circle next to the hours should represent the account color, and the
> notification icon should also be the same color as account color

I like that! Miller is reworking some of the account color code in his day view patch so I will land this and file a followup bug once Miller's work also lands.

> 2. Use a hollow circle if the event is tentative, and use a fully filled
> circle if the event is accepted

I like this also, but we aren't supporting invitations yet.

> 3. All day event should just display "All Day" and aligh vertically center
> to the event
> 4. If no location in a event, align the event title vertically center to the
> event
> 5. Please change the margin from left of the screen to event title/location
> to 105px instead of margin from time to title/location 20px, cause time's
> length will change
> 6. Change divider of event list to #e5e5e5
> 7. Change "No Event" text color to #b8b8b8

Ok will fix.

Comment 43

[Harly Hsu[:harly]]

(In reply to Gareth Aye [:gaye] from comment #42)
> (In reply to Harly Hsu from comment #40)
> > Hi Gareth, 
> > I must have install the wrong patch previously. Now I see the correct
> > version, and the list and the date is just what we want, great work! Below
> > are a few comments:
> > 1. The circle next to the hours should represent the account color, and the
> > notification icon should also be the same color as account color
> 
> I like that! Miller is reworking some of the account color code in his day
> view patch so I will land this and file a followup bug once Miller's work
> also lands.
> 
Got it, could you give me the bug number so I can keep track? Thanks
> > 2. Use a hollow circle if the event is tentative, and use a fully filled
> > circle if the event is accepted
> 
> I like this also, but we aren't supporting invitations yet.

In this case, could we use a fully filled circle before supporting invitations? Because the new drawer also use a fully filled circle representing accounts.
> 
> > 3. All day event should just display "All Day" and aligh vertically center
> > to the event
> > 4. If no location in a event, align the event title vertically center to the
> > event
> > 5. Please change the margin from left of the screen to event title/location
> > to 105px instead of margin from time to title/location 20px, cause time's
> > length will change
> > 6. Change divider of event list to #e5e5e5
> > 7. Change "No Event" text color to #b8b8b8
> 
> Ok will fix.

Comment 44

[Gareth Aye [:gaye] (back from PTO)]

I've updated the current work to switch out the open dots for closed ones on the month's day details view.

Comment 45

[Miller Medeiros [:millermedeiros]]

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

I landed the day view today, so you will need to rebase and make sure we don't have any VISUAL conflicts - there are no merge conflicts so far, but day view deleted/added some CSS rules that affected the month view (specially related to the event title and also the hours on the day child).

My wish was that the MonthDay view was less coupled to the the DayBased, but I'm OK with current approach.  - should not cause us "headaches" and we are probably going to change whole view structure during calendar refactor anyway. (amount of work/risk is too big)

I'll r+ as soon as the CSS conflicts are solved.

Comment 46

[Gareth Aye [:gaye] (back from PTO)]

CSS conflicts are fixed and https://travis-ci.org/mozilla-b2g/gaia/builds/25348765 passes (although the unit tests now run over 50 minutes which is someone else's problem : )

Comment 47

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

test case created : https://moztrap.mozilla.org/manage/case/13108/

Comment 48

[Miller Medeiros [:millermedeiros]]

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

just need to add ellipsis in case the event location text is too large. the other things looks good. r+ from me.

Comment 49

[Gareth Aye [:gaye] (back from PTO)]

https://github.com/mozilla-b2g/gaia/commit/ed5d408dc1120b035ebce9a809499c30fbfb4582

Comment 50

[Edward Chen[:edchen]]

This is a user story ticket, actually it was developed complete so I marked it to "VERIFIED". But depends on this tickets which has 2 bugs (1012796, 1013120) do not fixed. I will keep monitor ones until fixed.

[Environment]
Gaia      8a2352d5b7be27ec4b1ea18c680ebcd0b6d34348
Gecko     https://hg.mozilla.org/mozilla-central/rev/cb9f34f73ebe
BuildID   20140519160202
Version   32.0a1
ro.build.version.incremental=324
ro.build.date=Thu Dec 19 14:04:55 CST 2013

[Restult]
PASS