910854 - [Clock] Reduce analog face resizing from 4 sizes to 3 sizes

Status: RESOLVED FIXED

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

Description

[Evelyn Eastmond [:evhan55]]

According to Eric Pang:

"I was told the clock only has two sizes now.
You can see the two sizes in the review psd here:
https://mozilla.box.com/s/ls0pr46mxp49m8ccy3yn"

Comment 1

[[e-mail], [e-mail]]

Hi Eric,

If you don't have any concern, could you please provide the permission for me? I would like to access them. 
My account on app.box.com:
Name:  Ian Liu
Email: [e-mail] 
Thanks.

Comment 2

[Eric Pang [:epang] UX]

(In reply to Ian Liu [:ianliu] from comment #1)
> Hi Eric,
> 
> If you don't have any concern, could you please provide the permission for
> me? I would like to access them. 
> My account on app.box.com:
> Name:  Ian Liu
> Email: [e-mail] 
> Thanks.

Hi Ian, I've invited you to the folder now.  We've tried to open all the applications folders on box.  But we run into an issue since setting a top level folder doesn't change all sub folders.  In the future if you need to see a file in a rush you can use this link: 

https://mozilla.box.com/applications

From here you can navigate to the file you need to see :).  Sorry for the inconvenience, Stephany has contacted IT and ask if there's a to have all the folders default to open instead of on to collaborators.

Comment 3

[[e-mail], [e-mail]]

Eric,

I'm able to see these folders now. The new visual looks pretty good.:) Thanks for your helps. I have no confused for the request of the issue here.

Comment 4

[Eric Pang [:epang] UX]

(In reply to Ian Liu [:ianliu] from comment #3)
> Eric,
> 
> I'm able to see these folders now. The new visual looks pretty good.:)
> Thanks for your helps. I have no confused for the request of the issue here.

Ian/Evelyn,

Sorry for confirming to only send an update now.  I spoke with Przemek and we both think that the clock works better with 3 sizes instead of 2, since the large clock was pretty small.  Now we have a small, medium and large clock.  I've updated the files on box to demonstrate this.  I've also updated the name of this bug to "[Clock] Reduce analog face resizing from 4 sizes to 3 sizes". Thanks!  Hope this doesn't cause any issues!

Comment 6

[Evelyn Eastmond [:evhan55]]

Currently, the clock face doesn't correctly resize back to the largest size when all the alarms from the alarms list are deleted.  This is probably a bug that can be fixed as part of this ticket.

Comment 7

[Dylan Oliver [:doliver]]

triage: in a joint meeting with product/ux/dev, we agreed that this was critical functionality for clock in 1.2.

Comment 8

[Evelyn Eastmond [:evhan55]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/13099

`clock_view.js`
    - Changed to resize to 3 sizes instead of 4
    - Removed reference to 'marks' CSS
`clock.css`
    - Renamed 'marks' size classes to 'small', 'medium', 'large'
Issues
    - No unit tests for this?  Is there an integration test?

Comment 9

[Mike Pennisi [:jugglinmike]]

Comment on attachment 822473 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/13099

Hi Evelyn,

You're right to want integration tests for this behavior, but the tests have been removed because of a platform bug that caused them to fail [1]. Until we're able to add them back, I'm afraid we'll just have to add some less-than-maintainable unit tests to cover this behavior. Fortunately, the code you are changing is very functional in nature, so those tests should be easy to write.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=907177#c24

Comment 10

[Evelyn Eastmond [:evhan55]]

Comment on attachment 822473 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/13099

Resubmitting for review after adding unit tests for resizeAnalogClock.

Updated PR comment:

- `clock_view.js`
    - Changed to resize to 3 sizes instead of 4
    - Removed reference to 'marks' CSS
- `clock.css`
    - Renamed 'marks' size classes to 'small', 'medium', 'large'
- Tests
    - Tested manually on desktop and on Inari
    - Unit test in `clock_view_test.js` testing basic `resizeAnalogClock` functionality

Comment 11

[Mike Pennisi [:jugglinmike]]

Comment on attachment 822473 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/13099

This looks good to me! I'll land it as soon as the TravisCI run completes.

Comment 12

[Mike Pennisi [:jugglinmike]]

master: https://github.com/mozilla-b2g/gaia/commit/35f9bfb58c21b43b413eba97d35ae32d8690f4aa

Thank you kindly, Evelyn!

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 35f9bfb58c21b43b413eba97d35ae32d8690f4aa to:
v1.2: bc312456e14c323dc6595c575c492b1512882dfd

Comment 14

[Peter Bylenga [:PBylenga]]

Verified Fixed: Clock analog face utilizes 3 different sizes to fit multiple alarms.  Also clock analog face resizes as expected after alarms are deleted.

Environmental Variables
Device: Buri v1.2 COM RIL
Build ID: 20131105004003
Gecko: http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/3ba912717904
Gaia: be4ea00a50236b10eb0a03232a28ffd0048e0cb8
Platform Version: 26.0
RIL Version: 01.01.00.019.281 
Firmware Version: US_20131015