Closed Bug 870593 Opened 7 years ago Closed 6 years ago

Add a "tip section" in Australis Customization Mode

Categories

(Firefox :: Toolbars and Customization, defect)

x86
All
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 30
Tracking Status
firefox29 --- verified
firefox30 --- verified

People

(Reporter: zfang, Assigned: mikedeboer)

References

(Depends on 2 open bugs, Blocks 2 open bugs)

Details

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

Attachments

(11 files, 3 obsolete files)

260.99 KB, image/png
Details
6.71 MB, application/zip
Details
3.34 MB, image/png
Details
286 bytes, image/png
Details
501 bytes, image/png
Details
1.40 KB, patch
mikedeboer
: checkin+
Details | Diff | Splinter Review
1.65 KB, patch
Gijs
: review+
Details | Diff | Splinter Review
1.01 KB, application/zip
Details
41.52 KB, application/zip
Details
52.48 KB, patch
jaws
: review+
Details | Diff | Splinter Review
24.34 KB, application/zip
Details
When open customization page for the firs time, show a tip "drag and drop widgets to customize the menu panel and toolbar". Still need actual copy and visual.
No longer blocks: 770135
Whiteboard: [Australis:M7]
Whiteboard: [Australis:M7] → [Australis:M?]
Whiteboard: [Australis:M?] → [Australis:M?][Australis:P3]
(In reply to Zhenshuo Fang (:fang) - Firefox UX Team from comment #1)
> When open customization page for the firs time, show a tip "drag and drop
> widgets to customize the menu panel and toolbar". Still need actual copy and
> visual.

Is there copy and visual now?
Flags: needinfo?(zfang)
Attached image CustomizationTip1.png
Update on the tip section:

When opening customization for the first time, show a doorhanger panel with tip in side. Close button is on hover, user hover over the doorhanger to close the tip. (I believe the interface is intuitive enough so that we don't have to show a step by step tour or show it every time user opens customization.)

Also, based on some user testing, many users have no problem figuring out how to customize the menu panel but didn't realize they can also drag things to the toolbar, that's why the doorhanger comes down from the toolbar area in the mock up. The downside is the tip pushes the content of the page ("More tools..") down and leaves an empty space after closing it. Another alternative is pointing the tip to the menu panel. 
Alternative: http://people.mozilla.org/~zfang/Customization/CustomizationTip2.png

Shorlander, do you have any other comments? especially the visual style
Flags: needinfo?(zfang) → needinfo?(shorlander)
(In reply to Mike Conley (:mconley) from comment #3)
> Gentle shorlander ping here.

I have some thoughts. Will try and work it into a mockup this week.
Whiteboard: [Australis:M?][Australis:P3] → [Australis:P4]
This attachment contains:

• Design mock

• Design Spec

• Image assets
Flags: needinfo?(shorlander)
Whiteboard: [Australis:P4] → [Australis:P4][strings]
Whiteboard: [Australis:P4][strings] → [Australis:P2][strings]
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Whiteboard: [Australis:P2][strings] → [Australis:P2][strings] [feature] p=0
Assigning to Michael; I provided some feedback to the mockup and he promised to make the necessary adjustments.
Assignee: mdeboer → mmaslaney
Assignee: mmaslaney → mdeboer
I believe we can use this for a link to a support article:
https://support.mozilla.org/1/firefox/%VERSION%/%OS%/%LOCALE%/customize

Ricky can you set that up and point it to https://support.mozilla.org/kb/customize-firefox-controls-buttons-and-toolbars for now?
Flags: needinfo?(rrosario)
Ricky said on irc about 90 min ago, "I just added it but it takes some minutes to take effect due to caching." So you should be good to go with https://support.mozilla.org/1/firefox/%VERSION%/%OS%/%LOCALE%/customize
Flags: needinfo?(rrosario)
(In reply to Verdi [:verdi] from comment #11)
> Ricky said on irc about 90 min ago, "I just added it but it takes some
> minutes to take effect due to caching." So you should be good to go with
> https://support.mozilla.org/1/firefox/%VERSION%/%OS%/%LOCALE%/customize

Yep! It's good to go! \o/
Based on a meeting I had with Madhava today, here's what I came up with for this:

Hint: You can customize Firefox to work the way you do. Simply drag any of the above to the menu or toolbar. <a>Learn more</a> about customizing Firefox.
Getting the strings out of the way first.
Attachment #8367224 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8367224 [details] [diff] [review]
Patch 1: add string bundle to be used for tips in Customize Mode

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

I'd prefer not to add a new file and have to use a new string bundle just because of this. Also, I would have expected we would just insert the markup for this into customizeMode.inc.xul and use browser.dtd . That's what e.g. bug 932928 did. Is there a particular reason not to?
(In reply to :Gijs Kruitbosch from comment #15)
> I'd prefer not to add a new file and have to use a new string bundle just
> because of this. Also, I would have expected we would just insert the markup
> for this into customizeMode.inc.xul and use browser.dtd . That's what e.g.
> bug 932928 did. Is there a particular reason not to?

Reasons:

1) I can place the popup/ panel markup in customizeMode.inc.xul, but I'll have to reparent it on first show to make it appear. So I'm moving towards putting it in panelUI.inc.xul.
2) More importantly, the "Learn more" link is at the start of a sentence, right in the middle of the main tip string. Whether it stays is that position is entirely up to the locale and not something we can rely on. On top of that; string substitution in markup is a challenge, if not impossible.

I can put the strings in browser.properties... how does that sound?
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Mike de Boer [:mikedeboer] from comment #16)
> (In reply to :Gijs Kruitbosch from comment #15)
> > I'd prefer not to add a new file and have to use a new string bundle just
> > because of this. Also, I would have expected we would just insert the markup
> > for this into customizeMode.inc.xul and use browser.dtd . That's what e.g.
> > bug 932928 did. Is there a particular reason not to?
> 
> Reasons:
> 
> 1) I can place the popup/ panel markup in customizeMode.inc.xul, but I'll
> have to reparent it on first show to make it appear. So I'm moving towards
> putting it in panelUI.inc.xul.

Meh, same difference as far as I'm concerned. :-)

However, why does it need reparenting? It shouldn't need to be an actual panel, AIUI, just needs to be overlaid on top of the customization stuff.

> 2) More importantly, the "Learn more" link is at the start of a sentence,
> right in the middle of the main tip string. Whether it stays is that
> position is entirely up to the locale and not something we can rely on. On
> top of that; string substitution in markup is a challenge, if not impossible.

Typically this is done by having something like:

<label value="&fooLabel.first;"/><label class="text-link" href="whatever" value="&fooLabel.learnMoreLink;"/><label value="&fooLabel.second;"/>

But I don't care too much. browser.properties WFM.
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8367224 - Attachment is obsolete: true
Attachment #8367224 - Flags: review?(gijskruitbosch+bugs)
Attachment #8367286 - Flags: review?(gijskruitbosch+bugs)
Attachment #8367286 - Flags: review?(gijskruitbosch+bugs) → review+
so sorry, I had to update the string once more :/
Attachment #8367286 - Attachment is obsolete: true
Attachment #8367300 - Flags: review?(gijskruitbosch+bugs)
Attachment #8367300 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8367300 [details] [diff] [review]
Patch 1.2: add locale strings to be used for tips in Customize Mode

remote: https://hg.mozilla.org/integration/fx-team/rev/d0b34b1a097e
Attachment #8367300 - Flags: checkin+
Whiteboard: [Australis:P2][strings] [feature] p=0 → [Australis:P2][strings] [feature] p=0 [leave-open]
Comment on attachment 8367300 [details] [diff] [review]
Patch 1.2: add locale strings to be used for tips in Customize Mode

>diff --git a/browser/locales/en-US/chrome/browser/browser.properties b/browser/locales/en-US/chrome/browser/browser.properties

>+tipSection.tip0 = %1$S: You can customize Firefox to work the way you do. Simply drag any of the above to the menu or toolbar. %2$S about customizing Firefox.

Shouldn't this use brandShortName?
Comment on attachment 8367300 [details] [diff] [review]
Patch 1.2: add locale strings to be used for tips in Customize Mode

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

Yes, yes it should.
Attachment #8367300 - Flags: review+
Attachment #8367405 - Flags: review?(gijskruitbosch+bugs)
Attachment #8367405 - Flags: review?(gijskruitbosch+bugs) → review+
Blocks: 966167
Filed bug 966167 to fix the string change. I see this bug is still open, not sure if you want to use this one directly.
No longer blocks: 966167
Depends on: 966167
No longer blocks: fxdesktopbacklog
Whiteboard: [Australis:P2][strings] [feature] p=0 [leave-open] → [Australis:P2][strings] [leave-open]
Michael, I need two more assets, I'm afraid; on OSX we use PNGs for the panel arrow.

When you look at the directory listing at https://hg.mozilla.org/integration/fx-team/file/dde12ccefb19/toolkit/themes/osx/global/arrow, you'll see panelarrow-horizontal.png, panelarrow-horizontal@2x.png, panelarrow-vertical.png and panelarrow-vertical@2x.png. For those I'd like to have their equivalents in blue.

Thanks!
Flags: needinfo?(mmaslaney)
Attached file panelarrow.zip
These should do the job.
Flags: needinfo?(mmaslaney)
Jared, this all mostly does the right thing with two exceptions:

1) the panel opens on the wrong side for me on Windows, on the right side on OSX :/
2) the button with the text 'Learn more' has a disturbing margin at the left and right side, which DOMi can't explain to me. I'm hoping you could...?
Attachment #8377634 - Flags: feedback?(jaws)
Comment on attachment 8377634 [details] [diff] [review]
Patch 3: tip section in Customize Mode.

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

