Comment on attachment 9093101 [details] [diff] [review] bug1581305_observes_qf.mail.patch Review of attachment 9093101 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, I tested each of the 3 places where you can toggle the quickfilter toolbar, and it is still shown and hidden correctly and the toggles all get updated correctly as checked or not. r+ with a docstring added for the new function. ::: mail/base/content/quickFilterBar.js @@ +90,5 @@ > > this._updateCommands(); > }, > > + _setupToggleChecks(checked) { A brief doc string for this function would be good here.

Comment on attachment 9093102 [details] [diff] [review] bug1581305_observes_compose.mail.patch Review of attachment 9093102 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, r+ with a couple doc comments added. I tested it and all the various composer buttons/widgets were getting updated correctly. ::: mail/components/compose/content/ComposerCommands.js @@ +366,4 @@ > } > } > > function pokeStyleUI(uiID, aDesiredState) { Could you add a doc comment? Would be good to doc these parameters while we're here. @@ +419,5 @@ > } > return typedarray; > } > > function pokeMultiStateUI(uiID, cmdParams) { Same, would be good to add a doc comment while we're here.