DevTools: cleanup button styles

DevTools: cleanup button styles

- Migrates some buttons to be made with UI.createTextButton
- Use the same shade of 'blue' color on active buttons

This CL also prepares the way for new button styles:
https://codereview.chromium.org/2881453003

BUG=690239
Review-Url: https://codereview.chromium.org/2912563003
Cr-Commit-Position: refs/heads/master@{#476752}
Committed: https://chromium.googlesource.com/chromium/src/+/b73ea5c134104f1d27512c58ac88f60b7cdd889e

[luoe, 2017-05-26 23:37:08]

Patchset #1 (id:1) has been deleted

[luoe, 2017-05-26 23:40:23]

Description was changed from

==========
DevTools: cleanup button styles

BUG=690239
==========

to

==========
DevTools: cleanup button styles

- Migrates some buttons to be made with UI.createTextButton
- Use the same shade of 'blue' color on active buttons

This CL also prepares the way for new button styles:
https://codereview.chromium.org/2881453003

BUG=690239
==========

[luoe, 2017-05-26 23:41:05]

luoe@chromium.org changed reviewers:
+ dgozman@chromium.org, einbinder@chromium.org, pfeldman@chromium.org

[luoe, 2017-05-26 23:41:06]

To make things easier to review, this CL was split out, and only affects
buttons.

Please take a look

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/devices/devicesView.css
(right):

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/devices/devicesView.css:357: }
Without this, you can get the button to shrink.

[pfeldman, 2017-05-30 21:18:17]

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/devices/devicesView.css
(right):

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/devices/devicesView.css:357: }
On 2017/05/26 23:41:06, luoe wrote:
> Without this, you can get the button to shrink.

It should be done for all buttons thought, no?

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/profiler/profilesPanel.css
(right):

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/profiler/profilesPanel.css:203:
button {
We might have other buttons inside profiles panel that this change would affect.
Why is this changing?

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

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/searchableView.css:81:
min-height: 20px !important;
What else is setting min-height? Can we resolve the conflict?

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/searchableView.css:83: padding:
0;
ditto

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/toolbar.css (left):

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/toolbar.css:140:
background-color: hsl(218, 81%, 59%);
This was kind-of nice, hinting us at the fact that the hue and saturation were
the same with just the lightness changing. Why is this changing?

[luoe, 2017-05-30 22:39:45]

Ptal

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/devices/devicesView.css
(right):

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/devices/devicesView.css:357: }
On 2017/05/30 21:18:17, pfeldman wrote:
> On 2017/05/26 23:41:06, luoe wrote:
> > Without this, you can get the button to shrink.
> 
> It should be done for all buttons thought, no?

