feat(material-experimental/mdc-slide-toggle): switch to non-deprecated styles #23143

crisbeto · 2021-07-10T12:37:10Z

Switches the MDC-based slide toggle to the non-deprecated MDC styles.

Notable changes:

New markup which uses a button instead of an input for the interaction.
New icons inside the slide toggle's thumb. Technically we could opt out of them, but I think that they look better and they help with accessibility for color-blind users.
New theming system that uses a flat list of variables. There is a fallback for IE11, but I decided not to include it for now, because of the upcoming deprecation and the fact that the component is in experimental.
The component has some slightly different colors and it supports more states (e.g. hover).
Due to the switch from input to button, the required input is basically a noop now.

Here's what it looks like now:

jelbourn · 2021-07-13T18:46:24Z

scripts/check-mdc-tests-config.ts

+      // The MDC implementation uses a `button` instead of an `input` which can't be required.
+      'should forward the required attribute',
+      'should prevent the form from submit when being required',
+


Can we add Angular's required validation manually? We support required on MatSelect which isn't a native control

The only thing we were using the required input for was aria-required which doesn't make sense on a button. We also have the MatSlideToggleRequiredValidator directive which matches on it, but we don't actually need an input for it to be picked up.

role="switch" does support aria-required, though, so we should still support that

According to aXe it isn't supported so I had to leave it out, otherwise it might've broken the g3 aXe check.

Aha, aria-required is supported in ARIA 1.2, but not in ARIA 1.1. It seems like axe is out of date. I sent an email to Google's a11y list to see if anyone has a recommended here. My hunch is that axe should be updated to support this.

The feedback I got was to focus on how it actually works in the various assistive tech, so we should just test it with a few screen-readers and see how it works. I tested having aria-required on the <button> and it worked as expected on VoiceOver and NVDA- I think we should try to keep it and update the axe configuration to accept it.

jelbourn · 2021-07-13T18:53:48Z

src/material-experimental/mdc-slide-toggle/_slide-toggle-theme.scss

