Bug 1354127 introduced a new "more menu" in the new static hamburger menu. But the animation is not shippable in this state. Please see the attached screencast. ;)

The horizontal jumping of the header text I assume will go away as a result of bug 1363753. The jump in height after the 'more' view is first shown should go away after bug 1363756 (which, come to think of it, may be required for bug 1363756 as well). (In reply to Sören Hentzschel from comment #0) > But the animation is not shippable in this state. Can you please be explicit about what specifically you mean in that screencast, especially if you're seeing something else you object to besides what I already noted in this comment?

Hi Gijs, sorry, I was in a hurry. I think you described all the problems I saw. I'll test again when bug 1363753 and bug 1363756 are fixed. Thank you.

(In reply to :Gijs from comment #1) > The horizontal jumping of the header text I assume will go away as a result > of bug 1363753. I don't see this anymore on m-c now that this has landed, so I think this is fixed. Mike, I can reproduce the final vertical 'jump' at the end of the transition locally on OS X. Can you? Any idea what's responsible for that? Seems like we're underestimating the final height somehow, which might actually mean that bug 1363756 miiight not fix this. Not sure. Thoughts?

(In reply to :Gijs from comment #3) > Mike, I can reproduce the final vertical 'jump' at the end of the transition > locally on OS X. Can you? Any idea what's responsible for that? Seems like > we're underestimating the final height somehow, which might actually mean > that bug 1363756 miiight not fix this. Not sure. Thoughts? Indeed, let's keep this bug to fix that final glitch. It seems like we're getting that behavior _after_ I added the padding to the arrowpanel, to match the specs. Adding that padding to the `getBoundsWithoutFlushing()` result should get us back to where we need to be.

Comment on attachment 8868536 [details] Bug 1364672 - take the padding of the views into account when calculating the height of each sub view. https://reviewboard.mozilla.org/r/140148/#review143492 ::: browser/components/customizableui/PanelMultiView.jsm:389 (Diff revision 1) > - if (this.panelViews && !this._mainViewWidth) > + if (this.panelViews && !this._mainViewWidth) { > this._mainViewWidth = previousRect.width; > + let cs = window.getComputedStyle(previousViewNode); > + this._viewPadding = parseInt(cs.getPropertyValue("padding-top"), 10) + > + parseInt(cs.getPropertyValue("padding-bottom"), 10); > + } 3 notes here: - Can you add a comment that makes it clear this relies on the previousViewNode/previousRect to be the main view - Can we call this `_viewVerticalPadding` for good measure? - This is causing a sync style flush, right, because we're reading computed style? I don't know that I agree with the commit message that that doesn't affect perf... Instead, can we get the lazy bounds of the first and last child of the view node (which should be cheap and just use DOM + lazy bounds getting), and compare the diff between their top/bottom and the (lazy) height of the view rect? That should effectively get the padding (and border, but there isn't any), right? ::: browser/components/customizableui/PanelMultiView.jsm:515 (Diff revision 1) > // axis gets confused. I.e. it lies. > // So what we need to resort to here is count the height of each > // individual child element of the view. > viewRect.height = [viewNode.header, ...viewNode.children].reduce((acc, node) => { > return acc + dwu.getBoundsWithoutFlushing(node).height; > - }, 0); > + }, 0) + this._viewPadding; Nit: can also pass this as the base for the reduce() (instead of 0). Don't much care either way, but I figured I'd flag it up.

Comment on attachment 8868536 [details] Bug 1364672 - take the padding of the views into account when calculating the height of each sub view. https://reviewboard.mozilla.org/r/140148/#review143554 I haven't tested this, but assuming it fixes things, r=me

Pushed by mdeboer@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4dfebbea3897 take the padding of the views into account when calculating the height of each sub view. r=Gijs

Verified on Windows, Mac, Ubuntu.