DevTools: Use png icon for bezier editor

DevTools: Use png icon for bezier editor

The bezier icon should use an image asset like our other icons.

Committed: https://crrev.com/5bb6fcbd666d7e2ab1008f98de5828b34908c287
Cr-Commit-Position: refs/heads/master@{#412597}

[flandy, 2016-08-10 22:01:24]

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

[flandy, 2016-08-10 22:01:25]

Please take a look

[lushnikov, 2016-08-10 23:42:13]

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/elements/ColorSwatchPopoverIcon.js
(right):

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/elements/ColorSwatchPopoverIcon.js:38:
var root = WebInspector.createShadowRootWithCoreStyles(this._iconElement,
"ui/bezierSwatch.css");
let's put these three lines by themselves:


WebInspector.BezierSwatch = {}
WebInspector.BezierSwatch.create = function()
{
    ... these three lines...   
}

And the WebInspector.BezierSwatch could go into ColorSwatch.js for simplicity

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/ui/bezierSwatch.css (right):

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/bezierSwatch.css:11:
.bezier-swatch {
maybe also add a :hover, like in color swatch?..

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/bezierSwatch.css:21:
-webkit-mask-image: url(Images/toolbarButtonGlyphs.png);
you'd also need @media (-webkit-min-device-pixel-ratio: 1.5)

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/bezierSwatch.css:23:
-webkit-mask-position: -299px -144px;
let's also add entry in toolbar.css

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/bezierSwatch.css:23:
-webkit-mask-position: -299px -144px;
let's also center the icon inside its cell in svg file

[flandy, 2016-08-11 01:38:00]

Please take another look. This is dependent on the new image asset.

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/elements/ColorSwatchPopoverIcon.js
(right):

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/elements/ColorSwatchPopoverIcon.js:38:
var root = WebInspector.createShadowRootWithCoreStyles(this._iconElement,
"ui/bezierSwatch.css");
On 2016/08/10 23:42:13, lushnikov wrote:
> let's put these three lines by themselves:
> 
> 
> WebInspector.BezierSwatch = {}
> WebInspector.BezierSwatch.create = function()
> {
>     ... these three lines...   
> }
> 
> And the WebInspector.BezierSwatch could go into ColorSwatch.js for simplicity

Done.

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/ui/bezierSwatch.css (right):

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/bezierSwatch.css:11:
.bezier-swatch {
On 2016/08/10 23:42:13, lushnikov wrote:
> maybe also add a :hover, like in color swatch?..

Done.

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/bezierSwatch.css:21:
-webkit-mask-image: url(Images/toolbarButtonGlyphs.png);
On 2016/08/10 23:42:13, lushnikov wrote:
> you'd also need @media (-webkit-min-device-pixel-ratio: 1.5)

Thanks! Realized I forgot this.

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/bezierSwatch.css:23:
-webkit-mask-position: -299px -144px;
On 2016/08/10 23:42:13, lushnikov wrote:
> let's also center the icon inside its cell in svg file

I was planning to put the other swatches around it, but instead let's use a
separate svg file as Dmitry suggested.

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/bezierSwatch.css:23:
-webkit-mask-position: -299px -144px;
On 2016/08/10 23:42:13, lushnikov wrote:
> let's also add entry in toolbar.css

No need anymore?

[lushnikov, 2016-08-11 01:41:08]

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/ui/bezierSwatch.css (right):

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/bezierSwatch.css:23:
-webkit-mask-position: -299px -144px;
On 2016/08/11 01:37:59, flandy wrote:
> On 2016/08/10 23:42:13, lushnikov wrote:
> > let's also center the icon inside its cell in svg file
> 
> I was planning to put the other swatches around it, but instead let's use a
> separate svg file as Dmitry suggested.

yes, lets do smallIcons.svg first

[flandy, 2016-08-12 21:55:08]

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/ui/bezierSwatch.css (right):

https://codereview.chromium.org/2237663002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/bezierSwatch.css:23:
-webkit-mask-position: -299px -144px;
> yes, lets do smallIcons.svg first

Done.

[lushnikov, 2016-08-16 22:56:48]

why leave a free space before bezier icon?

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

https://codereview.chromium.org/2237663002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/ColorSwatchPopoverIcon.js:33:
_createDOM: function(text)
looks like no more reason to have this function. let's just inline it!

[flandy, 2016-08-16 23:53:58]

On 2016/08/16 22:56:48, lushnikov wrote:
> why leave a free space before bezier icon?

I left a space because this is where I put the shadow icon in the other patch.

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

https://codereview.chromium.org/2237663002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/ColorSwatchPopoverIcon.js:33:
_createDOM: function(text)
On 2016/08/16 22:56:48, lushnikov wrote:
> looks like no more reason to have this function. let's just inline it!

Done.

[vinodsonkusare77_gmail.com, 2016-08-17 10:08:01]

https://youtu.be/ZB29OFBS2w8?list=UUWQUZfAdokV83KKjLl-sDeQ

On Wed, Aug 17, 2016 at 5:23 AM, flandy via DevTools Reviews <
devtools-reviews@chromium.org> wrote:

> On 2016/08/16 22:56:48, lushnikov wrote:
> > why leave a free space before bezier icon?
>
> I left a space because this is where I put the shadow icon in the other
> patch.
>
>
> https://codereview.chromium.org/2237663002/diff/40001/
> third_party/WebKit/Source/devtools/front_end/elements/
> ColorSwatchPopoverIcon.js
> File
> third_party/WebKit/Source/devtools/front_end/elements/
> ColorSwatchPopoverIcon.js
> (right):
>
> https://codereview.chromium.org/2237663002/diff/40001/
> third_party/WebKit/Source/devtools/front_end/elements/
> ColorSwatchPopoverIcon.js#newcode33
> third_party/WebKit/Source/devtools/front_end/elements/
> ColorSwatchPopoverIcon.js:33:
> _createDOM: function(text)
> On 2016/08/16 22:56:48, lushnikov wrote:
> > looks like no more reason to have this function. let's just inline it!
>
> Done.
>
> https://codereview.chromium.org/2237663002/
>
> --
> You received this message because you are subscribed to the Google Groups
> "DevTools Reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to devtools-reviews+unsubscribe@chromium.org.
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vinodsonkusare77_gmail.com, 2016-08-17 10:08:02]

https://youtu.be/ZB29OFBS2w8?list=UUWQUZfAdokV83KKjLl-sDeQ

On Wed, Aug 17, 2016 at 5:23 AM, flandy via DevTools Reviews <
devtools-reviews@chromium.org> wrote:

> On 2016/08/16 22:56:48, lushnikov wrote:
> > why leave a free space before bezier icon?
>
> I left a space because this is where I put the shadow icon in the other
> patch.
>
>
> https://codereview.chromium.org/2237663002/diff/40001/
> third_party/WebKit/Source/devtools/front_end/elements/
> ColorSwatchPopoverIcon.js
> File
> third_party/WebKit/Source/devtools/front_end/elements/
> ColorSwatchPopoverIcon.js
> (right):
>
> https://codereview.chromium.org/2237663002/diff/40001/
> third_party/WebKit/Source/devtools/front_end/elements/
> ColorSwatchPopoverIcon.js#newcode33
> third_party/WebKit/Source/devtools/front_end/elements/
> ColorSwatchPopoverIcon.js:33:
> _createDOM: function(text)
> On 2016/08/16 22:56:48, lushnikov wrote:
> > looks like no more reason to have this function. let's just inline it!
>
> Done.
>
> https://codereview.chromium.org/2237663002/
>
> --
> You received this message because you are subscribed to the Google Groups
> "DevTools Reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to devtools-reviews+unsubscribe@chromium.org.
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[vinodsonkusare77_gmail.com, 2016-08-17 10:08:31]

https://youtu.be/s2mEASqcipA?list=UUWQUZfAdokV83KKjLl-sDeQ

On Wed, Aug 17, 2016 at 3:37 PM, Vinod kumar Sonkusare <
vinodsonkusare77@gmail.com> wrote:

> https://youtu.be/ZB29OFBS2w8?list=UUWQUZfAdokV83KKjLl-sDeQ
>
> On Wed, Aug 17, 2016 at 5:23 AM, flandy via DevTools Reviews <
> devtools-reviews@chromium.org> wrote:
>
>> On 2016/08/16 22:56:48, lushnikov wrote:
>> > why leave a free space before bezier icon?
>>
>> I left a space because this is where I put the shadow icon in the other
>> patch.
>>
>>
>> https://codereview.chromium.org/2237663002/diff/40001/third_
>> party/WebKit/Source/devtools/front_end/elements/ColorSwatchPopoverIcon.js
>> File
>> third_party/WebKit/Source/devtools/front_end/elements/ColorS
>> watchPopoverIcon.js
>> (right):
>>
>> https://codereview.chromium.org/2237663002/diff/40001/third_
>> party/WebKit/Source/devtools/front_end/elements/ColorSwatch
>> PopoverIcon.js#newcode33
>> third_party/WebKit/Source/devtools/front_end/elements/ColorS
>> watchPopoverIcon.js:33:
>> _createDOM: function(text)
>> On 2016/08/16 22:56:48, lushnikov wrote:
>> > looks like no more reason to have this function. let's just inline it!
>>
>> Done.
>>
>> https://codereview.chromium.org/2237663002/
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "DevTools Reviews" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to devtools-reviews+unsubscribe@chromium.org.
>>
>
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[vinodsonkusare77_gmail.com, 2016-08-17 10:08:33]

https://youtu.be/s2mEASqcipA?list=UUWQUZfAdokV83KKjLl-sDeQ

On Wed, Aug 17, 2016 at 3:37 PM, Vinod kumar Sonkusare <
vinodsonkusare77@gmail.com> wrote:

> https://youtu.be/ZB29OFBS2w8?list=UUWQUZfAdokV83KKjLl-sDeQ
>
> On Wed, Aug 17, 2016 at 5:23 AM, flandy via DevTools Reviews <
> devtools-reviews@chromium.org> wrote:
>
>> On 2016/08/16 22:56:48, lushnikov wrote:
>> > why leave a free space before bezier icon?
>>
>> I left a space because this is where I put the shadow icon in the other
>> patch.
>>
>>
>> https://codereview.chromium.org/2237663002/diff/40001/third_
>> party/WebKit/Source/devtools/front_end/elements/ColorSwatchPopoverIcon.js
>> File
>> third_party/WebKit/Source/devtools/front_end/elements/ColorS
>> watchPopoverIcon.js
>> (right):
>>
>> https://codereview.chromium.org/2237663002/diff/40001/third_
>> party/WebKit/Source/devtools/front_end/elements/ColorSwatch
>> PopoverIcon.js#newcode33
>> third_party/WebKit/Source/devtools/front_end/elements/ColorS
>> watchPopoverIcon.js:33:
>> _createDOM: function(text)
>> On 2016/08/16 22:56:48, lushnikov wrote:
>> > looks like no more reason to have this function. let's just inline it!
>>
>> Done.
>>
>> https://codereview.chromium.org/2237663002/
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "DevTools Reviews" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to devtools-reviews+unsubscribe@chromium.org.
>>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[lushnikov, 2016-08-17 16:43:33]

thanks, lgtm

[flandy, 2016-08-17 17:16:46]

The CQ bit was checked by flandy@google.com

[commit-bot: I haz the power, 2016-08-17 17:17:01]

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-17 18:42:45]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-17 18:47:10]

Description was changed from

==========
DevTools: Use png icon for bezier editor

The bezier icon should use an image asset like our other icons.
==========

to

==========
DevTools: Use png icon for bezier editor

The bezier icon should use an image asset like our other icons.

Committed: https://crrev.com/5bb6fcbd666d7e2ab1008f98de5828b34908c287
Cr-Commit-Position: refs/heads/master@{#412597}
==========

[commit-bot: I haz the power, 2016-08-17 18:47:11]

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