Skip to content

Commit

Permalink
fix(menu,list): new sys token mismatch between menu and list
Browse files Browse the repository at this point in the history 
in the latest set of tokens there was a mismatch betwee color in menu container and list-item container. In this PR we:

- Refactor menu and list token value function to have supported and unsupported token lists
- Apply menu-container-color to list-item-container-color
  - Do some appropriate token renames to reflect this
- Move menu elevation layer below the list items
  - We do this because the new v172 tokens fix the colors here by flattening elevation

PiperOrigin-RevId: 523531521
  • Loading branch information
@material-web-copybara @copybara-github
material-web-copybara authored and copybara-github committed Apr 11, 2023
1 parent 301eb9a commit 55df403
Show file tree
Hide file tree
Showing 10 changed files with 469 additions and 207 deletions.
55 changes: 6 additions & 49 deletions list/lib/_list.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,15 @@
// go/keep-sorted end

@mixin theme($tokens) {
$reference: tokens.md-comp-list-values();
$reference: resolve-tokens($reference);
$tokens: resolve-tokens($tokens);
$tokens: theme.validate-theme($reference, $tokens);
$tokens: theme.create-theme-vars($tokens, list);
$tokens: theme.validate-theme(tokens.md-comp-list-values(), $tokens);

@include theme.emit-theme-vars($tokens);
@each $token, $value in $tokens {
--md-list-#{$token}: #{$value};
}
}

