+++ This bug was initially created as a clone of Bug #1354126 +++ See https://mozilla.invisionapp.com/share/5ZAEYEW8M#/screens/225203398 . We should restyle the main content area to match the spec. This includes: - icon/margin/padding size adjustments - not showing a visible border on the main toolbars (it might be useful to still have a border to avoid additional reflows!) - making the background continuous with the toolbar (ie same colour) - remove the horizontal line under 'additional tools and features' and restyle the text itself per spec

Moving to reserve backlog for now.

Mockup has moved to here: https://mozilla.invisionapp.com/share/8HBR8ZD2B#/230606754_Customize_-_First_Run

Comment on attachment 8881520 [details] Bug 1354145 - fix background colour across customize mode as well as integration with the footer, https://reviewboard.mozilla.org/r/152674/#review157800 ::: browser/themes/shared/browser.inc.css:31 (Diff revision 1) > + --toolbar-background-color: #f9f9fa; > + --toolbar-foreground-color: #0c0c0d; Requesting review from Dale because I imagine we'll want to use this in bug 1367439, but I can't also fix that bug here because it depends on the tabstrip, and I don't want this fix to wait on the tabstrip changes.

Comment on attachment 8881520 [details] Bug 1354145 - fix background colour across customize mode as well as integration with the footer, https://reviewboard.mozilla.org/r/152674/#review157998 This looks great, I was considering creating a CSS var for this already, probably the best way, will update my patch to use it, cheers

Comment on attachment 8881518 [details] Bug 1354145 - set photon-structure attribute on root, update palette/panel icon sizes and layout, https://reviewboard.mozilla.org/r/152670/#review158136 ::: browser/components/customizableui/content/panelUI.js:124 (Diff revision 2) > this.overflowFixedList.hidden = false; > // Also unhide the separator. We use CSS to hide/show it based on the panel's content. > this.overflowFixedList.previousSibling.hidden = false; > CustomizableUI.registerMenuPanel(this.overflowFixedList, CustomizableUI.AREA_FIXED_OVERFLOW_PANEL); > this.navbar.setAttribute("photon-structure", "true"); > + document.documentElement.setAttribute("photon-structure", "true"); This looks like duplicating the thing for navbar, but it's probably better for perf in the interim. ::: browser/themes/shared/browser.inc:23 (Diff revision 2) > + > +%ifdef MOZ_PHOTON_THEME > +%define panelPaletteIconSize var(--panel-palette-icon-size) > +%else > +%define panelPaletteIconSize 32px > +%endif Why can't you use `var(--panel-palette-icon-size)` everywhere? It looks like you're overriding it properly in PanelUI.inc.css, so it should be possible... For perf? If so, why? ::: browser/themes/shared/customizableui/panelUI.inc.css:1172 (Diff revision 2) > panelview .toolbarbutton-1, > .subviewbutton, > .widget-overflow-list .toolbarbutton-1, > .panelUI-grid .toolbarbutton-1 > .toolbarbutton-menubutton-button, > .share-provider-button, > -.toolbaritem-combined-buttons@inAnyPanel@ > toolbarbutton { > +:root:not([photon-structure]) .toolbaritem-combined-buttons@inAnyPanel@ > toolbarbutton { cart is not going to like this, now is it? Not saying there's a way around it, but just raising awareness...

Comment on attachment 8881519 [details] Bug 1354145 - remove dashed outlines for customization targets for photon, https://reviewboard.mozilla.org/r/152672/#review158138 Good riddance ;)

Comment on attachment 8881521 [details] Bug 1354145 - adjust sizing of header and margins/paddings on palette, https://reviewboard.mozilla.org/r/152676/#review158140 It looks good, except for these two remarks: https://www.evernote.com/l/APu14LVh5SRFfKRuHIWz1p9eE-3ZQGSYVWo Please let me know what you think!

