Skip to content

Commit

Permalink
fix(material-experimental/theming): resolve M3 issues in mat-option (a…
Browse files Browse the repository at this point in the history
…ngular#28482)

Fixes the following issues in the M3 `mat-option`:
* Font size was too small and font weight too high. This appears to have been a bug in the spec that MDC is also working around. These changes apply the same workaround as MDC.
* Multi-select options in the selected state had a darker background, even though they shouldn't. This happened to work by accident in M2 and only became visible now in M3.
  • Loading branch information
crisbeto committed Jan 26, 2024
1 parent d963df9 commit 5f1a7ea
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 4 deletions.
16 changes: 12 additions & 4 deletions src/material-experimental/theming/_custom-tokens.scss
Original file line number Diff line number Diff line change
Expand Up @@ -822,9 +822,7 @@
/// @param {Boolean} $exclude-hardcoded Whether to exclude hardcoded token values
/// @return {Map} A set of custom tokens for the mat-option
@function option($systems, $exclude-hardcoded) {
@return (mat.private-merge-all(
_generate-typography-tokens($systems, label-text, label-large),
(
@return ((
selected-state-label-text-color: map.get($systems, md-sys-color, on-secondary-container),
label-text-color: map.get($systems, md-sys-color, on-surface),
hover-state-layer-color: mat.private-safe-color-change(
Expand All @@ -836,7 +834,17 @@
$alpha: map.get($systems, md-sys-state, focus-state-layer-opacity)
),
selected-state-layer-color: map.get($systems, md-sys-color, secondary-container),
),

// According to the spec the options have to be `label-large` in all typography
// dimensions, however this is inconsistent with the designs and with MDC's
// own implementation. This appears to be a bug in the spec, because MDC overrides
// the font size and weight to be `body-large` (see b/261838263). We make the same
// override here so the label looks correct.
label-text-size: map.get($systems, md-sys-typescale, body-large-size),
label-text-weight: map.get($systems, md-sys-typescale, body-large-weight),
label-text-font: map.get($systems, md-sys-typescale, label-large-font),
label-text-line-height: map.get($systems, md-sys-typescale, label-large-line-height),
label-text-tracking: map.get($systems, md-sys-typescale, label-large-tracking),
), (
// Color variants:
primary: (
Expand Down
13 changes: 13 additions & 0 deletions src/material/core/option/option.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
@use '@material/typography/typography' as mdc-typography;

@use '../tokens/m2/mat/option' as tokens-mat-option;
@use '../tokens/m2/mdc/list' as tokens-mdc-list;
@use '../tokens/token-utils';
@use '../mdc-helpers/mdc-helpers';
@use '../style/vendor-prefixes';
Expand Down Expand Up @@ -178,6 +179,18 @@
}
}

.mat-mdc-option-multiple {
// Multi-select options in the selected state aren't supposed to change their background color,
// because the checkbox already indicates that they're selected. This happened to work in M2,
// due to `list-item-selected-container-color` being the same as `list-item-container-color`,
// but that's no longer the case in M3. This overrides ensures that the appearance is consistent.
@include token-utils.use-tokens(tokens-mdc-list.$prefix, tokens-mdc-list.get-token-slots()) {
$selected-token: token-utils.get-token-variable(list-item-selected-container-color);
$base-token: token-utils.get-token-variable(list-item-container-color);
#{$selected-token}: var(#{$base-token}, transparent);
}
}

// For options, render the focus indicator when the class .mat-mdc-option-active is present.
.mat-mdc-option-active .mat-mdc-focus-indicator::before {
content: '';
Expand Down

0 comments on commit 5f1a7ea

Please sign in to comment.