Closed Bug 1053530 Opened 5 years ago Closed 5 years ago

Enhanced Tiles intro text popup

Categories

(Firefox :: New Tab Page, defect)

defect
Not set
Points:
5

Tracking

()

VERIFIED FIXED
Firefox 34
Iteration:
34.3
Tracking Status
firefox33 + verified
firefox34 + verified

People

(Reporter: clarkbw, Assigned: Mardak)

References

Details

Attachments

(5 files, 6 obsolete files)

Attached image What is this page.png
(this is a late addition late-l10n need)

As part of the first run for users updating to the new Enhanced Tiles we want to give a short intro text to explain what Enhanced Tiles is and inform them how they can control it.

Here's the acceptance criteria:
1. A link at the footer of the page with the text "What's this page?"
2. A doorhanger popup that originates from the link with text similar to what is seen in the mockup (exact text to follow in this bug)
3. The doorhanger should be opened the first time a user encounters the new tab page with the Enhanced Tiles pref'd on.

There are a number of best practice questions we have for the l10n community regarding the mockup as shown.

How to best split up the text?
Are there examples of something like the gear icon integrating?  We are short on time so we are considering dropping the icon integration for now but would appreciate guidance on it for the future.
From some discussion in #l10n, it sounds like one option is to have 3 string parts for paragraphs that have a link: before-link, link, after-link. And 2 string parts for an image: before-image, after-image. Where it could be possible that after-link or after-image is "" for en-US.
I'm scared by the late-l10n part of comment 0. My understanding is that we're going to display this only once, and that's going to be in English for most of our locales given the timing of the request. And that's a huge and scary chunk of text for people who don't understand English. Are you comfortable with it? Personally I'm not.

To answer your technical questions.

We have icons like this in devtools (Network panel), you need two strings (before, after) and a clear comment explaining what's going to happen.

