MD Settings: fix settings-toggle-button label text overflow.

MD Settings: fix settings-toggle-button label text overflow.

This is to fix appearance page's "[Show home button] long URLs breaks page" bug.
Once this lands, consumers of settings-toggle-button will be able to specify label-class and sub-label-class to be passed in and applied to the label elements.

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

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

[scottchen, 2017-01-27 00:55:43]

Description was changed from

==========
MD Settings: fix settings-toggle-button label text overflow.

This is to fix appearance page's "[Show home button] long URLs breaks page" bug.
Once this lands, consumers of settings-toggle-button will be able to specify
label-class and sub-label-class to be passed in and applied to the label
elements.

BUG=684152
==========

to

==========
MD Settings: fix settings-toggle-button label text overflow.

This is to fix appearance page's "[Show home button] long URLs breaks page" bug.
Once this lands, consumers of settings-toggle-button will be able to specify
label-class and sub-label-class to be passed in and applied to the label
elements.

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

[scottchen, 2017-01-27 01:01:15]

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

[scottchen, 2017-01-27 01:01:16]

The ultimate goal is to get .text-elide onto the labels. And also for that to do
anything, we'll need to add min-width: 0 to all of it's flex-child ancestors. 

I wasn't too happy to have to add that to the <settings-toggle-button> element
code since it'll be added to wherever <settings-toggle-button> is used, but I
can't think of a better way to apply the ellipsis, and adding min-width: 0 to
the containers doesn't seem to have done any visual harms.

Thoughts?

[Dan Beam, 2017-01-27 01:40:25]

FIRST, we should probably make this text-elide stuff more composable, as in:
split the text elide stuff up.

/* in settings_vars_css.html */
--text-elide: {
  overflow: hidden;
  text-overflow: ellipsis;
  white-space: nowrap;
};

/* in settings_shared_css.html */
.text-elide {
  @apply(--text-elide);
}

and then second, we should do either:

1) add <settings-toggle-button elide-label> or something

properties: {
  elideLabel: {
    type: Boolean,
    reflectToAttribute: true,
  },
},

<link rel="import" href="/settings_vars_css.html">
<style>

:host([elide-label]) {
  min-width: 0;
}

:host([elide-label]) #label {
  @apply(--text-elide);
}

</style>

OR

2) just add a mixin, i.e.

#label {
  @apply(--settings-toggle-button-label);
}

and set the mixin from the external scope:

--settings-toggle-button-label: var(--text-elide);

or something

[scottchen, 2017-01-27 17:59:27]

On 2017/01/27 01:40:25, Dan Beam wrote:
> FIRST, we should probably make this text-elide stuff more composable, as in:
> split the text elide stuff up.
> 
> /* in settings_vars_css.html */
> --text-elide: {
>   overflow: hidden;
>   text-overflow: ellipsis;
>   white-space: nowrap;
> };
> 
> /* in settings_shared_css.html */
> .text-elide {
>   @apply(--text-elide);
> }
> 
> and then second, we should do either:
> 
> 1) add <settings-toggle-button elide-label> or something
> 
> properties: {
>   elideLabel: {
>     type: Boolean,
>     reflectToAttribute: true,
>   },
> },
> 
> <link rel="import" href="/settings_vars_css.html">
> <style>
> 
> :host([elide-label]) {
>   min-width: 0;
> }
> 
> :host([elide-label]) #label {
>   @apply(--text-elide);
> }
> 
> </style>
> 
> OR
> 
> 2) just add a mixin, i.e.
> 
> #label {
>   @apply(--settings-toggle-button-label);
> }
> 
> and set the mixin from the external scope:
> 
> --settings-toggle-button-label: var(--text-elide);
> 
> or something

Didn't realize you could do:

:host([elide-label]) {
  min-width: 0;
}

Cool ideas though, a lot more flexible! Will give it a shot and patch again.

[scottchen, 2017-01-27 22:19:50]

@dbeam your suggestion worked great: http://imgur.com/a/TvL5n. So clean! love
it.

[dpapad, 2017-01-27 22:22:19]

On 2017/01/27 at 22:19:50, scottchen wrote:
> @dbeam your suggestion worked great: http://imgur.com/a/TvL5n. So clean! love
it.

Letting @dbeam review this one ;>

[dpapad, 2017-01-27 22:22:32]

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

[Dan Beam, 2017-01-27 22:49:26]

sweeeet! lgtm

you could attempt to write a test for this.  i've previously done it for
downloads, but it's a bit of work.  people will probabbbbbly notice this pretty
quickly.

[scottchen, 2017-01-27 23:06:40]

The CQ bit was checked by scottchen@chromium.org

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

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-28 00:45:42]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485558400214380,
"parent_rev": "af2cb5d0ffa185621767d2e390023ee381e4e79d", "commit_rev":
"fafa62de998f555b86b12040d2c776297aa38567"}

[commit-bot: I haz the power, 2017-01-28 00:46:13]

Description was changed from

==========
MD Settings: fix settings-toggle-button label text overflow.

This is to fix appearance page's "[Show home button] long URLs breaks page" bug.
Once this lands, consumers of settings-toggle-button will be able to specify
label-class and sub-label-class to be passed in and applied to the label
elements.

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

to

==========
MD Settings: fix settings-toggle-button label text overflow.

This is to fix appearance page's "[Show home button] long URLs breaks page" bug.
Once this lands, consumers of settings-toggle-button will be able to specify
label-class and sub-label-class to be passed in and applied to the label
elements.

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

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

[commit-bot: I haz the power, 2017-01-28 00:46:14]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/fafa62de998f555b86b12040d2c7...