Closed Bug 704415 Opened 8 years ago Closed 8 years ago

Style the Add-on Manager

Categories

(Firefox for Android :: General, defect, P2)

ARM
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 12
Tracking Status
firefox11 --- verified
firefox12 --- verified
firefox13 --- verified
fennec 11+ ---

People

(Reporter: mfinkle, Assigned: mfinkle)

References

Details

(Keywords: late-l10n)

Attachments

(9 files)

Blocks: aom
Assignee: nobody → padamczyk
Spec for standard (480x800) screens.
http://www.flickr.com/photos/patrykdesign/6395720527/

Spec for high resolution (720x1280) screens
http://www.flickr.com/photos/patrykdesign/6391226459/
Assignee: padamczyk → mark.finkle
Priority: -- → P2
Attached patch WIPSplinter Review
The beginnings of new style and structure:

* Has basic CSS added. Needs more tweaks
* Has the list main page
* Has the detail page. Tap a list item to open the detail page
* Back/Forward works between pages
* Enable/Disable and Uninstall work
* Only the beginnings of listing the Options on the details page. Needs much more work.
Comment on attachment 580470 [details] [diff] [review]
WIP

Actually, this patch is better than the current code and the style spec is changing, so I'll need another refresh anyway.

Bug 696533 will be used to get the options working better too.
Attachment #580470 - Flags: review?(mbrubeck)
Comment on attachment 580470 [details] [diff] [review]
WIP

