MD Settings: clean up text controls (links, buttons)

MD Settings: clean up text controls (links, buttons)

Making styles and behaviors consistent for <a href>, .action-link, .list-button, and other varieties of text-based action items.

BUG=668307
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
NOTRY=true  # blame dbeam@

Review-Url: https://codereview.chromium.org/2639373002
Cr-Commit-Position: refs/heads/master@{#446236}
Committed: https://chromium.googlesource.com/chromium/src/+/a41c7aae93287688ee2e3b345b46d70b1e5f8e4b

[scottchen, 2017-01-18 19:14:41]

Description was changed from

==========
MD Settings: clean up text controls (links, buttons)

Making styles and behaviors consistent for <a href>, .action-link, .list-button,
and other varieties of text-based action items.

BUG=668307
==========

to

==========
MD Settings: clean up text controls (links, buttons)

Making styles and behaviors consistent for <a href>, .action-link, .list-button,
and other varieties of text-based action items.

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

[scottchen, 2017-01-20 21:18:22]

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

https://codereview.chromium.org/2639373002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:230:
.list-item.list-button > [is='action-link'] {
This is to make the focus border the same size as the clickable area.

[scottchen, 2017-01-20 22:17:17]

The CQ bit was checked by scottchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-20 22:17:43]

Dry run: 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-01-20 23:05:36]

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

[commit-bot: I haz the power, 2017-01-20 23:05:36]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[scottchen, 2017-01-21 01:34:21]

The CQ bit was checked by scottchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-21 01:34:41]

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

[scottchen, 2017-01-21 01:49:49]

scottchen@chromium.org changed reviewers:
+ dbeam@chromium.org, michaelpg@chromium.org

[commit-bot: I haz the power, 2017-01-21 03:04:55]

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

[commit-bot: I haz the power, 2017-01-21 03:04:55]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[michaelpg, 2017-01-23 06:51:19]

i'd trust dan's opinion on this

https://codereview.chromium.org/2639373002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/stylus.html (right):

https://codereview.chromium.org/2639373002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/stylus.html:25: <!--
TODO(scottchen): Make a proper a[href].settings-box with
this is hard to understand, can you include a crbug.com link?

[scottchen, 2017-01-23 18:49:07]

https://codereview.chromium.org/2639373002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/stylus.html (right):

https://codereview.chromium.org/2639373002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/stylus.html:25: <!--
TODO(scottchen): Make a proper a[href].settings-box with
On 2017/01/23 06:51:19, michaelpg wrote:
> this is hard to understand, can you include a http://crbug.com link?

Done.

[scottchen, 2017-01-23 18:49:20]

The CQ bit was checked by scottchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-23 18:50:02]

Dry run: 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-01-23 20:27:09]

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

[commit-bot: I haz the power, 2017-01-23 20:27:10]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Dan Beam, 2017-01-23 20:40:09]

ok, so, i'd like us to think about how to pull in action links in a more
import-friendly way now that we're sure we're gonna use them.

I think it'd make sense to audit where we use <a is="action-link"> and verify
we're importing action_link.html (which calls document.registerElement() so that
the is="action-link" part actually works).  this is especially important in
tests.

secondly, I'd like a better way to load styles.  right now, the only way that
works (afaik) to load action_link.css (which we should be doing anywhere that we
house an action link) is

<link rel="import" type="css" href="chrome://resources/css/action_link.css">

which isn't awesome because it's deprecated and flickery (Polymer JS actually
queries the DOM for 'link[rel=import][type=css]' and loads the resource and
injects it into a <style> or something.  it's not natively supported by the
browser.)

so instead, I think we should make it a style module like everything else in
settings is now.  the easiest way to accomplish this might be to simply use 

  <include src="../../../../ui/webui/resources/css/action_link.css">

inside the <style> tag in settings_shared_css.html.

does that make sense?

