Closed Bug 1198935 Opened 4 years ago Closed 4 years ago

Set about: pages header height to 48px

Categories

(Firefox for Android :: Theme and Visual Design, defect)

All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 48
Tracking Status
firefox48 --- verified

People

(Reporter: antlam, Assigned: ralin)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files)

Attached image spec_aboutheader.png
I noticed our headers in the about:logins, about:addons, etc, are taller than in the spec.

Let's update these and see how they look!
Assignee: nobody → ralin
Attached image header_output.png
Hi, Anthony.

I've made some changes in the screenshot:
1. change border color
2. set fixed height(48px) for header
3. made right hand side button to 48px square

Could you give me some feedback? 

Thanks
Attachment #8741256 - Flags: feedback?(alam)
Comment on attachment 8741256 [details]
header_output.png

Looks great, thanks Ray!
Attachment #8741256 - Flags: feedback?(alam) → feedback+
Comment on attachment 8741399 [details]
MozReview Request: Bug 1198935 - Set about: pages header height to 48px. r?margaret

https://reviewboard.mozilla.org/r/46463/#review43043

I'm not sure if ally is still active in Bugzilla. This looks good to me, so I'll steal the review and give this an r+!.

::: mobile/android/themes/core/aboutBase.css:41
(Diff revision 1)
>  }
>  
>  .header > div {
>    flex: 1;
>    padding: 1em;
> -  -moz-padding-start: 1.5em;
> +  -moz-padding-start: 16px;

It's odd to have inconsistency between em and px. Can we change the generic padding rule to px as well?
Attachment #8741399 - Flags: review+
Comment on attachment 8741399 [details]
MozReview Request: Bug 1198935 - Set about: pages header height to 48px. r?margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46463/diff/1-2/
Attachment #8741399 - Attachment description: MozReview Request: Bug 1198935 - Set about: pages header height to 48px. r?ally → MozReview Request: Bug 1198935 - Set about: pages header height to 48px. r?Margaret
Attachment #8741399 - Flags: review?(a.m.naaktgeboren)
Comment on attachment 8741399 [details]
MozReview Request: Bug 1198935 - Set about: pages header height to 48px. r?margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46463/diff/2-3/
Attachment #8741399 - Attachment description: MozReview Request: Bug 1198935 - Set about: pages header height to 48px. r?Margaret → MozReview Request: Bug 1198935 - Set about: pages header height to 48px. r?margaret
https://reviewboard.mozilla.org/r/46463/#review43043

> It's odd to have inconsistency between em and px. Can we change the generic padding rule to px as well?

No problem! I changed generic padding to 10px, and it looks no difference to 1em in this case. Thanks.
Thanks!
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/cfdf31074891
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Verified as fixed in build 48.0a2 2016-05-30;
Device: LG G4 (Android 5.1).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.