1396423 - Should be able to drop items anywhere in toolbox area (including underneath the panel) in customize mode

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect, P1)

Description

[Alice0775 White]

[Tracking Requested - why for this release]:No good photon UI

Reproducible: always

Steps To Reproduce:
1. Enter customize mode
2. Attempt to remove a toolbutton with drag & drop

Actual Results:
I often fail.
Because Pallet area(drop-able zone) is not obvious. See attached screenshot.

Expected Results:
The pallet area should be obvious.
It needs border and background color to make it distinguishable by users.


UI should not expect user's skill level!!!

Comment 1

[Alice0775 White]

Attachment: screenshot

Comment 2

[:Gijs (he/him)]

This also came up in bug 1390601, I think we just need to extend the entire area to work.

Comment 3

[Julien Cristau [:jcristau]]

Marked as P4 by photon team -> not tracking for 57.

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1396423 - make it possible to drop items in what visually looks like the palette but isn't,

Review commit: https://reviewboard.mozilla.org/r/185108/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/185108/

Comment 5

[Mike de Boer [:mikedeboer]]

Comment on attachment 8913691 [details]
Bug 1396423 - make it possible to drop items in what visually looks like the palette but isn't,

https://reviewboard.mozilla.org/r/185108/#review190158

::: browser/components/customizableui/CustomizeMode.jsm:2214
(Diff revision 1)
>        }
>      }
>  
>      let areas = CustomizableUI.areas;
>      areas.push(kPaletteId);
> -    while (aElement) {
> +    return aElement.closest(areas.map(a => "#" + CSS.escape(a)).join(","));

Nice!

::: browser/components/customizableui/test/browser_drag_outside_palette.js:1
(Diff revision 1)
> +/* Any copyright is dedicated to the Public Domain.

CC comment not required anymore.

Comment 6

[:Gijs (he/him)]

(In reply to Mike de Boer [:mikedeboer] from comment #5)
> ::: browser/components/customizableui/test/browser_drag_outside_palette.js:1
> (Diff revision 1)
> > +/* Any copyright is dedicated to the Public Domain.
> 
> CC comment not required anymore.

Per IRC discussion re: https://bugzilla.mozilla.org/show_bug.cgi?id=1316187#c39 , leaving this as-is.

Comment 7

[:Gijs (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f8ca714c5bf5

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a9af2009e406
make it possible to drop items in what visually looks like the palette but isn't, r=mikedeboer

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/a9af2009e406

Comment 10

[:Gijs (he/him)]

Comment on attachment 8913691 [details]
Bug 1396423 - make it possible to drop items in what visually looks like the palette but isn't,

Approval Request Comment
[Feature/Bug causing the regression]: we made the palette in customize mode look like it extended throughout the window in bug 1354123 and friends. Before, the area below the panel had a different background and was visually clearly distinct from the palette. That is no longer the case, and so it's not obvious that only part of the screen is/was a 'valid' drag target for the palette.
[User impact if declined]: removing items from the toolbar or panel is trickier than it may seem - it seems to work in some places and not others. This feels buggy / makes the user experience confusing.
[Is this code covered by automated tests?]: yes, plus new ones for this change
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]:
0. enter customize mode 
1. drag an item (home button, library button, flexible space, etc.) from the navbar down to the area underneath the panel
2. drop it there.

Expected: the item is removed from the navbar and put in the palette
Actual (pre-patch): the item is reset back to the navbar

(Note: the back/forward button and url bar cannot be removed from the navbar. This is intentional.)
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: not very
[Why is the change risky/not risky?]: while it's not a tiny patch, the effects are relatively straightforward to understand, we have good automated test coverage, and added tests for this specific change, too, and we still have a considerable beta runway.
[String changes made/needed]: none

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8913691 [details]
Bug 1396423 - make it possible to drop items in what visually looks like the palette but isn't,

Improve the UX, taking it.
Should be in 57b5

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2a998e14f67b

Comment 13

[Emil Ghitta, QA [:emilghitta]]

I have reproduced the issue mentioned in comment 0, using an affected Firefox 57.0a1 build (BuildId:20170903220032).

I have verified that the issue is not reproducible using Firefox 57.0b6 (Build Id:20171005195903) and Firefox 58.0a1 (BuildId:20171008220130) on Windows 10 64bit, macOS 10.11.6 and Ubuntu 16.04 64bit.