I'm not sure why the panel is showing up on the wrong side on Windows. I attached a debugger and the values coming in look correct to me. Maybe Enn will have a better idea here.

::: browser/components/customizableui/src/CustomizeMode.jsm
@@ +523,5 @@
> +      messageNode.innerHTML = bundle.getFormattedString("customizeTips.tip0", [
> +        "<label class=\"customization-tipPanel-em\" value=\"" +
> +          bundle.getString("customizeTips.tip0.hint") + "\"/>",
> +        this.document.getElementById("bundle_brand").getString("brandShortName"),
> +        "<button class=\"" + kButtonClass + "\" label=\"" +

You should use a <label class="text-link" style="margin:0; border-width: 0;" value="Learn more"/> to fix the internal padding on the button.
Attachment #8377634 - Flags: feedback?(jaws)
Attachment #8377634 - Flags: feedback?(enndeakin)
Attachment #8377634 - Flags: feedback+
Comment on attachment 8377634 [details] [diff] [review]
Patch 3: tip section in Customize Mode.

>+<panel id="customization-tipPanel"
>+       type="arrow"
>+       flip="slide"
>+       noautohide="true">

Try adding side="left" (or right) here.
Attachment #8377634 - Flags: feedback?(enndeakin)
(In reply to Neil Deakin from comment #32)
> Try adding side="left" (or right) here.

That didn't work, unfortunately.
Mike, can we replace the customize images with the attached? The updated focus more on "feature location" rather than "user-action", which I feel better illustrates the capabilities of the customize-mode.

The onboarding tour will feature the same illustration in the "Customize" step.
Sure! Newer assets are no-problemo, the implementation is the trouble maker for this one!
> > Try adding side="left" (or right) here.
> 
> That didn't work, unfortunately.

That did work when I tried it back last week.
Attachment #8377634 - Attachment is obsolete: true
Comment on attachment 8386278 [details] [diff] [review]
Patch 3.1: tip section in Customize Mode.

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

I talked with mmaslaney and he said that we should switch back to the image with the arrows showing that you can move items from the palette to the panel or the toolbar.

r=me with that change and the nits below fixed.

::: browser/components/customizableui/src/CustomizeMode.jsm
@@ +545,5 @@
> +    try {
> +      shown = Services.prefs.getBoolPref(kShownPref);
> +    } catch (ex) {}
> +    // if (shown)
> +    //   return;

you can delete this dead code

@@ +549,5 @@
> +    //   return;
> +
> +    let anchorNode = aAnchor || this.document.getElementById("customization-panelHolder");
> +    let messageNode = this.tipPanel.querySelector(".customization-tipPanel-contentMessage");
> +    if (!messageNode.childNodes.length) {

s/messageNode.childNodes.length/messageNode.childElementCount/

::: browser/themes/shared/customizableui/customizeTip.inc.css
@@ +8,5 @@
> +  min-width: 400px;
> +  max-width: 1000px;
> +  min-height: 200px;
> +  border-radius: 3px;
> +  background-image: linear-gradient(90deg,#a0dfff 0%,#ceeeff 100%);

nit, spaces after commas in the linear-gradient function. only do no spaces after commas within a color macro.

@@ +41,5 @@
> +  list-style-image: url(chrome://browser/skin/customizableui/customize-illustration.png);
> +  min-width: 298px;
> +  max-width: 298px;
> +  min-height: 189px;
> +  max-height: 189px;

can we just use width and height here?

@@ +60,5 @@
> +  margin: 0 !important;
> +}
> +
> +.customization-tipPanel-link:hover {
> +  text-decoration: underline;

i don't think this is necessary with class="text-link" on the label.
Attachment #8386278 - Flags: review?(jaws) → review+
Landed as:

https://hg.mozilla.org/integration/fx-team/rev/37308abb18a8
Whiteboard: [Australis:P2][strings] [leave-open] → [Australis:P2][strings][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/37308abb18a8
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P2][strings][fixed-in-fx-team] → [Australis:P2][strings]
Target Milestone: --- → Firefox 30
Depends on: 980373
Depends on: 980433
(In reply to mmaslaney from comment #41)
> Created attachment 8386847 [details]
> customize_illustration.zip

Hi Michael, we probably need an image for right-to-left locales. I think we can just flip the current picture horizontally, also the icons and check marks in the menu and "more tools" section. The URL bar looks right after flipped.
Depends on: 936265
posted the right-to-left illustration in Bug 980369
Comment on attachment 8386278 [details] [diff] [review]
Patch 3.1: tip section in Customize Mode.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a; new feature
User impact if declined: Australis onboarding tip panel will not be shown when a user enters customize mode for the first time.
Testing completed (on m-c, etc.): merged to m-c
Risk to taking this patch (and alternatives if risky): minor
String or IDL/UUID changes made by this patch: n/a
Attachment #8386278 - Flags: approval-mozilla-aurora?
Attachment #8386278 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Depends on: 980369
Landed in one combined patch on aurora, together with bug 870593 and bug 980369:

https://hg.mozilla.org/releases/mozilla-aurora/rev/92c7529e9169
Keywords: verifyme
Status: RESOLVED → VERIFIED
Depends on: 1018159
Blocks: 1017202
You need to log in before you can comment on or make changes to this bug.