Closed Bug 594795 Opened 9 years ago Closed 9 years ago

Add a "What's an Add-on" block to the Add-ons Manager

Categories

(Firefox for Android Graveyard :: General, defect)

x86
macOS
defect
Not set

Tracking

(fennec2.0b2+)

VERIFIED FIXED
Tracking Status
fennec 2.0b2+ ---

People

(Reporter: madhava, Assigned: mfinkle)

Details

Attachments

(1 file, 1 obsolete file)

We've wanted to have some explanation of what add-ons are, for new users, in the product for a while.  The Add-ons Manager redesign for desktop Firefox 4 includes a section with a brief explanation and a button that leads to a more informative webpage.  We should include a slimmed down version of this section in the Fennec Add-ons Manager.

Here's a mockup:
http://www.flickr.com/photos/madhava_work/4971673433/sizes/l/

For reference, here's what the desktop version looks like:
http://www.flickr.com/photos/madhava_work/4971680897/in/photostream/

We'll need for the "Learn More" button to point somewhere; a mobilized version of the one in production for desktop firefox would be ideal.
tracking-fennec: --- → ?
Flags: in-litmus?
Attached patch patch (obsolete) — Splinter Review
sub text is now black too. just need the background color/style
Assignee: nobody → mark.finkle
We'll need to have some way to dismiss the row, too, so it's not permanent.

Brainstorm:
- a "done" button next to "Learn More"
- close button that looks like a tab close button at right, learn more moved to left
Do we need to dismiss it? It collapses to a small-ish row. We would remove it when viewing a search result.
Attached patch patch 2Splinter Review
This patch is the same as the previous, with the following additions:
* Strings added to .properties file
* Added support for bug 596594
** Breaks apart the displaySearchResults into displayRecommendedResults and displaySearchResult - instead of trying to make the single function more complex.
** Does two search requests for the recommended list: the recommended items and the top X items sorted by rating

This implements much of the plan for bug 596594. We can tweak the specifics in future bugs.
Attachment #473946 - Attachment is obsolete: true
Attachment #475716 - Flags: review?(21)
Comment on attachment 475716 [details] [diff] [review]
patch 2

>diff --git a/chrome/content/bindings/extensions.xml b/chrome/content/bindings/extensions.xml
>--- a/chrome/content/bindings/extensions.xml
>+++ b/chrome/content/bindings/extensions.xml
>@@ -236,9 +236,30 @@

>+  <binding id="extension-search-banner" extends="chrome://browser/content/bindings.xml#richlistitem">
>+    <content orient="vertical" nohighlight="true">

Nit: I'm not sure why I've missed that last time but "nohighlight" seems a crazy name to me.
I would rather prefer "ignorehighlight"

>+  appendSearchResults: function(aAddons, aShowRating) {

Nit: let's keep the nice debugging name that are everywhere into this file
>+  displayRecommendedResults: function ev_displaySearchResults(aRecommendedAddons, aBrowseAddons) {
>+    this.clearSection("repo");
>+    let whatare = document.createElement("richlistitem");
>+    whatare.setAttribute("typeName", "banner");
>+
>+    whatare.setAttribute("label", strings.getString("addonsWhatAre.label"));

Nit: no need for the middle line break

>+///////////////////////////////////////////////////////////////////////////////
>+// callback for the browse search
>+var BrowseSearchResults = {
>+  cache: null,
>+
>+  searchSucceeded: function(aAddons, aAddonCount, aTotalResults) {
>+    this.cache = aAddons;
>+    ExtensionsView.displayRecommendedResults(RecommendedSearchResults.cache, aAddons);
>   },

What is the "cache" property for?


>diff --git a/themes/core/platform.css b/themes/core/platform.css
>--- a/themes/core/platform.css
>+++ b/themes/core/platform.css
>@@ -545,16 +545,22 @@ richlistitem description.title {
> 
> richlistitem label.normal,
> richlistitem description.normal {
>   color: gray;
>   font-size: 18px !important;
>   white-space: pre-wrap;
> }
> 
>+richlistitem label.normal-black,
>+richlistitem description.normal-black {
>+  font-size: 18px !important;
>+  white-space: pre-wrap;
>+}
>+

The name is a bit misleading since normal-black sounds like this is the normal style + black text but 
in fact this is just normal

Maybe we should replace this by normal and switch "normal" to "normal-gray"?



r+ with nits adressed.
Attachment #475716 - Flags: review?(21) → review+
tracking-fennec: ? → 2.0b2+
Version: 1.9.2 Branch → Trunk
(In reply to comment #6)

> >+  <binding id="extension-search-banner" extends="chrome://browser/content/bindings.xml#richlistitem">
> >+    <content orient="vertical" nohighlight="true">
> 
> Nit: I'm not sure why I've missed that last time but "nohighlight" seems a
> crazy name to me.
> I would rather prefer "ignorehighlight"

We use this in other places so we should file a new bug to change it

> >+  appendSearchResults: function(aAddons, aShowRating) {
> 
> Nit: let's keep the nice debugging name that are everywhere into this file

I'll add the name

> >+  displayRecommendedResults: function ev_displaySearchResults(aRecommendedAddons, aBrowseAddons) {
> >+    this.clearSection("repo");
> >+    let whatare = document.createElement("richlistitem");
> >+    whatare.setAttribute("typeName", "banner");
> >+
> >+    whatare.setAttribute("label", strings.getString("addonsWhatAre.label"));
> 
> Nit: no need for the middle line break

Will remove

> >+var BrowseSearchResults = {
> >+  cache: null,
> >+
> >+  searchSucceeded: function(aAddons, aAddonCount, aTotalResults) {
> >+    this.cache = aAddons;
> 
> What is the "cache" property for?

Nothing. I'll remove it

> >+richlistitem label.normal-black,
> >+richlistitem description.normal-black {

> The name is a bit misleading since normal-black sounds like this is the normal
> style + black text but 
> in fact this is just normal
> 
> Maybe we should replace this by normal and switch "normal" to "normal-gray"?

I'll file a new bug
Whiteboard: [fennec-checkin-postb1]
pushed:
http://hg.mozilla.org/mobile-browser/rev/9bfef4e850e6
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fennec-checkin-postb1]
verified FIXED on builds:
Mozilla/5.0 (Maemo; Linux armv71; rv:2.0b6pre) Gecko/20100930 Namoroka/4.0b7pre Fennec/4.0b1pre

and

Mozilla/5.0 (Android; Linux armv71; rv:2.0b6pre) Gecko/20100930 Namoroka/4.0b7pre Fennec/4.0b1pre


Follow-up bug: https://bugzilla.mozilla.org/show_bug.cgi?id=600862
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
Flags: in-litmus? → in-litmus?(mozaakash)
litmus testcase https://litmus.mozilla.org/show_test.cgi?id=13630 created to regression test this bug
Flags: in-litmus?(mozaakash) → in-litmus+
You need to log in before you can comment on or make changes to this bug.