[Clock] Adjust Pause/Cancel buttons size and style in Timer

VERIFIED FIXED in 1.4 S1 (14feb)

Status

Firefox OS
Gaia::Clock
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: HHuang, Assigned: mcav)

Tracking

unspecified
1.4 S1 (14feb)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(feature-b2g:2.0)

Details

(Whiteboard: [p=1])

Attachments

(6 attachments, 3 obsolete attachments)

(Reporter)

Description

4 years ago
Buttons size are inconsistent with system UI.
(Reporter)

Updated

4 years ago
Blocks: 950212

Comment 1

4 years ago
(In reply to Helen Huang from comment #0)
> Buttons size are inconsistent with system UI.

Is stopwatch controls size can be taken as reference to adjust timer controls?
Flags: needinfo?(hhuang)

Comment 2

4 years ago
Just looked at this Bug 951096 and come to know that stopwatch controls size also incorrect.
Could you share the details of button sizes?
(Reporter)

Comment 3

4 years ago
Created attachment 8358270 [details]
Clock_Timer_VisualSpecV2_Dec10.jpg

Visual spec has uploaded. Buttons size please follow the spec, thanks!
Flags: needinfo?(hhuang)
(Reporter)

Comment 4

4 years ago
Created attachment 8358275 [details]
Clock_Timer_VisualSpecV2_Dec10.jpg

Spec updated.
Attachment #8358270 - Attachment is obsolete: true

Comment 5

4 years ago
In the spec, sizes of buttons are given in pixels. So alignment of the buttons will change depending upon the display size of the device. ( Ex: It will be different in INARI and Nexus 4 as the aspect ratio is different)
It seems that instead of absolute values of the buttons (like 140 x 40px ) we need to give values in % so that UI remains consistent for all the form factors or display sizes.

Please let me know your views
Flags: needinfo?(hhuang)
(Reporter)

Comment 6

4 years ago
Created attachment 8359681 [details]
Clock_Timer_VisualSpecV3_Dec14.jpg

Thanks for the reminder.
I’ve modified the spec. Please take a look.
I used rem values and marked the distances between buttons and screen edges. I also checked buttons.css on GitHub, the button width is always 100%.
Any question please let me know.
Flags: needinfo?(hhuang)
(Reporter)

Updated

4 years ago
Attachment #8358275 - Attachment is obsolete: true

Comment 7

4 years ago
Created attachment 8360309 [details] [diff] [review]
0001-Bug-951092-Clock-Adjust-Pause-Cancel-buttons-size-an.patch

Thanks for the spec.

As per the spec, I tried setting each button width to calc((100%-1rem)/2), but each button is occupying entire screen. As a result, pause and cancel buttons are being displayed in two different rows.Hence I didn't set those values.

Instead, I have set the width to 45% of total value.Height and margin values are also set in the attached patch.

Please let me know your comments on the patch.
Attachment #8360309 - Flags: review?(hhuang)
(Reporter)

Comment 8

4 years ago
Thank you for trying.
I think each button width to 44% of total value is closer to the spec. 
Could you attach the screenshot if you've done? That will be helpful!

Comment 9

4 years ago
Created attachment 8361062 [details]
2014-01-16-13-46-43.png

Thanks for the comments.

Attached screen shot with width as 44% for button.

please let me know your comments
Attachment #8360309 - Attachment is obsolete: true
Attachment #8360309 - Flags: review?(hhuang)

Comment 10

4 years ago
Created attachment 8361064 [details] [diff] [review]
Updated patch with width set to 44%

Attached updated patch.
Attachment #8361064 - Flags: review?(hhuang)

Comment 11

4 years ago
Created attachment 8361088 [details]
2014-01-16-14-02-06.png

Attached screen shot for 45% width

Comment 12

4 years ago
Would like to know your comments.
Flags: needinfo?(m)
(Assignee)

Comment 13

4 years ago
You _should_ use the calc() function; the problem before is that you need to have whitespace around the minus operator, otherwise it interprets the width as "-1rem".
Flags: needinfo?(m)

Comment 14

4 years ago
Thanks for the comments!

I have placed white space. Though width is decreased,  due to margins( of 1.5 rem) on both ends and spacing of 1 rem in b/w buttons made the second button appear below the first button instead of next to each other.

It seems I can't use the width of calc((100% - 1rem)/2).

Comment 15

4 years ago
Hi,

Maybe you could center the buttons and use button:first-child/last-child to fine-tune the button position.

And please also make sure the tablet layout is not missed since these styles will be inherited.
(Assignee)

Comment 16

4 years ago
Created attachment 8369604 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/15923

James, this is pretty much identical to the other review request, don't be alarmed. :-)
Attachment #8369604 - Flags: review?(jrburke)
(Assignee)

Comment 17

4 years ago
Created attachment 8369606 [details]
Screen Shot 2014-02-02 at 12.15.13.png

Helen, does this look correct?
Assignee: nobody → m
Status: NEW → ASSIGNED
Attachment #8369606 - Flags: ui-review?(hhuang)

Updated

4 years ago
Attachment #8369604 - Flags: review?(jrburke) → review+
(Reporter)

Comment 18

4 years ago
Comment on attachment 8369606 [details]
Screen Shot 2014-02-02 at 12.15.13.png

Looks great! Thanks a lot :)
Attachment #8369606 - Flags: ui-review?(hhuang) → ui-review+

Comment 19

4 years ago
It seems that as per spec, margin-top to be set to 27.8rem. 

Setting 27.8rem also may not be correct as it will look differently on different form-factor devices.

As discussed with Marcus on IRC, space between buttons and timer numbers should match.
As margin-top is not set in attached patch, it seems this needs to be incorporated.
(Assignee)

Updated

4 years ago
Whiteboard: [p=1]
(Assignee)

Updated

4 years ago
Target Milestone: --- → 1.4 S1 (14feb)
(Assignee)

Comment 20

4 years ago
Landed in master:

https://github.com/mozilla-b2g/gaia/commit/07384e046d7023ad00e3fa2872d1647a7911bc15
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
[Verified info]
Gaia      6e71ab4da1b08586ea0c758edb7aa199ee34cd2f
Gecko     https://hg.mozilla.org/mozilla-central/rev/bb030d47c946
BuildID   20140219160202
Version   30.0a1

[Testing result]
PASS
New UI is changed and no other side effects. I mark it to "VERIFIED"
Status: RESOLVED → VERIFIED
(Reporter)

Comment 22

4 years ago
Comment on attachment 8361064 [details] [diff] [review]
Updated patch with width set to 44%

Review of attachment 8361064 [details] [diff] [review]:
-----------------------------------------------------------------

Bug fixed
Attachment #8361064 - Flags: review?(hhuang) → review-
feature-b2g: --- → 2.0
You need to log in before you can comment on or make changes to this bug.