there have been constant issues where folks are not actually loading JS or CSS
required but things still kinda work :(.  let's fix that if we're going forward
with action-link.

yes, we could possibly re-implement it as a Polymer element, but it's still used
on old webui pages that i don't really want to break/update.

https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/stylus.html (right):

https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/stylus.html:1: <link rel="import"
href="chrome://resources/html/action_link.html">
drop this import

https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_summary_item.html
(right):

https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_summary_item.html:107:
<a is="action-link" class="list-button"
where is the import that defines action link?

https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:236: min-height:
inherit;
alphabetize

[scottchen, 2017-01-23 22:46:07]

On 2017/01/23 20:40:09, Dan Beam wrote:
> ok, so, i'd like us to think about how to pull in action links in a more
> import-friendly way now that we're sure we're gonna use them.
> 
> I think it'd make sense to audit where we use <a is="action-link"> and verify
> we're importing action_link.html (which calls document.registerElement() so
that
> the is="action-link" part actually works).  this is especially important in
> tests.
> 
> secondly, I'd like a better way to load styles.  right now, the only way that
> works (afaik) to load action_link.css (which we should be doing anywhere that
we
> house an action link) is
> 
> <link rel="import" type="css" href="chrome://resources/css/action_link.css">
> 
> which isn't awesome because it's deprecated and flickery (Polymer JS actually
> queries the DOM for 'link[rel=import][type=css]' and loads the resource and
> injects it into a <style> or something.  it's not natively supported by the
> browser.)
> 
> so instead, I think we should make it a style module like everything else in
> settings is now.  the easiest way to accomplish this might be to simply use 
> 
>   <include src="../../../../ui/webui/resources/css/action_link.css">
> 
> inside the <style> tag in settings_shared_css.html.
> 
> does that make sense?
> 
> there have been constant issues where folks are not actually loading JS or CSS
> required but things still kinda work :(.  let's fix that if we're going
forward
> with action-link.
> 
> yes, we could possibly re-implement it as a Polymer element, but it's still
used
> on old webui pages that i don't really want to break/update.
> 
>
https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
> File chrome/browser/resources/settings/device_page/stylus.html (right):
> 
>
https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
> chrome/browser/resources/settings/device_page/stylus.html:1: <link
rel="import"
> href="chrome://resources/html/action_link.html">
> drop this import
> 
>
https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
> File chrome/browser/resources/settings/internet_page/network_summary_item.html
> (right):
> 
>
https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
> chrome/browser/resources/settings/internet_page/network_summary_item.html:107:
> <a is="action-link" class="list-button"
> where is the import that defines action link?
> 
>
https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
> File chrome/browser/resources/settings/settings_shared_css.html (right):
> 
>
https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
> chrome/browser/resources/settings/settings_shared_css.html:236: min-height:
> inherit;
> alphabetize

Yeah, that sounds good. 
And also should be pretty straight forward now that the usage is unified in
these patches. 
I'll submit another patch to move all the action-link related css to a separate
file and import it where used.

[scottchen, 2017-01-23 23:08:37]

https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_summary_item.html
(right):

https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_summary_item.html:107:
<a is="action-link" class="list-button"
On 2017/01/23 20:40:09, Dan Beam wrote:
> where is the import that defines action link?

It's just inside settings_shared_css.html currently. But per our discussion on
the review thread, I'll move it to a style module to be imported instead.

[scottchen, 2017-01-24 00:24:40]

Patchset #6 (id:100001) has been deleted

[scottchen, 2017-01-24 02:01:03]

On 2017/01/23 23:08:37, scottchen wrote:
>
https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
> File chrome/browser/resources/settings/internet_page/network_summary_item.html
> (right):
> 
>
https://codereview.chromium.org/2639373002/diff/80001/chrome/browser/resource...
> chrome/browser/resources/settings/internet_page/network_summary_item.html:107:
> <a is="action-link" class="list-button"
> On 2017/01/23 20:40:09, Dan Beam wrote:
> > where is the import that defines action link?
> 
> It's just inside settings_shared_css.html currently. But per our discussion on
> the review thread, I'll move it to a style module to be imported instead.

dbeam@ Is patch #6 more aligned with what you were thinking?

[scottchen, 2017-01-24 02:02:28]

The CQ bit was checked by scottchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 02:03:57]

Dry run: 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-01-24 02:52:31]

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

[commit-bot: I haz the power, 2017-01-24 02:52:32]

Dry run: 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_...)

