Closed Bug 951092 Opened 8 years ago Closed 7 years ago
[Clock] Adjust Pause/Cancel buttons size and style in Timer
255.92 KB, image/jpeg
20.71 KB, image/png
845 bytes, patch
|Details | Diff | Splinter Review|
21.48 KB, image/png
46 bytes, text/x-github-pull-request
|Details | Review|
14.35 KB, image/png
Buttons size are inconsistent with system UI.
(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?
Just looked at this Bug 951096 and come to know that stopwatch controls size also incorrect. Could you share the details of button sizes?
Visual spec has uploaded. Buttons size please follow the spec, thanks!
Attachment #8358270 - Attachment is obsolete: true
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
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.
Attachment #8358275 - Attachment is obsolete: true
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.
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!
Thanks for the comments. Attached screen shot with width as 44% for button. please let me know your comments
Attached updated patch.
Attachment #8361064 - Flags: review?(hhuang)
Attached screen shot for 45% width
Would like to know your comments.
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".
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).
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.
James, this is pretty much identical to the other review request, don't be alarmed. :-)
Attachment #8369604 - Flags: review?(jrburke)
Helen, does this look correct?
Assignee: nobody → m
Status: NEW → ASSIGNED
Attachment #8369606 - Flags: ui-review?(hhuang)
Attachment #8369604 - Flags: review?(jrburke) → review+
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+
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.
Landed in master: https://github.com/mozilla-b2g/gaia/commit/07384e046d7023ad00e3fa2872d1647a7911bc15
Status: ASSIGNED → RESOLVED
Closed: 7 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
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-
You need to log in before you can comment on or make changes to this bug.