+
+// Generates all color mapping for the properties that only change based on the theme.
+@function _get-theme-base-map($is-dark) {
+  $on-surface: if($is-dark, mdc-color-palette.$grey-100, mdc-color-palette.$grey-800);


Could you add a comment that explains where these color choices come from? E.g. why is $on-surface gray-100/gray-800?

So this is the part where I had to make some assumptions. The light theme values come from MDC's $light-theme variable one-to-one, but since MDC didn't provide a dark theme, I had to come up with the values myself. I did try to follow a pattern (e.g. 800 and 100 are on different ends of the palette), but it's not foolproof.

I chatted with the MDC team about the status/plan here. Summary is:

For the new version of Material Design, there will be a set of token values OSS we can use, but they're not ready yet.

We have a set of token values internally for Google's specific brand configuration, but those aren't public

The MDC team is picking as reasonable values as they can for 2018-era Material Design (M2) tokens, but they didn't do dark theme values since we're the only ones using them, so it's reasonable for us to decide them on our own.

The tokens are split into two parts: system tokens (used across multiple components) and component-specific tokens. I think we should create a Sass file in material/core for the baseline token values, and then create a another file per component with the tokens for that specific spec version. So, something like

├── material/core/theming │ ├── _system-tokens-2018.scss ├── material/slide-toggle │ ├── _slide-toggle-tokens-2018.scss

We should make sure to comment which values we're just picking ourselves and which we're sourcing from somewhere else.

Do we know which ones are system tokens and which ones are slide toggle-specific? I can make some reasonable guesses, but it would be better if there was a list.

The format is a little rough, but this is the stuff currently considered system styles that we care about (there was also some shape stuff that we don't care about yet)

/** COLOR */
// Background
$background;

// Error
$error;

// Error state content
$error-state-content;

// Error state layer
$error-state-layer;

// Hairline
$hairline;

// Inverse on surface
$inverse-on-surface;

// Inverse primary
$inverse-primary;

// Inverse primary state content
$inverse-primary-state-content;

// Inverse primary state layer
$inverse-primary-state-layer;

// Inverse surface
$inverse-surface;

// On background
$on-background;

// On error
$on-error;

// On error state content
$on-error-state-content;

// On error state layer
$on-error-state-layer;

// On primary
$on-primary;

// On primary state content
$on-primary-state-content;

// On primary state layer
$on-primary-state-layer;

// On secondary
$on-secondary;

// On secondary state content
$on-secondary-state-content;

// On secondary state layer
$on-secondary-state-layer;

// On surface
$on-surface;

// On surface state content
$on-surface-state-content;

// On surface state layer
$on-surface-state-layer;

// On surface variant
$on-surface-variant;

// Primary
$primary;

// Primary state content
$primary-state-content;

// Primary state layer
$primary-state-layer;

// Primary variant
$primary-variant;

// Secondary
$secondary;

// Secondary variant
$secondary-variant;

// Shadow color
$shadow;

// Surface
$surface;

// Textfield error
$textfield-error;

// Textfield hairline
$textfield-hairline;

// Textfield on surface variant
$textfield-on-surface-variant;

// Textfield primary
$textfield-primary;

// Textfield state layer
$textfield-state-layer;

// Textfield surface
$textfield-surface;

/** ANIMATION */

// Casual duration
$duration-long1;

// Slow duration
$duration-long2;

// Quick duration
$duration-medium1;

// Standard duration
$duration-medium2;

// Flash duration
$duration-short1;

// Rapid duration
$duration-short2;

// Accelerated easing (in)
$easing-accelerated;

// Decelerated easing (out)
$easing-decelerated;

// Emphasized
$easing-emphasized;

// Linear easing
$easing-linear;

// Standard easing (in and out)
$easing-standard;

/** TYPOGRAPHY */

// Body 1
$body1: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Body 1 font name
$body1-font;

// Body 1 line height
$body1-line-height;

// Body 1 font size
$body1-size;

// Body 1 font tracking
$body1-tracking;

// Body 1 font weight
$body1-weight;

// Body 2
$body2: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Body 2 font name
$body2-font;

// Body 2 line height
$body2-line-height;

// Body 2 font size
$body2-size;

// Body 2 font tracking
$body2-tracking;

// Body 2 font weight
$body2-weight;

// Button
$button: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Button font name
$button-font;

// Button line height
$button-line-height;

// Button font size
$button-size;

// Button font tracking
$button-tracking;

// Button font weight
$button-weight;

// Caption
$caption: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Caption font name
$caption-font;

// Caption line height
$caption-line-height;

// Caption font size
$caption-size;

// Caption font tracking
$caption-tracking;

// Caption font weight
$caption-weight;

// Display 1
$display1: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Display 1 font name
$display1-font;

// Display 1 line height
$display1-line-height;

// Display 1 font size
$display1-size;

// Display 1 font tracking
$display1-tracking;

// Display 1 font weight
$display1-weight;

// Display 2
$display2: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Display 2 font name
$display2-font;

// Display 2 line height
$display2-line-height;

// Display 2 font size
$display2-size;

// Display 2 font tracking
$display2-tracking;

// Display 2 font weight
$display2-weight;

// Display 3
$display3: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Display 3 font name
$display3-font;

// Display 3 line height
$display3-line-height;

// Display 3 font size
$display3-size;

// Display 3 font tracking
$display3-tracking;

// Display 3 font weight
$display3-weight;

// Headline 1
$headline1: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Headline 1 font name
$headline1-font;

// Headline 1 line height
$headline1-line-height;

// Headline 1 font size
$headline1-size;

// Headline 1 font tracking
$headline1-tracking;

// Headline 1 font weight
$headline1-weight;

// Headline 2
$headline2: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Headline 2 font name
$headline2-font;

// Headline 2 line height
$headline2-line-height;

// Headline 2 font size
$headline2-size;

// Headline 2 font tracking
$headline2-tracking;

// Headline 2 font weight
$headline2-weight;

// Headline 3
$headline3: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Headline 3 font name
$headline3-font;

// Headline 3 line height
$headline3-line-height;

// Headline 3 font size
$headline3-size;

// Headline 3 font tracking
$headline3-tracking;

// Headline 3 font weight
$headline3-weight;

// Headline 4
$headline4: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Headline 4 font name
$headline4-font;

// Headline 4 line height
$headline4-line-height;

// Headline 4 font size
$headline4-size;

// Headline 4 font tracking
$headline4-tracking;

// Headline 4 font weight
$headline4-weight;

// Headline 5
$headline5: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Headline 5 font name
$headline5-font;

// Headline 5 line height
$headline5-line-height;

// Headline 5 font size
$headline5-size;

// Headline 5 font tracking
$headline5-tracking;

// Headline 5 font weight
$headline5-weight;

// Headline 6
$headline6: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Headline 6 font name
$headline6-font;

// Headline 6 line height
$headline6-line-height;

// Headline 6 font size
$headline6-size;

// Headline 6 font tracking
$headline6-tracking;

// Headline 6 font weight
$headline6-weight;

// Overline
$overline: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Overline font name
$overline-font;

// Overline line height
$overline-line-height;

// Overline font size
$overline-size;

// Overline text transform
$overline-text-transform;

// Overline font tracking
$overline-tracking;

// Overline font weight
$overline-weight;

// Subhead 1
$subhead1: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Subhead 1 font name
$subhead1-font;

// Subhead 1 line height
$subhead1-line-height;

// Subhead 1 font size
$subhead1-size;

// Subhead 1 font tracking
$subhead1-tracking;

// Subhead 1 font weight
$subhead1-weight;

// Subhead 2
$subhead2: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Subhead 2 font name
$subhead2-font;

// Subhead 2 line height
$subhead2-line-height;

// Subhead 2 font size
$subhead2-size;

// Subhead 2 font tracking
$subhead2-tracking;

// Subhead 2 font weight
$subhead2-weight;

// Subtitle 1
$subtitle1: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Subtitle 1 font name
$subtitle1-font;

// Subtitle 1 line height
$subtitle1-line-height;

// Subtitle 1 font size
$subtitle1-size;

// Subtitle 1 font tracking
$subtitle1-tracking;

// Subtitle 1 font weight
$subtitle1-weight;

// Subtitle 2
$subtitle2: (
font-family,
font-size,
font-weight,
letter-spacing,
line-height,
text-transform,
);

// Subtitle 2 font name
$subtitle2-font;

// Subtitle 2 line height
$subtitle2-line-height;

// Subtitle 2 font size
$subtitle2-size;

// Subtitle 2 font tracking
$subtitle2-tracking;

// Subtitle 2 font weight
$subtitle2-weight;

/** ELEVATION */

// 0
$level0;

// +1
$level1;

// +2
$level2;

// +3
$level3;

// +4
$level4;

// +5
$level5;

Thanks for listing it out. Since this PR is pretty large already, would it be ok if I added these in a follow-up?

Yeah, I think that's reasonable, I just want to make sure we do this before adding another component on top of the tokens system

src/material-experimental/mdc-slide-toggle/_slide-toggle-theme.scss

src/material-experimental/mdc-slide-toggle/slide-toggle.scss

jelbourn · 2021-07-13T19:10:42Z

src/material-experimental/mdc-slide-toggle/slide-toggle.ts

+    }
+
+    // Even though we have a `label` element with a `for` pointing to the button, we need the
+    // `aria-labelledby`, because the button gets flagged as not having a label by tools like axe.


Might be worth filing an issue with axe

src/material-experimental/mdc-slide-toggle/_slide-toggle-theme.scss

src/material-experimental/mdc-slide-toggle/slide-toggle.scss

src/material-experimental/mdc-slide-toggle/slide-toggle.ts

crisbeto · 2021-07-15T20:15:36Z

All of the feedback should be addressed now. I'll wait for a green master build before rebasing.

crisbeto · 2021-07-16T06:41:43Z

Good to go now.

jelbourn · 2021-07-20T20:50:00Z

scripts/check-mdc-tests-config.ts

+      // The MDC implementation uses a `button` instead of an `input` which can't be required.
+      'should forward the required attribute',
+      'should prevent the form from submit when being required',
+


The feedback I got was to focus on how it actually works in the various assistive tech, so we should just test it with a few screen-readers and see how it works. I tested having aria-required on the <button> and it worked as expected on VoiceOver and NVDA- I think we should try to keep it and update the axe configuration to accept it.

jelbourn · 2021-07-20T20:52:37Z

src/material-experimental/mdc-slide-toggle/_slide-toggle-theme.scss

+
+// Generates all color mapping for the properties that only change based on the theme.
+@function _get-theme-base-map($is-dark) {
+  $on-surface: if($is-dark, mdc-color-palette.$grey-100, mdc-color-palette.$grey-800);


Yeah, I think that's reasonable, I just want to make sure we do this before adding another component on top of the tokens system

src/material-experimental/mdc-slide-toggle/slide-toggle.scss

jelbourn · 2021-07-21T15:06:16Z

src/material-experimental/mdc-slide-toggle/slide-toggle.html

+    [class.mdc-switch--selected]="checked"
+    [class.mdc-switch--unselected]="!checked"
+    [tabIndex]="tabIndex"
+    [disabled]="disabled"


When I tested the demo (looking at aria-required), I was able to toggle the disabled slide-toggles with the keyboard.

I just tried it and I couldn't reproduce. Are you sure that you weren't interacting with the top slide toggle which also changes the value of the disabled one?

Ahh, I see what happened. With VoiceOver on, screen-reader navigation moved onto the disabled toggle, but browser focus stayed on the enabled one. Nvm.

crisbeto · 2021-07-22T14:28:33Z

I've added aria-required and un-deprecated the input.

jelbourn

LGTM

src/material/slide-toggle/testing/slide-toggle-harness.ts

…d styles Switches the MDC-based slide toggle to the non-deprecated MDC styles. Notable changes: * New markup which uses a `button` instead of an `input` for the button. * New icons inside the slide toggle's thumb. Technically we could opt out of them, but I think that they look better and they help with accessibility for color-blind users. * New theming system that uses a flat list of variables. There is a fallback for IE11, but I opted not to include it for now, because of the upcoming deprecation and the fact that the component is in experimental. * The component has some slightly different colors and it supports more states (e.g. hover). * Due to the switch from `input` to `button`, the `required` input is basically a noop now.

angular-automatic-lock-bot · 2021-09-06T00:02:14Z

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

crisbeto added P2 The issue is important to a large percentage of users, with a workaround target: minor This PR is targeted for the next minor release labels Jul 10, 2021

crisbeto requested review from jelbourn and mmalerba July 10, 2021 12:37

crisbeto requested review from devversion and a team as code owners July 10, 2021 12:37

google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jul 10, 2021

crisbeto force-pushed the mdc-slide-toggle-styles branch from 0405b0e to 3b2e7cb Compare July 10, 2021 14:05

josephperrott removed the request for review from a team July 12, 2021 22:05

jelbourn reviewed Jul 13, 2021

View reviewed changes

crisbeto force-pushed the mdc-slide-toggle-styles branch 2 times, most recently from d6d2806 to 8f10f50 Compare July 14, 2021 18:02

asyncLiz reviewed Jul 14, 2021

View reviewed changes

src/material-experimental/mdc-slide-toggle/_slide-toggle-theme.scss Outdated Show resolved Hide resolved

asyncLiz reviewed Jul 14, 2021

View reviewed changes

src/material-experimental/mdc-slide-toggle/slide-toggle.scss Show resolved Hide resolved

asyncLiz reviewed Jul 14, 2021

View reviewed changes

src/material-experimental/mdc-slide-toggle/slide-toggle.scss Outdated Show resolved Hide resolved

asyncLiz reviewed Jul 15, 2021

View reviewed changes

src/material-experimental/mdc-slide-toggle/slide-toggle.ts Outdated Show resolved Hide resolved

crisbeto force-pushed the mdc-slide-toggle-styles branch from 8f10f50 to 83ef5af Compare July 15, 2021 20:14

crisbeto force-pushed the mdc-slide-toggle-styles branch 2 times, most recently from 1e3548a to 300cedb Compare July 16, 2021 06:45

jelbourn reviewed Jul 21, 2021

View reviewed changes

crisbeto force-pushed the mdc-slide-toggle-styles branch from 300cedb to 0855875 Compare July 22, 2021 14:27

jelbourn approved these changes Jul 22, 2021

View reviewed changes

jelbourn added the action: merge The PR is ready for merge by the caretaker label Jul 22, 2021

crisbeto force-pushed the mdc-slide-toggle-styles branch 2 times, most recently from c3ecae0 to eb14fa3 Compare July 22, 2021 16:15

crisbeto mentioned this pull request Jul 24, 2021

fix(material-experimental/mdc-checkbox): switch to non-deprecated styles #23218

Merged

mmalerba reviewed Jul 26, 2021

View reviewed changes

src/material/slide-toggle/testing/slide-toggle-harness.ts Outdated Show resolved Hide resolved

crisbeto force-pushed the mdc-slide-toggle-styles branch from eb14fa3 to eeedcf8 Compare July 27, 2021 05:40

jelbourn mentioned this pull request Aug 6, 2021

Support theming with CSS Custom Properties #4352

Open

andrewseguin merged commit a4ae3b8 into angular:master Aug 6, 2021

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(material-experimental/mdc-slide-toggle): switch to non-deprecated styles #23143

feat(material-experimental/mdc-slide-toggle): switch to non-deprecated styles #23143

crisbeto commented Jul 10, 2021 •

edited

jelbourn Jul 13, 2021

crisbeto Jul 13, 2021

jelbourn Jul 14, 2021

crisbeto Jul 14, 2021

jelbourn Jul 14, 2021

jelbourn Jul 20, 2021

jelbourn Jul 13, 2021

crisbeto Jul 13, 2021

jelbourn Jul 14, 2021

crisbeto Jul 14, 2021

jelbourn Jul 14, 2021

crisbeto Jul 15, 2021

jelbourn Jul 20, 2021

jelbourn Jul 13, 2021

crisbeto commented Jul 15, 2021

crisbeto commented Jul 16, 2021

jelbourn Jul 20, 2021

jelbourn Jul 20, 2021

jelbourn Jul 21, 2021

crisbeto Jul 21, 2021

jelbourn Jul 21, 2021

crisbeto commented Jul 22, 2021

jelbourn left a comment

angular-automatic-lock-bot bot commented Sep 6, 2021

feat(material-experimental/mdc-slide-toggle): switch to non-deprecated styles #23143

feat(material-experimental/mdc-slide-toggle): switch to non-deprecated styles #23143

Conversation

crisbeto commented Jul 10, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crisbeto commented Jul 15, 2021

crisbeto commented Jul 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crisbeto commented Jul 22, 2021

jelbourn left a comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 6, 2021

crisbeto commented Jul 10, 2021 •

edited