MD Settings: make custom UIs look "actionable"

MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>

FYI: This CL is semi-automatedly generated via:

  vim -p `git grep -l on-tap -- chrome/browser/resources/settings`

and lots of hacking.

R=dpapad@chromium.org
BUG=611826
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/a298aa9c3276b78073a61613787909a0b2427302
Committed: https://crrev.com/ed68883804c25965c677a99278d1ab4c2d98af96
Cr-Original-Commit-Position: refs/heads/master@{#401763}
Cr-Commit-Position: refs/heads/master@{#402042}

[Dan Beam, 2016-06-23 19:19:31]

Description was changed from

==========
MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>

R=tommycli@chromium.org
BUG=611826
==========

to

==========
MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>


FYI: This CL is semi-automatedly generated via:

  vi -p `git grep -l on-tap -- chrome/browser/resources/settings`

and lots of hacking.


R=dpapad@chromium.org
BUG=611826
==========

[Dan Beam, 2016-06-23 19:19:31]

dbeam@chromium.org changed reviewers:
+ dpapad@chromium.org
- tommycli@chromium.org

[Dan Beam, 2016-06-23 19:19:48]

Description was changed from

==========
MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>


FYI: This CL is semi-automatedly generated via:

  vi -p `git grep -l on-tap -- chrome/browser/resources/settings`

and lots of hacking.


R=dpapad@chromium.org
BUG=611826
==========

to

==========
MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>


FYI: This CL is semi-automatedly generated via:

  vim -p `git grep -l on-tap -- chrome/browser/resources/settings`

and lots of hacking.


R=dpapad@chromium.org
BUG=611826
==========

[Dan Beam, 2016-06-23 19:20:01]

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/about_page/about_page.html (right):

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/about_page/about_page.html:30: margin-top:
auto;
presubmit made me do it (it was fixed to start scanning <style.*> instead of
<style> recently[1], so it's finding a few things in the CL)

[1] https://codereview.chromium.org/2073343003/

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/about_page/about_page.html:45:
iron-icon[icon='settings:error'] {
both presubmit

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/internet_page/network_proxy_exclusions.html
(right):

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy_exclusions.html:3:
<link rel="import" href="/settings_root_css.html">
this particular file didn't use settings-shared already, so just using the
--variable is [arguably] lighter

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/people_page/people_page.html (left):

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/people_page/people_page.html:51: </if>
redundant

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/people_page/people_page.html:90: <if expr="not
chromeos">
talked with tommycli@: this was reversed (as I suspected)

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/people_page/people_page.html (right):

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/people_page/people_page.html:51:
iron-icon[icon='settings:sync-problem'] {
presubmit

[dpapad, 2016-06-23 20:51:45]

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/appearance_page/appearance_page.html
(right):

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/appearance_page/appearance_page.html:55:
<paper-button id="resetTheme" on-tap="resetTheme_" actionable
Given that paper-buttons are by default having the correct cursor, do we need
this? Or are you adding it just for consistency? Personally I would be fine with
only adding it in divs with on-tap (or in general elements that don't already
have the correct cursor).

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.html
(right):

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.html:39:
<!-- TODO(dbeam): this is not [actionable] because it does nothing. -->
This TODO does not state explicitly what needs to be done. Can you elaborate a
bit.

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html
(right):

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html:51:
<div id="menuEditAddress" class="list-item menu-item" actionable
We could also add the @apply(--settings-actionable) at .list-item menu-item CSS
selector instead. Perhaps better since it would only be in one place VS four?

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html
(right):

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html:92:
<div id="menuEditPassword" class="list-item menu-item" actionable
Same question/suggestion here.

[Dan Beam, 2016-06-23 21:28:00]

Description was changed from

==========
MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>


FYI: This CL is semi-automatedly generated via:

  vim -p `git grep -l on-tap -- chrome/browser/resources/settings`

and lots of hacking.


R=dpapad@chromium.org
BUG=611826
==========

to

==========
MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>


FYI: This CL is semi-automatedly generated via:

  vim -p `git grep -l on-tap -- chrome/browser/resources/settings`

and lots of hacking.


R=dpapad@chromium.org
BUG=611826
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2016-06-23 21:28:03]

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/appearance_page/appearance_page.html
(right):

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/appearance_page/appearance_page.html:55:
<paper-button id="resetTheme" on-tap="resetTheme_" actionable
On 2016/06/23 20:51:45, dpapad wrote:
> Given that paper-buttons are by default having the correct cursor, do we need
> this? Or are you adding it just for consistency? Personally I would be fine
with
> only adding it in divs with on-tap (or in general elements that don't already
> have the correct cursor).

no, this is wrong.  Reverted.

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.html
(right):

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.html:39:
<!-- TODO(dbeam): this is not [actionable] because it does nothing. -->
On 2016/06/23 20:51:45, dpapad wrote:
> This TODO does not state explicitly what needs to be done. Can you elaborate a
> bit.

dropped "TODO"

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html
(right):

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html:51:
<div id="menuEditAddress" class="list-item menu-item" actionable
On 2016/06/23 20:51:45, dpapad wrote:
> We could also add the @apply(--settings-actionable) at .list-item menu-item
CSS
> selector instead. Perhaps better since it would only be in one place VS four?

guess that's OK with me.  i'll apply this logic to site settings as well

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html
(right):

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html:92:
<div id="menuEditPassword" class="list-item menu-item" actionable
On 2016/06/23 20:51:45, dpapad wrote:
> Same question/suggestion here.

Done.

[dpapad, 2016-06-23 22:07:46]

LGTM overall. Some comments along the lines of previous batch of comments, and a
question.

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html
(right):

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html:51:
<div id="menuEditAddress" class="list-item menu-item" actionable
On 2016/06/23 at 21:28:03, Dan Beam wrote:
> On 2016/06/23 20:51:45, dpapad wrote:
> > We could also add the @apply(--settings-actionable) at .list-item menu-item
CSS
> > selector instead. Perhaps better since it would only be in one place VS
four?
> 
> guess that's OK with me.  i'll apply this logic to site settings as well

password_section.html could also use similar method.

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.html
(right):

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.html:41:
<div hidden$="[[!device.paired]]" on-tap="doNothing_">
Instead of calling e.stopPropagation() inside doNothing_, could we add
"pointer-events: none"  in CSS? If that works then we can also remove this
comment.

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/internet_page/network_proxy_exclusions.html
(right):

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy_exclusions.html:12:
iron-icon {
Wondering what is the difference of using iron-icon with the pointer cursor
instead of using paper-icon-button. If the former is better for performance,
perhaps we should migrate all paper-icon-buttons to iron-icons? If there are
other reasons to use paper-icon-buttons (ripple effect?) maybe this one should
be paper-icon-button too.

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/languages_page.html
(right):

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/languages_page.html:48: <div
class$="list-item [[getLanguageItemClass_(
Similarly to menu-item, this could be done as follows,

.list-item {
 --@apply(--settings-actionable);
}

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/on_startup_page/startup_urls_page.html
(right):

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/on_startup_page/startup_urls_page.html:24:
<div class="list-item list-button" id="useCurrentPages" actionable
list-button could also be a good selector for applying the cursor in CSS.

[Dan Beam, 2016-06-23 23:31:49]

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html
(right):

https://codereview.chromium.org/2094813002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html:51:
<div id="menuEditAddress" class="list-item menu-item" actionable
On 2016/06/23 22:07:45, dpapad wrote:
> On 2016/06/23 at 21:28:03, Dan Beam wrote:
> > On 2016/06/23 20:51:45, dpapad wrote:
> > > We could also add the @apply(--settings-actionable) at .list-item
menu-item
> CSS
> > > selector instead. Perhaps better since it would only be in one place VS
> four?
> > 
> > guess that's OK with me.  i'll apply this logic to site settings as well
> 
> password_section.html could also use similar method.

Done.

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.html
(right):

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.html:41:
<div hidden$="[[!device.paired]]" on-tap="doNothing_">
On 2016/06/23 22:07:45, dpapad wrote:
> Instead of calling e.stopPropagation() inside doNothing_, could we add
> "pointer-events: none"  in CSS? If that works then we can also remove this
> comment.

eh, i'll let you try that in a followup

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/internet_page/network_proxy_exclusions.html
(right):

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy_exclusions.html:12:
iron-icon {
On 2016/06/23 22:07:45, dpapad wrote:
> Wondering what is the difference of using iron-icon with the pointer cursor
> instead of using paper-icon-button. If the former is better for performance,
> perhaps we should migrate all paper-icon-buttons to iron-icons? If there are
> other reasons to use paper-icon-buttons (ripple effect?) maybe this one should
> be paper-icon-button too.

ehhhhhh, thought about that, but i'm trying to reduce change in this CL

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/languages_page.html
(right):

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/languages_page.html:48: <div
class$="list-item [[getLanguageItemClass_(
On 2016/06/23 22:07:45, dpapad wrote:
> Similarly to menu-item, this could be done as follows,
> 
> .list-item {

there's nothing about being an item in a list that is inherently actionable, but
being a list-button is

>  --@apply(--settings-actionable);
> }

Done.

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/on_startup_page/startup_urls_page.html
(right):

https://codereview.chromium.org/2094813002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/on_startup_page/startup_urls_page.html:24:
<div class="list-item list-button" id="useCurrentPages" actionable
On 2016/06/23 22:07:46, dpapad wrote:
> list-button could also be a good selector for applying the cursor in CSS.

Done.

[dpapad, 2016-06-23 23:44:23]

https://codereview.chromium.org/2094813002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/languages_page.js (right):

https://codereview.chromium.org/2094813002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/languages_page.js:236:
getLanguageItemClass_: function(languageCode, prospectiveUILanguage,
Nit (optional): You could split this method in two separate methods,

getClassA: function(/* params needed to determine classA existence*/) {...}
getClassB: function(/* params needed to determine classB existence*/) {...}

Then in HTML (I think the following would work).
class="[[getClassA()]] [[getClassB()]]"

This avoids the need to call join(), as well it makes it more explicit which CSS
classes depend on which Polymer property.

[Dan Beam, 2016-06-23 23:48:26]

https://codereview.chromium.org/2094813002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/languages_page.js (right):

https://codereview.chromium.org/2094813002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/languages_page.js:236:
getLanguageItemClass_: function(languageCode, prospectiveUILanguage,
On 2016/06/23 23:44:22, dpapad wrote:
> Nit (optional): You could split this method in two separate methods,
> 
> getClassA: function(/* params needed to determine classA existence*/) {...}
> getClassB: function(/* params needed to determine classB existence*/) {...}
> 
> Then in HTML (I think the following would work).
> class="[[getClassA()]] [[getClassB()]]"
> 
> This avoids the need to call join(), as well it makes it more explicit which
CSS
> classes depend on which Polymer property.

eh, it's a little bit better but i don't know of a time were only some of the
dependencies are ready (they're pretty much atomic, AFAIK).  and if they aren't
set at the same time, there might be flicker as the class name changes a couple
times.

[Dan Beam, 2016-06-23 23:48:45]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2016-06-23 23:48:46]

The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2094813002/#ps40001
(title: "dpapad@ review")

[commit-bot: I haz the power, 2016-06-23 23:50:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2094813002/40001

[commit-bot: I haz the power, 2016-06-24 00:58:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-24 01:00:19]

Description was changed from

==========
MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>


FYI: This CL is semi-automatedly generated via:

  vim -p `git grep -l on-tap -- chrome/browser/resources/settings`

and lots of hacking.


R=dpapad@chromium.org
BUG=611826
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>

FYI: This CL is semi-automatedly generated via:

  vim -p `git grep -l on-tap -- chrome/browser/resources/settings`

and lots of hacking.

R=dpapad@chromium.org
BUG=611826
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/a298aa9c3276b78073a61613787909a0b2427302
Cr-Commit-Position: refs/heads/master@{#401763}
==========

[commit-bot: I haz the power, 2016-06-24 01:00:21]

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

[blundell, 2016-06-24 09:57:27]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2089313005/ by blundell@chromium.org.

The reason for reverting is: This CL causes
SettingLanguagesBrowserTest.LanguagesPage to fail on ChromiumOS and Win7
(confirmed by building ChromeOS locally in Release, observing the failure,
reverting the CL, and seeing that the failure goes away). Example failure:
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%....

Relevant snippet from test output:

[11504:11504:0624/023830:INFO:CONSOLE(1196)] "Running TestCase
SettingsLanguagesPageBrowserTest.MAYBE_LanguagesPage", source: test_api.js
(1196)
[11504:11504:0624/023830:ERROR:CONSOLE(44)] "Mocha test failed: languages page
manage languages
AssertionError: expected false to be true
    at Function.assert.isTrue (chai.js:2423:31)
    at assertTrue (test_api.js:851:17)
    at Context.<anonymous> (languages_page_browsertest.js:81:7)
", source: mocha_adapter.js (44).

[mazigh.imazighan11hakim, 2016-06-24 10:23:14]

mazigh.imazighan11hakim@gmail.com changed reviewers:
+ mazigh.imazighan11hakim@gmail.com

[michaelpg, 2016-06-24 17:04:29]

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

https://codereview.chromium.org/2094813002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/languages_page.html:54:
item.language.supportsUI)]]"
test needs to change '.list-button' to '.list-button:last-of-type' or something

Wonder how the CQ passed this.

[michaelpg, 2016-06-24 17:20:10]

On 2016/06/24 17:04:29, michaelpg wrote:
>
https://codereview.chromium.org/2094813002/diff/40001/chrome/browser/resource...
> File chrome/browser/resources/settings/languages_page/languages_page.html
> (right):
> 
>
https://codereview.chromium.org/2094813002/diff/40001/chrome/browser/resource...
> chrome/browser/resources/settings/languages_page/languages_page.html:54:
> item.language.supportsUI)]]"
> test needs to change '.list-button' to '.list-button:last-of-type' or
something

https://codereview.chromium.org/2095993003/

> 
> Wonder how the CQ passed this.

[michaelpg, 2016-06-24 18:36:11]

On 2016/06/24 17:20:10, michaelpg wrote:
> On 2016/06/24 17:04:29, michaelpg wrote:
> >
>
https://codereview.chromium.org/2094813002/diff/40001/chrome/browser/resource...
> > File chrome/browser/resources/settings/languages_page/languages_page.html
> > (right):
> > 
> >
>
https://codereview.chromium.org/2094813002/diff/40001/chrome/browser/resource...
> > chrome/browser/resources/settings/languages_page/languages_page.html:54:
> > item.language.supportsUI)]]"
> > test needs to change '.list-button' to '.list-button:last-of-type' or
> something
> 
> https://codereview.chromium.org/2095993003/
> 
> > 
> > Wonder how the CQ passed this.

Maybe some of the bots have funky language settings?

[Dan Beam, 2016-06-24 19:24:57]

Description was changed from

==========
MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>

FYI: This CL is semi-automatedly generated via:

  vim -p `git grep -l on-tap -- chrome/browser/resources/settings`

and lots of hacking.

R=dpapad@chromium.org
BUG=611826
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/a298aa9c3276b78073a61613787909a0b2427302
Cr-Commit-Position: refs/heads/master@{#401763}
==========

to

==========
MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>

FYI: This CL is semi-automatedly generated via:

  vim -p `git grep -l on-tap -- chrome/browser/resources/settings`

and lots of hacking.

R=dpapad@chromium.org
BUG=611826
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/a298aa9c3276b78073a61613787909a0b2427302
Cr-Commit-Position: refs/heads/master@{#401763}
==========

[Dan Beam, 2016-06-24 19:25:07]

dbeam@chromium.org changed reviewers:
- mazigh.imazighan11hakim@gmail.com

[Dan Beam, 2016-06-25 00:52:09]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2016-06-25 00:52:21]

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

[Dan Beam, 2016-06-25 00:52:50]

The CQ bit was unchecked by dbeam@chromium.org

[Dan Beam, 2016-06-25 01:01:01]

let's try again after this has landed:
https://codereview.chromium.org/2095993003/

[Dan Beam, 2016-06-25 01:01:07]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2016-06-25 01:01:29]

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-06-25 01:33:27]

