Just like we currently show the main hamburger panel in customize mode, in the new design we should be showing the overflow menu instead. In customize mode, at least initially the extra dynamically inserted/removed items (rather than the sticky/user-customized ones) won't be shown). We should leave extra room at the bottom of the panel to insert items. If no items are in the panel, a placeholder message with user instructions should be displayed. The panel arrow should line up to the overflow button.

Comment on attachment 8864955 [details] Bug 1354082 - part 1: make toggling the photon structure pref at runtime not break the panels, https://reviewboard.mozilla.org/r/136608/#review140304 OK, we discussed this work f2f as well. Ideally, I'd like to see this patch accompanied with a try push that emulated the situation to come: 'browser.photon.structure.enabled' toggled to `true`.

Comment on attachment 8864956 [details] Bug 1354082 - part 2: make overflow panel show up in customize mode and make dragging work correctly, https://reviewboard.mozilla.org/r/136610/#review140554 ::: browser/components/customizableui/CustomizeMode.jsm:359 (Diff revision 1) > > this._addDragHandlers(this.visiblePalette); > > window.gNavToolbox.addEventListener("toolbarvisibilitychange", this); > > + if (gPhotonStructure) { Huh, here and below the checks are clearly wrong... :-\

(In reply to Mike de Boer [:mikedeboer] from comment #3) > Comment on attachment 8864955 [details] > Bug 1354082 - part 1: make toggling the photon structure pref at runtime not > break the panels, > > https://reviewboard.mozilla.org/r/136608/#review140304 > > OK, we discussed this work f2f as well. Ideally, I'd like to see this patch > accompanied with a try push that emulated the situation to come: > 'browser.photon.structure.enabled' toggled to `true`. remote: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6d6f4cf2a63ed86365738f5c03f18eadeac2800 I'm reasonably confident the CUI tests should be green. I haven't looked for other tests. I expect there will be some others that will need adjusting for when we really flip the pref, but this should be the bulk of the work, and I would prefer not to block this bug on fixing the remaining tests, if that's OK. :-)

Comment on attachment 8866303 [details] Bug 1354082 - part 0: set prefs in all the tests that need it, https://reviewboard.mozilla.org/r/137938/#review141090 r=me with comments addressed. Thanks! ::: browser/components/customizableui/test/browser_890140_orphaned_placeholders.js:26 (Diff revision 1) > ok(!CustomizableUI.inDefaultState, "Should not be in default state if on DevEdition."); > } > > // This test relies on an exact number of widgets being in the panel. > // Remove the buttons to satisfy that. (bug 1229236) > + Cu.reportError("removing original buttons"); Please remove this line. ::: browser/components/customizableui/test/browser_938995_indefaultstate_nonremovable.js:10 (Diff revision 1) > "use strict"; > > const kWidgetId = "test-non-removable-widget"; > > // Adding non-removable items to a toolbar or the panel shouldn't change inDefaultState > -add_task(function() { > +add_task(function*() { Please use `async` and `await` here. ::: browser/components/customizableui/test/browser_981305_separator_insertion.js:65 (Diff revision 1) > CustomizableUI.reset(); > } > }; > } > > +add_task(function* () { Please use `async` and `await` here.

Comment on attachment 8864956 [details] Bug 1354082 - part 2: make overflow panel show up in customize mode and make dragging work correctly, https://reviewboard.mozilla.org/r/136610/#review141102 Also here, r=me with comments addressed. Nice work! ::: browser/components/customizableui/CustomizeMode.jsm:1564 (Diff revision 3) > } > // Drawing in the titlebar means not showing the titlebar, hence the negation: > Services.prefs.setBoolPref(kDrawInTitlebarPref, !aShouldShowTitlebar); > }, > > + _getDOMWindowUtils() { Why not just: ```js get _dwu() { if (!this.__dwu) { //... } return this.__dwu; }, get _dir() { if (!this.__dir) { //... } return this.__dir; }, ``` ? ::: browser/components/customizableui/CustomizeMode.jsm:2032 (Diff revision 3) > let draggedWrapper = this.document.getElementById("wrapper-" + aDraggedItemId); > let originArea = this._getCustomizableParent(draggedWrapper); > makeSpaceImmediately = originArea == targetArea; > } > - // Calculate width of the item when it'd be dropped in this position > - let width = this._getDragItemSize(aItem, draggedItem).width; > + let propertyToMeasure = aAreaType == "toolbar" ? "width" : "height"; > + // Calculate width/height of the item when it'd be dropped in this position nit: missing dot. ::: browser/components/customizableui/CustomizeMode.jsm:2034 (Diff revision 3) > makeSpaceImmediately = originArea == targetArea; > } > - // Calculate width of the item when it'd be dropped in this position > - let width = this._getDragItemSize(aItem, draggedItem).width; > - let direction = window.getComputedStyle(aItem).direction; > + let propertyToMeasure = aAreaType == "toolbar" ? "width" : "height"; > + // Calculate width/height of the item when it'd be dropped in this position > + let borderWidth = this._getDragItemSize(aItem, draggedItem)[propertyToMeasure]; > + let direction = aAreaType == "toolbar" ? "Inline" : "Block"; nit: `direction` reminds me of 'ltr' or 'rtl' values; perhaps change it to `side` or `layoutSide` or `orientation`?

hg error in cmd: hg rebase -s ae3b5973065d -d ed073a4c343e: abort: can't rebase public changeset ae3b5973065d (see 'hg help phases' for details)

Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a92f82ecbf10 part 0: set prefs in all the tests that need it, r=mikedeboer https://hg.mozilla.org/integration/mozilla-inbound/rev/59cd845e4cb0 part 1: make toggling the photon structure pref at runtime not break the panels, r=mikedeboer https://hg.mozilla.org/integration/mozilla-inbound/rev/119ce372e28e part 2: make overflow panel show up in customize mode and make dragging work correctly, r=mikedeboer

Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9621b63fc745 rm space because it's 'async function()' but 'function* ()', rs=bustage on a CLOSED TREE

https://hg.mozilla.org/mozilla-central/rev/a92f82ecbf10 https://hg.mozilla.org/mozilla-central/rev/59cd845e4cb0 https://hg.mozilla.org/mozilla-central/rev/119ce372e28e https://hg.mozilla.org/mozilla-central/rev/9621b63fc745

Verified on Windows, Mac, and Ubuntu.