@mixin styles() {
$tokens: resolve-tokens(tokens.md-comp-list-values());
$tokens: theme.create-theme-vars($tokens, list);
$tokens: tokens.md-comp-list-values();

:host {
@each $token, $value in $tokens {
Expand All @@ -42,6 +39,7 @@

.md3-list {
background-color: var(--_container-color);
border-radius: inherit;
display: block;
list-style-type: none;
margin: 0;
Expand All @@ -53,44 +51,3 @@
position: relative;
}
}

/// Resolves the tokens that are specific to list.
///
/// The tokenset for list include list plus all of list item. We do not want to
/// duplicate tokens and custom properties across md-list and md-list-item.
/// So here we resolve list-items tokens, and resolve the difference between
/// the full set of tokens and the ones specific to list-item.
@function resolve-tokens($tokens) {
// Accept container-color token in the theme() mixin.
$container-color: map.get($tokens, container-color);

@if not $container-color {
// If not defined, rename the list-item-container-color token. This is
// useful for our styles() mixin where we simply feed this function the comp
// tokens from the compiler and no `container-color` token.
$container-color: map.get($tokens, list-item-container-color);
$tokens: map.set($tokens, container-color, $container-color);
}

$list-tokens: remove-unused-tokens($tokens);

@return $list-tokens;
}

// removes unused tokens
@function remove-unused-tokens($tokens) {
$unused-tokens: ();
@each $token in map-keys($tokens) {
$index: string.index($token, 'list-item');

@if $index {
$unused-tokens: list.append($unused-tokens, $token);
}
}

@each $token in $unused-tokens {
$tokens: map.remove($tokens, $token);
}

@return $tokens;
}
74 changes: 5 additions & 69 deletions list/lib/listitem/_list-item.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,15 @@
// go/keep-sorted end

@mixin theme($tokens) {
$tokens: theme.validate-theme(
resolve-tokens(tokens.md-comp-list-values()),
resolve-tokens($tokens)
);
$tokens: theme.create-theme-vars($tokens, 'list');
$tokens: theme.validate-theme(tokens.md-comp-list-item-values(), $tokens);

@include theme.emit-theme-vars($tokens);
@each $token, $value in $tokens {
--md-list-item-#{$token}: #{$value};
}
}

@mixin styles() {
$tokens: resolve-tokens(tokens.md-comp-list-values());
$tokens: theme.create-theme-vars($tokens, 'list');
$tokens: tokens.md-comp-list-item-values();

:host {
@each $token, $value in $tokens {
Expand Down Expand Up @@ -417,64 +414,3 @@
height: var(--_list-item-large-leading-video-height);
}
}

/// Resolves the tokens that are specific to list-item.
///
/// The tokenset for list include list plus all of list item. We do not want to
/// duplicate tokens and custom properties across md-list and md-list-item.
/// So here we resolve list-items tokens by selecting only tokens that have
/// 'list-item' in its name.
@function resolve-tokens($tokens) {
// Values pulled from b/198759625 spreadsheet
$list-item-tokens: (
'list-item-leading-space': 16px,
'list-item-trailing-space': 16px,
'list-item-leading-element-leading-space': 16px,
'list-item-leading-video-leading-space': 0px,
// Commented out until the comments in the spreadsheet linked above are
// resolved regarding vertical alignment.
// 'list-item-leading-item-top-space': 8px,
// 'list-item-leading-video-top-space': 0px,
// 'list-item-leading-item-bottom-space': 8px,
// 'list-item-leading-video-bottom-space': 0px,
'list-item-trailing-element-headline-trailing-element-space': 16px,
);

@each $token, $value in $tokens {
$index: string.index($token, 'list-item');

@if $index {
$list-item-tokens: map.set($list-item-tokens, $token, $value);
}
}

$list-item-tokens: _remove-unused-tokens($list-item-tokens);

@return $list-item-tokens;
}

// removes unused tokens
@function _remove-unused-tokens($tokens) {
$unused-tokens: (
'list-item-container-elevation',
'list-item-disabled-state-layer-color',
'list-item-disabled-state-layer-opacity',
'list-item-dragged-container-elevation',
'list-item-dragged-label-text-color',
'list-item-dragged-leading-icon-icon-color',
'list-item-dragged-state-layer-color',
'list-item-dragged-state-layer-opacity',
'list-item-dragged-trailing-icon-icon-color',
'list-item-overline-color',
'list-item-overline-type',
'list-item-selected-trailing-icon',
'list-item-selected-trailing-icon-color',
'list-item-unselected-trailing-icon-color'
);

@each $token in $unused-tokens {
$tokens: map.remove($tokens, $token);
}

@return $tokens;
}
34 changes: 6 additions & 28 deletions menu/lib/_menu.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,16 @@
@use '../../tokens';
// go/keep-sorted end

$_custom-property-prefix: 'menu';
@mixin theme($tokens) {
$tokens: theme.validate-theme(tokens.md-comp-menu-values(), $tokens);

@mixin theme($theme) {
$theme: theme.validate-theme(tokens.md-comp-menu-values(), $theme);
$theme: _resolve-tokens($theme);
$theme: theme.create-theme-vars($theme, $_custom-property-prefix);

@include theme.emit-theme-vars($theme);
@each $token, $value in $tokens {
--md-menu-#{$token}: #{$value};
}
}

@mixin styles() {
$tokens: tokens.md-comp-menu-values();
$tokens: _resolve-tokens($tokens);
$tokens: theme.create-theme-vars($tokens, $_custom-property-prefix);

:host {
@each $token, $value in $tokens {
Expand All @@ -39,7 +35,7 @@ $_custom-property-prefix: 'menu';

@include md-list.theme(
(
'list-item-container-color': var(--_container-color),
'container-color': var(--_container-color),
)
);

Expand Down Expand Up @@ -103,21 +99,3 @@ $_custom-property-prefix: 'menu';
}
}
}

@function _resolve-tokens($tokens) {
// removes unused tokens
$unused-tokens: ();
@each $token in map-keys($tokens) {
$index: string.index($token, 'list-item');

@if $index {
$unused-tokens: list.append($unused-tokens, $token);
}
}

@each $token in $unused-tokens {
$tokens: map.remove($tokens, $token);
}

@return $tokens;
}
2 changes: 1 addition & 1 deletion menu/lib/menu.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,8 @@ export abstract class Menu extends LitElement {
class="menu ${classMap(this.getSurfaceClasses())}"
style=${styleMap(this.menuPositionController.surfaceStyles)}
@focusout=${this.handleFocusout}>
${this.renderList()}
${this.renderElevation()}
${this.renderList()}
${this.renderFocusRing()}
</div>
`;
Expand Down
45 changes: 13 additions & 32 deletions menu/lib/menuitem/_menu-item.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,33 @@
@use 'sass:string';
// go/keep-sorted end
// go/keep-sorted start
@use '../../../list/list-item';
@use '../../../ripple/ripple';
@use '../../../sass/theme';
@use '../../../tokens';
// go/keep-sorted end

$_custom-property-prefix: 'menu';
@mixin theme($tokens) {
$tokens: theme.validate-theme(tokens.md-comp-menu-item-values(), $tokens);

@mixin theme($theme) {
$theme: theme.validate-theme(tokens.md-comp-menu-values(), $theme);
$theme: _resolve-tokens($theme);
$theme: theme.create-theme-vars($theme, $_custom-property-prefix);

@include theme.emit-theme-vars($theme);
@each $token, $value in $tokens {
--md-menu-item-#{$token}: #{$value};
}
}

@mixin styles() {
$tokens: tokens.md-comp-menu-values();
$tokens: _resolve-tokens($tokens);
$tokens: theme.create-theme-vars($tokens, $_custom-property-prefix);
$tokens: tokens.md-comp-menu-item-values();

:host {
@each $token, $value in $tokens {
--_#{$token}: #{$value};
}

@include list-item.theme(
(
'list-item-container-color': var(--_list-item-container-color),
)
);
}

.list-item {
Expand All @@ -53,25 +56,3 @@ $_custom-property-prefix: 'menu';
);
}
}

@function _resolve-tokens($tokens) {
@return _remove-unused-tokens($tokens);
}

// removes unused tokens
@function _remove-unused-tokens($tokens) {
$unused-tokens: ();
@each $token in map-keys($tokens) {
$index: string.index($token, 'list-item');

@if not $index {
$unused-tokens: list.append($unused-tokens, $token);
}
}

@each $token in $unused-tokens {
$tokens: map.remove($tokens, $token);
}

@return $tokens;
}
2 changes: 2 additions & 0 deletions tokens/_index.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@
@forward './md-comp-linear-progress-indicator' as
md-comp-linear-progress-indicator-*;
@forward './md-comp-list' as md-comp-list-*;
@forward './md-comp-list-item' as md-comp-list-item-*;
@forward './md-comp-menu' as md-comp-menu-*;
@forward './md-comp-menu-item' as md-comp-menu-item-*;
@forward './md-comp-navigation-bar' as md-comp-navigation-bar-*;
@forward './md-comp-navigation-drawer' as md-comp-navigation-drawer-*;
@forward './md-comp-navigation-rail' as md-comp-navigation-rail-*;
Expand Down
Loading

0 comments on commit 55df403

Please sign in to comment.