1023662 - [User story] Show 5 day week view

Status: VERIFIED FIXED

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

Comment 1

[Harly Hsu[:harly]]

Attachment: [2.1 Calendar] 5 Days Week View_Draft.pdf

Comment 2

[Miller Medeiros [:millermedeiros]]

I'm currently working on a prototype for the current spec, will let you guys know about the progress. Hopefully we will have something next week to try out (even if still not working as expected we will get an idea about performance and challenges). I'm assuming this will take longer than 1 sprint to implement properly.

Comment 3

[Miller Medeiros [:millermedeiros]]

you guys can see the prototype here: https://github.com/millermedeiros/prototype-week-view - it was REALLY hard to get good performance.

I'll try to push a new branch with the `position:sticky` on Monday so you guys can try it out, it's way faster than this version but I wasn't able to make the "snap" to work properly. Maybe I'll need to ask someone from the APZ team if they have any "tricks".

at first I tried to adapt the calendar app code but it was causing so many errors and was so hard to edit that I gave up and started an empty project, that's why it doesn't look the same as the app. - please ignore the visual, focus on the interaction and performance!

Comment 4

[Harly Hsu[:harly]]

Attachment: [2.1 Calendar] 5 Days Week View_v1.0.pdf

Comment 5

[Harly Hsu[:harly]]

Attachment: [2.1 Calendar] 5 Days Week View_v1.0.pdf

Comment 6

[Miller Medeiros [:millermedeiros]]

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

still work in progress, but should be enough for some UI feedback. I did not lock the vertical scroll while doing the drag because of Bug 1050789, I tried to do it in many different ways and failed...

I'm going to clean up the code and abstract it further to reduce duplications and also to make it easier to reuse same structure for the day view.. I'm thinking that we should probably keep both the "same" code, only difference is that on the day view the events are rendered differently and that we only show a single day at a time.

PS: I replaced the whole week view with this changes, so don't assume things still work as they used before.

Comment 7

[Harly Hsu[:harly]]

Hi Miller,
Great work! The performance of the patch is really better than the prototype you did last time. The only UX concern I have besides locking vertical scroll is that after using it for a while, I found out that the scrolling is not so smooth when I do a quick swipe, it felt more like a quick jump rather than a continuous swipe. But if I drag slowly from left to right, the scrolling felt good and continuous. Do you know what's causing it? Also is it possible to add an ease animation to the auto snap?

Comment 8

[Edward Chen[:edchen]]

Test case: https://moztrap.mozilla.org/manage/cases/?pagenumber=1&pagesize=20&sortfield=created_on&sortdirection=desc&filter-tag=2964

Comment 9

[Miller Medeiros [:millermedeiros]]

Harly, I updated the patch. That's basically what I did:

 - improved the fast swipe performance A LOT.
 - implemented a "fast swipe" logic, so if the "momentum" is bigger than 0.4 (momentum = drag distance / duration) it will move by 5 days, which I think is a better behavior than what we had before.
 - improved snapping logic to only snap to "next day" if the position is very close to the border of next day; before I was doing a regular rounding, so 50% would round up, now I only round up if above 70% (near the second digit of the date).
 - increased the transition duration and changed the easing equation so user can see the the animation better (I had easing before as well, it was just too fast/subtle).
 - improved the scroll lock logic (now I only re-enable the vertical scroll after the animation finishes).
 - and also improved the regular drag speed/feedback.

now it's getting into a point where I don't think we can improve the performance that much, but we can still tweak the values to get better easing or to get a better snap/rounding/swipe behavior... But I would rather spend the next weeks doing other work (redoing the Day view to use same code/logic and also finish the AMD module conversion), and do polish work during the stabilization sprints (if that is possible).

PS: if you swipe really fast you might still get a "quick jump" (specially on the hamachi), that's because we build the dates as needed (we always have 15 days on the DOM); but now that's way less perceptible since the new logic tries to be "smart" about it (10 fast swipes will ALWAYS move 50 days, even if we skip the animation of a few dates).

