Closed Bug 518468 Opened 11 years ago Closed 11 years ago

no easy way to undo after installation of a lightweight theme

Categories

(Firefox :: General, defect)

3.6 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 3.7a1
Tracking Status
status1.9.2 --- beta2-fixed

People

(Reporter: jane, Assigned: dao)

References

Details

(Keywords: late-l10n, verified1.9.2)

Attachments

(1 file, 1 obsolete file)

It came to my attention that whilst we are making it very easy for our users to enjoy the personas experience, and to easily install (without a restart etc), that this may lead to problems with users who try out the Persona feature for fun, and are left wondering how to un-install the theme they have just added. 

An example: I install the 3.6 update and on the What's new page I see personas featured. I hover over one, and see what it does and then I decide to click and have it installed. Its a Chicago Bears persona, and I just wanted to try out what it was like, but am not a bears fan. I've already clicked away from the get started page, now it is not clear / easy for me to understand where I should go to immediately to revert my browser back to what it was / or change to another personas. (e.g. Tools, Addons, Themes and so on).

It would be good talk about how best to resolve or communicate this out to users.
Three strategies come immediately to mind:

1. An information bar with an Undo option which switches back to the default theme.

2. The first time a Persona is installed, we open the EM with the Themes pane focused, which shows the button for switching back.

3. When a Persona is applied, decorate the status bar with an icon; clicking on the icon removes the persona (this is by far my least favourite option)

The Personas add-on currently provides no real support for this issue, either, though because the little switcher UI is always in the corner, it's a touch more evident.
Summary: Personas - "undo" experience UI suggestion → no easy way to undo after installation of a lightweight theme
FWIW, for the add-on we're looking at implementing #1 for the first time a specific theme is selected through a DOM event from web event, and likely with a checkbox to subsequently suppress. It doesn't feel like this is as much of an issue when selecting a persona from the primary UI.
Thanks for filing, Jane. Other options that come to mind:

 - A (Tools?) menu item called "reset appearance" that, well, you know... (requires strings)
 - A smarter web gallery experience that, for the first couple personas a person installs, poofs up a prompt along the lines of "Want to manage your personas?" with a link to a guide.  "User education" typically sucks as a solution to incomplete UI, but in this case it could be a piece of the puzzle.