[Dan Beam, 2017-01-24 05:25:34]

regarding the CSS: are we actually loading action_link.css for any of these
pages now?  don't we want to load those default styles, or are they not that
helpful?

https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/settings/privacy_page/privacy_page.html (right):

https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/settings/privacy_page/privacy_page.html:353: 
why so spacious?

https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/settings/settings_shared_css.html:219:
.list-button[is='action-link'] {
why can't this be .list-item [is='action-link']?

https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/settings/site_settings/site_details.html (right):

https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/settings/site_settings/site_details.html:113: <div
raised class="list-item">
what does this raised do?

https://codereview.chromium.org/2639373002/diff/120001/ui/webui/resources/htm...
File ui/webui/resources/html/action_link_css.html (right):

https://codereview.chromium.org/2639373002/diff/120001/ui/webui/resources/htm...
ui/webui/resources/html/action_link_css.html:7: cursor: pointer;
this should use @apply(--settings-actionable) or --cr-actionable

[scottchen, 2017-01-24 19:39:12]

https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/settings/privacy_page/privacy_page.html (right):

https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/settings/privacy_page/privacy_page.html:353: 
On 2017/01/24 05:25:33, Dan Beam wrote:
> why so spacious?

Done. That batman reference though :)

https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/settings/settings_shared_css.html:219:
.list-button[is='action-link'] {
On 2017/01/24 05:25:33, Dan Beam wrote:
> why can't this be .list-item [is='action-link']?

I'm thinking there could be cases in which action-link inside .list-item is not
intended to be a full-row button.

e.g. :

<div class="list-button> 
   Hello 
  <a is="action-link">Manage Something</a>
</div>

https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/settings/site_settings/site_details.html (right):

https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/settings/site_settings/site_details.html:113: <div
raised class="list-item">
On 2017/01/24 05:25:33, Dan Beam wrote:
> what does this raised do?

I'm guessing this div was originally a paper-button.
https://elements.polymer-project.org/elements/paper-button#property-raised

It doesn't seem to do anything anymore, will remove.

https://codereview.chromium.org/2639373002/diff/120001/ui/webui/resources/htm...
File ui/webui/resources/html/action_link_css.html (right):

https://codereview.chromium.org/2639373002/diff/120001/ui/webui/resources/htm...
ui/webui/resources/html/action_link_css.html:7: cursor: pointer;
On 2017/01/24 05:25:33, Dan Beam wrote:
> this should use @apply(--settings-actionable) or --cr-actionable

This is outside of settings, so I'll use --cr-actionable.

[scottchen, 2017-01-24 19:43:56]

On 2017/01/24 05:25:34, Dan Beam wrote:
> regarding the CSS: are we actually loading action_link.css for any of these
> pages now?  don't we want to load those default styles, or are they not that
> helpful?
> 
>
https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
> File chrome/browser/resources/settings/privacy_page/privacy_page.html (right):
> 
>
https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
> chrome/browser/resources/settings/privacy_page/privacy_page.html:353: 
> why so spacious?
> 
>
https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
> File chrome/browser/resources/settings/settings_shared_css.html (right):
> 
>
https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
> chrome/browser/resources/settings/settings_shared_css.html:219:
> .list-button[is='action-link'] {
> why can't this be .list-item [is='action-link']?
> 
>
https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
> File chrome/browser/resources/settings/site_settings/site_details.html
(right):
> 
>
https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
> chrome/browser/resources/settings/site_settings/site_details.html:113: <div
> raised class="list-item">
> what does this raised do?
> 
>
https://codereview.chromium.org/2639373002/diff/120001/ui/webui/resources/htm...
> File ui/webui/resources/html/action_link_css.html (right):
> 
>
https://codereview.chromium.org/2639373002/diff/120001/ui/webui/resources/htm...
> ui/webui/resources/html/action_link_css.html:7: cursor: pointer;
> this should use @apply(--settings-actionable) or --cr-actionable

action_link_css.html is a full replacement of action_link.css for the md pages.

[Dan Beam, 2017-01-24 19:47:28]

On 2017/01/24 19:43:56, scottchen wrote:
> On 2017/01/24 05:25:34, Dan Beam wrote:
> > regarding the CSS: are we actually loading action_link.css for any of these
> > pages now?  don't we want to load those default styles, or are they not that
> > helpful?
> > 
> >
>
https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
> > File chrome/browser/resources/settings/privacy_page/privacy_page.html
(right):
> > 
> >
>
https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
> > chrome/browser/resources/settings/privacy_page/privacy_page.html:353: 
> > why so spacious?
> > 
> >
>
https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
> > File chrome/browser/resources/settings/settings_shared_css.html (right):
> > 
> >
>
https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
> > chrome/browser/resources/settings/settings_shared_css.html:219:
> > .list-button[is='action-link'] {
> > why can't this be .list-item [is='action-link']?
> > 
> >
>
https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
> > File chrome/browser/resources/settings/site_settings/site_details.html
> (right):
> > 
> >
>
https://codereview.chromium.org/2639373002/diff/120001/chrome/browser/resourc...
> > chrome/browser/resources/settings/site_settings/site_details.html:113: <div
> > raised class="list-item">
> > what does this raised do?
> > 
> >
>
https://codereview.chromium.org/2639373002/diff/120001/ui/webui/resources/htm...
> > File ui/webui/resources/html/action_link_css.html (right):
> > 
> >
>
https://codereview.chromium.org/2639373002/diff/120001/ui/webui/resources/htm...
> > ui/webui/resources/html/action_link_css.html:7: cursor: pointer;
> > this should use @apply(--settings-actionable) or --cr-actionable
> 
> action_link_css.html is a full replacement of action_link.css for the md
pages.

you should probably give people a prayer at understanding this with comments at
the top of each file or something ;)

[scottchen, 2017-01-24 21:50:21]

The CQ bit was checked by scottchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 21:50:49]

Dry run: 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-01-24 23:00:24]

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

