DevTools: Box-shadow editor initial implementation

DevTools: Box-shadow editor initial implementation

This is dependent on the model in https://codereview.chromium.org/2230183004/
and will be changed accordingly.

BUG=639095
Committed: https://crrev.com/325c00a303eb2074a76f861cb84787bb37e6e5dd
Cr-Commit-Position: refs/heads/master@{#414575}

[flandy, 2016-08-16 23:26:41]

Patchset #1 (id:1) has been deleted

[flandy, 2016-08-16 23:32:19]

flandy@google.com changed reviewers:
+ dgozman@chromium.org, lushnikov@chromium.org

[flandy, 2016-08-16 23:32:20]

So far I've added some basic sliders and text inputs.

I just wanted to get some feedback on the overall direction for creating the
editor and UI components.

Please take a look when convenient. Thanks!

[lushnikov, 2016-08-17 17:07:21]

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js (right):

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:13: this._shadow
= shadow;
this._model = ..

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:15:
this.registerRequiredCSS("ui/shadowEditor.css");
this should go as the first line after super-call

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:30: field =
this.contentElement.createChild("div", "shadow-editor-field");
these look strikingly similar. Maybe it's possible to make a helper function
with parameters which does just this?

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:41:
WebInspector.installDragHandle(slider, this._dragStart.bind(this),
this._dragX.bind(this), this._dragX.bind(this), "default");
can you please attach a screenshot - it's hard to
figure out what it will look like

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:54:
WebInspector.installDragHandle(slider, this._dragStart.bind(this),
this._dragY.bind(this), this._dragY.bind(this), "default");
Why not listen to default slider changed events?

[dgozman, 2016-08-17 17:42:09]

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js (right):

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:98:
this._sliderTrackWidth = this._blurSliderTrack.offsetWidth;
Should we use measureElement instead?

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:118: if
(this._type === "box-shadow")
We should either make "box-shadow" a named constant, or have a boolean flag
"_isBoxShadow" set in constructor.

Or even better, we can add "hasSpread()" method to the model instead of working
with type.

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:190:
_dragSpread: function(event)
I'm curious why default slider handling doesn't work here.

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/shadowEditor.css (right):

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/shadowEditor.css:22:
font-family: Menlo, Consolas, 'dejavu sans mono', monospace;
Fonts are usually OS-specific? Do we really need anything custom here?

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/shadowEditor.css:44:
/*line-height: 22px;*/
Commented code.

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/shadowEditor.css:56:
/*line-height: 16px;*/
Commented code.

[flandy, 2016-08-24 17:52:32]

Description was changed from

==========
DevTools: Box-shadow editor initial implementation

This is dependent on the model in https://codereview.chromium.org/2230183004/
and will be changed accordingly.

Do not push.
==========

to

==========
DevTools: Box-shadow editor initial implementation

This is dependent on the model in https://codereview.chromium.org/2230183004/
and will be changed accordingly.

BUG=639095
==========

[flandy, 2016-08-24 18:10:07]

Please take a look. See bug for screenshot

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js (right):

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:13: this._shadow
= shadow;
On 2016/08/17 17:07:21, lushnikov wrote:
> this._model = ..

Done.

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:15:
this.registerRequiredCSS("ui/shadowEditor.css");
On 2016/08/17 17:07:21, lushnikov wrote:
> this should go as the first line after super-call

Done.

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:30: field =
this.contentElement.createChild("div", "shadow-editor-field");
On 2016/08/17 17:07:21, lushnikov wrote:
> these look strikingly similar. Maybe it's possible to make a helper function
> with parameters which does just this?

Done.

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:41:
WebInspector.installDragHandle(slider, this._dragStart.bind(this),
this._dragX.bind(this), this._dragX.bind(this), "default");
On 2016/08/17 17:07:21, lushnikov wrote:
> can you please attach a screenshot - it's hard to
> figure out what it will look like

Done.

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:54:
WebInspector.installDragHandle(slider, this._dragStart.bind(this),
this._dragY.bind(this), this._dragY.bind(this), "default");
On 2016/08/17 17:07:21, lushnikov wrote:
> Why not listen to default slider changed events?

Done.

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:98:
this._sliderTrackWidth = this._blurSliderTrack.offsetWidth;
On 2016/08/17 17:42:09, dgozman wrote:
> Should we use measureElement instead?

Removed.

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:118: if
(this._type === "box-shadow")
On 2016/08/17 17:42:09, dgozman wrote:
> We should either make "box-shadow" a named constant, or have a boolean flag
> "_isBoxShadow" set in constructor.
> 
> Or even better, we can add "hasSpread()" method to the model instead of
working
> with type.

Let's hide fields that are not needed.

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ShadowEditor.js:190:
_dragSpread: function(event)
On 2016/08/17 17:42:09, dgozman wrote:
> I'm curious why default slider handling doesn't work here.

Changed to use a default range input with custom styling.

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/shadowEditor.css (right):

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/shadowEditor.css:22:
font-family: Menlo, Consolas, 'dejavu sans mono', monospace;
On 2016/08/17 17:42:09, dgozman wrote:
> Fonts are usually OS-specific? Do we really need anything custom here?

Removed.

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/shadowEditor.css:44:
/*line-height: 22px;*/
On 2016/08/17 17:42:09, dgozman wrote:
> Commented code.

Removed.

https://codereview.chromium.org/2252913002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/shadowEditor.css:56:
/*line-height: 16px;*/
On 2016/08/17 17:42:09, dgozman wrote:
> Commented code.

Removed.

[dgozman, 2016-08-24 19:18:31]

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js
(right):

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2030:
var shadows, isBoxShadow;
style: one variable per declaration

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js (right):

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:20:
label.textContent = WebInspector.UIString("Type");
Inline |label| here.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:21:
this._outsetButton = this._typeField.createChild("button", "shadow-editor-button
left");
"left" is too generic for css class. Let's do shadow-editor-button-left

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:25:
this._insetButton.createTextChild(WebInspector.UIString("Inset"));
Just textContent = ... ?

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:29: inputs =
this._createSliderField("X offset", true);
Always UIString the literal, do not pass it raw.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:67:
label.htmlFor = propertyName;
Use setAttribute.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:71:
textInput.addEventListener("input", this._onTextInput.bind(this), true);
Why do we capture all events?

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:74: var
slider = negativeAllowed ? createSliderLabel(halfRange * -1, halfRange) :
createSliderLabel(0, WebInspector.CSSShadowEditor.maxRange);
-halfRange

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:80:
hideBoxShadowProperties: function()
Isn't this class reused? We should.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:82:
this._typeField.hidden = true;
Figure that out from this._model.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:134:
this._model.setOffset(length, this._model.offsetY());
Why don't we have separate setOffsetX and setOffsetY?

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:141:
this._updateSliders();
Let's update them separately as we do for inputs.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:149: var
length = this._changedElement.value === "" ? WebInspector.CSSLength.zero() :
WebInspector.CSSLength.parse(this._changedElement.value);
x === ""   should be  !x

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ColorSwatch.js (right):

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ColorSwatch.js:205:
cssShadowModel: function()
model

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right):

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:1561:
root.createChild("content");
You only need content if you append children (e.g. label) to the slider.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/slider.css (right):

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/slider.css:39: box-shadow: 0 0 0
2px rgba(66, 133, 244, 0.4);
style: indentation

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/slider.css:43: background-color:
#bdbdbd;
style: indentation

[flandy, 2016-08-24 22:08:42]

Please take another look

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js
(right):

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/StylesSidebarPane.js:2030:
var shadows, isBoxShadow;
On 2016/08/24 19:18:30, dgozman wrote:
> style: one variable per declaration

Done.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js (right):

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:20:
label.textContent = WebInspector.UIString("Type");
On 2016/08/24 19:18:30, dgozman wrote:
> Inline |label| here.

Done.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:21:
this._outsetButton = this._typeField.createChild("button", "shadow-editor-button
left");
On 2016/08/24 19:18:30, dgozman wrote:
> "left" is too generic for css class. Let's do shadow-editor-button-left

Done.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:25:
this._insetButton.createTextChild(WebInspector.UIString("Inset"));
On 2016/08/24 19:18:30, dgozman wrote:
> Just textContent = ... ?

Done.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:29: inputs =
this._createSliderField("X offset", true);
On 2016/08/24 19:18:30, dgozman wrote:
> Always UIString the literal, do not pass it raw.

I called UIString in the function, but I suppose I can call it here instead.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:67:
label.htmlFor = propertyName;
On 2016/08/24 19:18:30, dgozman wrote:
> Use setAttribute.

Done.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:71:
textInput.addEventListener("input", this._onTextInput.bind(this), true);
On 2016/08/24 19:18:30, dgozman wrote:
> Why do we capture all events?

We want any valid changes to be reflected immediately in the style panel and the
web page. However, if the change is invalid, we don't want to translate the
value until the text input blurs.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:74: var
slider = negativeAllowed ? createSliderLabel(halfRange * -1, halfRange) :
createSliderLabel(0, WebInspector.CSSShadowEditor.maxRange);
On 2016/08/24 19:18:30, dgozman wrote:
> -halfRange

Done.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:80:
hideBoxShadowProperties: function()
On 2016/08/24 19:18:30, dgozman wrote:
> Isn't this class reused? We should.

Is CSSShadowEditor re-used for text-shadows? Yes.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:82:
this._typeField.hidden = true;
On 2016/08/24 19:18:30, dgozman wrote:
> Figure that out from this._model.

Done.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:134:
this._model.setOffset(length, this._model.offsetY());
On 2016/08/24 19:18:30, dgozman wrote:
> Why don't we have separate setOffsetX and setOffsetY?

Done.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:141:
this._updateSliders();
On 2016/08/24 19:18:30, dgozman wrote:
> Let's update them separately as we do for inputs.

Done.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:149: var
length = this._changedElement.value === "" ? WebInspector.CSSLength.zero() :
WebInspector.CSSLength.parse(this._changedElement.value);
On 2016/08/24 19:18:30, dgozman wrote:
> x === ""   should be  !x

Done.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ColorSwatch.js (right):

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ColorSwatch.js:205:
cssShadowModel: function()
On 2016/08/24 19:18:30, dgozman wrote:
> model

Done.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right):

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:1561:
root.createChild("content");
On 2016/08/24 19:18:31, dgozman wrote:
> You only need content if you append children (e.g. label) to the slider.

Removed.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/slider.css (right):

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/slider.css:39: box-shadow: 0 0 0
2px rgba(66, 133, 244, 0.4);
On 2016/08/24 19:18:31, dgozman wrote:
> style: indentation

Done.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/slider.css:43: background-color:
#bdbdbd;
On 2016/08/24 19:18:31, dgozman wrote:
> style: indentation

Done.

[dgozman, 2016-08-24 23:26:59]

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js (right):

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:71:
textInput.addEventListener("input", this._onTextInput.bind(this), true);
On 2016/08/24 22:08:41, flandy wrote:
> On 2016/08/24 19:18:30, dgozman wrote:
> > Why do we capture all events?
> 
> We want any valid changes to be reflected immediately in the style panel and
the
> web page. However, if the change is invalid, we don't want to translate the
> value until the text input blurs.

I was talking about last parameter being |true|. Should be |false| unless you
have reasons.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:80:
hideBoxShadowProperties: function()
On 2016/08/24 22:08:41, flandy wrote:
> On 2016/08/24 19:18:30, dgozman wrote:
> > Isn't this class reused? We should.
> 
> Is CSSShadowEditor re-used for text-shadows? Yes.

What I meant is that creating an editor every time is unnecessary. Let's create
it once in SSP (lazily), and just call setModel on it before editing. Kind of
similar to spectrum.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right):

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:1537:
root.createChild("content");
Removed content in wrong element.

