Closed Bug 1428994 Opened 3 years ago Closed 3 years ago

Make the top and bottom padding size of dividers on Preferences be equal as 16px

Categories

(Firefox :: Preferences, enhancement, P1)

55 Branch
enhancement

Tracking

()

RESOLVED FIXED
Firefox 59
Tracking Status
firefox59 --- fixed

People

(Reporter: evanxd, Assigned: evanxd)

Details

Attachments

(2 files)

Attached image 16px-dividers.png
After discussed with Helen, we would like to make the top and bottom padding size of dividers on Preferences be equal as 16px. It will make preferences page looks more balance.
Hi Helen,

Could you confirm the padding size as 16px? Thank you.
Flags: needinfo?(hhuang)
Assignee: nobody → evan
Status: NEW → ASSIGNED
Yes, the top/bottom padding of dividers is 16px, I've updated the spec accordingly. Please take a look at it https://mozilla.invisionapp.com/share/X8BGCX9PD#/244683138_Content_-_General
Flags: needinfo?(hhuang)
Priority: -- → P1
Comment on attachment 8941349 [details]
Bug 1428994 - Set dividers' margin-top as 16px to make its above and below space be equal and match visual spec.

https://reviewboard.mozilla.org/r/211662/#review217642

::: browser/themes/shared/incontentprefs/preferences.inc.css:134
(Diff revision 1)
> -  margin-top: 32px;
> +  margin-top: 16px;
>    padding-top: 15px;
>    border-top: 1px solid rgba(12, 12, 13, 0.15);

Since the border is between the margin and padding, would it make more sense to set the margin-top to 15px so it matches the padding-top?
Comment on attachment 8941349 [details]
Bug 1428994 - Set dividers' margin-top as 16px to make its above and below space be equal and match visual spec.

https://reviewboard.mozilla.org/r/211662/#review217728

::: browser/themes/shared/incontentprefs/preferences.inc.css:134
(Diff revision 1)
> -  margin-top: 32px;
> +  margin-top: 16px;
>    padding-top: 15px;
>    border-top: 1px solid rgba(12, 12, 13, 0.15);

Good point. Let's make the margin-top and padding-top as 16px to match the visual spec[1].

[1]: https://mozilla.invisionapp.com/share/X8BGCX9PD#/screens/244683138_Content_-_General
Comment on attachment 8941349 [details]
Bug 1428994 - Set dividers' margin-top as 16px to make its above and below space be equal and match visual spec.

Hi Jared,

Could you help review the patch?

Thank you.
Attachment #8941349 - Flags: review?(jaws)
Comment on attachment 8941349 [details]
Bug 1428994 - Set dividers' margin-top as 16px to make its above and below space be equal and match visual spec.

https://reviewboard.mozilla.org/r/211662/#review217932
Attachment #8941349 - Flags: review?(jaws) → review+
Thank you for the review, Jared. Let's land it.
Keywords: checkin-needed
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/022ee7de0098
Set dividers' margin-top as 16px to make its above and below space be equal and match visual spec. r=jaws
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/022ee7de0098
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
You need to log in before you can comment on or make changes to this bug.