Comment 10

[Harly Hsu[:harly]]

Comment on attachment 8470830 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/22697/files

Miller, awesome work! The performance has improved greatly, and just as you've said, there is still room for improvement like better easing or better snap/rounding/swipe behavior that we could tweak on. But for now, it is more than acceptable. Also, I would be happy to work with you with the tweak value during the stabilization when you are available. Thanks.

Comment 12

[Miller Medeiros [:millermedeiros]]

Comment on attachment 8470830 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/22697/files

I did a mix of unit & marionette tests (some things are only tested by the unit tests and others are only tested by marionette), it should be enough to cover basic regressions.

Comment 13

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

Comment on attachment 8470830 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/22697/files

Nice job Miller and thank you, as always, for being patient through all of my nits :). Squash and land when ready!

Comment 14

[Miller Medeiros [:millermedeiros]]

landed on master: https://github.com/mozilla-b2g/gaia/commit/9e5cdf38347c00ae96e91e4e28d406cc5cb73f8c

Comment 15

[Harly Hsu[:harly]]

Hi Miller,
Have just checked the calendar on master, it looks great. Just one thing that puzzle me, is that it seems one of the previous behavior of week view was not included in the new 5 days week view. It is that when user swipe to other days in week view, for example 9/1~9/5. Then go back to month view, it will not highlight the first day in previous week view, which is 9/1. It would still highlight the date before entering week view.

Comment 16

[Miller Medeiros [:millermedeiros]]

Harly, I created a new bug for it Bug 1060542. Let's create new bugs for each regression that we find (or improvements).

Comment 17

[John Thomas [:Johnt]]

This issue is verified fixed on Flame 2.1(319mb)

Environmental Variables:
Device: Flame Master
BuildID: 20140902040205
Gaia: 44bf2e3bc5ddea9db9a8c851bd353cb234aa883c
Gecko: c360f3d1c00d
Version: 34.0a1 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

The Calender is now showing a 5 days week view.

Comment 18

[Jordan de Geus(JordanD)]

Adding additional information for Flame 2.1 (319mb)

Bug 1050789 seems to no longer occur - Users are NOT able to scroll vertically while scrolling horizontally
Bug 1060542 also seems to no longer occur as when users switch back to month view from week view, users will see the date they had prior selected. 

STR:
1. Launch Calendar App
2. Select Week view
3. Users are brought to the week selected while autoscrolled to 1 hour prior to current time
4. Scroll left and right multiple weeks - Scrolling functions as a continious horizontal pane
5. Switching back to month view after scrolling multiple weeks - users see the date they had previously selected prior to entering week view.

Actual: 
Verifed when switching to Week view, user sees 5 days in a week. Users also see they are autoscrolled to the current time with 1 hour prior to displaying if current day in view. Switching back to the month view from week view, users will see the date they had previously selected. 

Environmental Variables:
Device: Flame 2.1 (319mb)
Build ID: 20140903000204
Gaia: fbb297c39aab5f17b179533d2a9a6c5166b2c197
Gecko: fb5e796da813
Version: 34.0a2 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 19

[Miller Medeiros [:millermedeiros]]

step 5 should be: switching back to month view after scrolling multiple weeks - should update the selected day to match the "last" day displayed on the week view. (in fact the oldest date displayed on the week view, if viewing Oct 06 till Oct 10 it should highlight Oct 06)

this is being worked on Bug 1060542 (we did not land it yet since I'm waiting for review).

Comment 20

[Jordan de Geus(JordanD)]

I cannot verify this feature landed user story as Bug 1060542 is still set to NEW and has yet to be landed

Comment 21

[Jason Smith [:jsmith]]

bug 1060542 was deemed as a non-blocker, which means it shouldn't block QA signoff.

Comment 22

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

The calendar shows 5 days when looking at it in week view, as shown in the PDF attached to this bug