680441 - Make fennecomb "more" menu pretty

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, enhancement, P1)

Description

[Ian Barlow (:ibarlow)]

Attachment: assets

Let's make the menu on the far right look like this: http://www.flickr.com/photos/61892693@N03/6059293024/in/photostream

Notes:
* We're differentiating page actions from the rest of the menu with the use of icons.
* These icons are just FPO, I'll post some more polished ones a little later on. I just wanted to show the basic design to get people started. 

Specs: http://cl.ly/0a3e1N0T151W433Z0D04

Placeholder assets are attached here

Comment 1

[Wesley Johnston (:wesj)]

Attachment: Patch v1

This applies on top of my patch in bug 680212. Build is at:

http://dl.dropbox.com/u/72157/fennec-honeycomb1.apk

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 556718 [details] [diff] [review]
Patch v1

>diff --git a/mobile/chrome/content/AppMenu.js b/mobile/chrome/content/AppMenu.js
>       let listbox = document.getElementById("appmenu-popup-commands");
>       while (listbox.firstChild)
>         listbox.removeChild(listbox.firstChild);
>+      let siteCommandsBox = document.getElementById("appmenu-popup-sitecommands");
>+      while (siteCommandsBox.firstChild)
>+        siteCommandsBox.removeChild(siteCommandsBox.firstChild);
>+
> 

Add a blank line above | let siteCommandsBox = ... | and remove the extra blank line below

>diff --git a/mobile/chrome/content/browser.xul b/mobile/chrome/content/browser.xul

>     <arrowbox id="appmenu-popup" hidden="true">
>+      <box id="appmenu-popup-sitecommands"/>
>       <richlistbox id="appmenu-popup-commands"/>

I might be OCD enough to change "appmenu-popup-commands" to "appmenu-popup-appcommands". We haven't officially released this yet, so add-ons won't be hurt.

>     <hbox id="appmenu" bottom="0" hidden="true" align="stretch" oncommand="AppMenu.hide();">
>-      <toolbarbutton class="appmenu-site-button appmenu-button"
>+      <toolbarbutton class="appmenu-site-button appmenu-button appmenu-pageaction"
>         label="&appMenu.siteOptions;"
>         oncommand="getIdentityHandler().show(); event.stopPropagation();"/>
>       <toolbarbutton class="appmenu-preferences-button appmenu-button"
>         label="&prefsHeader.label;"
>         oncommand="BrowserUI.showPanel('prefs-container');"/>
>       <toolbarbutton class="appmenu-addons-button appmenu-button"
>         label="&addonsHeader.label;"
>         oncommand="BrowserUI.showPanel('addons-container');"/>
>-      <toolbarbutton class="appmenu-share-button appmenu-button"
>+      <toolbarbutton class="appmenu-share-button appmenu-button appmenu-pageaction"
>         label="&pageactions.share.page;"
>         oncommand="SharingUI.show(getBrowser().currentURI.spec, getBrowser().contentTitle);"/>
>-      <toolbarbutton class="appmenu-findinpage-button appmenu-button"
>+      <toolbarbutton class="appmenu-findinpage-button appmenu-button appmenu-pageaction"
>         label="&pageactions.findInPage;"
>         oncommand="FindHelperUI.show();"/>
>       <toolbarbutton class="appmenu-downloads-button appmenu-button"
>         label="&downloadsHeader.label;"
>         oncommand="BrowserUI.showPanel('downloads-container');"/>
> #ifdef NS_PRINTING
>-      <toolbarbutton class="appmenu-saveas-button appmenu-button"
>+      <toolbarbutton class="appmenu-saveas-button appmenu-button appmenu-pageaction"
>         label="&pageactions.saveas.pdf;"
>         oncommand="PageActions.savePageAsPDF();"/>
> #endif
>       <toolbarbutton class="appmenu-quit-button appmenu-button"
>         label="&pageactions.quit;"
>         oncommand="Browser.quit();"/>

Why did you not add the class to a few of the items?

>diff --git a/mobile/themes/core/honeycomb/browser.css b/mobile/themes/core/honeycomb/browser.css

>+#appmenu-popup-sitecommands > .appmenu-pageaction {

>+  min-height: -moz-calc(1.5 * @touch_button_xlarge@) !important;
>+  min-width: -moz-calc(1.5 * @touch_button_xlarge@) !important;
>+  max-width: -moz-calc(1.5 * @touch_button_xlarge@) !important;
>+  width: -moz-calc(1.5 * @touch_button_xlarge@) !important;

We could probably add this to defines.inc and save the calc time

>+#appmenu-popup-sitecommands > .appmenu-pageaction:not(:nth-child(3n+3)):-moz-locale-dir(ltr) {
>+  border-right: @border_width_tiny@ solid @color_button_border@;
>+}
>+
>+#appmenu-popup-sitecommands > .appmenu-pageaction:not(:nth-child(3n+3)):-moz-locale-dir(rtl) {
>+  border-left: @border_width_tiny@ solid @color_button_border@;
>+}
>+
>+#appmenu-popup-sitecommands > .appmenu-pageaction:nth-child(3n+1):last-child,
>+#appmenu-popup-sitecommands > .appmenu-pageaction:nth-child(3n+1):nth-last-child(-3n+2),
>+#appmenu-popup-sitecommands > .appmenu-pageaction:nth-child(3n+1):nth-last-child(-3n+3),
>+#appmenu-popup-sitecommands > .appmenu-pageaction:nth-child(3n+2):last-child,
>+#appmenu-popup-sitecommands > .appmenu-pageaction:nth-child(3n+2):nth-last-child(-3n+2),
>+#appmenu-popup-sitecommands > .appmenu-pageaction:nth-child(3n+3):last-child {
>+  border-bottom: none;
>+}

This looks like it's designed to handle more items, if we ever decide to add them, or if add-ons add them, to the sitecommands

r+ with the nits fixed.

I do worry that someone can turn on tablet mode while in froyo or gingerbread themes, there was another theme bug where I wanted to backport some changes to the other themes. The color_background_* changes, iirc. We should file some bugs to do that soon. Good first bug for someone.

Comment 3

[Wesley Johnston (:wesj)]

> I might be OCD enough to change "appmenu-popup-commands" to
> "appmenu-popup-appcommands". We haven't officially released this yet, so
> add-ons won't be hurt.

> Why did you not add the class to a few of the items?

I only added it to things that should go into the "site" section of the menu. 
 
> We could probably add this to defines.inc and save the calc time

I think this is nice and makes it easy to retheme. One measurement rules, rather than 3 or 4 that have to be updated on every change. 

> I do worry that someone can turn on tablet mode while in froyo or
> gingerbread themes, there was another theme bug where I wanted to backport
> some changes to the other themes. The color_background_* changes, iirc. We
> should file some bugs to do that soon. Good first bug for someone.
I made a few small changes to make sure we maintain the current appearance on these themes. We can move more of this over sometime if we want to be more consistent.

Comment 4

[Wesley Johnston (:wesj)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/d52877bb7ed9

Comment 5

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/d52877bb7ed9

Comment 6

[Aaron Train [:aaronmt]]

Samsung Galaxy Tab 10.1
Mozilla/5.0 (Android, Linux armv7l; rv:9.0a1) Gecko/20110906 Firefox/9.0a1 Fennec/9.0a1