940387 - Wide widgets wideness messes up DnD placeholders in the palette

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[u428464]

First of all I wanna say the new DnD stuff is pretty awesome. Sadly I ran into three main issues. 
Firstly the caption buttons flicker on Windows when you drag and drop stuff in the panel or the palette (sometimes they seem disabled and then they are normal again).
Secondly the placeholders can be taken off the panel and then they disappear.
Thirdly the wide widget aren't animated into the palette and they overlap other icons when a normal widget are dragged upon them.

Comment 1

[u428464]

I also wanna say that the placeholders could maybe be done differently : it's pretty strange to have the third one being pushed to the next line.

Comment 2

[:Gijs (he/him)]

(In reply to Guillaume C. [:ge3k0s] from comment #2)
> Thirdly the wide widget aren't animated into the palette and they overlap
> other icons when a normal widget are dragged upon them.

I'm sorry, I don't really understand what you mean here. Can you elaborate and/or provide a screenshot?

Comment 3

[u428464]

Attachment: DnD issue.png

Yeap here's a screenshot of the issue

Comment 4

[:Gijs (he/him)]

(In reply to Guillaume C. [:ge3k0s] from comment #3)
> Created attachment 8334543 [details]
> DnD issue.png
> 
> Yeap here's a screenshot of the issue

I thought we fixed the look of wide items in the palette, Mike? Do we still need the rest of bug 878065

Comment 6

[:Gijs (he/him)]

The wide widget issue is our own fault - we specifically style the edit and zoom controls to be this wide in the panel, see bug 879974. This throws off all the logic in the dragging code.

Stephen, would it be OK if we styled the wide items similar to how they are shown in the navbar, to ensure they're not so wide when in the palette?

Alternatively, we could disable the animation effects within the palette - having variable-width items in there which reflow organically (rather than the distinct row-filling items in the panel) makes the problem of reflowing items correctly much harder.

Still alternatively... we could try to rework the animation to make it work with variable-width items. But that is significantly more complex than what it currently does.

Comment 7

[:Gijs (he/him)]

(In reply to Guillaume C. [:ge3k0s] from comment #7)
> Firstly the caption buttons flicker on Windows when you drag and drop stuff
> in the panel or the palette (sometimes they seem disabled and then they are
> normal again).

I expect this is the same as bug 940455. Does that sound plausible?

Comment 8

[:Gijs (he/him)]

Attachment: Australis customize mode: shouldn't be able to drag placeholders,

Trivial fix.

Comment 9

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/5e91ba03a3df

Comment 10

[u428464]

(In reply to :Gijs Kruitbosch from comment #7)
> (In reply to Guillaume C. [:ge3k0s] from comment #7)
> > Firstly the caption buttons flicker on Windows when you drag and drop stuff
> > in the panel or the palette (sometimes they seem disabled and then they are
> > normal again).
> 
> I expect this is the same as bug 940455. Does that sound plausible?

That seems to be it ;)

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5e91ba03a3df

Comment 12

[:Gijs (he/him)]

Attachment: change Australis' wide widget styling in palette to fit into grid,

I've just gone ahead and written up a patch for this. This looks nice, and no longer messes up DnD stuff. Mike, what do you think?

Comment 13

[:Gijs (he/him)]

Attachment: Screen shot of new wide widget styling

Comment 14

[:Gijs (he/him)]

Attachment: when dragging wide items in Australis' palette, don't fix insertion point to first item in row,

Just noticed this 'oops' issue in the code here... wide items snap to the first item in their row in the palette, too. That's obviously wrong...

Comment 15

[:Gijs (he/him)]

Discussed this with Stephen on IRC, and he said this should be alright.

Comment 16

[Mike de Boer [:mikedeboer]]

Comment on attachment 8341695 [details] [diff] [review]
change Australis' wide widget styling in palette to fit into grid,

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

Did you mean to make the icons look stretched?

Comment 17

[Mike de Boer [:mikedeboer]]

Comment on attachment 8341700 [details] [diff] [review]
when dragging wide items in Australis' palette, don't fix insertion point to first item in row,

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

LGTM!

Comment 18

[:Gijs (he/him)]

(In reply to Mike de Boer [:mikedeboer] from comment #16)
> Comment on attachment 8341695 [details] [diff] [review]
> change Australis' wide widget styling in palette to fit into grid,
> 
> Review of attachment 8341695 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Did you mean to make the icons look stretched?

No; they should be 16x16px, sourced from files in which they are 32x32 or 16x16, IIRC. I had to fix the width to make them not look stretched on OS X (because without that style they ended up being 18x13px or something weird). What platform do they look stretched on?

Comment 19

[Mike de Boer [:mikedeboer]]

Erm, on OSX...

Comment 20

[:Gijs (he/him)]

Oh, I see, this is because there's padding set on the buttons somehow, and I seem to have missed that. New patch coming up.

Comment 21

[:Gijs (he/him)]

Attachment: change Australis' wide widget styling in palette to fit into grid,

So this helps... I can't just adjust the rule (in customizemode.inc.css) that's causing the padding to not apply here, because I'd still have to override the padding, as the default for toolbarbuttons is 2px 6px, which actually distorts the icons worse still.

Comment 22

[:Gijs (he/him)]

Comment on attachment 8341700 [details] [diff] [review]
when dragging wide items in Australis' palette, don't fix insertion point to first item in row,

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

Comment 23

[Ryan VanderMeulen [:RyanVM]]

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

Comment 24

[Mike de Boer [:mikedeboer]]

Comment on attachment 8343922 [details] [diff] [review]
change Australis' wide widget styling in palette to fit into grid,

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

LGTM! Thanks!

Comment 25

[Mike de Boer [:mikedeboer]]

Gijs, I think it'd be a good idea to close this bug after you checked in attachment 8343922 [details] [diff] [review] and use proper follow-up bugs for any DnD-related future work.

Comment 26

[:Gijs (he/him)]

Comment on attachment 8343922 [details] [diff] [review]
change Australis' wide widget styling in palette to fit into grid,

(In reply to Mike de Boer [:mikedeboer] from comment #25)
> Gijs, I think it'd be a good idea to close this bug after you checked in
> attachment 8343922 [details] [diff] [review] and use proper follow-up bugs
> for any DnD-related future work.

Yes, I agree. This is the last thing to address from comment #0; the fog overlay in Win7 has its own bug (bug 940455).

https://hg.mozilla.org/integration/fx-team/rev/27dfeba05309

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/27dfeba05309

Comment 28

[Andrei Vaida [:avaida]]

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

1. Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0
2. Mozilla/5.0 (X11; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0
3. Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:29.0) Gecko/20100101 Firefox/29.0