[flandy, 2016-08-25 00:17:47]

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js (right):

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:71:
textInput.addEventListener("input", this._onTextInput.bind(this), true);
On 2016/08/24 23:26:58, dgozman wrote:
> On 2016/08/24 22:08:41, flandy wrote:
> > On 2016/08/24 19:18:30, dgozman wrote:
> > > Why do we capture all events?
> > 
> > We want any valid changes to be reflected immediately in the style panel and
> the
> > web page. However, if the change is invalid, we don't want to translate the
> > value until the text input blurs.
> 
> I was talking about last parameter being |true|. Should be |false| unless you
> have reasons.

Oh I see, done!

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:80:
hideBoxShadowProperties: function()
On 2016/08/24 23:26:58, dgozman wrote:
> On 2016/08/24 22:08:41, flandy wrote:
> > On 2016/08/24 19:18:30, dgozman wrote:
> > > Isn't this class reused? We should.
> > 
> > Is CSSShadowEditor re-used for text-shadows? Yes.
> 
> What I meant is that creating an editor every time is unnecessary. Let's
create
> it once in SSP (lazily), and just call setModel on it before editing. Kind of
> similar to spectrum.

Thanks for clarifying. Done.

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right):

https://codereview.chromium.org/2252913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:1537:
root.createChild("content");
On 2016/08/24 23:26:59, dgozman wrote:
> Removed content in wrong element.

