Closed Bug 858056 Opened 8 years ago Closed 7 years ago

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

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: mconley, Assigned: jaws)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:M5])

Attachments

(2 files)

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).
Heh, "all customization areas glow by default" - that was me adding what I considered to be debug code :)
Attached patch PatchSplinter Review
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.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attachment #747570 - Flags: review?(mconley)
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"?
Attachment #747570 - Flags: review?(mconley) → review+
Attached patch Patch v2Splinter Review
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?
Attachment #747745 - Flags: review?(bmcbride)
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.
Attachment #747745 - Flags: review?(shorlander)
Attachment #747745 - Flags: review?(bmcbride)
Attachment #747745 - Flags: review+
No longer blocks: 770135
Whiteboard: [Australis:M5]
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.
Whiteboard: [Australis:M5] → [Australis:M5][fixed-in-ux]
Summary: Customization areas should only glow when an item is grabbed → Customization areas should only get a visible border when an item is grabbed
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.
https://hg.mozilla.org/mozilla-central/rev/7543c000a5a3
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M5][fixed-in-ux] → [Australis:M5]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.