MD WebUI: Move some settings CSS to shared_style_css

MD WebUI: Move some settings CSS to shared_style_css

This CL makes the separator line and hover color shared for MD.
It also shares the [actionable] styling.

BUG=521040
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/3ec487498ee6035b89e02b07cf03b7725de2ccc3
Cr-Commit-Position: refs/heads/master@{#407217}

[stevenjb, 2016-07-20 21:05:44]

Description was changed from

==========
MD: Move some settings CSS to shared_style_css

This CL makes the separator line and hover color shared for MD.
It also shares the [actionable] styling.

BUG=521040
==========

to

==========
MD: Move some settings CSS to shared_style_css

This CL makes the separator line and hover color shared for MD.
It also shares the [actionable] styling.

BUG=521040
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[stevenjb, 2016-07-20 21:06:17]

Description was changed from

==========
MD: Move some settings CSS to shared_style_css

This CL makes the separator line and hover color shared for MD.
It also shares the [actionable] styling.

BUG=521040
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD WebUI: Move some settings CSS to shared_style_css

This CL makes the separator line and hover color shared for MD.
It also shares the [actionable] styling.

BUG=521040
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[stevenjb, 2016-07-20 21:13:26]

Patchset #2 (id:20001) has been deleted

[stevenjb, 2016-07-20 21:14:13]

stevenjb@chromium.org changed reviewers:
+ dschuyler@chromium.org, michaelpg@chromium.org

[michaelpg, 2016-07-20 23:40:47]

seems sound enough, lgtm

[dschuyler, 2016-07-21 00:04:48]

https://codereview.chromium.org/2169603002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_root_css.html (right):

https://codereview.chromium.org/2169603002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/settings_root_css.html:5: <style
is="custom-style" include="cr-shared-style">
Can this include be done elsewhere?

I see now that the comment above is vague. The intention
here was to have a file that had only variables and no
other items. i.e. if none of the variables were used then
the contents (hmm, 'heaviness'?) of the file is essentially
zero.

https://codereview.chromium.org/2169603002/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/shared_style_css.html (right):

https://codereview.chromium.org/2169603002/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/shared_style_css.html:15: }
Suggestion: the :root only needs to be included
once to affect the whole app/page. Please consider
making a shared_root_css.html (or similar) to hold
the :root variable definitions.

[Dan Beam, 2016-07-21 00:08:46]

dbeam@chromium.org changed reviewers:
+ dbeam@chromium.org

[Dan Beam, 2016-07-21 00:08:47]

don't make :hover easier to do yet

https://codereview.chromium.org/2169603002/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list_item_css.html
(right):

https://codereview.chromium.org/2169603002/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list_item_css.html:23:
[hoverable]:hover {
we recently (yesterday) got the guidance to drop :hover

please don't add more

[Dan Beam, 2016-07-21 00:33:22]

https://codereview.chromium.org/2169603002/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list_item_css.html
(right):

https://codereview.chromium.org/2169603002/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list_item_css.html:23:
[hoverable]:hover {
On 2016/07/21 00:08:46, Dan Beam wrote:
> we recently (yesterday) got the guidance to drop :hover
> 
> please don't add more

https://bugs.chromium.org/p/chromium/issues/detail?id=610630#c14

[michaelpg, 2016-07-21 04:26:51]

https://codereview.chromium.org/2169603002/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/shared_style_css.html (right):

https://codereview.chromium.org/2169603002/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/shared_style_css.html:15: }
On 2016/07/21 00:04:48, dschuyler wrote:
> Suggestion: the :root only needs to be included
> once to affect the whole app/page. Please consider
> making a shared_root_css.html (or similar) to hold
> the :root variable definitions.

I think the comment added on line 4 is reasonable, though. It's not obvious
whether it's slower to import an additional file just for three variables, or to
include a small :root rule in all of our custom style instances when it only
needs to be in one. I'd *guess* there's very little difference, in which case as
long as this rule and the whole file stay small, this seems simpler.

[stevenjb, 2016-07-21 15:46:32]

https://codereview.chromium.org/2169603002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_root_css.html (right):

https://codereview.chromium.org/2169603002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/settings_root_css.html:5: <style
is="custom-style" include="cr-shared-style">
On 2016/07/21 00:04:48, dschuyler wrote:
> Can this include be done elsewhere?
> 
> I see now that the comment above is vague. The intention
> here was to have a file that had only variables and no
> other items. i.e. if none of the variables were used then
> the contents (hmm, 'heaviness'?) of the file is essentially
> zero.

So just to be clear, you would advocate for creating shared_root_css.html now,
and including that here instead?

https://codereview.chromium.org/2169603002/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list_item_css.html
(right):

https://codereview.chromium.org/2169603002/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list_item_css.html:23:
[hoverable]:hover {
On 2016/07/21 00:33:22, Dan Beam wrote:
> On 2016/07/21 00:08:46, Dan Beam wrote:
> > we recently (yesterday) got the guidance to drop :hover
> > 
> > please don't add more
> 
> https://bugs.chromium.org/p/chromium/issues/detail?id=610630#c14

Is there a tracking issue for changing that in Settings? The network menu will
be badly broken without hover effects so we need to allocate some time to change
it (i.e. not here, this is just making the color consistent).

[stevenjb, 2016-07-21 16:58:28]

PTAL

[Dan Beam, 2016-07-21 18:04:16]

https://codereview.chromium.org/2169603002/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list_item_css.html
(right):

https://codereview.chromium.org/2169603002/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list_item_css.html:23:
[hoverable]:hover {
On 2016/07/21 15:46:32, stevenjb wrote:
> On 2016/07/21 00:33:22, Dan Beam wrote:
> > On 2016/07/21 00:08:46, Dan Beam wrote:
> > > we recently (yesterday) got the guidance to drop :hover
> > > 
> > > please don't add more
> > 
> > https://bugs.chromium.org/p/chromium/issues/detail?id=610630#c14
> 
> Is there a tracking issue for changing that in Settings? The network menu will
> be badly broken without hover effects so we need to allocate some time to
change
> it (i.e. not here, this is just making the color consistent).

https://bugs.chromium.org/p/chromium/issues/detail?id=630067

[stevenjb, 2016-07-21 21:17:50]

After discussing online, I went ahead and renamed settings_root_css ->
settings_vars_css.

PTAL

[Dan Beam, 2016-07-22 01:16:45]

dbeam@chromium.org changed reviewers:
- dbeam@chromium.org

[stevenjb, 2016-07-22 16:16:32]

On 2016/07/21 21:17:50, stevenjb wrote:
> After discussing online, I went ahead and renamed settings_root_css ->
> settings_vars_css.
> 
> PTAL

dschuyler@ - ping.

[dschuyler, 2016-07-22 17:46:35]

lgtm

[stevenjb, 2016-07-22 17:48:41]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2016-07-22 17:48:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/2169603002/#ps140001
(title: "Rebase with hover removal changes")

[commit-bot: I haz the power, 2016-07-22 17:49:03]

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-07-22 18:47:22]

Description was changed from

==========
MD WebUI: Move some settings CSS to shared_style_css

This CL makes the separator line and hover color shared for MD.
It also shares the [actionable] styling.

BUG=521040
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD WebUI: Move some settings CSS to shared_style_css

This CL makes the separator line and hover color shared for MD.
It also shares the [actionable] styling.

BUG=521040
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-07-22 18:47:24]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2016-07-22 18:49:11]

Description was changed from

==========
MD WebUI: Move some settings CSS to shared_style_css

This CL makes the separator line and hover color shared for MD.
It also shares the [actionable] styling.

BUG=521040
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD WebUI: Move some settings CSS to shared_style_css

This CL makes the separator line and hover color shared for MD.
It also shares the [actionable] styling.

BUG=521040
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/3ec487498ee6035b89e02b07cf03b7725de2ccc3
Cr-Commit-Position: refs/heads/master@{#407217}
==========

[commit-bot: I haz the power, 2016-07-22 18:49:13]

Patchset 7 (id:??) landed as
https://crrev.com/3ec487498ee6035b89e02b07cf03b7725de2ccc3
Cr-Commit-Position: refs/heads/master@{#407217}