879981 - Need feedback when an item is not removable from an area

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

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

The URL bar / navigation widget will not be removable by default from the nav-bar. We currently don't do much to demonstrate this to the user, except to snap it back into place after they're done dragging.

We should indicate somehow that it can't be moved.

Comment 1

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

Attachment: Patch

Hm, I don't know if we've figured out what a removable=false UI should look like. I put together a patch that fades and blurs out the customizable regions that are not potential targets for a customizable item.

To test this out, try dragging the location bar and you'll see that the customization palette and the menu panel contents will fade out and blur.

This effect is not applied if you move a removable=true item.

Comment 2

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

Attachment: Screencast of patch

Stephen, what do you think about this effect?

Comment 3

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

Comment on attachment 763323 [details] [diff] [review]
Patch

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

Just a drive-by while we wait on shorlander feedback.

::: browser/components/customizableui/src/CustomizeMode.jsm
@@ +709,5 @@
>      }
>  
> +    document.documentElement.removeAttribute("customizing-movingItem");
> +    let customizationBlockedElements = document.getElementsByClassName(kCustomizationBlocked);
> +    while (customizationBlockedElements.length) {

This block appears several times in this patch, which suggests to me that it'd probably be best to put it into its own function.

::: browser/themes/shared/customizableui/customizeMode.inc.css
@@ +39,5 @@
>    padding-right: 10px;
>  }
>  
> +.customization-blocked {
> +  position: relative;

Nit - put these in alphabetical order.

Comment 4

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

(In reply to Mike Conley (:mconley) from comment #3)
> > +.customization-blocked {
> > +  position: relative;
> 
> Nit - put these in alphabetical order.

Actually that position: relative; can be elided, it's just flotsam.

Comment 5

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

Comment on attachment 763323 [details] [diff] [review]
Patch

According to dolske and shorlander, this isn't the effect we want. Instead, we want to try to "trap" the non-removable item inside its container, much the way we trap tabs inside the tabstrip when dragging them.

Comment 6

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

These bugs didn't make it into the UR Build that went out in bug 879846. Clearing the [User Research Build+] flag.

Comment 7

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

Removing the items from M7 that do not block landing on m-c.

Comment 8

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

Attachment: WIP patch (doesn't work yet)

Checkpointing my work here. The basic idea is to get away from using the dragImage and to make the dragged element position:absolute and change the top/left of the element while dragging. We can then clamp the values of top/left to the customizable area that it is home to if it is removable=false.

Comment 9

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

Right now when one of the palette items is set as position:absolute, it ends up getting rendered as though it is position:static/display:block, creating its own block and moving other palette items to separate rows, also not moving when top/left are adjusted. I need to figure this out before proceeding further with this bug.

Comment 12

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

We've got a tentative plan to grey out non-removable items and not allow dragging of them.

phlsa is going to work on a mock-up.

Comment 13

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Attachment: Mockup

Mockup is attached.
While we are at it, we can also implement Stephens suggestions for shrinking the location bar to make space for new items and that seems to be related.
The opacity of the location bar in the Mockup is 40%.

Comment 14

[Mike de Boer [:mikedeboer]]

After discussing this with Philipp, the spec is formalized a bit more:

Additional space is needed at the start of a non-movable widget when it's the first widget inside the cust. area, to indicate clearly that widgets may be placed before it.
The width of the space needs to be roughly the same as a standard button widgets' width.

Comment 15

[Peter Retzer (:pretzer)]

(In reply to Mike Conley (:mconley) from comment #12)
> We've got a tentative plan to grey out non-removable items and not allow
> dragging of them.

What's the plan for the menu button? The mockup doesn't grey it out, although it's not removable, in fact not even movable.

Comment 16

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

(In reply to Peter Retzer (:pretzer) from comment #15)
> (In reply to Mike Conley (:mconley) from comment #12)
> > We've got a tentative plan to grey out non-removable items and not allow
> > dragging of them.
> 
> What's the plan for the menu button? The mockup doesn't grey it out,
> although it's not removable, in fact not even movable.

The menu button will not be movable, but the mockup probably didn't grey it out because it is shown in the "open" state.

Comment 17

[:Gijs (out for now; he/him)]

(In reply to Philipp Sackl [:phlsa] from comment #13)
> Created attachment 8361108 [details]
> Mockup
> 
> Mockup is attached.
> While we are at it, we can also implement Stephens suggestions for shrinking
> the location bar to make space for new items and that seems to be related.
> The opacity of the location bar in the Mockup is 40%.

I don't know how easy it is to keep the rest of the items right-aligned, but not the location bar, though. Additionally, what happens if the user customizes all the items to be to the left of the location bar?

Comment 18

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

(In reply to :Gijs Kruitbosch from comment #17)
> I don't know how easy it is to keep the rest of the items right-aligned, but
> not the location bar, though.

A temporary spacer placed in the customization-target would get the alignment as requested.

> Additionally, what happens if the user
> customizes all the items to be to the left of the location bar?

If nothing is on the right-side of the location bar, we can place a similar padding (compared to the empty left-side) on the right-side to show that it is possible to place an item to the right of the location bar.

Comment 19

[:Gijs (out for now; he/him)]

(In reply to Jared Wein [:jaws] from comment #18)
> (In reply to :Gijs Kruitbosch from comment #17)
> > I don't know how easy it is to keep the rest of the items right-aligned, but
> > not the location bar, though.
> 
> A temporary spacer placed in the customization-target would get the
> alignment as requested.

That's weird, though, because then dropped items will either hug the location bar or the other items, depending on where they're dropped. I'm not even entirely sure how our DnD code would deal with it.

> > Additionally, what happens if the user
> > customizes all the items to be to the left of the location bar?
> 
> If nothing is on the right-side of the location bar, we can place a similar
> padding (compared to the empty left-side) on the right-side to show that it
> is possible to place an item to the right of the location bar.

I guess I would just much prefer to implement this without the space between the location bar and the buttons. It's a lot fewer headaches, and I don't see a UX benefit to adding the space between the items.

Comment 20

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

(In reply to :Gijs Kruitbosch from comment #19)
> I guess I would just much prefer to implement this without the space between
> the location bar and the buttons. It's a lot fewer headaches, and I don't
> see a UX benefit to adding the space between the items.

That is exactly what we should do at this point. The shifting does have the benefit of making the customizable areas clearer, but that really should be a different bug.

So the actions here should be:
1) Reduce the opacity of the location bar in customization mode
2) Make it non-draggable
3) Don't show a hand cursor when hovering it

Comment 21

[u428464]

Note that the list-all tabs dropdown arrow is also not removable, but it can be moved inside the tabbar.

Comment 22

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

(In reply to Guillaume C. [:ge3k0s] from comment #21)
> Note that the list-all tabs dropdown arrow is also not removable, but it can
> be moved inside the tabbar.

Good point!
Given that the arrow is far less prominent though, I'd say that the current behavior is good enough here (it simply snaps back into place if you drag it outside the tab bar).

Comment 23

[:Gijs (out for now; he/him)]

Attachment: make non-removable items fade out and non-draggable and adjust their cursor for Australis,

This works. I think. Even for the dropmarker in the tabs toolbar.

Comment 24

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

(In reply to :Gijs Kruitbosch from comment #23)
> Created attachment 8371424 [details] [diff] [review]
> make non-removable items fade out and non-draggable and adjust their cursor
> for Australis,
> 
> This works. I think. Even for the dropmarker in the tabs toolbar.

That's good to hear!
As for the drop marker: I think fixing it in place as you suggest is actually the right way to go. In case someone tries to move it, having a clear indication that this is not possible is better than the frustration of only having a few (unindicated) possible drop zones.

Comment 25

[Mike de Boer [:mikedeboer]]

Comment on attachment 8371424 [details] [diff] [review]
make non-removable items fade out and non-draggable and adjust their cursor for Australis,

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

The JS part looks excellent! Only the styling bit needs a little more TLC...

::: browser/base/content/browser.css
@@ +249,5 @@
>    display: none;
>  }
>  
> +toolbarpaletteitem[removable="false"] {
> +  opacity: 0.5;

Why .5? We use .4 for disabled items AFAIK...

@@ +250,5 @@
>  }
>  
> +toolbarpaletteitem[removable="false"] {
> +  opacity: 0.5;
> +  cursor: default !important;

Why the !important here?

Comment 26

[:Gijs (out for now; he/him)]

(In reply to Mike de Boer [:mikedeboer] from comment #25)
> Comment on attachment 8371424 [details] [diff] [review]
> make non-removable items fade out and non-draggable and adjust their cursor
> for Australis,
> 
> Review of attachment 8371424 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The JS part looks excellent! Only the styling bit needs a little more TLC...
> 
> ::: browser/base/content/browser.css
> @@ +249,5 @@
> >    display: none;
> >  }
> >  
> > +toolbarpaletteitem[removable="false"] {
> > +  opacity: 0.5;
> 
> Why .5? We use .4 for disabled items AFAIK...

http://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/browser.css#1206

We use .5 on OSX > 10.6, so either of them doesn't match in all cases.

I don't think the 0.1 difference matters. Precious few people would even be able to eyeball the difference on things like the dropmarker icon.

However, if you prefer, I can make this a combination of ifdefs and media queries so we match Windows and 10.6 when they're used.

As for the cursor styling, because I wanted to avoid it being overridden by more specific styles for items inside the toolbarpaletteitem. However, it seems we don't do that for -moz-grabbing either.

Comment 27

[:Gijs (out for now; he/him)]

Comment on attachment 8371424 [details] [diff] [review]
make non-removable items fade out and non-draggable and adjust their cursor for Australis,

Per discussion on IRC, r=mike with the !important removed.

remote:   https://hg.mozilla.org/integration/fx-team/rev/68778cb67084

Comment 29

[:Gijs (out for now; he/him)]

This broke the skipintoolbarset drag test. I landed a simple test fix:

remote:   https://hg.mozilla.org/integration/fx-team/rev/167f977b7b9c

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/68778cb67084
https://hg.mozilla.org/mozilla-central/rev/167f977b7b9c

Comment 31

[:Gijs (out for now; he/him)]

Attachment: Patch for Aurora

Comment 32

[:Gijs (out for now; he/him)]

Comment on attachment 8372899 [details] [diff] [review]
Patch for Aurora

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: less-than-optimal feedback trying to move non-removable items to different areas
Testing completed (on m-c, etc.): local, now on m-c
Risk to taking this patch (and alternatives if risky): low, mostly CSS changes. This patch effectively restores the behaviour that non-removable items had when customizing pre-Australis
String or IDL/UUID changes made by this patch: none

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8fe51382be60

Comment 34

[Mike Hommey [:glandium]]

This has the unfortunate side effect of making the urlbar not movable at all. Before, you could take it and move it, say on the right-most location. Now, to do the same, you need to move every single other button. Should I file a followup bug?

Comment 35

[:Gijs (out for now; he/him)]

(In reply to Mike Hommey [:glandium] from comment #34)
> This has the unfortunate side effect of making the urlbar not movable at
> all. Before, you could take it and move it, say on the right-most location.
> Now, to do the same, you need to move every single other button. Should I
> file a followup bug?

This was the design decision made here. Short of backing out the entire fix, or going with a completely different fix, I don't see what could be done about it. Pretending you could move it anywhere and then snapping it back to the navbar was also bad UX - that's why we've done what we've done. You could file a followup bug to only allow dragging the item within the confines of the navbar, but that's very hard to fix, and won't be done for 29 (or probably even 30).

Comment 36

[Andrei Vaida [:avaida]]

I was able to confirm the fix for this issue on Windows 7 64-bit [1], Mac OS X 10.9.1 [2] and Ubuntu 13.10 64-bit [3], using:
- the latest Nightly (Build ID: 20140306030201)
- the latest Aurora (Build ID: 20140306004001)

Note: it's also worth mentioning that on Nightly, the Location Bar displays a dashed border at all times and the Title Bar displays it on hover, while Aurora does not display any kind of borders for its elements (all of this happens in customization mode only).

Please let me know if the note above does not depict an intended behavior.

[1] Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:30.0) Gecko/20100101 Firefox/30.0
[2] Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:30.0) Gecko/20100101 Firefox/30.0
[3] Mozilla/5.0 (X11; Linux x86_64; rv:30.0) Gecko/20100101 Firefox/30.0