907412 - Depopulate the customization palette before scaling the customization window upon exiting

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

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

I've noticed on my local builds (which happen to run a bit slower), that the items in the customization palette will adjust positions as the chrome window padding changes, meaning that layout and graphics has to do extra work.

We can hide the palette or remove it from the DOM before beginning the animation.

Comment 1

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

Attachment: Patch

This patch makes exiting feel smoother to me. What do you think?

Comment 2

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

Attachment: Patch v.1

Removed an extra newline.

Comment 3

[:Gijs (he/him)]

Comment on attachment 793124 [details] [diff] [review]
Patch v.1

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

LGTM. While here, I noticed those Task.spawns (inside depopulatePalette) don't have an error reporter. Not that I really expect errors, but if you see my point (see also recent fx-dev/m.d.platform discussion), can you file a separate bug about auditing our code so we have at least a basic console.error reporter everywhere? I'm sure add-ons will find ways to break "something". Can be P4 or something along those lines, not really urgent, but we should do it, IMHO.

Comment 4

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

https://hg.mozilla.org/projects/ux/rev/237d2879d89d

Comment 5

[:Gijs (he/him)]

I was thinking about this more last night... wouldn't it be better for "snappiness" to reverse this patch, and just set the palette to hidden before starting the animation, and then doing the clearing of the palette afterwards?

Comment 6

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

(In reply to :Gijs Kruitbosch from comment #5)
> I was thinking about this more last night... wouldn't it be better for
> "snappiness" to reverse this patch, and just set the palette to hidden
> before starting the animation, and then doing the clearing of the palette
> afterwards?

That could be better if we think that depopulating the palette requires a noticeable amount of work. I'm not sure if that is the case though.

Comment 7

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] from comment #6)
> (In reply to :Gijs Kruitbosch from comment #5)
> > I was thinking about this more last night... wouldn't it be better for
> > "snappiness" to reverse this patch, and just set the palette to hidden
> > before starting the animation, and then doing the clearing of the palette
> > afterwards?
> 
> That could be better if we think that depopulating the palette requires a
> noticeable amount of work. I'm not sure if that is the case though.

When moving a lot of items to the palette, I can visually see it clearing before the transition starts. :-(

Comment 8

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/237d2879d89d