Description was changed from

==========
MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>

FYI: This CL is semi-automatedly generated via:

  vim -p `git grep -l on-tap -- chrome/browser/resources/settings`

and lots of hacking.

R=dpapad@chromium.org
BUG=611826
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/a298aa9c3276b78073a61613787909a0b2427302
Cr-Commit-Position: refs/heads/master@{#401763}
==========

to

==========
MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>

FYI: This CL is semi-automatedly generated via:

  vim -p `git grep -l on-tap -- chrome/browser/resources/settings`

and lots of hacking.

R=dpapad@chromium.org
BUG=611826
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/a298aa9c3276b78073a61613787909a0b2427302
Cr-Commit-Position: refs/heads/master@{#401763}
==========

[commit-bot: I haz the power, 2016-06-25 01:33:29]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-25 01:35:51]

Description was changed from

==========
MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>

FYI: This CL is semi-automatedly generated via:

  vim -p `git grep -l on-tap -- chrome/browser/resources/settings`

and lots of hacking.

R=dpapad@chromium.org
BUG=611826
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/a298aa9c3276b78073a61613787909a0b2427302
Cr-Commit-Position: refs/heads/master@{#401763}
==========

to

==========
MD Settings: make custom UIs look "actionable"

Currently, that just entails a cursor: pointer; but may involve e.g. a
:hover effect in the future.

This introduces the shared variable:

  --settings-actionable: {
    cursor: pointer;
  };

which can be used from places that don't want all the settings shared
style (which generally applies to the sections in the middle).

  .my-custom-thing {
    @apply(--settings-actionable);
  }

For stuff inside of <settings-main> (or code otherwise using
settings-shared), it's probably easier to just use the new:

  [actionable] {
    @apply(--settings-actionable);
  }

and just annotate your element with that attribute, i.e.

  <div id="frobber" on-tap="onFrobberTap_" actionable>

FYI: This CL is semi-automatedly generated via:

  vim -p `git grep -l on-tap -- chrome/browser/resources/settings`

and lots of hacking.

R=dpapad@chromium.org
BUG=611826
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/a298aa9c3276b78073a61613787909a0b2427302
Committed: https://crrev.com/ed68883804c25965c677a99278d1ab4c2d98af96
Cr-Original-Commit-Position: refs/heads/master@{#401763}
Cr-Commit-Position: refs/heads/master@{#402042}
==========

[commit-bot: I haz the power, 2016-06-25 01:35:52]

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