This button will look and behave the same as the Account button in the current menu panel. The icon/ gravatar picture will be smaller; the size to be the same as all other menu item icons. The caption will no longer be the name of the user who signed in, but will become static: 'Firefox Account'. The associated command will stay the same as it currently is. Please take a possible future hotkey assignment into account. (no pun intended.)

(I haven't added in keyboard support yet, but this should otherwise be ready for review, I hope...)

Bryan, can you provide final normal/hover/active colours for the sync 'warning' states? I'm purposefully keeping the current iconography, with the assumption that we'll bulk-replace things in bug 1355455.

Works (and looks) great

Comment on attachment 8869444 [details] Bug 1354084 - add sync / Firefox account button to hamburger panel, https://reviewboard.mozilla.org/r/141076/#review145056 Nice work! It looks really good in the new menu. I have a few comments here and there, but nothing that requires another round. Thanks! ::: browser/base/content/browser-sync.js:52 (Diff revision 1) > get remoteClients() { > return Weave.Service.clientsEngine.remoteClients > .sort((a, b) => a.name.localeCompare(b.name)); > }, > > + _generateGetters(usePhoton) { nit: `_generateNodeGetters` might be a bit more descriptive. Also, I think we wanna keep this even when we clean up the non-photon stuff later, by moving it to the `init()` method. ::: browser/base/content/browser-sync.js:66 (Diff revision 1) > + }); > + } > + }, > + > + _maybeUpdateUIState() { > + // Update the UI nit: missing dot. ::: browser/base/content/browser-sync.js:70 (Diff revision 1) > + _maybeUpdateUIState() { > + // Update the UI > + if (UIState.isReady()) { > + const state = UIState.get(); > + // If we are not configured, the UI is already in the right state when > + // we open the window. We can avoid a repaint. Well, you're probably right in this case :/ Normally one would expect that setting the state with its current value(s) does not cause a repaint. Oh well! ::: browser/base/content/browser-sync.js:94 (Diff revision 1) > + XPCOMUtils.defineLazyPreferenceGetter(this, "gPhotonStructure", > + "browser.photon.structure.enabled", (pref, old, newValue) => { > + this._generateGetters(newValue); > + this._maybeUpdateUIState(); > + }); > + this._generateGetters(this.gPhotonStructure); Since you're not using the 'lazy' part of `defineLazyPreferenceGetter`, but because of its observer utilirty, can you tell us about that above its declaration in a comment? ::: browser/themes/shared/customizableui/panelUI.inc.css:717 (Diff revision 1) > - > .panel-banner-item > .toolbarbutton-text { > width: 0; /* Fancy cropping solution for flexbox. */ > } > > +/* fxa indicator bits */ nit: missing dot. ::: browser/themes/shared/customizableui/panelUI.inc.css:744 (Diff revision 1) > + > +#appMenu-fxa-avatar, > +#appMenu-fxa-label > .toolbarbutton-icon, > +#appMenu-fxa-icon > .toolbarbutton-icon { > + width: 16px; > + height: 16px; Please add these selectors to `#PanelUI-remotetabs-tabslist > toolbarbutton[itemtype="tab"] > .toolbarbutton-icon,` and friends below. You might want to move `#PanelUI-containersItems > .subviewbutton > .toolbarbutton-icon` on line 1682 as well. ::: browser/themes/shared/customizableui/panelUI.inc.css:752 (Diff revision 1) > +#appMenu-fxa-icon > .toolbarbutton-text { > + display: none; > +} > + > +#appMenu-fxa-icon[syncstatus="active"] { > + list-style-image: url(chrome://browser/skin/syncProgress-horizontalbar.png); I think we'll want to make this SVG using CSS anims too, eventually. Certainly because it looks like a rotate transform with different fill color! Can you file a follow-up for that? ::: browser/themes/shared/customizableui/panelUI.inc.css:760 (Diff revision 1) > +#appMenu-fxa-avatar { > + pointer-events: none; > + list-style-image: url(chrome://browser/skin/fxa/default-avatar.svg); > +} > + > +@media (min-resolution: 1.1dppx) { ...because I'm sooo happy to see less and less of these blocks :) ::: browser/themes/shared/customizableui/panelUI.inc.css:800 (Diff revision 1) > +} > + > +#appMenu-fxa-container[fxastatus="signedin"] > toolbarseparator { > + -moz-appearance: none; > + height: 24px; > + margin: 0; nit: trailing space. ::: browser/themes/shared/customizableui/panelUI.inc.css:830 (Diff revision 1) > +} > + > +#appMenu-fxa-container[fxastatus="login-failed"] > #appMenu-fxa-status:hover:active, > +#appMenu-fxa-container[fxastatus="unverified"] > #appMenu-fxa-status:hover:active { > + background-color: hsl(56, 96%, 60%); > + box-shadow: 0 1px 0 hsla(210, 4%, 10%, .05) inset; Can we put these state color values in CSS variables, please? I think that'd make things a bit easier when Bryan decides to change them later on.

Comment on attachment 8869444 [details] Bug 1354084 - add sync / Firefox account button to hamburger panel, https://reviewboard.mozilla.org/r/141076/#review145238 ::: browser/base/content/browser-sync.js:70 (Diff revision 1) > + _maybeUpdateUIState() { > + // Update the UI > + if (UIState.isReady()) { > + const state = UIState.get(); > + // If we are not configured, the UI is already in the right state when > + // we open the window. We can avoid a repaint. FWIW, this was pre-existing and moved out from the init() code. I do know that re-setting the same (XUL) attribute does count as e.g. a mutation for a mutationobserver, and I wouldn't be at all surprised if it dirtied layout state.

Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/e7575b0dd085 add sync / Firefox account button to hamburger panel, r=mikedeboer

I ended up landing this as-is but reusing the add-on banner colours. I expect it still needs looking at, but we can do that in a separate bug if necessary. I used variables as requested so it should be easy to do. Leaving ni to check.

Verified on Windows, Mac, and Ubuntu.