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

Status: RESOLVED FIXED

(Firefox :: Settings UI, enhancement, P1)

Description

[Evan Tseng [:evanxd]]

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

Comment 1

[Evan Tseng [:evanxd]]

Hi Helen,

Could you confirm the padding size as 16px? Thank you.

Comment 2

[Helen Huang [:HHuang]]

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

Comment 3

[Evan Tseng [:evanxd]]

Attachment: Bug 1428994 - Set dividers' margin-top as 16px to make its above and below space be equal and match visual spec.

Review commit: https://reviewboard.mozilla.org/r/211662/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/211662/

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

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 5

[Evan Tseng [:evanxd]]

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 6

[Evan Tseng [:evanxd]]

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.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/211662/diff/1-2/

Comment 7

[Evan Tseng [:evanxd]]

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.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

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

Comment 9

[Evan Tseng [:evanxd]]

Thank you for the review, Jared. Let's land it.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
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

Comment 11

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/022ee7de0098