Sorry. Fixed.

[dgozman, 2016-08-25 17:36:21]

lgtm

[flandy, 2016-08-25 20:52:38]

The CQ bit was checked by flandy@google.com

[commit-bot: I haz the power, 2016-08-25 20:53:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-25 20:57:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-25 20:57:30]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[flandy, 2016-08-25 21:34:39]

The CQ bit was checked by flandy@google.com

[flandy, 2016-08-25 21:34:39]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2252913002/#ps100001
(title: "Merge")

[commit-bot: I haz the power, 2016-08-25 21:35:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-25 23:08:03]

Description was changed from

==========
DevTools: Box-shadow editor initial implementation

This is dependent on the model in https://codereview.chromium.org/2230183004/
and will be changed accordingly.

BUG=639095
==========

to

==========
DevTools: Box-shadow editor initial implementation

This is dependent on the model in https://codereview.chromium.org/2230183004/
and will be changed accordingly.

BUG=639095
==========

[commit-bot: I haz the power, 2016-08-25 23:08:04]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-08-25 23:09:45]

Description was changed from

==========
DevTools: Box-shadow editor initial implementation

This is dependent on the model in https://codereview.chromium.org/2230183004/
and will be changed accordingly.

BUG=639095
==========

to

==========
DevTools: Box-shadow editor initial implementation

This is dependent on the model in https://codereview.chromium.org/2230183004/
and will be changed accordingly.

BUG=639095

Committed: https://crrev.com/325c00a303eb2074a76f861cb84787bb37e6e5dd
Cr-Commit-Position: refs/heads/master@{#414575}
==========

[commit-bot: I haz the power, 2016-08-25 23:09:46]

Patchset 5 (id:??) landed as
https://crrev.com/325c00a303eb2074a76f861cb84787bb37e6e5dd
Cr-Commit-Position: refs/heads/master@{#414575}