Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mat-slide-toggle styling inconsistent #14192

Closed
arciisine opened this issue Nov 18, 2018 · 10 comments · Fixed by #14230
Closed

mat-slide-toggle styling inconsistent #14192

arciisine opened this issue Nov 18, 2018 · 10 comments · Fixed by #14230
Assignees

Comments

@arciisine
Copy link

arciisine commented Nov 18, 2018

Bug, feature request, or proposal:

As of #12698, changes were made to slide toggle styling to support the theme properly.

The issue is, that in the dark theme, the slide toggle styling relies on a key of 200 to be in the palette for coloring.
https://angular-kh6bg8.stackblitz.io

As far as I can tell, this is the only location that makes this choice. Everywhere else is dependent upon default, lighter, or darker.

What is the expected behavior?

That the slide toggle would rely upon default/lighter/darker for the checked state, in the dark theme.

What is the current behavior?

The slide toggle relies upon the palette's 200 value for the checked state in the dark theme.

Below is the current input I have to construct for mat-pallete to allow sass to compile

$mat-primary: (
  main: #cc33ca,
  lighter: #f0c2ef,
  darker: #b920b7,
  200: #cc33ca, // For slide toggle,
  contrast : (
    main: $light-primary-text,
    lighter: $dark-primary-text,
    darker: $light-primary-text,
  )
);
$theme-primary: mat-palette($mat-primary, main, lighter, darker);

$theme: mat-dark-theme($theme-primary, ... );

What are the steps to reproduce?

https://angular-kh6bg8.stackblitz.io

What is the use-case or motivation for changing an existing behavior?

That by default, when defining a theme, the only colors that are needed are lighter, darker, and default. There should not a need to define all the other colors in a custom theme.

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Material, latest

Is there anything else we should know?

@crisbeto
Copy link
Member

@arciisine your Stackblitz link doesn't seem to load.

@arciisine
Copy link
Author

Sorry about that, I forgot to fork before making changes, description has been updated with new links.

@devversion devversion self-assigned this Nov 18, 2018
@arciisine
Copy link
Author

screen shot 2018-11-19 at 8 32 49 am

Attaching an image as the stackblitz link seems to be slow to load.

@devversion
Copy link
Member

I'm thinking about this, but somehow don't feel comfortable changing the dark hue to default. This is because:

  • The new specs don't cover switches in dark mode at all
  • The old specs explicitly stated that the hue 200 should be used in dark themes
  • Using the default hue causes a bad contrast between the bar and thumb.
Following old specs Using default hue
image image

Note that MDC also seems to use the default (500) hue in dark mode. Though this does not necessarily mean that it's the intended color for dark themes.

@arciisine
Copy link
Author

I added this to the description, but basically this toggle is unique in it's behavior. I am only adding provisions for the slide-toggle, and nothing else.

$mat-primary: (
  main: #cc33ca,
  lighter: #f0c2ef,
  darker: #b920b7,
  200: #cc33ca, // For slide toggle,
  contrast : (
    main: $light-primary-text,
    lighter: $dark-primary-text,
    darker: $light-primary-text,
  )
);
$theme-primary: mat-palette($mat-primary, main, lighter, darker);

$theme: mat-dark-theme($theme-primary, ... );

@devversion
Copy link
Member

I see the issue about having to define the 200 hue just for the slide-toggle, but I think, usually a custom theme should include updated colors for all hues.

Even though developer experience is an important point, following the specs is also an important part. I'll try to collect more information, but I'm tending to just follow what MDC does (removing the hue 200)

@arciisine
Copy link
Author

Looking at https://material.io/tools/color/#!/, and https://material.io/tools/theme-editor/, it looks like majority of the tools have knowledge of all the colors in the default Material palettes, but only require 3 colors per palette. Is this a fair assumption that the material angular project will follow that as well?

I'm currently working on a theme editor (http://materialtheme.arcsine.org) and I'm trying to align first with Material's theming, and then secondarily fit in with the angular implementation.

@devversion
Copy link
Member

True, some implementations like MDC apparently just expect a single color for primary, secondary. This is different from our implementation because we expect primary and secondary to be a full palette of colors.

Quote from the specs

These palettes provide additional ways to use your primary and secondary colors, by providing lighter and darker options to separate surfaces and provide colors that meet accessibility standards.

I think we are good to change this away from 200 to default in the future, but since this is a breaking change, it won't be anytime soon.

The Material Design specifications also provide a tool to generate all hues based on a picked color. See https://material.io/design/color/the-color-system.html#tools-for-picking-colors

@arciisine
Copy link
Author

And I have seen these tools, the issue for me is that knowing what 100-A700 mean within angular material can be challenging. I'm attempting to make it simpler for people who are not using the standard color palettes, as defining all those colors, and knowing how they will be used is not clear (apart from the default, lighter and darker).

devversion added a commit to devversion/material2 that referenced this issue Nov 21, 2018
* The thumb should use the `default` hue in dark themes to match the MDC behavior and also make it easier for people to create a custom theme without needing to explicitly specify the `200` hue just for the slide-toggle. This also makes the slide-toggle more consistent with the checkbox and radio (which are considered selection controls as well)

Fixes angular#14192
vivian-hu-zz pushed a commit that referenced this issue Jan 16, 2019
* The thumb should use the `default` hue in dark themes to match the MDC behavior and also make it easier for people to create a custom theme without needing to explicitly specify the `200` hue just for the slide-toggle. This also makes the slide-toggle more consistent with the checkbox and radio (which are considered selection controls as well)

Fixes #14192
s2-abdo pushed a commit to s2-abdo/material2 that referenced this issue Jan 18, 2019
…4230)

* The thumb should use the `default` hue in dark themes to match the MDC behavior and also make it easier for people to create a custom theme without needing to explicitly specify the `200` hue just for the slide-toggle. This also makes the slide-toggle more consistent with the checkbox and radio (which are considered selection controls as well)

Fixes angular#14192
s2-abdo pushed a commit to s2-abdo/material2 that referenced this issue Jan 18, 2019
…4230)

* The thumb should use the `default` hue in dark themes to match the MDC behavior and also make it easier for people to create a custom theme without needing to explicitly specify the `200` hue just for the slide-toggle. This also makes the slide-toggle more consistent with the checkbox and radio (which are considered selection controls as well)

Fixes angular#14192
vivian-hu-zz pushed a commit that referenced this issue Jan 18, 2019
* The thumb should use the `default` hue in dark themes to match the MDC behavior and also make it easier for people to create a custom theme without needing to explicitly specify the `200` hue just for the slide-toggle. This also makes the slide-toggle more consistent with the checkbox and radio (which are considered selection controls as well)

Fixes #14192
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants