871204 - Auto fit panel height after exiting customization mode

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Alejandro Rodriguez [:Alopepeo]]

Attachment: Auto fit issue

1. Enter customization mode.
2. Drag and drop three items of the panel back to the palette.
3. Exit customization mode.
4. Open the panel.

Result:
The space that was previously used by the removed items is still there. It will not fit until you restart jamun.

Comment 1

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

Going to steal this - hopefully you hadn't already started on this, Mike!

Comment 2

[Mike de Boer [:mikedeboer]]

I did start on it, but I was only investigating atm. So far I didn't come to any conclusions worth sharing here.

Comment 3

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

Attachment: Patch v1

Trial and error gave me this. Then again, trial and error gave me the whole panelmultiview implementation in the first place, so I guess it's fitting.

Anyhow, this seems to do the job - we hide the main view spring and remove the flex on the main view when the panelmultiview has view="main". That shrinks down the main view to the right height, and then _syncContainerToMainView does the right thing.

Bonus, we stop animating the height of the panel unless the panelmultiview has view="subview". This should stop those short instances where the panel seems to expand when it's first deployed.

Comment 4

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

The only regression I see from this patch is that when the History widget is put into the toolbar, at least on Ubuntu, the "Show All History" button is getting slightly cut off on the bottom. But I'm willing to take that for now and fix down the road.

Comment 5

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

Bonus - this also appears to fix the bug on Retina displays where the menu panel would sometimes "run away" as you moved your mouse over it after customizing.

Comment 6

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

Though I can still reproduce it when opening the History subview. :/

Comment 7

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

Comment on attachment 762970 [details] [diff] [review]
Patch v1

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

::: browser/components/customizableui/content/panelUI.xml
@@ +132,5 @@
>  
>          if (this._mainView) {
>            this.setMainView(this._mainView);
>          }
> +        this.setAttribute("view", "main");

nit, can we change this to "viewType"? I think view= might imply that the value will define which view is actually being shown, {main, historysubview, bookmarkssubview, ...}.

Comment 8

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

(In reply to Jared Wein [:jaws] from comment #7)
> Comment on attachment 762970 [details] [diff] [review]
> Patch v1
> 
> Review of attachment 762970 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/components/customizableui/content/panelUI.xml
> @@ +132,5 @@
> >  
> >          if (this._mainView) {
> >            this.setMainView(this._mainView);
> >          }
> > +        this.setAttribute("view", "main");
> 
> nit, can we change this to "viewType"? I think view= might imply that the
> value will define which view is actually being shown, {main, historysubview,
> bookmarkssubview, ...}.

Sure - gonna go with the monocased "viewtype" though, to stick with convention.

Comment 9

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

Attachment: Patch v1.1 (r+'d by jaws)

Switched to viewtype. Also, I just noticed that my old patch was causing the height to snap and not transition when switching from the subview back to the main view. I've added a transitioning attribute and modified the transition rule to compensate.

Comment 10

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

Landed in UX as https://hg.mozilla.org/projects/ux/rev/ce7a6a715dc2

Comment 11

[Mike de Boer [:mikedeboer]]

Mike, when watching the resizing behavior on OSX - of course, for me, but I'm sure it's noticeable on the other platforms as well - I noticed it causes the height of the panel to yank when

1. Entering & leaving customization mode
2. Opening the panel the first time after I made changes to the panel layout in customization mode

Is that something to address as part of this bug, or should another bug be filed?

Comment 12

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

(In reply to Mike de Boer [:mikedeboer] from comment #11)
> Mike, when watching the resizing behavior on OSX - of course, for me, but
> I'm sure it's noticeable on the other platforms as well - I noticed it
> causes the height of the panel to yank when
> 
> 1. Entering & leaving customization mode
> 2. Opening the panel the first time after I made changes to the panel layout
> in customization mode
> 
> Is that something to address as part of this bug, or should another bug be
> filed?

This is *after* this patch has been applied? This patch was supposed to make the height not animate unless the panel was already open.

Comment 13

[Mike de Boer [:mikedeboer]]

Weird, now I don't see it anymore after a rebuild... me very happy! :)

Comment 14

[:Gijs (he/him)]

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