angular / components

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

bug(Toolbar): Defined background colors does not get applied with version 17.3.1 #28805

Open Asac2142 opened 3 months ago

Asac2142 commented 3 months ago

Is this a regression?

The previous version in which this bug was not present was

16

Description

If you go to the official Angular Material docs, and select component Toolbar, examples tab, you see in action the usage of toolbar with a color defined such as primary but you don't see any color applied to it.

If you open up any of the examples to Stackblitz, you can clearly see that any of the defined colors "warn", "primary", "accent" does not get applied in the toolbar property

<mat-toolbar color="primary">
 ...
</mat-toolbar>

<mat-toolbar color="warn">
 ...
</mat-toolbar>

<mat-toolbar color="accent">
 ...
</mat-toolbar>

image

Reproduction

StackBlitz link: https://stackblitz.com/edit/ga5498?file=src%2Fexample%2Ftoolbar-overview-example.html

Steps to reproduce:

  1. Specify one of the colors predefined such as warn, primary, accent to the toolbar component

Expected Behavior

If the developer specifies a color to a toolbar, either a predefined one or a custom one, then the color should take effect into the material component (e.g toolbar)

Actual Behavior

Specifying any of the predefined colors does not get applied in the toolbar component

Environment

BaronVonPerko commented 3 months ago

I've experienced this as well playing with the new M3 styles.

BaronVonPerko commented 3 months ago

I think I may have found the issue, if I can fix it I will raise a PR

amysorto commented 3 months ago

This is intentional as it is a part of the M3 spec. Althought yes the docs and examples should be updated to remove these custom colors that used to work in M2.

Asac2142 commented 3 months ago

This is intentional as it is a part of the M3 spec. Althought yes the docs and examples should be updated to remove these custom colors that used to work in M2.

Your answer quite scares me, you are saying this is intentional, so the other bugs reported based on missing colors applied in other Angular Material components are okay, is not a bug, is a feature. M3 doesn't look correct.

What's the point of having a toolbar with the attribute color if none of the colors will be applied? in order words, what is the purpose of doing this

<mat-toolbar color="warn">...</mat-toolbar>

If neither warn, primary, accent, or any custom color is applied? Don't tell me the color attribute is going to be deprecated.

BaronVonPerko commented 3 months ago

This is intentional as it is a part of the M3 spec. Althought yes the docs and examples should be updated to remove these custom colors that used to work in M2.

Your answer quite scares me, you are saying this is intentional, so the other bugs reported based on missing colors applied in other Angular Material components are okay, is not a bug, is a feature. M3 doesn't look correct.

What's the point of having a toolbar with the attribute color if none of the colors will be applied? in order words, what is the purpose of doing this

<mat-toolbar color="warn">...</mat-toolbar>

If neither warn, primary, accent, or any custom color is applied? Don't tell me the color attribute is going to be deprecated.

Agreed. If this is being deprecated, it needs to be made known in the docs. I understand that m2 will be supported for quite some time, but this is a pretty big change.

wagnermaciel commented 3 months ago

Hi, maybe it would be good to take a step back and add some context here

The purpose of the color input is to allow users to switch which palette is being used for a specific component. This functionality will still exist in M3 via the optional backwards compatibility api

https://material.angular.io/guide/material-3#optional-add-backwards-compatibility-styles-for-color-variants

The important distinction here is that in M3, the background color of the toolbar (or app bar as it is called on their docs) comes from the "neutral" palette. Because of this, the background color of the toolbar is always a neutral tone, even when switching between different palettes

https://m3.material.io/components/top-app-bar/specs

This is different from the M2 toolbar which used a color for it's background https://m2.material.io/components/app-bars-top

Edit: Also, here is an example showcasing that the color input still works in M3 https://stackblitz.com/edit/ga5498-wp2khz?file=src%2Fexample%2Ftoolbar-overview-example.html

BaronVonPerko commented 3 months ago

Thank you for the clarification @wagnermaciel. I'm happy to learn about this backwards compatibility!

@include matx.color-variants-back-compat($theme);

BaerMitUmlaut commented 3 months ago

The important distinction here is that in M3, the background color of the toolbar (or app bar as it is called on their docs) comes from the "neutral" palette. Because of this, the background color of the toolbar is always a neutral tone, even when switching between different palettes

grafik

The material docs are a great example why this doesn't work well with a global navbar. Remove the light blue background and it looks awful.

Peperud commented 2 months ago

The important distinction here is that in M3, the background color of the toolbar (or app bar as it is called on their docs) comes from the "neutral" palette. Because of this, the background color of the toolbar is always a neutral tone, even when switching between different palettes

