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

Status: VERIFIED FIXED

Description

[Helen Huang [:HHuang]]

Buttons size are inconsistent with system UI.

Comment 1

[Gnaneswar]

(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?

Comment 2

[Gnaneswar]

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

Comment 3

[Helen Huang [:HHuang]]

Created attachment 8358270 [details]
Clock_Timer_VisualSpecV2_Dec10.jpg

Visual spec has uploaded. Buttons size please follow the spec, thanks!

Comment 4

[Helen Huang [:HHuang]]

Created attachment 8358275 [details]
Clock_Timer_VisualSpecV2_Dec10.jpg

Spec updated.

Comment 5

[Gnaneswar]

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

Comment 6

[Helen Huang [:HHuang]]

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.

Comment 7

[Gnaneswar]

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.

Comment 8

[Helen Huang [:HHuang]]

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

[Gnaneswar]

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

Comment 10

[Gnaneswar]

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

Attached updated patch.

Comment 11

[Gnaneswar]

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

Attached screen shot for 45% width

Comment 12

[Gnaneswar]

Would like to know your comments.

Comment 13

[Marcus Cavanaugh [:mcav]]

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".

Comment 14

[Gnaneswar]

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

[[e-mail]]

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.

Comment 16

[Marcus Cavanaugh [:mcav]]

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

Comment 17

[Marcus Cavanaugh [:mcav]]

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

Helen, does this look correct?

Comment 18

[Helen Huang [:HHuang]]

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

Looks great! Thanks a lot :)

Comment 19

[Gnaneswar]

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.

Comment 20

[Marcus Cavanaugh [:mcav]]

Landed in master:

https://github.com/mozilla-b2g/gaia/commit/07384e046d7023ad00e3fa2872d1647a7911bc15

Comment 21

[Edward Chen[:edchen]]

[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"

Comment 22

[Helen Huang [:HHuang]]

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

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

Bug fixed