858660 - Remove "Done" button from customization mode

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

Summary says it all. UX has requested that we try to squeeze this into M2.

Blake - do you think you could take this? The "Done" button is, I believe, within browser/base/content/customization.inc.

Comment 1

[Blake Winton (:bwinton) (:☕️)]

Sure.  I'm poking around in that file already…  :)

Comment 2

[Blake Winton (:bwinton) (:☕️)]

Attachment: A patch to remove the Done button, and move the other button to the left (so that people don't think _it's_ a Done button).

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 733952 [details] [diff] [review]
A patch to remove the Done button, and move the other button to the left (so that people don't think _it's_ a Done button).

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

r=me with the string also removed.

I think moving the Restore Defaults button to the far left is fine for now, but I don't know if it'll be the final resting place...

::: browser/base/content/customize.inc
@@ -33,2 @@
>      <button oncommand="gCustomizeMode.reset();" label="&customizeMode.restoreDefaults;"/>
> -    <button oncommand="BrowserToolboxCustomizeDone();" label="&customizeMode.done;"/>

We should remove the string while we're at it - browser/locale/chrome/browser/browser.dtd.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

I also think this should be blocked by bug 858662 and bug 858597.

Comment 5

[Blake Winton (:bwinton) (:☕️)]

Attachment: The patch, with nits fixed.

I agree, but that's where Madhava asked for it to be moved, at least for now…

Thanks,
Blake.

Comment 6

[Blake Winton (:bwinton) (:☕️)]

Attachment: The _actual_ patch, with nits fixed.

(I'm not very smart sometimes…)

Comment 7

[Blake Winton (:bwinton) (:☕️)]

I agree with it depending on bug 858662, but I think it's important enough to not wait for bug 858597 (which seems harder, and less likely to land in this milestone ;).

(The importance here is that the Done button makes people think that none of their changes will be applied until they hit "Done", which is really not the case.  And there are a bunch of other ways to exit this mode (closing the tab, clicking the blueprint, clicking the hamburger again), so the lack of a Done button should hopefully not be a huge problem.)

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Hmm, I have some concerns about doing this. With about:newaddon, we've found that people don't realize they can/should close the tab to get rid of that UI - I'm worried we'll have the same issue, as this doesn't behave quite like what you'd traditionally have in a tab either.

(In reply to Blake Winton (:bwinton) from comment #7)
> (The importance here is that the Done button makes people think that none of
> their changes will be applied until they hit "Done", which is really not the
> case.

Labeling it "Close" might solve that somewhat.

Comment 9

[Blake Winton (:bwinton) (:☕️)]

(Adding Madhava, so that he can explain why he asked for it in a more eloquent way than I can. ;)

Comment 10

[Blake Winton (:bwinton) (:☕️)]

So, normally I would suggest we remove it for this milestone, and see via Test Pilot whether people get into trouble.  But that's probably not going to work for a variety of reasons, so my alternative plan is this: Remove it for this milestone, and file a bug to re-add it with the text "Close" for the next milestone.

How does that sound to everybody?

Comment 11

[Mike Conley (:mconley) (:⚙️)]

(In reply to Blake Winton (:bwinton) from comment #10)
> So, normally I would suggest we remove it for this milestone, and see via
> Test Pilot whether people get into trouble.  But that's probably not going
> to work for a variety of reasons, so my alternative plan is this: Remove it
> for this milestone, and file a bug to re-add it with the text "Close" for
> the next milestone.
> 
> How does that sound to everybody?

I don't mind that plan.

Comment 12

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Yea ok, as long as there's something on file before this bug lands.

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Rebased and landed on Jamun:
https://hg.mozilla.org/projects/jamun/rev/48b0d65af283

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Landed on UX as https://hg.mozilla.org/projects/ux/rev/48b0d65af283

Comment 15

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/48b0d65af283