858056 - Customization areas should only get a visible border when an item is grabbed

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

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

Currently when entering Australis customization mode, all customization areas glow by default. They should only glow when an item starts to be dragged (and only the allowed areas should glow).

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Heh, "all customization areas glow by default" - that was me adding what I considered to be debug code :)

Comment 2

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

Attachment: Patch

The mouseup event handler isn't being hit when the dragdrop event is fired (I'm not sure why), and the mouseup event handler is needed so if the user only clicks (not drags) on the item then the attribute will be removed.

This currently highlights all customization areas. There is another bug on file to get it to only highlight allowed areas, but that would require a little more refactoring.

Comment 3

[Blair McBride [:Unfocused] (UNAVAILABLE)]

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

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

::: browser/components/customizableui/src/CustomizeMode.jsm
@@ +531,5 @@
>  
>    _onDragDrop: function(aEvent) {
>      __dumpDragData(aEvent);
>  
> +    let doc = aEvent.target.ownerDocument;

This function is already getting the ownerDocument a couple of lines below.

@@ +668,5 @@
>    },
>  
>    _onMouseDown: function(aEvent) {
> +    let doc = aEvent.target.ownerDocument;
> +    doc.documentElement.setAttribute("movingItem", "");

Set to "true" - makes it look far more intentional.

Also, not sure about "movingItem" - its a bit too generic. Maybe something like "customizing-movingItem"?

Comment 4

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

Attachment: Patch v2

Blair, this patch is similar to the previous one but I switched to an animated border image, which I think looks better. Does this look OK to you?

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 747745 [details] [diff] [review]
Patch v2

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

The length of the little ants varies by a pixel as they move along - I think that might be some kind of bug in the border-image code :\

r+ on the code, but I want shorlander's input on the visual effect.

Comment 6

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

https://hg.mozilla.org/projects/ux/rev/7543c000a5a3

Landed with part of the patch from bug 866978 since it was more robust at removing the customizing-movingItem attribute.

Comment 7

[Stephen Horlander [:shorlander]]

Probably a follow-up bug, but this only sort of works. Initially it doesn't show until you start dragging, but after you start dragging it shows the areas but doesn't hide them again.

Also just clicking will show the border, you don't even have to drag.

Comment 8

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/7543c000a5a3