Closed Bug 932928 Opened 6 years ago Closed 6 years ago

[Australis] Show something when "More Tools" section is empty in customization mode

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 29

People

(Reporter: zfang, Assigned: jaws)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:P5][strings])

Attachments

(2 files, 1 obsolete file)

In customization mode, we should show something when user moved all the tools and the "more tools" section is empty. The message could be something like "Want more tools? Go and install more Add-ons"
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [Australis:M?] [Australis:P?] → [Australis:M?][Australis:P?]
Version: unspecified → Trunk
Zhenshuo, can you tell us what we should implement here? Preferably the link text (could be the same as you mentioned in comment 0 earlier) and the action of what happens when you click it.

Thanks!
Flags: needinfo?(zfang)
Whiteboard: [Australis:M?][Australis:P?] → [Australis:M?][Australis:P5]
Attached image MoreTools.jpg
OK, here's what I have:
"Want more tools? Choose from thousands of add-ons"
The link opens about:addons in a new tab.
Flags: needinfo?(zfang)
Specifically, open the Disco pane in about:addons:
  BrowserOpenAddonsMgr("addons://discovery/");
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Whiteboard: [Australis:M?][Australis:P5] → [Australis:M?][Australis:P5][strings]
Attached patch Patch (obsolete) — Splinter Review
Attachment #8363059 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8363059 [details] [diff] [review]
Patch

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

f+ but I'm concerned about being able to move items back to the palette if it's empty.

::: browser/components/customizableui/content/customizeMode.inc.xul
@@ +9,5 @@
>      </label>
>      <vbox id="customization-palette" flex="1"/>
> +    <hbox id="customization-empty" hidden="true">
> +      <label>
> +        &customizeMode.menuAndToolbars.empty;

Nit: does this need to have whitespace on either side? I suspect not, but just checking.

::: browser/components/customizableui/src/CustomizeMode.jsm
@@ +273,5 @@
>      let document = this.document;
>      let documentElement = document.documentElement;
>  
>      // Hide the palette before starting the transition for increased perf.
>      this.visiblePalette.hidden = true;

If this is hidden first, won't that destroy the binding on the nodes inside it? I am so paranoid about these XBL bindings these days...

@@ +920,5 @@
>    },
>  
> +  _updateEmptyPaletteNotice: function() {
> +    let paletteEmpty = !this.visiblePalette.childElementCount;
> +    this.visiblePalette.hidden = paletteEmpty;

Why do we need to hide the palette if it's empty? Does it have a minimum size or something? Separately, I'm pretty sure our drag handlers are on that thing, so how can people move items back if the palette's hidden?
Attachment #8363059 - Flags: review?(gijskruitbosch+bugs) → feedback+
(In reply to :Gijs Kruitbosch from comment #5)
> ::: browser/components/customizableui/src/CustomizeMode.jsm
> @@ +273,5 @@
> >      let document = this.document;
> >      let documentElement = document.documentElement;
> >  
> >      // Hide the palette before starting the transition for increased perf.
> >      this.visiblePalette.hidden = true;
> 
> If this is hidden first, won't that destroy the binding on the nodes inside
> it? I am so paranoid about these XBL bindings these days...

Eh, disregard this comment.
Attached patch Patch v2Splinter Review
Attachment #8363059 - Attachment is obsolete: true
Attachment #8363097 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8363097 [details] [diff] [review]
Patch v2

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

r=me
Attachment #8363097 - Flags: review?(gijskruitbosch+bugs) → review+
https://hg.mozilla.org/integration/fx-team/rev/be932ecd964e
Whiteboard: [Australis:M?][Australis:P5][strings] → [Australis:P5][strings]
https://hg.mozilla.org/mozilla-central/rev/be932ecd964e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Depends on: 980090
You need to log in before you can comment on or make changes to this bug.