sergeyv@chromium.org changed reviewers: + lushnikov@chromium.org, pfeldman@chromium.org
intermediate comments https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... File Source/devtools/front_end/elements/ElementStatePane.js (right): https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... Source/devtools/front_end/elements/ElementStatePane.js:60: * @override param https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... Source/devtools/front_end/elements/ElementStatePane.js:105: this._button.setEnabled(!!enabled); no need for !! https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... File Source/devtools/front_end/elements/StylesSidebarPane.js (right): https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... Source/devtools/front_end/elements/StylesSidebarPane.js:826: this._currentToolbarPane.detach(); lets detach is hidden https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... Source/devtools/front_end/elements/StylesSidebarPane.js:3164: stray line https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... Source/devtools/front_end/elements/StylesSidebarPane.js:3174: * @param {?WebInspector.DOMNode} newNode protected? https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... Source/devtools/front_end/elements/StylesSidebarPane.js:3182: this.element.classList.remove("expanded"); ?
https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... File Source/devtools/front_end/elements/ElementStatePane.js (right): https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... Source/devtools/front_end/elements/ElementStatePane.js:60: * @override On 2015/05/29 10:53:12, lushnikov wrote: > param Done. https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... Source/devtools/front_end/elements/ElementStatePane.js:105: this._button.setEnabled(!!enabled); On 2015/05/29 10:53:12, lushnikov wrote: > no need for !! Done. https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... File Source/devtools/front_end/elements/StylesSidebarPane.js (right): https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... Source/devtools/front_end/elements/StylesSidebarPane.js:826: this._currentToolbarPane.detach(); On 2015/05/29 10:53:12, lushnikov wrote: > lets detach is hidden Done. https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... Source/devtools/front_end/elements/StylesSidebarPane.js:3164: On 2015/05/29 10:53:12, lushnikov wrote: > stray line Done. https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... Source/devtools/front_end/elements/StylesSidebarPane.js:3174: * @param {?WebInspector.DOMNode} newNode On 2015/05/29 10:53:12, lushnikov wrote: > protected? Done. https://codereview.chromium.org/1149373004/diff/1/Source/devtools/front_end/e... Source/devtools/front_end/elements/StylesSidebarPane.js:3182: this.element.classList.remove("expanded"); On 2015/05/29 10:53:12, lushnikov wrote: > ? Done.
lgtm given functionality regressions are blessed by @pfeldman! https://codereview.chromium.org/1149373004/diff/40001/Source/devtools/front_e... File Source/devtools/front_end/elements/ElementStatePaneWidget.js (right): https://codereview.chromium.org/1149373004/diff/40001/Source/devtools/front_e... Source/devtools/front_end/elements/ElementStatePaneWidget.js:4: stray line https://codereview.chromium.org/1149373004/diff/40001/Source/devtools/front_e... Source/devtools/front_end/elements/ElementStatePaneWidget.js:107: var node = WebInspector.context.flavor(WebInspector.DOMNode); lets inline https://codereview.chromium.org/1149373004/diff/40001/Source/devtools/front_e... File Source/devtools/front_end/elements/StylesSidebarPane.js (right): https://codereview.chromium.org/1149373004/diff/40001/Source/devtools/front_e... Source/devtools/front_end/elements/StylesSidebarPane.js:3167: willHide: function() @override? https://codereview.chromium.org/1149373004/diff/40001/Source/devtools/front_e... Source/devtools/front_end/elements/StylesSidebarPane.js:3172: wasShown: function() ditto https://codereview.chromium.org/1149373004/diff/40001/Source/devtools/front_e... Source/devtools/front_end/elements/StylesSidebarPane.js:3175: var elementNode = WebInspector.SharedSidebarModel.elementNode(WebInspector.context.flavor(WebInspector.DOMNode)); this._nodeChanged() ?
https://codereview.chromium.org/1149373004/diff/40001/Source/devtools/front_e... File Source/devtools/front_end/elements/ElementStatePaneWidget.js (right): https://codereview.chromium.org/1149373004/diff/40001/Source/devtools/front_e... Source/devtools/front_end/elements/ElementStatePaneWidget.js:4: On 2015/05/29 13:38:47, lushnikov wrote: > stray line Done. https://codereview.chromium.org/1149373004/diff/40001/Source/devtools/front_e... Source/devtools/front_end/elements/ElementStatePaneWidget.js:107: var node = WebInspector.context.flavor(WebInspector.DOMNode); On 2015/05/29 13:38:47, lushnikov wrote: > lets inline Done. https://codereview.chromium.org/1149373004/diff/40001/Source/devtools/front_e... File Source/devtools/front_end/elements/StylesSidebarPane.js (right): https://codereview.chromium.org/1149373004/diff/40001/Source/devtools/front_e... Source/devtools/front_end/elements/StylesSidebarPane.js:3167: willHide: function() On 2015/05/29 13:38:47, lushnikov wrote: > @override? Done. https://codereview.chromium.org/1149373004/diff/40001/Source/devtools/front_e... Source/devtools/front_end/elements/StylesSidebarPane.js:3172: wasShown: function() On 2015/05/29 13:38:47, lushnikov wrote: > ditto Done. https://codereview.chromium.org/1149373004/diff/40001/Source/devtools/front_e... Source/devtools/front_end/elements/StylesSidebarPane.js:3175: var elementNode = WebInspector.SharedSidebarModel.elementNode(WebInspector.context.flavor(WebInspector.DOMNode)); On 2015/05/29 13:38:47, lushnikov wrote: > this._nodeChanged() ? Done.
lgtm given UI approval from @pfeldman https://codereview.chromium.org/1149373004/diff/120001/Source/devtools/front_... File Source/devtools/front_end/elements/ElementStatePaneWidget.js (right): https://codereview.chromium.org/1149373004/diff/120001/Source/devtools/front_... Source/devtools/front_end/elements/ElementStatePaneWidget.js:73: inputs[i].disabled = newNode.pseudoType(); !! https://codereview.chromium.org/1149373004/diff/120001/Source/devtools/front_... File Source/devtools/front_end/elements/StylesSidebarPane.js (right): https://codereview.chromium.org/1149373004/diff/120001/Source/devtools/front_... Source/devtools/front_end/elements/StylesSidebarPane.js:812: if (this.animatedToolbarPane !== undefined) private? https://codereview.chromium.org/1149373004/diff/120001/Source/devtools/front_... Source/devtools/front_end/elements/StylesSidebarPane.js:844: this._toolbarPaneElement.style.animationName = ''; removeProperty https://codereview.chromium.org/1149373004/diff/120001/Source/devtools/front_... Source/devtools/front_end/elements/StylesSidebarPane.js:855: delete this._nextWidget; pendingWidget
https://codereview.chromium.org/1149373004/diff/120001/Source/devtools/front_... File Source/devtools/front_end/elements/ElementStatePaneWidget.js (right): https://codereview.chromium.org/1149373004/diff/120001/Source/devtools/front_... Source/devtools/front_end/elements/ElementStatePaneWidget.js:73: inputs[i].disabled = newNode.pseudoType(); On 2015/06/03 12:10:35, lushnikov wrote: > !! Done. https://codereview.chromium.org/1149373004/diff/120001/Source/devtools/front_... File Source/devtools/front_end/elements/StylesSidebarPane.js (right): https://codereview.chromium.org/1149373004/diff/120001/Source/devtools/front_... Source/devtools/front_end/elements/StylesSidebarPane.js:812: if (this.animatedToolbarPane !== undefined) On 2015/06/03 12:10:36, lushnikov wrote: > private? Done. https://codereview.chromium.org/1149373004/diff/120001/Source/devtools/front_... Source/devtools/front_end/elements/StylesSidebarPane.js:844: this._toolbarPaneElement.style.animationName = ''; On 2015/06/03 12:10:36, lushnikov wrote: > removeProperty Done. https://codereview.chromium.org/1149373004/diff/120001/Source/devtools/front_... Source/devtools/front_end/elements/StylesSidebarPane.js:855: delete this._nextWidget; On 2015/06/03 12:10:36, lushnikov wrote: > pendingWidget Done.
The CQ bit was checked by sergeyv@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from lushnikov@chromium.org Link to the patchset: https://codereview.chromium.org/1149373004/#ps140001 (title: "Address comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1149373004/140001
The CQ bit was unchecked by sergeyv@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from lushnikov@chromium.org Link to the patchset: https://codereview.chromium.org/1149373004/#ps160001 (title: "Fix test")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1149373004/160001
Committed patchset #9 (id:160001) as https://src.chromium.org/viewvc/blink?view=rev&revision=196415
Chromium Code Reviews
