494200 - Simplify header of Get-addons section

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Madhava Enros [:madhava]]

If we remove the [ Recommended | Search Catalog ] toggle in the header with just a search field containing grey text ("Search all add-ons"), we can just show 3-5 recommended add-ons in the area below (marked as recommended in the upper right of each row) whenever a search is not being performed.

Comment 1

[Madhava Enros [:madhava]]

Attachment: this is what I mean

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.1

The patch should resolve bug 499843 too

Comment 3

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

Comment on attachment 384616 [details] [diff] [review]
Patch v0.1

>diff -r e96cc7361c5e chrome/content/browser.css
>--- a/chrome/content/browser.css	Fri Jun 19 17:48:13 2009 -0400
>+++ b/chrome/content/browser.css	Tue Jun 23 16:05:41 2009 +0200
>@@ -142,6 +142,17 @@
>   background-color: lightgray;
> }
> 
>+richlistitem.section-recommended {
>+  border-bottom:0px;
>+  border-top:1px solid grey;
>+  min-height:30px;
>+  margin:0px;
>+}

I don't think Madhava wanted a message row for "Recommended". I think he just wanted the results to be displayed.

>+
>+richlistitem[typeName="message"] {
>+  border-bottom:0px;
>+}

Add space after the ":"

>diff -r e96cc7361c5e chrome/content/browser.xul
>               <richlistitem id="addons-repo" class="section-header" orient="vertical" pack="center">
>-                <hbox>
>+                <hbox align="center">
>                   <label value="&addonsRepo.label;" flex="1"/>
>-                  <radiogroup id="addons-repo-mode" oncommand="ExtensionsView.toggleMode();">
>-                    <radio label="&addonsRecommended.label;" value="recommended" selected="true"/>
>-                    <radio label="&addonsSearch.label;" value="search"/>
>-                  </radiogroup>
>-                </hbox>
>-                <hbox id="addons-search-box" pack="end" collapsed="true">
>-                  <textbox id="addons-search-text" emptytext="&addonsSearch.emptytext;" type="search" searchbutton="false"
>-                           oncommand="ExtensionsView.getAddonsFromRepo(this.value);"/>
>+                  <hbox id="addons-search-box" pack="end" collapsed="false">
>+                    <textbox id="addons-search-text" emptytext="&addonsSearch.emptytext;" type="search" searchbutton="false"
>+                             oncommand="ExtensionsView.getAddonsFromRepo(this.value);"/>
>+                  </hbox>
>                 </hbox>

I don't think we need the <hbox> around the <textbox> anymore. The flex on the <label> should push the <textbox> to the right.

>diff -r e96cc7361c5e chrome/content/extensions.js

>+  displayRecommendedHeader: function ev_displayRecommendedHeader() {
>+    let item = document.createElement("richlistitem");
>+    item.setAttribute("class", "section-header section-recommended");
>+
>+    let strings = document.getElementById("bundle_browser");
>+    let label = document.createElement("label");
>+    label.setAttribute("value", strings.getString("addonsSearchRecommended.label"));
>+    item.appendChild(label);
>+
>+    this._list.insertBefore(item, this._repoItem.nextSibling);
>+  },

I don't think we need this function (and the CSS style)

>   displaySearchResults: function ev_displaySearchResults(aAddons, aTotalResults, aIsRecommended) {
>     this.clearSection("repo");
> 
>+    // Add a bannner saying that theses addons are recommended if needed
>+    if (aIsRecommended)
>+      this.displayRecommendedHeader();
>+

No need

>diff -r e96cc7361c5e locales/en-US/chrome/browser.properties
>+addonsSearchRecommended.label=Recommended

No need

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2

Addresses comments.

In a first draft I've try to add a xbl:inherits attribute on extensions.xml to display a 'recommended' header on the top right corner of each row but it seems redundant.

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2

Always the same old wrong options stuff. I have no head...

Comment 6

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

Comment on attachment 384635 [details] [diff] [review]
Patch v0.2

>+              <richlistitem id="addons-repo" class="section-header" orient="horizontal" pack="center">

changed pack="center" to align="center"

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.3

Changed.

Comment 8

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

pushed:
https://hg.mozilla.org/mobile-browser/rev/376af942ad8c

Comment 9

[Dão Gottwald [:dao]]

bugspam