Yes!  Done.

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/profiler/profilesPanel.css
(right):

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/profiler/profilesPanel.css:203:
button {
On 2017/05/30 21:18:17, pfeldman wrote:
> We might have other buttons inside profiles panel that this change would
affect.
> Why is this changing?

I want these margin/width styles to apply to both buttons.  Perhaps it's better
to add 'profile-launcher-button' class to both for specificity.

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

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/searchableView.css:81:
min-height: 20px !important;
On 2017/05/30 21:18:17, pfeldman wrote:
> What else is setting min-height? Can we resolve the conflict?

Done.  We can resolve this by defining `height` for all buttons in
textButton.css.

Before, we put the same !important min-height in textButton.css to override the
universal selector in inspectorCommon.css that gives everything min-height: 0.
https://codereview.chromium.org/1085603003

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/searchableView.css:83: padding:
0;
On 2017/05/30 21:18:17, pfeldman wrote:
> ditto

Done.  We can avoid this by letting these buttons use the default button
padding.  The width will adjust, accordingly.

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/toolbar.css (left):

https://codereview.chromium.org/2912563003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/toolbar.css:140:
background-color: hsl(218, 81%, 59%);
On 2017/05/30 21:18:17, pfeldman wrote:
> This was kind-of nice, hinting us at the fact that the hue and saturation were
> the same with just the lightness changing. Why is this changing?

The only reason is to be more consistent and use fewer shades of 'blue'.  I
think a better solution would involve CSS variables, say --devtools-blue and
--devtools-lighter-blue.  But that seems out of scope for this CL.

I'll revert these changes.

https://codereview.chromium.org/2912563003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/help/releaseNote.css (right):

https://codereview.chromium.org/2912563003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/help/releaseNote.css:78:
box-shadow: none;
I realized that converting these buttons to UI.createTextButton's now leaves a
box-shadow.  This hides it until the followup CL.

[einbinder, 2017-05-31 21:20:11]

https://codereview.chromium.org/2912563003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/SearchableView.js (left):

https://codereview.chromium.org/2912563003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/SearchableView.js:131: var
uniqueId = ++UI.SearchableView._lastUniqueId;
What was this doing? Looks interesting.

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

https://codereview.chromium.org/2912563003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:1155: * @param
{string=} title
Not against this, just curious why remove title?

[pfeldman, 2017-05-31 21:40:37]

lgtm

https://codereview.chromium.org/2912563003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/help/releaseNote.css (right):

https://codereview.chromium.org/2912563003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/help/releaseNote.css:78:
box-shadow: none;
Is there anything we need to do on the branch?

[luoe, 2017-05-31 21:42:50]

Thanks for the reviews!

https://codereview.chromium.org/2912563003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/help/releaseNote.css (right):

https://codereview.chromium.org/2912563003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/help/releaseNote.css:78:
box-shadow: none;
On 2017/05/31 21:40:36, pfeldman wrote:
> Is there anything we need to do on the branch?

Nope!  I haven't landed any other harmony/cleanup-related style changes.

https://codereview.chromium.org/2912563003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/SearchableView.js (left):

https://codereview.chromium.org/2912563003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/SearchableView.js:131: var
uniqueId = ++UI.SearchableView._lastUniqueId;
On 2017/05/31 21:20:10, einbinder wrote:
> What was this doing? Looks interesting.

It looks like there used to be a bug when multiple SearchableViews were shown on
the screen, and clicking 'Replace' on one would affect another, and this logic
ensured uniqueness (https://codereview.chromium.org/232133008).

At the time, SearchableView made its own checkbox, and then later migrated to
the UIUtils component (https://codereview.chromium.org/805853002).

Todays UIUtils.CheckboxLabel already has unique, incrementing IDs built in as of
(https://codereview.chromium.org/2776263006), so these lines are old.

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

https://codereview.chromium.org/2912563003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:1155: * @param
{string=} title
On 2017/05/31 21:20:10, einbinder wrote:
> Not against this, just curious why remove title?

I plan to add a 'primary' boolean param in the followup CL
(https://codereview.chromium.org/2881453003), and didn't want the number of
params to balloon. 
 Since `title` is only used in one site, and no other UIUtils component takes it
as a param, I'd rather take it out.

[luoe, 2017-06-01 21:14:01]

The CQ bit was checked by luoe@chromium.org

[commit-bot: I haz the power, 2017-06-01 21:14:43]

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, 2017-06-01 22:24:32]

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

[commit-bot: I haz the power, 2017-06-01 22:24:34]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[luoe, 2017-06-01 23:42:27]

The CQ bit was checked by luoe@chromium.org

[luoe, 2017-06-01 23:42:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/2912563003/#ps60001
(title: "fix test")

[commit-bot: I haz the power, 2017-06-01 23:43:13]

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, 2017-06-02 01:30:15]

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

[commit-bot: I haz the power, 2017-06-02 01:30:16]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[luoe, 2017-06-02 01:44:59]

The CQ bit was checked by luoe@chromium.org

[commit-bot: I haz the power, 2017-06-02 01:45:37]

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, 2017-06-02 01:45:40]

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

[commit-bot: I haz the power, 2017-06-02 01:45:41]

CQ cannot get SignCLA result. Please try later.

[luoe, 2017-06-02 17:30:54]

The CQ bit was checked by luoe@chromium.org

[commit-bot: I haz the power, 2017-06-02 17:31:06]

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, 2017-06-02 19:28:17]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1496424654828350,
"parent_rev": "cf255632485f5f62553bbc8ca434e116c7d60f14", "commit_rev":
"b73ea5c134104f1d27512c58ac88f60b7cdd889e"}

[commit-bot: I haz the power, 2017-06-02 19:28:26]

Description was changed from

==========
DevTools: cleanup button styles

- Migrates some buttons to be made with UI.createTextButton
- Use the same shade of 'blue' color on active buttons

This CL also prepares the way for new button styles:
https://codereview.chromium.org/2881453003

BUG=690239
==========

to

==========
DevTools: cleanup button styles

- Migrates some buttons to be made with UI.createTextButton
- Use the same shade of 'blue' color on active buttons

This CL also prepares the way for new button styles:
https://codereview.chromium.org/2881453003

BUG=690239

Review-Url: https://codereview.chromium.org/2912563003
Cr-Commit-Position: refs/heads/master@{#476752}
Committed:
https://chromium.googlesource.com/chromium/src/+/b73ea5c134104f1d27512c58ac88...
==========

[commit-bot: I haz the power, 2017-06-02 19:28:33]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/b73ea5c134104f1d27512c58ac88...