[commit-bot: I haz the power, 2017-01-24 23:00:25]

Dry run: 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...)

[scottchen, 2017-01-25 21:21:09]

The CQ bit was checked by scottchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-25 21:22:00]

Dry run: 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-01-25 21:24:24]

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

[commit-bot: I haz the power, 2017-01-25 21:24:27]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[scottchen, 2017-01-25 21:33:55]

The CQ bit was checked by scottchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-25 21:34:40]

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

[scottchen, 2017-01-25 21:38:28]

The CQ bit was checked by scottchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-25 21:39:12]

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

[Dan Beam, 2017-01-25 23:47:47]

lgtm

https://codereview.chromium.org/2639373002/diff/180001/chrome/browser/resourc...
File chrome/browser/resources/settings/internet_page/network_summary_item.html
(right):

https://codereview.chromium.org/2639373002/diff/180001/chrome/browser/resourc...
chrome/browser/resources/settings/internet_page/network_summary_item.html:5:
<!-- "action_link_css.html" replaces "action_link.css" for MD pages. -->
don't repeat this everywhere, just put it in the top of action_link_css.html,
IMO

[scottchen, 2017-01-26 01:06:20]

The CQ bit was checked by scottchen@chromium.org

[scottchen, 2017-01-26 01:06:20]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/2639373002/#ps300001
(title: "move comment about action_link_css to action_likn_css.html")

[commit-bot: I haz the power, 2017-01-26 01:07:23]

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-01-26 04:08:33]

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

[commit-bot: I haz the power, 2017-01-26 04:08:34]

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...)

[Dan Beam, 2017-01-26 05:11:11]

Description was changed from

==========
MD Settings: clean up text controls (links, buttons)

Making styles and behaviors consistent for <a href>, .action-link, .list-button,
and other varieties of text-based action items.

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

to

==========
MD Settings: clean up text controls (links, buttons)

Making styles and behaviors consistent for <a href>, .action-link, .list-button,
and other varieties of text-based action items.

BUG=668307
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
NOTRY=true  # blame dbeam@
==========

[Dan Beam, 2017-01-26 05:11:19]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2017-01-26 05:11:47]

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-01-26 05:41:04]

CQ is committing da patch.

Bot data: {"patchset_id": 300001, "attempt_start_ts": 1485407479138070,
"parent_rev": "2fbc5bb6bdc66f31173fab53372c69a97c39eccb", "commit_rev":
"a41c7aae93287688ee2e3b345b46d70b1e5f8e4b"}