For the links part (but also for the icon), you can either use:
* 3 strings: before-link, link text, after-link
* 2 strings: full sentence with a placeholder for the link, link text (https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_best_practices#Avoid_concatenations.2C_use_placeholders_instead)

Personally I find the second clearer.
(In reply to Francesco Lodolo [:flod] from comment #2)
> And that's a huge and scary chunk of text for people who don't understand English.
The text doesn't need to be shown for non en-US users who wouldn't see the Directory Tiles content anyway. If we did show the message, it could be even more confusing as there's no "Sponsored" content.

This would then be similar to bug 1040369 where the [SPONSORED] indicator happens to only show for en-US users because there's only en-US sponsored content.

(In reply to Francesco Lodolo [:flod] from comment #2)
> we're going to display this only once
I believe comment 0 was referring to automatically showing it once, but the text/link will be clickable in later opened newtabs as well.
Flags: firefox-backlog+
(In reply to Bryan Clark (Firefox PM) [:clarkbw] from comment #0)
> Created attachment 8472712 [details]
> What is this page.png
This mockup only shows 2 rows of tiles and 3 rows barely fit on a 900px height screen given the new spacing between tiles. Adding in the "What is this page" text at the bottom will most likely allow only 2 rows ever to be shown.

1366x768	19.27%
1024x768	11.56%
[1920x1080	6.90%]
1280x800	5.95%
[768x1024	5.62%]
[1280x1024	5.04%]
1440x900	4.65%
1600x900	4.43%
http://www.w3counter.com/globalstats.php

Is it intended to only be able to show 2 rows for roughly half of our users?

Although I believe most of those users should be able to fit 4 columns, so people would go from 9 tiles to 8.

Alternatively, should the link be moved next to the gear icon to save some vertical space? I'm not sure if this would be confusing but the first run experience could even have the doorhanger appear from the gear icon. Then later invocations could be done through a menu item within the gear's menu?
Flags: needinfo?(athornburgh)
Notes: we'll want to only display enhanced tiles messaging and enhanced related stuff (menu item) for en-US when uplifting to aurora because there's only going to be en-US content and not require localizers to get strings ready for beta.
(In reply to Ed Lee :Mardak from comment #4)
> have the doorhanger appear from the gear icon
This sounds like it's good to do for first run.

> Then later invocations could be done through a menu item within the gear's menu?
This is less clear. dcrobot, would it be okay to have a menu item be "what's this page?" that when clicked, closes the existing menu list and opens up a panel? (As opposed to a panel pointing to the menu item that's in a menu pointing to the gear.)
That sounds good to me, Ed. Please implement and we'll work on a better solution for next round.
Flags: needinfo?(athornburgh)
clarkbw, what page should things link to? I see https://wiki.mozilla.org/Tiles but that's not as localizable. We could use that for now, but in the past we had a link to SUMO: https://support.mozilla.org/kb/how-do-sponsored-tiles-work
Blocks: 1042214
Flags: qe-verify?
(In reply to Ed Lee :Mardak from comment #8)
> clarkbw, what page should things link to? I see
> https://wiki.mozilla.org/Tiles but that's not as localizable. We could use
> that for now, but in the past we had a link to SUMO:
> https://support.mozilla.org/kb/how-do-sponsored-tiles-work

It is planned to be a www.mozilla.org link, the wiki link is just temporary.
Blocks: 1056279
Attached image v1 screenshot (obsolete) —
(In reply to Bryan Clark (Firefox PM) [:clarkbw] from comment #9)
> (In reply to Ed Lee :Mardak from comment #8)
> > clarkbw, what page should things link to? I see
> > https://wiki.mozilla.org/Tiles but that's not as localizable. We could use
> > that for now, but in the past we had a link to SUMO:
> > https://support.mozilla.org/kb/how-do-sponsored-tiles-work
> It is planned to be a www.mozilla.org link, the wiki link is just temporary.
If it's temporary, should we just point it to the SUMO article? (It at least has some info.)

Also, what's the privacy notice link? I'm guessing it'll be something like:
https://www.mozilla.org/privacy/firefox/#tiles
Flags: needinfo?(clarkbw)
Please plan ahead for way much longer text, especially for the last item.

The current menu enlarge to fit the text (from a quick test with devtools), not sure if it's possible to wrap it after a certain limit and display the text on two lines.
Attached patch v1 (obsolete) — Splinter Review
Working on tests right now. Seems like newtab passes even with the panel showing, but we'll probably want to set introShown true globally.
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #8476903 - Flags: review?(adw)
Attached image v1 screenshot
Attachment #8476866 - Attachment is obsolete: true
Looks good to me, Ed. Thanks for showing.
(In reply to Ed Lee :Mardak from comment #11)
> (In reply to Bryan Clark (Firefox PM) [:clarkbw] from comment #9)
> > (In reply to Ed Lee :Mardak from comment #8)
> > > clarkbw, what page should things link to? I see
> > > https://wiki.mozilla.org/Tiles but that's not as localizable. We could use
> > > that for now, but in the past we had a link to SUMO:
> > > https://support.mozilla.org/kb/how-do-sponsored-tiles-work
> > It is planned to be a www.mozilla.org link, the wiki link is just temporary.
> If it's temporary, should we just point it to the SUMO article? (It at least
> has some info.)
> 
> Also, what's the privacy notice link? I'm guessing it'll be something like:
> https://www.mozilla.org/privacy/firefox/#tiles

https://www.mozilla.org/en-US/privacy/
Flags: needinfo?(clarkbw)
Can we adjust the text styles? Here's what I'd like to propose...

Headline ("What is this page?"
Open Sans, Light, 30 px, HEX #343F48

Text
Mac = Lucida Grande, Regular, 15 px, HEX #6A7B86, line height = 20 px
PC = Segoe UI, Normal, 15 px, HEX #6A7B86, line height = 20 px
Use that general link.  But you are correct that the privacy notice will appear here: https://www.mozilla.org/privacy/firefox/
Also, text links should be HEX# 4A90E2
Attached patch v2 (obsolete) — Splinter Review
Attachment #8476903 - Attachment is obsolete: true
Attachment #8476903 - Flags: review?(adw)
Attachment #8477039 - Flags: review?(adw)
Attached patch aurora delta (obsolete) — Splinter Review
This patch is on top of the one landing in m-c, but when landing this patch for aurora, it'll be squashed into the same commit so there's no churn of locales files.
Points: --- → 5
Attached patch aurora delta (obsolete) — Splinter Review
Attachment #8477111 - Attachment is obsolete: true
Attached patch aurora delta (obsolete) — Splinter Review
Attachment #8477154 - Attachment is obsolete: true
Attached patch v3Splinter Review
Fixed test. The original one was only passing locally and on try linux.
Attachment #8477039 - Attachment is obsolete: true
Attachment #8477039 - Flags: review?(adw)
Attachment #8477483 - Flags: review?(adw)
Blocks: 1057602
Comment on attachment 8477483 [details] [diff] [review]
v3

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

::: browser/base/content/newtab/intro.js
@@ +33,5 @@
> +  showPanel: function() {
> +    // Open the customize menu first
> +    gCustomize.showPanel().then(nodes => {
> +      // Point the panel at the 'what' menu item
> +      this._nodes.panel.openPopup(nodes.what);

Interesting.

@@ +37,5 @@
> +      this._nodes.panel.openPopup(nodes.what);
> +    });
> +  },
> +
> +  _setUpPanel: function() {

Why not include the markup in the XUL?  Why insert it dynamically like this?  Guess I could have asked the same thing in the other bug with the sponsored explanation overlays.

::: browser/base/content/newtab/newTab.xul
@@ +21,5 @@
>              title="&newtab.pageTitle;">
>  
> +  <xul:panel id="newtab-intro-panel" orient="vertical" type="arrow"
> +             noautohide="true" position="leftcenter topright">
> +    <h1>&newtab.intro.header;</h1>

Hmm, an h1 inside of a XUL panel seems a little weird, but if it works, it works.

::: browser/base/content/newtab/page.js
@@ +215,5 @@
>  
>      DirectoryLinksProvider.reportSitesAction(gGrid.sites, "view", lastIndex);
> +
> +    // Show the panel now that anchors are sized
> +    gIntro.showIfNecessary();

One of the points of the preloader was to avoid jarring changes to the page when you open it.  In that light, how's the UX with this -- opening both the customization and intro panels when the page becomes visible?  (I haven't tested.)

::: browser/base/content/test/newtab/browser_newtab_intro.js
@@ +33,5 @@
> +  is(panel.state, "closed", "intro not shown on second opening");
> +
> +  // Test with preload true
> +  Services.prefs.setBoolPref(INTRO_PREF, false);
> +  Services.prefs.setBoolPref(PRELOAD_PREF, true);

For future-proofing and general soundness, should probably clear these prefs at the end of the test, even though the preload pref is true by default.  Or would clearing the intro pref undo the testing/profiles/prefs_general.js change?  If so, then please clear only the preload pref.
Attachment #8477483 - Flags: review?(adw) → review+
Attached video v3 screencast
(In reply to Drew Willcoxon :adw from comment #25)
> > +  _setUpPanel: function() {
> Why not include the markup in the XUL?
Partially it's to defer loading, but yes arguably the purpose of preloading is to load it earlier. ;) In the common case, this panel is not being shown except for the very first opening. Also, there are strings that need to be dynamically created for localization purposes. Inserting links and images in a localizable string using the (text text %S more text as opposed to &textbefore; <insert> &textafter;).

> One of the points of the preloader was to avoid jarring changes to the page
> when you open it.  In that light, how's the UX with this -- opening both the
> customization and intro panels when the page becomes visible?  (I haven't
> tested.)
See attached 1sec wemb video. This is from a debug build.

> ::: browser/base/content/test/newtab/browser_newtab_intro.js
> > +  Services.prefs.setBoolPref(INTRO_PREF, false);
> > +  Services.prefs.setBoolPref(PRELOAD_PREF, true);
> For future-proofing and general soundness, should probably clear these prefs
> at the end of the test, even though the preload pref is true by default.
I'll make it reset to what it was originally.
Attached patch aurora deltaSplinter Review
Here's a patch on top of what landed in m-c but to make it string-freeze-friendly for aurora uplift.
Attachment #8477155 - Attachment is obsolete: true
Attachment #8477760 - Flags: review?(adw)
Tracking because enhanced tile is a new feature.
https://hg.mozilla.org/mozilla-central/rev/f1b80fce0e7a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Comment on attachment 8477760 [details] [diff] [review]
aurora delta

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

Thanks for posting these deltas.
Attachment #8477760 - Flags: review?(adw) → review+
MarcoM is this in the current iteration?
Flags: qe-verify?
Flags: qe-verify+
Flags: needinfo?(mmucci)
QA Contact: cornel.ionce
Iteration: --- → 34.3
Flags: needinfo?(mmucci)
Verified fixed on Windows 7 64bit, Mac OS X 10.8.5, Ubuntu 14.04 32bit and Windows 7 64bit using:
1. latest Nightly, build ID: 20140828030205
2. latest Aurora, build ID: 20140829004003

User Agents:
1. Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0
   Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:34.0) Gecko/20100101 Firefox/34.0
   Mozilla/5.0 (X11; Linux i686; rv:34.0) Gecko/20100101 Firefox/34.0

2. Mozilla/5.0 (Windows NT 6.1; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0
   Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:33.0) Gecko/20100101 Firefox/33.0
   Mozilla/5.0 (X11; Linux i686; rv:33.0) Gecko/20100101 Firefox/33.0
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.