Review of attachment 580470 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/chrome/content/aboutAddons.xhtml
@@ +358,5 @@
> +            for (let i = 0; i < prefs.length; i++)
> +              box.appendChild(prefs.item(i));
> +  /*
> +            // Send an event so add-ons can prepopulate any non-preference based
> +            // settings

Why is this section commented out?

::: mobile/android/chrome/content/browser.js
@@ +1275,5 @@
>      }
>    },
>  
>    onLocationChange: function(aWebProgress, aRequest, aLocationURI, aFlags) {
> +    dump("*** location change ***")

Remove before checkin (unless you think this will be generally useful for development).

::: mobile/android/themes/core/aboutAddons.css
@@ +110,5 @@
> +  -moz-box-flex: 1;
> +}
> +
> +.tag {
> +  text-align: right;

Is this correct for RTL?  (I'm having trouble picturing which parts of the .details box change in RTL and which don't.)
Attachment #580470 - Flags: review?(mbrubeck) → review+
(In reply to Matt Brubeck (:mbrubeck) from comment #7)

> ::: mobile/android/chrome/content/aboutAddons.xhtml

> > +            for (let i = 0; i < prefs.length; i++)
> > +              box.appendChild(prefs.item(i));
> > +  /*
> > +            // Send an event so add-ons can prepopulate any non-preference based
> > +            // settings
> 
> Why is this section commented out?

Add-on options are ready to go yet. Waiting for the next patch, probably in bug 696533.

> ::: mobile/android/chrome/content/browser.js

> >    onLocationChange: function(aWebProgress, aRequest, aLocationURI, aFlags) {
> > +    dump("*** location change ***")
> 
> Remove before checkin (unless you think this will be generally useful for
> development).

Will remove

> ::: mobile/android/themes/core/aboutAddons.css

> > +.tag {
> > +  text-align: right;
> 
> Is this correct for RTL?  (I'm having trouble picturing which parts of the
> .details box change in RTL and which don't.)

| text-align: end | is the RTL safe way. Changing.
https://hg.mozilla.org/mozilla-central/rev/87145c060348

will leave open for some follow on CSS work
We are aligning everything to a more unique mozilla style and turn away from being so native related. The layout and font size hasn't changed, but the colours and backgrounds have. They are more aligned with the updated header and awesome screen.

Mocks are found here:
http://www.flickr.com/photos/patrykdesign/6501493853/in/photostream
http://www.flickr.com/photos/patrykdesign/6501356635/in/photostream

These changes should only be CSS HEX value related.
Attached image Addons Layout Bugs
Attached is a screenshot from the Galaxy Nexus showing some difference in the proposed layout and the implementation.
OS: Linux → Android
Hardware: x86 → ARM
tracking-fennec: --- → 11+
This patch uses the specs to create a "light" style for the add-on manager
* Adds new background images
* Adds new header image
* If you tap the header image you open an AMO tab. The URL is pulled from the mobile.js prefs
* Updated the CSS URLs for about: background images too. They were using a mix of browser/skin and browser/skin/images and I made them all use browser/skin/images which is what we use everywhere else.

Screenshots coming
Attachment #588680 - Flags: review?(wjohnston)
Oops the screenshot is one "hg qref" too old. The header text "ADD-ONS" is now "Your-Add-ons"
Comment on attachment 588680 [details] [diff] [review]
patch - update to a light theme

Review of attachment 588680 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. I played with centering the text a bit, but you're right, its a pain and I have higher priority bugs to work on. I am a bit worried there's no "active" state for tapped items. Quickly trying to hack that in doesn't work either... I wouldn't hold this up waiting for that either though.

::: mobile/android/chrome/content/aboutAddons.xhtml
@@ +89,5 @@
>    <div id="addons-header">
> +    <div>
> +      <div>&aboutAddons.header2;</div>
> +      <img src="chrome://browser/skin/images/addons-amo-hdpi.png" pref="extensions.getAddons.browseAddons" onclick="openLink(this);"/>
> +    </div>

So only clicking the image will take you to AMO? That seems.... odd to me, and difficult to discover? Patryk says that's normal for Android apps that link to the market. I think maybe we should add a subtle arrow, or a border on the left?

::: mobile/android/themes/core/aboutAddons.css
@@ +40,2 @@
>    font-size: 18px;
> +  background-image: url("chrome://browser/skin/about-bg-lightblue.png");

chrome://browser/skin/IMAGES/about-bg-lightblue.png
Attachment #588680 - Flags: review?(wjohnston) → review+
https://hg.mozilla.org/mozilla-central/rev/b7ca6c0493dd
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
Finkle, can you request aurora approval?
Comment on attachment 580470 [details] [diff] [review]
WIP

[Approval Request Comment]Patch has bug fixes and theme updates for add-on manager we want in fx11
Attachment #580470 - Flags: approval-mozilla-aurora?
Comment on attachment 588680 [details] [diff] [review]
patch - update to a light theme

[Approval Request Comment]
Additional fixes and theme work needed for fx11
Attachment #588680 - Flags: approval-mozilla-aurora?
Comment on attachment 580470 [details] [diff] [review]
WIP

[Triage Comment]
Mobile only - approved for Aurora.
Attachment #580470 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #588680 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This has string impact and still needs to land on aurora?
Keywords: late-l10n
https://hg.mozilla.org/releases/mozilla-aurora/rev/1e9d420263f8
Status: RESOLVED → REOPENED
tracking-fennec: 11+ → 12+
Resolution: FIXED → ---
Target Milestone: Firefox 12 → ---
Version: unspecified → Trunk
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
tracking-fennec: 12+ → 11+
Resolution: FIXED → ---
Target Milestone: --- → Firefox 12
Mark, did you reopen this bug on purpose?
(In reply to Axel Hecht [:Pike] from comment #26)
> Mark, did you reopen this bug on purpose?

No. Only wanted to set the tracking-fennec flag
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Verified fixed on:

Firefox 13.0a1 (2012-02-17)
20120217031227
http://hg.mozilla.org/mozilla-central/rev/2271cb92cc05

Firefox 12.0a2 (2012-02-16)
20120216042010
http://hg.mozilla.org/releases/mozilla-aurora/rev/c6fcc091279c

Firefox 11.0 (2012-02-15)
20120215185359
http://hg.mozilla.org/releases/mozilla-beta/rev/f21c6aa0f8c2

--
Device: Motorola Droid PRO
OS: Android 2.3.3
Depends on: 1008825
You need to log in before you can comment on or make changes to this bug.