OS: Mac OS X → All
Hardware: x86 → All
(In reply to comment #1)
> Three strategies come immediately to mind:
> 
> 1. An information bar with an Undo option which switches back to the default
> theme.

I prefer this option though I might suggest two buttons, one to undo and one to "manage themes" that opens the add-ons manager making it more obvious where users can find them in the future.
(In reply to comment #4)

> I prefer this option though I might suggest two buttons, one to undo and one to
> "manage themes" that opens the add-ons manager making it more obvious where
> users can find them in the future.

In a world without string freezes, I'm pretty sure I agree that this is the option I would choose. It might be a touch annoying to have this thing descending all the time though, while you're clicking around the gallery, though? Maybe not - the undo is a nice option to have every single time.

cc'ng l10n: Nobody's breaking string freeze at the moment, but there is a discussion of options and that's one of them; I want to make sure no one is surprised by that.
(In reply to comment #1)

> The Personas add-on currently provides no real support for this issue, either,
> though because the little switcher UI is always in the corner, it's a touch
> more evident.

Yup, no way to undo is not good, nor did I see the corner ui either.  I think this is an example where it doesn't make sense as a corner ui icon down there because there is no clear indication after installing Personas that its down there or something I should click on to find out, but its in tools instead.  Where it either looks like part of a theme or its just invisible with dark themes.  In contrast, Paddlelock does currently make sense because we've seen it for ages before themes.

(In reply to comment #4)

Using the Personas Add-on on top of having lightweight themes installed, seems like a UX issue where the user must know the difference between the two.  So I think the "lightweight theme was installed" notification bar dual button suggestion seems like a good option.  

(In reply to comment #5)
 
If I remember correctly (and correct me if I'm wrong here) I believe with the recent changes to the notification bar event handling, it should stay in place using the most current event (and no longer have all events thrown) if the user doesn't click on it but installed theme after theme, not just previewed them.  They would rollup if so, and the user would only get to undo once.  If lightweight themes allow previews and installation like Personas, then ideally it should not or wouldn't drop on a hover preview, but after its installed, unless it was the UI to also install or apply theme.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #404418 - Flags: ui-review?(beltzner)
Attachment #404418 - Flags: review?(dtownsend)
Flags: wanted-firefox3.6+
Keywords: late-l10n
FWIW, I don't think that wanted-firefox36 for a string-impact change is OK. It's either blocking or off, IMHO.
The patch looks good, however I wonder if for this particular case we should be switching back to the previous lightweight theme (if there was one) if the user uninstalls from the notification bar. While I don't think we should remember that long term this is a special case and I think it's a bit odd that we don't go back to the way the browser looked before here. What do you think Dão?
The downside of this would be that if you tested x themes and undid the last selection, you would get x-1 with no easy way to undo (i.e. this bug). I imagine this could happen quite easily on the first-run page.
There has been some discussion whether "undo" is the best way to describe the behavior of returning to default. The sentiment is that it is not. Two options are:

1. keep the word "undo" and set the action to (x-1) or your previous design, whether that be default or a different persona. 

2. change the word "undo" to something more descriptive (eg, "back to original") and set the action to be default. 

Option 2 will help the greatest number of people because it primarily supports the non-personas user discovering personas for the first time, whereas option 1 which primarily supports existing personas users that click on an image on the first-run page. 

FWIW, I *recommend option 2* because: 1) it supports the greatest number of people (millions of non-personas users that update to 3.6), and 2) option 1 has a workaround for existing personas users, which is to go to "recently selected" in the switcher.
(In reply to comment #11)
> Option 2 will help the greatest number of people because it primarily supports
> the non-personas user discovering personas for the first time, whereas option 1
> which primarily supports existing personas users that click on an image on the
> first-run page. 

In the case of a first-time user, though, going to X-1 would take them back to their previous setting, which was "no persona, default theme." So Option 1 seems to primarily support the non-personas user discovering them for the first time.

> FWIW, I *recommend option 2* because: 1) it supports the greatest number of
> people (millions of non-personas users that update to 3.6), and 2) option 1 has
> a workaround for existing personas users, which is to go to "recently selected"
> in the switcher.

This makes me think you're misinterpreting what the "Undo" option will do; it doesn't switch back to your previously selected persona. It switches back to your previously selected theme, which can be:

 - a theme (such as the default theme)
 - a persona (the last one you decided to wear)
(In reply to comment #11)
> 2. change the word "undo" to something more descriptive (eg, "back to
> original") and set the action to be default. 

The current patch uses "Remove Theme".

(In reply to comment #12)
> In the case of a first-time user, though, going to X-1 would take them back to
> their previous setting, which was "no persona, default theme." So Option 1
> seems to primarily support the non-personas user discovering them for the first
> time.

Except if the user clicked on two themes...
(In reply to comment #10)
> The downside of this would be that if you tested x themes and undid the last
> selection, you would get x-1 with no easy way to undo (i.e. this bug). I
> imagine this could happen quite easily on the first-run page.

Nick Ngyuen was saying that what will likely happen on the first-run page is that after a single click, we'll install the Persona and also take the user to a web page that explains what happened, how they can manage them (hopefully with screenshots of the Add-ons manager) etc.
Comment on attachment 404418 [details] [diff] [review]
patch

Some string changes ...

>+lwthemePostInstallNotification.message=A new theme has been selected.

A new theme has been installed.

>+lwthemePostInstallNotification.removeButton=Remove Theme

Undo

>+lwthemePostInstallNotification.removeButton.accesskey=R

U

>+lwthemePostInstallNotification.manageButton=Manage Themes

Manage Themes... <-- use the proper ellipsis character, though

>+lwthemePostInstallNotification.manageButton.accesskey=M

thanks, uir+ with those changes
Attachment #404418 - Flags: ui-review?(beltzner) → ui-review+
Attachment #404418 - Flags: review?(dtownsend)
flagging for litmus testcase when patch is ready.
Flags: in-litmus?
Attached patch patch v2Splinter Review
Attachment #404418 - Attachment is obsolete: true
Attachment #405107 - Flags: review?(dtownsend)
Comment on attachment 405107 [details] [diff] [review]
patch v2

A couple of comments but this is fine. I presume we'll need additional approval to land this on branch since it changes strings.

>+  _postInstallNotification: function (newTheme, previousTheme) {
>+    function text(id) {
>+      return gNavigatorBundle.getString("lwthemePostInstallNotification." + id);
>+    }

Might just be worth pulling this function out into the main scope and using it in the other place this module pulls strings.

>+    var buttons = [

Just a nit, but elsewhere in browser we seem to prefer the style:

var buttons = [{
  ...
}, {
  ...
}];
Attachment #405107 - Flags: review?(dtownsend) → review+
(In reply to comment #18)
> (From update of attachment 405107 [details] [diff] [review])
> A couple of comments but this is fine. I presume we'll need additional approval
> to land this on branch since it changes strings.
> 
> >+  _postInstallNotification: function (newTheme, previousTheme) {
> >+    function text(id) {
> >+      return gNavigatorBundle.getString("lwthemePostInstallNotification." + id);
> >+    }
> 
> Might just be worth pulling this function out into the main scope and using it
> in the other place this module pulls strings.

The other place doesn't use the "lwthemePostInstallNotification." prefix, so that function wouldn't be as handy as it is now. One of the other three strings also needs getFormattedString rather than getString.
(In reply to comment #19)
> (In reply to comment #18)
> > (From update of attachment 405107 [details] [diff] [review] [details])
> > A couple of comments but this is fine. I presume we'll need additional approval
> > to land this on branch since it changes strings.
> > 
> > >+  _postInstallNotification: function (newTheme, previousTheme) {
> > >+    function text(id) {
> > >+      return gNavigatorBundle.getString("lwthemePostInstallNotification." + id);
> > >+    }
> > 
> > Might just be worth pulling this function out into the main scope and using it
> > in the other place this module pulls strings.
> 
> The other place doesn't use the "lwthemePostInstallNotification." prefix, so
> that function wouldn't be as handy as it is now. One of the other three strings
> also needs getFormattedString rather than getString.

Fair enough, as it is is fine then.
http://hg.mozilla.org/mozilla-central/rev/ce326f411aed
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
Attachment #405107 - Flags: approval1.9.2?
Verified fixed with Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091008 Minefield/3.7a1pre (.NET CLR 3.5.30729)
(In reply to comment #8)
> FWIW, I don't think that wanted-firefox36 for a string-impact change is OK.
> It's either blocking or off, IMHO.

I hear that. I think we will make that determination once we have some beta feedback. It certainly won't land for the beta (perhaps obviously).
Also verified on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091008 Minefield/3.7a1pre.
Status: RESOLVED → VERIFIED
Axel said on today's engineering call that we are clear to land this on branch, if it lands this week. Dao or Dave, please make it so.
Attachment #405107 - Flags: approval1.9.2? → approval1.9.2+
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b2pre) Gecko/20091021 Namoroka/3.6b2pre
and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b2pre) Gecko/20091021 Namoroka/3.6b2pre
Keywords: verified1.9.2
Duplicate of this bug: 523744
You need to log in before you can comment on or make changes to this bug.