Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mat-icon color doesn't follow the guidelines #9664

Open
philip-firstorder opened this issue Jan 29, 2018 · 5 comments
Open

mat-icon color doesn't follow the guidelines #9664

philip-firstorder opened this issue Jan 29, 2018 · 5 comments
Labels
area: material/icon material spec Issue related to the Material Design spec https://material.io/design/ P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@philip-firstorder
Copy link

Bug, feature request, or proposal:

mat-icon color doesn't follow the guidelines

This example added some extra css to partially fixed the problem, it does so only for the white background theme.
screen shot 2018-01-29 at 18

What is the expected behavior?

All icons should be initialised according to the guidelines when color attribute is not explicitly set to "primary", "accent" or "warning".
screen shot 2018-01-29 at 18 05 05

What is the current behavior?

Icons inherit the properties of the container, most of the time black with 87% opacity.

What are the steps to reproduce?

https://material.angular.io/components/icon/overview
https://material.angular.io/components/list/examples (folder icon)
https://material.angular.io/components/datepicker/examples (datepicker icon)
https://material.angular.io/components/form-field/overview (prefixes, suffixes: both text and icon)
https://material.angular.io/components/button/examples (basic icon button, basic fab buttons: which should have also white backgrounds)

What is the use-case or motivation for changing an existing behavior?

Consistency with the guidelines, which means less css to write afterwards.

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

material 5.1.0

Is there anything else we should know?

Progress controls should also have a basic color of black with 54% opacity, so that it will work on the basic fab button. Now the default if primary, which in my opinion is wrong.

@philip-firstorder
Copy link
Author

philip-firstorder commented Jan 29, 2018

This is my temporary css fix in global styles.scss, works also for the input prefix and suffix

// mat-icon color doesn't follow the guidelines 
// https://github.com/angular/material2/issues/9664
.mat-icon,
.mat-form-field-prefix,
.mat-form-field-suffix {
  color: rgba(0, 0, 0, 0.54);
  &:focus {
    color: rgba(0, 0, 0, 0.87);
  }
}

.mat-icon[disabled] {
  color: rgba(0, 0, 0, 0.38);
}

.mat-primary,
.mat-accent {
  .mat-icon {
    color: inherit;
  }
}

.mat-form-field-disabled {
  .mat-form-field-prefix,
  .mat-form-field-suffix,
  .mat-form-field-label,
  .mat-hint {
    color: rgba(0, 0, 0, 0.38);
  }
}

@Klaster1
Copy link

The linked guideline page does not list icon colors as on screenshot. Does the issue still stand? I found it because icons had too much contrast for my taste, other Material apps like YouTube do have lower contrast that seems to match the values above.

To address the issue in my app, I resorted to the following mixin, but it's not comprehensive and will require modification for particular use cases:

@mixin fix-icon-color($active-focused, $active-unfocused) {
  .mat-icon-no-color.fix-icon-color {
    color: $active-focused
  }

  .mat-list-base .mat-list-item,
  .mat-option,
  .mat-icon-button {
    .mat-icon-no-color {
      color: $active-focused
    }
  }

  .mat-list-item:hover,
  .mat-option.mat-active,
  .mat-option:hover,
  .mat-icon-button:hover {
    .mat-icon-no-color {
      color: $active-unfocused
    }
  }
}
@include angular-material-theme($theme);
@include fix-icon-color(
  map-get($mat-light-theme-foreground, 'icon'),
  map-get($mat-light-theme-foreground, 'text')
);

Another thing to note is the fact that foreground/background palettes require an "icon" value, but the value is used only once to style the mat-menu icons.

@mmalerba mmalerba added the needs triage This issue needs to be triaged by the team label May 20, 2020
@mmalerba mmalerba added area: material/icon material spec Issue related to the Material Design spec https://material.io/design/ P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed needs triage This issue needs to be triaged by the team labels May 28, 2020
@mrmokwa
Copy link

mrmokwa commented Jul 29, 2020

I think it was made this way because mat-list doesn't have a default background, like mat-select for example, that have a white background color when ligth theme and therefore have the correct icon color.

But yeah, I agree that this issue should be addressed and IMO, other components, that suffer the same problem (like table), should have it fixed too.

@Char2sGu
Copy link

@debug: map.get($theme, 'colors', 'foreground');
  foreground: (
    base: black,
    divider: rgba(0, 0, 0, 0.12),
    dividers: rgba(0, 0, 0, 0.12),
    disabled: rgba(0, 0, 0, 0.38),
    disabled-button: rgba(0, 0, 0, 0.26),
    disabled-text: rgba(0, 0, 0, 0.38),
    elevation: black,
    hint-text: rgba(0, 0, 0, 0.38),
    secondary-text: rgba(0, 0, 0, 0.54),
    icon: rgba(0, 0, 0, 0.54), //              <----------
    icons: rgba(0, 0, 0, 0.54), //            <----------
    text: rgba(0, 0, 0, 0.87),
    slider-min: rgba(0, 0, 0, 0.87),
    slider-off: rgba(0, 0, 0, 0.26),
    slider-off-active: rgba(0, 0, 0, 0.38),
  ),

Actually the color of icon is already defined in the stylesheet, but not applied to the component. I wonder why.

@stevebor1
Copy link

Any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: material/icon material spec Issue related to the Material Design spec https://material.io/design/ P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

No branches or pull requests

8 participants