[commit-bot: I haz the power, 2017-01-26 05:41:45]

Description was changed from

==========
MD Settings: clean up text controls (links, buttons)

Making styles and behaviors consistent for <a href>, .action-link, .list-button,
and other varieties of text-based action items.

BUG=668307
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
NOTRY=true  # blame dbeam@
==========

to

==========
MD Settings: clean up text controls (links, buttons)

Making styles and behaviors consistent for <a href>, .action-link, .list-button,
and other varieties of text-based action items.

BUG=668307
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
NOTRY=true  # blame dbeam@

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

[commit-bot: I haz the power, 2017-01-26 05:41:51]

Committed patchset #15 (id:300001) as
https://chromium.googlesource.com/chromium/src/+/a41c7aae93287688ee2e3b345b46...

[Dan Beam, 2017-01-26 05:43:37]

https://codereview.chromium.org/2639373002/diff/300001/ui/webui/resources/htm...
File ui/webui/resources/html/action_link_css.html (right):

https://codereview.chromium.org/2639373002/diff/300001/ui/webui/resources/htm...
ui/webui/resources/html/action_link_css.html:3: <link rel="import"
href="chrome://resources/polymer/v1_0/paper-styles/color.html">
this needs a <link rel="import" href="chrome://resources/html/polymer.html">

you should not often need to the order of the imports (like you did) for a test
or something.  typically alphabetical should work.  this file is not loading
polymer.html so it's loading polymer directly without setting up a config.

[scottchen, 2017-01-26 19:51:40]

On 2017/01/26 05:43:37, Dan Beam wrote:
>
https://codereview.chromium.org/2639373002/diff/300001/ui/webui/resources/htm...
> File ui/webui/resources/html/action_link_css.html (right):
> 
>
https://codereview.chromium.org/2639373002/diff/300001/ui/webui/resources/htm...
> ui/webui/resources/html/action_link_css.html:3: <link rel="import"
> href="chrome://resources/polymer/v1_0/paper-styles/color.html">
> this needs a <link rel="import" href="chrome://resources/html/polymer.html">
> 
> you should not often need to the order of the imports (like you did) for a
test
> or something.  typically alphabetical should work.  this file is not loading
> polymer.html so it's loading polymer directly without setting up a config.

Hmm, I've had to load polymer.html before action_link_css.html for
lock_screen.html and search_engine.html, otherwise their corresponding
browser_tests will fail when checking visibility/existence of certain elements.
I thought it was because the <style> module import doesn't work
deterministically if polymer's not included first.. was talking to dpapad@
yesterday and he mentioned having to put polymer.html on top of file before to
get tests working as well.

[Dan Beam, 2017-02-01 02:18:49]

On 2017/01/26 19:51:40, scottchen wrote:
> On 2017/01/26 05:43:37, Dan Beam wrote:
> >
>
https://codereview.chromium.org/2639373002/diff/300001/ui/webui/resources/htm...
> > File ui/webui/resources/html/action_link_css.html (right):
> > 
> >
>
https://codereview.chromium.org/2639373002/diff/300001/ui/webui/resources/htm...
> > ui/webui/resources/html/action_link_css.html:3: <link rel="import"
> > href="chrome://resources/polymer/v1_0/paper-styles/color.html">
> > this needs a <link rel="import" href="chrome://resources/html/polymer.html">
> > 
> > you should not often need to the order of the imports (like you did) for a
> test
> > or something.  typically alphabetical should work.  this file is not loading
> > polymer.html so it's loading polymer directly without setting up a config.
> 
> Hmm, I've had to load polymer.html before action_link_css.html for
> lock_screen.html and search_engine.html,

you shouldn't if you put a polymer.html import in action_link_css.html itself

> otherwise their corresponding
> browser_tests will fail when checking visibility/existence of certain
elements.
> I thought it was because the <style> module import doesn't work
> deterministically if polymer's not included first.. was talking to dpapad@
> yesterday and he mentioned having to put polymer.html on top of file before to
> get tests working as well.

try adding the import first and let's see if it works