(In reply to Mike de Boer [:mikedeboer] from comment #14) > ::: browser/themes/shared/browser.inc:23 > (Diff revision 2) > > + > > +%ifdef MOZ_PHOTON_THEME > > +%define panelPaletteIconSize var(--panel-palette-icon-size) > > +%else > > +%define panelPaletteIconSize 32px > > +%endif > > Why can't you use `var(--panel-palette-icon-size)` everywhere? It looks like > you're overriding it properly in PanelUI.inc.css, so it should be possible... > For perf? If so, why? Yeah, variables are known to have... issues, perf-wise. But this won't need to be a variable long-term - it'll just be 16px when we ship photon. The only reason we need a variable now is because you can turn it on/off. I would expect that when we're dumping the MOZ_PHOTON_THEME flag for 57 we just hardcode it to 16px and remove the variable. Are you OK with this, or do you feel strongly that we should just use a variable and not worry about it? > ::: browser/themes/shared/customizableui/panelUI.inc.css:1172 > (Diff revision 2) > > panelview .toolbarbutton-1, > > .subviewbutton, > > .widget-overflow-list .toolbarbutton-1, > > .panelUI-grid .toolbarbutton-1 > .toolbarbutton-menubutton-button, > > .share-provider-button, > > -.toolbaritem-combined-buttons@inAnyPanel@ > toolbarbutton { > > +:root:not([photon-structure]) .toolbaritem-combined-buttons@inAnyPanel@ > toolbarbutton { > > cart is not going to like this, now is it? Not saying there's a way around > it, but just raising awareness... Err, not sure why cart particularly would care? I guess it makes things slightly worse for now, but if a single descendant selector like this would make a noticeable impact on cart I think we have worse problems. Anyway, we're getting rid of things completely anyway (bug 1354123) so I don't think it matters. Note also that this selector doesn't apply on current m-c as-is, so I wouldn't expect the numbers to change when this lands.

(In reply to :Gijs from comment #17) > Yeah, variables are known to have... issues, perf-wise. But this won't need > to be a variable long-term - it'll just be 16px when we ship photon. The > only reason we need a variable now is because you can turn it on/off. I > would expect that when we're dumping the MOZ_PHOTON_THEME flag for 57 we > just hardcode it to 16px and remove the variable. Are you OK with this, or > do you feel strongly that we should just use a variable and not worry about > it? Oooh now I see that you're trying to affect release as little as possible. It results in hard to follow non directional logic, but we'll learn to live with it. ;-) Alright, ship it.

Comment on attachment 8881518 [details] Bug 1354145 - set photon-structure attribute on root, update palette/panel icon sizes and layout, https://reviewboard.mozilla.org/r/152670/#review158272 Nice, thanks!

Comment on attachment 8881962 [details] Bug 1354145 - Make customize mode deal with drops in the padding around the panel contents, https://reviewboard.mozilla.org/r/153026/#review158276 LGTM! ::: browser/components/customizableui/CustomizeMode.jsm:1109 (Diff revision 1) > await this._wrapToolbarItem(area); > } > }, > > _addDragHandlers(aTarget) { > + // Allow dropping on the padding of the arrow panel nit: missing dot. ::: browser/components/customizableui/CustomizeMode.jsm:1130 (Diff revision 1) > } > }, > > _removeDragHandlers(aTarget) { > + // Remove handler from different target if it was added to > + // allow dropping on the padding of the arrow panel nit: missing dot.

Comment on attachment 8881962 [details] Bug 1354145 - Make customize mode deal with drops in the padding around the panel contents, https://reviewboard.mozilla.org/r/153026/#review158280 ::: browser/components/customizableui/CustomizeMode.jsm:2210 (Diff revision 1) > return aElement; > } > aElement = aElement.parentNode; > } > + > + if (gPhotonStructure) { Nope! Please make this `gPhotonStructure && aElement`

Comment on attachment 8881962 [details] Bug 1354145 - Make customize mode deal with drops in the padding around the panel contents, https://reviewboard.mozilla.org/r/153026/#review158282 Restore default (empty overflow panel) and try dragging something into it. You’ll never get an item in there!

Comment on attachment 8881521 [details] Bug 1354145 - adjust sizing of header and margins/paddings on palette, https://reviewboard.mozilla.org/r/152676/#review158372 Goodgood!

Comment on attachment 8881962 [details] Bug 1354145 - Make customize mode deal with drops in the padding around the panel contents, https://reviewboard.mozilla.org/r/153026/#review158374 Alright, ready to ship!

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 3b5768fbbe41 -d ed1964e4da87: rebasing 404572:3b5768fbbe41 "Bug 1354145 - set photon-structure attribute on root, update palette/panel icon sizes and layout, r=mikedeboer" merging browser/base/content/browser.css merging browser/themes/shared/customizableui/customizeMode.inc.css merging browser/themes/shared/customizableui/panelUI.inc.css rebasing 404573:77dddfa4a4cf "Bug 1354145 - remove dashed outlines for customization targets for photon, r=mikedeboer" merging browser/themes/shared/customizableui/customizeMode.inc.css rebasing 404574:aaf968aa9ca5 "Bug 1354145 - fix background colour across customize mode as well as integration with the footer, r=daleharvey" merging browser/base/content/browser.css merging browser/themes/shared/customizableui/customizeMode.inc.css warning: conflicts while merging browser/themes/shared/customizableui/customizeMode.inc.css! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s e0ce32deb2e1 -d ed1964e4da87: rebasing 404572:e0ce32deb2e1 "Bug 1354145 - set photon-structure attribute on root, update palette/panel icon sizes and layout, r=mikedeboer" merging browser/base/content/browser.css merging browser/themes/shared/customizableui/customizeMode.inc.css rebasing 404573:d2265ecc7d29 "Bug 1354145 - remove dashed outlines for customization targets for photon, r=mikedeboer" merging browser/themes/shared/customizableui/customizeMode.inc.css rebasing 404574:5fcb730fdcff "Bug 1354145 - fix background colour across customize mode as well as integration with the footer, r=daleharvey" merging browser/base/content/browser.css merging browser/themes/shared/customizableui/customizeMode.inc.css warning: conflicts while merging browser/themes/shared/customizableui/customizeMode.inc.css! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)

Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/55fd64a8515a set photon-structure attribute on root, update palette/panel icon sizes and layout, r=mikedeboer https://hg.mozilla.org/integration/autoland/rev/0896f6d55565 remove dashed outlines for customization targets for photon, r=mikedeboer https://hg.mozilla.org/integration/autoland/rev/518a396f9e78 fix background colour across customize mode as well as integration with the footer, r=daleharvey https://hg.mozilla.org/integration/autoland/rev/8b016e1da34d Make customize mode deal with drops in the padding around the panel contents, r=mikedeboer https://hg.mozilla.org/integration/autoland/rev/99e8731d94f0 adjust sizing of header and margins/paddings on palette, r=mikedeboer

https://hg.mozilla.org/mozilla-central/rev/55fd64a8515a https://hg.mozilla.org/mozilla-central/rev/0896f6d55565 https://hg.mozilla.org/mozilla-central/rev/518a396f9e78 https://hg.mozilla.org/mozilla-central/rev/8b016e1da34d https://hg.mozilla.org/mozilla-central/rev/99e8731d94f0

Improvements noticed! Thank you for landing these == Change summary for alert #7681 (as of July 04 2017 07:19 UTC) == Improvements: 8% cart summary linux64 opt e10s 21.77 -> 19.92 5% cart summary windows7-32 opt e10s 26.37 -> 25.06 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=7681

Verified on Windows, Mac, and Ubuntu