pfeldman@chromium.org changed reviewers: + dgozman@chromium.org
http://grab.by/I03C
http://grab.by/I03I
This patch asks to remove material experiment. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/emulation/responsiveDesignView.css (right): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/emulation/responsiveDesignView.css:320: /* border-right: 2px solid rgb(0, 0, 0); */ commented css https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/main/module.json (right): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/main/module.json:163: "order": 100, 100 is for close button. 99? https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/main/module.json:164: "actionId": "main.toggle-element-search" Remove actionId. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/main/module.json:174: "className": "WebInspector.Main.WarningErrorCounter", Why swapped with toggle drawer? https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/sidebarPane.css (right): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/sidebarPane.css:75: /* border-top: none; */ commented css https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/sources/sourcesPanel.css (right): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/sources/sourcesPanel.css:39: height: 28px; You've removed material from inspectorViewToolbar, and left it here. Let's align. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/sources/sourcesPanel.css:44: overflow: hidden; merge into first rule https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/timeline/timelinePanel.css (right): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/timeline/timelinePanel.css:31: border-bottom: 1px solid #f3f3f3; Border of the same color as background? Remove it then? https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/ui/Toolbar.js (right): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/ui/Toolbar.js:113: this._items[i].setVisible(!previousIsSeparator); What if I explicitly call setVisible on a separator? https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/ui/toolbar.css (left): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/ui/toolbar.css:95: .narrow .long-click-glyph { Remove makeNarrow. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/ui/toolbar.css (right): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/ui/toolbar.css:9: padding: 0 2px; This may strike when two toolbars are near each other. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/ui/toolbar.css:49: border: none; Two "border" declarations in the same rule. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/ui/toolbar.css:95: .toolbar-item.checkbox:hover, Should this be checkbox:enabled?
https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/emulation/responsiveDesignView.css (right): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/emulation/responsiveDesignView.css:320: /* border-right: 2px solid rgb(0, 0, 0); */ On 2015/06/11 16:14:11, dgozman wrote: > commented css Done. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/main/module.json (right): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/main/module.json:163: "order": 100, On 2015/06/11 16:14:12, dgozman wrote: > 100 is for close button. 99? These are different toolbars. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/main/module.json:164: "actionId": "main.toggle-element-search" On 2015/06/11 16:14:12, dgozman wrote: > Remove actionId. Done. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/main/module.json:174: "className": "WebInspector.Main.WarningErrorCounter", On 2015/06/11 16:14:11, dgozman wrote: > Why swapped with toggle drawer? Done. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/sidebarPane.css (right): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/sidebarPane.css:75: /* border-top: none; */ On 2015/06/11 16:14:12, dgozman wrote: > commented css Done. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/sources/sourcesPanel.css (right): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/sources/sourcesPanel.css:39: height: 28px; On 2015/06/11 16:14:12, dgozman wrote: > You've removed material from inspectorViewToolbar, and left it here. Let's > align. Done. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/sources/sourcesPanel.css:44: overflow: hidden; On 2015/06/11 16:14:12, dgozman wrote: > merge into first rule Done. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/timeline/timelinePanel.css (right): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/timeline/timelinePanel.css:31: border-bottom: 1px solid #f3f3f3; On 2015/06/11 16:14:12, dgozman wrote: > Border of the same color as background? Remove it then? Those are different elements. Timeline toolbar is white. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/ui/Toolbar.js (right): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/ui/Toolbar.js:113: this._items[i].setVisible(!previousIsSeparator); On 2015/06/11 16:14:12, dgozman wrote: > What if I explicitly call setVisible on a separator? stack overflow! https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... File Source/devtools/front_end/ui/toolbar.css (right): https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/ui/toolbar.css:9: padding: 0 2px; On 2015/06/11 16:14:12, dgozman wrote: > This may strike when two toolbars are near each other. That's probably fine for now? https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/ui/toolbar.css:49: border: none; On 2015/06/11 16:14:12, dgozman wrote: > Two "border" declarations in the same rule. Done. https://codereview.chromium.org/1176343002/diff/20001/Source/devtools/front_e... Source/devtools/front_end/ui/toolbar.css:95: .toolbar-item.checkbox:hover, Input is even deeper there + we don't disable them (yet?!)
lgtm
The CQ bit was checked by pfeldman@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1176343002/60001
Committed patchset #4 (id:60001) as https://src.chromium.org/viewvc/blink?view=rev&revision=196980
Chromium Code Reviews
