Elim cr_button from md-settings

Elim cr_button from md-settings

cr-button doesn't have any behaviors that we are concerned about and
only minor styling that can be done in settings_page.css.

BUG=532540
Committed: https://crrev.com/90e01f99ec58a64e339a2d87983f0fdd7fb4bde1
Cr-Commit-Position: refs/heads/master@{#350384}

[stevenjb, 2015-09-22 17:15:47]

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

[Dan Beam, 2015-09-22 17:22:47]

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

https://codereview.chromium.org/1346823005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/downloads_page/downloads_page.html:21:
<paper-button id="changeDownloadsPath" on-tap="selectDownloadLocation_"
did we decide on-tap instead of on-click yet?

https://codereview.chromium.org/1346823005/diff/1/ui/webui/resources/cr_eleme...
File ui/webui/resources/cr_elements/v1_0/cr_button/cr_button.css (left):

https://codereview.chromium.org/1346823005/diff/1/ui/webui/resources/cr_eleme...
ui/webui/resources/cr_elements/v1_0/cr_button/cr_button.css:21: min-width: auto;
where are we doing this?

[stevenjb, 2015-09-22 17:36:29]

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

https://codereview.chromium.org/1346823005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/downloads_page/downloads_page.html:21:
<paper-button id="changeDownloadsPath" on-tap="selectDownloadLocation_"
On 2015/09/22 17:22:47, Dan Beam wrote:
> did we decide on-tap instead of on-click yet?

I thought we did. Maybe discuss in today's meeting?

https://codereview.chromium.org/1346823005/diff/1/ui/webui/resources/cr_eleme...
File ui/webui/resources/cr_elements/v1_0/cr_button/cr_button.css (left):

https://codereview.chromium.org/1346823005/diff/1/ui/webui/resources/cr_eleme...
ui/webui/resources/cr_elements/v1_0/cr_button/cr_button.css:21: min-width: auto;
On 2015/09/22 17:22:47, Dan Beam wrote:
> where are we doing this?

I thought you added this for md-downloads then didn't use it? I haven't seen any
places where this is important for settings yet, I'd prefer to add it
if/when/where we need it.

[michaelpg, 2015-09-22 22:03:46]

https://codereview.chromium.org/1346823005/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/search_page/search_page.css (right):

https://codereview.chromium.org/1346823005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_page/search_page.css:15: display: flex;
these used to apply to a <cr-button> (a container that had a <papre-button>) but
now will apply to the <paper-button> itself, so my guess is this looks weird

[Dan Beam, 2015-09-22 22:22:23]

https://codereview.chromium.org/1346823005/diff/1/ui/webui/resources/cr_eleme...
File ui/webui/resources/cr_elements/v1_0/cr_button/cr_button.css (left):

https://codereview.chromium.org/1346823005/diff/1/ui/webui/resources/cr_eleme...
ui/webui/resources/cr_elements/v1_0/cr_button/cr_button.css:21: min-width: auto;
On 2015/09/22 17:36:29, stevenjb wrote:
> On 2015/09/22 17:22:47, Dan Beam wrote:
> > where are we doing this?
> 
> I thought you added this for md-downloads then didn't use it? I haven't seen
any
> places where this is important for settings yet, I'd prefer to add it
> if/when/where we need it.

downloads still does this, but so should settings, IMO:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

[stevenjb, 2015-09-22 23:00:18]

ptal

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

https://codereview.chromium.org/1346823005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/downloads_page/downloads_page.html:21:
<paper-button id="changeDownloadsPath" on-tap="selectDownloadLocation_"
On 2015/09/22 17:36:29, stevenjb wrote:
> On 2015/09/22 17:22:47, Dan Beam wrote:
> > did we decide on-tap instead of on-click yet?
> 
> I thought we did. Maybe discuss in today's meeting?

Confirmed on-tap in today's Settings meeting.

https://codereview.chromium.org/1346823005/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/search_page/search_page.css (right):

https://codereview.chromium.org/1346823005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_page/search_page.css:15: display: flex;
On 2015/09/22 22:03:46, michaelpg wrote:
> these used to apply to a <cr-button> (a container that had a <papre-button>)
but
> now will apply to the <paper-button> itself, so my guess is this looks weird

The button has no content, so this really just applies to the button forcing a
new layout block, i.e. it doesn't mater whether or not there is an extra
wrapper. (The display: flex is the property that matters, without it it looks
weird). I'll leave any re-styling to whomever owns this now.

https://codereview.chromium.org/1346823005/diff/1/ui/webui/resources/cr_eleme...
File ui/webui/resources/cr_elements/v1_0/cr_button/cr_button.css (left):

https://codereview.chromium.org/1346823005/diff/1/ui/webui/resources/cr_eleme...
ui/webui/resources/cr_elements/v1_0/cr_button/cr_button.css:21: min-width: auto;
On 2015/09/22 22:22:23, Dan Beam wrote:
> On 2015/09/22 17:36:29, stevenjb wrote:
> > On 2015/09/22 17:22:47, Dan Beam wrote:
> > > where are we doing this?
> > 
> > I thought you added this for md-downloads then didn't use it? I haven't seen
> any
> > places where this is important for settings yet, I'd prefer to add it
> > if/when/where we need it.
> 
> downloads still does this, but so should settings, IMO:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

I was reluctant to propagate --paper-button, but copying the styling from
Downloads to settings_page.css seems fine; done.

[Dan Beam, 2015-09-22 23:09:56]

lgtm

https://codereview.chromium.org/1346823005/diff/1/ui/webui/resources/cr_eleme...
File ui/webui/resources/cr_elements/v1_0/cr_button/cr_button.css (left):

https://codereview.chromium.org/1346823005/diff/1/ui/webui/resources/cr_eleme...
ui/webui/resources/cr_elements/v1_0/cr_button/cr_button.css:21: min-width: auto;
On 2015/09/22 23:00:18, stevenjb wrote:
> On 2015/09/22 22:22:23, Dan Beam wrote:
> > On 2015/09/22 17:36:29, stevenjb wrote:
> > > On 2015/09/22 17:22:47, Dan Beam wrote:
> > > > where are we doing this?
> > > 
> > > I thought you added this for md-downloads then didn't use it? I haven't
seen
> > any
> > > places where this is important for settings yet, I'd prefer to add it
> > > if/when/where we need it.
> > 
> > downloads still does this, but so should settings, IMO:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> 
> I was reluctant to propagate --paper-button, but copying the styling from
> Downloads to settings_page.css seems fine; done.

this is fine for now, I suppose, but we should start a polymer_shared.css or
polymer_wigets.css that handles stuff like this for all pages

[michaelpg, 2015-09-23 00:24:40]

lgtm

https://codereview.chromium.org/1346823005/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/search_page/search_page.css (right):

https://codereview.chromium.org/1346823005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_page/search_page.css:15: display: flex;
On 2015/09/22 23:00:18, stevenjb wrote:
> On 2015/09/22 22:03:46, michaelpg wrote:
> > these used to apply to a <cr-button> (a container that had a <papre-button>)
> but
> > now will apply to the <paper-button> itself, so my guess is this looks weird
> 
> The button has no content, so this really just applies to the button forcing a
> new layout block, i.e. it doesn't mater whether or not there is an extra
> wrapper. (The display: flex is the property that matters, without it it looks
> weird). I'll leave any re-styling to whomever owns this now.

I see.... "display: flex" make the paper-button not take up the full width,
which I guess is valid if not ideal.

[stevenjb, 2015-09-23 17:22:49]

https://codereview.chromium.org/1346823005/diff/1/ui/webui/resources/cr_eleme...
File ui/webui/resources/cr_elements/v1_0/cr_button/cr_button.css (left):

https://codereview.chromium.org/1346823005/diff/1/ui/webui/resources/cr_eleme...
ui/webui/resources/cr_elements/v1_0/cr_button/cr_button.css:21: min-width: auto;
On 2015/09/22 23:09:55, Dan Beam wrote:
> On 2015/09/22 23:00:18, stevenjb wrote:
> > On 2015/09/22 22:22:23, Dan Beam wrote:
> > > On 2015/09/22 17:36:29, stevenjb wrote:
> > > > On 2015/09/22 17:22:47, Dan Beam wrote:
> > > > > where are we doing this?
> > > > 
> > > > I thought you added this for md-downloads then didn't use it? I haven't
> seen
> > > any
> > > > places where this is important for settings yet, I'd prefer to add it
> > > > if/when/where we need it.
> > > 
> > > downloads still does this, but so should settings, IMO:
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> > 
> > I was reluctant to propagate --paper-button, but copying the styling from
> > Downloads to settings_page.css seems fine; done.
> 
> this is fine for now, I suppose, but we should start a polymer_shared.css or
> polymer_wigets.css that handles stuff like this for all pages

I agree that would be better, but it would also mean adding an additional .css
include to every settings page. Probably worth discussing, documenting, and
implementing before we start on too many additional WebUI pages. (Either at a
standup, or next week's meeting seems fine).

[stevenjb, 2015-09-23 17:23:26]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2015-09-23 17:23:28]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org,
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/1346823005/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2015-09-23 17:24:33]

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

[commit-bot: I haz the power, 2015-09-23 18:15:10]

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

[commit-bot: I haz the power, 2015-09-23 18:15:11]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[stevenjb, 2015-09-23 19:09:42]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2015-09-23 19:11:39]

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

[commit-bot: I haz the power, 2015-09-23 20:25:13]

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

[commit-bot: I haz the power, 2015-09-23 20:25:15]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[stevenjb, 2015-09-23 22:14:10]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2015-09-23 22:16:07]

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

[commit-bot: I haz the power, 2015-09-23 23:26:42]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-09-23 23:27:47]

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