grafik

The material docs are a great example why this doesn't work well with a global navbar. Remove the light blue background and it looks awful.

Touché!

Just upgraded an app and ran into this (and not only). My eyes hurt...

probert94 commented 1 month ago

It looks like the toolbar uses --mat-toolbar-container-background-color as background and --mat-toolbar-container-text-color as color. That said, you should be able to change this properties to primary and primary-contrast etc. using theming. If you use a similar approach to what I described here, it should be as simple as this:

mat-toolbar[color="primary"] {
  --mat-toolbar-container-background-color: var(--mat-primary);
  --mat-toolbar-container-text-color: var(--mat-on-primary);
}

I haven't tried this yet, so I am not sure if it works like this!

PinkTiu commented 1 month ago

The separation between toolbar and content looks so bad because the scrolling behavior of the toolbar does not meet the v3 specification.

Only upon scrolling, the top app bar container fills with a contrasting color that provides visual separation from the background.

https://m3.material.io/components/top-app-bar/guidelines#4eab4f50-4a3e-4189-bce2-a46514cde1da

You probably have to implement the color change during scrolling by yourself.

blogcraft commented 1 month ago

The important distinction here is that in M3, the background color of the toolbar (or app bar as it is called on their docs) comes from the "neutral" palette. Because of this, the background color of the toolbar is always a neutral tone, even when switching between different palettes

https://m3.material.io/components/top-app-bar/specs

How can we make the toolbar color be the primary color of the pallete? Because a neutral color for the bar is kind of useless and not even demoed in the angular material page.

blogcraft commented 1 month ago

Material theming is so broken, or the docs are so confusing/incomplete that I did not find a "correct" way to do this, so I have these 2 alternatives that I don't really like, but given what Angular has provided i guess it is what it is:

Option 1:

@mixin app-theme($theme, $type: "light") {
  @if $type == "light" {
    @include mat.all-component-themes($theme);
    .mat-toolbar {
      --mat-toolbar-container-text-color: #{mat.get-theme-color(
          $theme,
          primary,
          100
        )};
      --mat-toolbar-container-background-color: #{mat.get-theme-color(
          $theme,
          primary
        )};
      --mdc-icon-button-icon-color: #{mat.get-theme-color(
          $theme,
          primary,
          100
        )};
    }
 } @else {
    @include mat.all-component-colors($theme);
    .mat-toolbar {
      --mat-toolbar-container-text-color: #{mat.get-theme-color(
          $theme,
          primary,
          20
        )};
      --mat-toolbar-container-background-color: #{mat.get-theme-color(
          $theme,
          primary
        )};
      --mdc-icon-button-icon-color: #{mat.get-theme-color($theme, primary, 20)};
    }
  }
}

html {
  // .app-light-theme (default)
  @include app-theme($app-light-theme, "light");

  .app-dark-theme {
    @include app-theme($app-dark-theme, "dark");
  }

  @media (prefers-color-scheme: light) {
    .app-auto-theme {
      @include app-theme($app-light-theme, "light");
    }
  }

  @media (prefers-color-scheme: dark) {
    .app-auto-theme {
      @include app-theme($app-dark-theme, "dark");
    }
  }
}

Option 2:

@mixin app-theme($theme, $type: "light") {
  @if $type == "light" {
    @include mat.all-component-themes($theme);
    .mat-toolbar {
      background: mat.get-theme-color($theme, primary);
      color: mat.get-theme-color($theme, primary, 100);
      .mat-mdc-icon-button {
        color: mat.get-theme-color($theme, primary, 100);
      }
    }
 } @else {
    @include mat.all-component-colors($theme);
    .mat-toolbar {
      background: mat.get-theme-color($theme, primary);
      color: mat.get-theme-color($theme, primary, 20);
      .mat-mdc-icon-button {
        color: mat.get-theme-color($theme, primary, 20);
      }
    }
  }
}

html {
  // .app-light-theme (default)
  @include app-theme($app-light-theme, "light");

  .app-dark-theme {
    @include app-theme($app-dark-theme, "dark");
  }

  @media (prefers-color-scheme: light) {
    .app-auto-theme {
      @include app-theme($app-light-theme, "light");
    }
  }

  @media (prefers-color-scheme: dark) {
    .app-auto-theme {
      @include app-theme($app-dark-theme, "dark");
    }
  }
}

Any Thoughts?

tomnar commented 1 month ago

