angular / components

Component infrastructure and Material Design components for Angular
https://material.angular.io
MIT License
24.32k stars 6.73k forks source link

bug(material/select): Material 3 mat-selects use the secondary color for selected item backgrounds (when the component is styled to primary) #29553

Closed bdirito closed 3 days ago

bdirito commented 1 month ago

Is this a regression?

Material 2 uses a static color for this highlight.

The previous version in which this bug was not present was

No response

Description

The styling of a select (mat-select) highlights the background of a selected element. However even when you are using the default 'primary' styling of a select (as documented https://material.angular.io/guide/theming#using-component-color-variants) the background highlighting of the selected option in the select uses the secondary palette.

Screenshot 2024-08-07 at 10 23 41 AM

Reproduction

This is a stackblitz forked from the mat-select documentation. It has been adjusted to use custom styles as produced by the output of ng generate @angular/material:m3-theme with the following colors: primary: ff0000, secondary: 00ff00, tertiary: 0000ff. The generated m3-theme.scss was copied over into that stack blitz and m3-theme.$light-theme was then used for @include mat.all-component-themes(m3-theme.$light-theme) in styles.scss

StackBlitz link: https://stackblitz.com/edit/nngoxu-ggvcsg?file=src%2Fstyles.scss Steps to reproduce:

  1. click to select element to open up the options list; note the background of the selected element is miscolored

Expected Behavior

I would expect that the selected background color of a given palette to either be static (as in M2's rgba(0, 0, 0, 0.04)) or to be based on the colors in the palette chosen to style that component.

Possible work arounds:

For my code I went with option 3; --mat-option-selected-state-layer-color: rgba(0, 0, 0, 0.04);. Note for option 3, as this is styling something in a cdk-overlay-container you have to make sure that your override still applies.

Actual Behavior

The background color of a 'primary' (default) styled select appears to be using the color as specified in the secondary palette at the '90' level. This 'works' when you use the included scss material palettes because the 'secondary' palette is bundled as a subelement of that palette. A trimmed version of the 'rose' palette (rose/red is one of the styling options on the angular material page), as found in @angular/material/core/themeing/_palettes.scss looks like

/// Rose color palette to be used as primary or tertiary palette.
$rose-palette: _apply-patches((
  0: #000000,
  10: #3f001b,
  <... color levels ...>
  secondary: (
    <... color levels ...>
    90: #ffd9e1, // this is what the selected element actually uses on material.angular.io when using the 'Rose and Red' theme
    <... more color levels>
  ),
  neutral: (
    <... color levels...>
  ),
  neutral-variant: (
     <... color levels ...>
  ),
));

So even for the documentation stylings all of the secondary palettes are mutated forms of the primary palettes and so the selected element's background looks good. When using the ng generate @angular/material:m3-theme no such binding between the primary palette & secondary palette is implied or generated. So when it selects a secondary.90 for styling the background of a primary styled mat-select component it looks wrong.

Environment

amysorto commented 2 weeks ago

The M3 spec for select (https://m3.material.io/components/menus/specs), has the menu container color as the surface-container color and secondary-container for when it is selected.

For the colors in your theme it seems to be the correct colors you are seeing (the screenshot is from Material's Theme Builder):

Image

The surface-container color role comes from the neutral palette. But if not overriden the neutral palette key color comes from your primary color, which is why it has a pink tint since the primary color is a red.

Image

As mentioned you can use some of our predefined palettes or generate a theme with colors that are maybe less drastic than each other. It seems that maybe a triadic color scheme like the one above doesn't fit well into Material's specs for certain use cases like this. Overriding the CSS variables seems like the best case scenario for your colors (ex. --mat-option-selected-state-layer-color: red).

Also as a note for choosing a different color, you can read colors from a specific tonal palette or read color roles from your theme.

bdirito commented 2 weeks ago

@amysorto Thanks for the response; but I don't think this is completely resolved.

The M3 docs you linked don't talk about color theming variants at all. However when we look at the colors chosen its all 'secondary' colors. So for example Menu list item with trailing icon - icon color (ie the red checkmark next to pizza in the original image) is specified as md.sys.color.on-secondary-container. I split the colors in this demo into rgb as an easier way to track what color palette stuff comes from; red is the primary palette (green secondary & blue tertiary). So even though the M3 docs are specifying on-secondary-container we end up using on-primary-container for that checkmark. Other elements of the component also use the primary palette; such as for non-selected background (which is red tinted) etc.

This inconsistency of 'secondary' vs 'primary' I believe is due to the introduction of component color variants as via https://material.angular.io/guide/theming#using-component-color-variants . The docs for M3 menu (https://m3.material.io/components/menus/specs) seem to be using variants of the 'secondary' (or neutral) color for everything. Thus when using a 'select' styled to use the 'primary' component color variant, all of these 'secondary' colors have been shifted to instead be 'primary' colors.

Thus I'd argue that if configuring select to be 'primary' color variant updates the Menu list item with trailing icon - icon color from md.sys.color.on-secondary-container to md.sys.color.on-primary-container then configuring the select to be the 'primary' color variant should also update Menu list item selected container color from md.sys.color.secondary-container to md.sys.color.primary-container.

amysorto commented 2 weeks ago

The M3 docs have a table with color roles, thats what I was referring to initially for where these color roles are coming from.

Ahh I see what you mean. It seems the checkmark should be on-surface-variant from the spec. It may just be an incorrect mapping rather than related to having the color variants support.

I'll reopen this issue and take a closer look! Thanks!