Closed Bug 956946 Opened 6 years ago Closed 6 years ago
Gap below URL bar and menu
There's a small gap between the action bar and the menu itself. All other Android apps align the menu directly under the action bar, which looks cleaner, and we should do the same to follow convention. Though we've always had a gap here, we used to have an arrow between the menu button and the menu, so it made more sense before. Screenshot attached.
tracking-fennec: ? → 29+
The menu margin/shadow is set by menu_panel_bg.9.png: http://mxr.mozilla.org/mozilla-central/find?text=&string=menu_panel_bg.9.png Ian, I assume we want to flatten the menu out like the rest of our UI? If so, can you provide updated assets?
Or maybe it's menu_popup_bg.9.png: http://mxr.mozilla.org/mozilla-central/find?text=&string=menu_popup_bg.9.png I'm not sure what the difference is -- maybe we need to change both.
It looks like the Android resources have this padding: http://androidxref.com/4.4.2_r1/xref/frameworks/base/core/res/res/drawable-xhdpi/menu_dropdown_panel_holo_light.9.png Maybe we just need to offset this popup by the top padding?
Hmm...I couldn't find any style where we set padding or margin, so I thought the spacing in the 9 png was creating the gap. That's weird that Android's 9 png has visible top padding/shadow, yet the menu is flush with the action bar...
Eugen is taking a look at this, so assigning to him.
Assignee: nobody → esawin
Comment on attachment 8360782 [details] [diff] [review] URL_bar_gap.patch Passing to Brian for a better review. I worry that menu_popup_offset is used for MenuPopup and ArrowPopup http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/menu/MenuPopup.java#33 http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/widget/ArrowPopup.java#49 I don't know that we want to change the ArrowPopup, since it still has the little arrow pointing up. MenuPopup used to have the little arrow too, so the same offset worked for both. Maybe we should change menu_popup_offset, but add menu_popup_arrow_offset which has the old value and would be used in ArrowPopup?
Attachment #8360782 - Flags: review?(mark.finkle) → review?(bnicholson)
Comment on attachment 8360782 [details] [diff] [review] URL_bar_gap.patch I agree with Mark; menu_popup_offset is used by both MenuPopup and ArrowPopup, but we want to change only MenuPopup. Please create another dimens value as Mark said to keep these separate.
Attachment #8360782 - Flags: review?(bnicholson) → review-
I have added menu_popup_arrow_offset as a separate dimens value for the ArrowPopup with the old value and updated the menu_popup_offset for MenuPopup to the new value to keep them separated.
Comment on attachment 8361339 [details] [diff] [review] URL_bar_gap.patch Review of attachment 8361339 [details] [diff] [review]: ----------------------------------------------------------------- Nice, this looks good.
Attachment #8361339 - Flags: review?(bnicholson) → review+
Patch for check-in.
Congratulations on getting your first patch landed! https://hg.mozilla.org/integration/fx-team/rev/b290c1d5630f
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
You need to log in before you can comment on or make changes to this bug.