It is hard for me to see how going away from color="primary" is an improvement. A white toolbar looks broken and the suggested fixes are much more involved. I understand that the team is bound by the specification, but what did they think when it was written? Just compare https://m3.material.io/ to https://m2.material.io/. It has gotten so much worse! the website looks bad, is hard to use and is confusing. So sad.

probert94 commented 1 month ago

@blogcraft In the dark theme you are using the same color for text and background, this is probably wrong. If you can use the same properties (can still be different values!) from the theme for both light and dark theme (f.e. in both cases the primary 100 as text color) it would be a lot simpler. If you really need to use different properties (not only different values for those properties), then you could probably still simpify if by using scss build-in if function. Other then that your solution seems to be more or less as described in Theming your own components.

However, I personally think that theming in Angular Material is pretty complex (imho too complex) in general:

In a new project I experimented with a new approach which I described here which basically defines css custom properties for all possible theme variables. That way I can easily use them everywhere, and defining the color for the toolbar can be simplified to:

.mat-toolbar {
    --mat-toolbar-container-text-color: var(--mat-on-primary);
    --mat-toolbar-container-background-color: var(--mat-primary);
    --mdc-icon-button-icon-color: var(--mat-on-primary);
}
blogcraft commented 1 month ago

It is hard for me to see how going away from color="primary" is an improvement. A white toolbar looks broken and the suggested fixes are much more involved. I understand that the team is bound by the specification, but what did they think when it was written? Just compare https://m3.material.io/ to https://m2.material.io/. It has gotten so much worse! the website looks bad, is hard to use and is confusing. So sad.

They are trying to androdify the web and it's just ridiculous.... And the worst part is that they are "bound" to the spec, but they don't even follow it (toolbar color on scroll, card dynamic color, real tooltips, progress bar and spinner, etc...)! Also there are components defined in the spec that have never arrived to Angular Material and they don't seem to care to create them (navigation rail, carousel, time pickers...).

@blogcraft In the dark theme you are using the same color for text and background, this is probably wrong. If you can use the same properties (can still be different values!) from the theme for both light and dark theme (f.e. in both cases the primary 100 as text color) it would be a lot simpler. If you really need to use different properties (not only different values for those properties), then you could probably still simpify if by using scss build-in if function. Other then that your solution seems to be more or less as described in Theming your own components.

I had an error I updated, in text and icon y use 100 for light and 20 for dark trying to mimic the theme. But still don't like it very much.

Also trying to figure out all the tokens and mat functions and how to use them is confusing and a waste of time IMO.

artwaha commented 4 weeks ago

I truly thought i was the only who finds m3 weidly bad. I've been using the navigation schematic for setting my main layout in previous versions...after i updated to material v18, the navigation schematic just outputs a white navigation component...link buttons are white, toolbar is white...you cant even customise the toolbar color without defining some comples themes...I'm new to this angular ecosystem (started last year on v17) but materia v18 seems like a downgrade really!

jvdavim commented 2 days ago

I'm not used to front-end stuff, but I made it work with CSS classes in the theme file. I think it's simpler and easier to understand for my use case. But, I don't know if it scales well (I'm working on a small project).

@use '@angular/material' as mat;
@use 'src/styles/m3-theme';

@include mat.core();

$theme: m3-theme.$light-theme;

@mixin color($theme) {
  .tertiary-button {
    @include mat.button-color($theme, $color-variant: tertiary);
  }

  .tertiary-progress-bar {
    @include mat.progress-bar-color($theme, $color-variant: tertiary);
  }

  .tertiary-icon {
    @include mat.icon-color($theme, $color-variant: tertiary);
  }

  .primary-toolbar {
    background-color: mat.get-theme-color($theme, primary);
    color: mat.get-theme-color($theme, neutral, 100);
  }
}

@mixin typography($theme) {
  -webkit-font-smoothing: antialiased;

  .headline-medium-black {
    font: mat.get-theme-typography($theme, headline-medium, font);
    color: mat.get-theme-color($theme, neutral, 0);
  }

  .headline-small-black {
    font: mat.get-theme-typography($theme, headline-small, font);
    color: mat.get-theme-color($theme, neutral, 0);
  }

  .headline-small-white {
    font: mat.get-theme-typography($theme, headline-small, font);
    color: mat.get-theme-color($theme, neutral, 100);
  }

  .label-medium-white {
    font: mat.get-theme-typography($theme, label-medium, font);
    color: mat.get-theme-color($theme, neutral, 100);
  }
}

html {
  @include mat.all-component-themes($theme);
  @if mat.theme-has($theme, color) {
    @include color($theme);
  }

  @if mat.theme-has($theme, typography) {
    @include typography($theme);
  }
}

Then I use these global CSS classes to change my components styles.