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

ClrControlContainer component use wrong icon color #305

Closed
2 of 7 tasks
rbirkgit opened this issue Sep 6, 2022 · 7 comments · Fixed by #311
Closed
2 of 7 tasks

ClrControlContainer component use wrong icon color #305

rbirkgit opened this issue Sep 6, 2022 · 7 comments · Fixed by #311

Comments

@rbirkgit
Copy link

rbirkgit commented Sep 6, 2022

This is a bug report for the @clr Angular or UI versions of the design system.
For the web-component implementation of Clarity (@cds), visit vmware-clarity/core.

Describe the bug

ClrControlContainer component use wrong icon color. This is because it sets the icon status to "danger" which overrides the css for forms invalid color. Other container components don't set the status and why they work and use correct red color.

How to reproduce

Use the clr-control-container component as described here:
https://clarity.design/documentation/forms#:~:text=Custom%20and%20non%2DClarity%20Controls

Expected behavior

Error icon should have the same red color as the form invalid red, i.e. same red as the input control underline red.

Versions

Clarity version:

  • v12.x
  • v13.x
  • Other:

Framework:

  • Angular
  • React
  • Vue
  • Other:

Framework version:
Angular 13

Additional notes

Here is part of the source code of it. As you can see it adds the status. That line should be removed. See the clr-input-container as example as that one works.

@Component({
  selector: 'clr-control-container',
  template: `
        ...
        <cds-icon
          *ngIf="showInvalid"
          class="clr-validate-icon"
          status="danger"
          shape="exclamation-circle"
          aria-hidden="true"
        ></cds-icon>
@ashleyryan
Copy link

CAn you please provide a stackblitz so we can see the visual discrepancies

rbirkgit pushed a commit to rbirkgit/ng-clarity that referenced this issue Sep 6, 2022
The ClrControlContainer had wrong color on error icon as it sets the icon status to danger. This overrides the forms invalid color set by the form css. Other form containers such as ClrInputContainer don't set it either for this reason.

Close: vmware-clarity#305

Signed-off-by: Ron Birk <rbirk@vmware.com>
@rbirkgit
Copy link
Author

rbirkgit commented Sep 6, 2022

Here is what it looks like by default:
image

This is what it should be:
image

@rbirkgit
Copy link
Author

rbirkgit commented Sep 6, 2022

Here you can see how the danger status overrides the correct color:
image

@kevinbuhmann
Copy link
Member

kevinbuhmann commented Sep 6, 2022

That's not overriding the color of the icon. That's just setting the --color CSS variable which is not used in the .clr-validate-icon rules.

kevinbuhmann added a commit that referenced this issue Sep 13, 2022
The `status="danger"` attribute sets the icon color and fill to red 700,
but the form invalid color in ng-clarity is different. I think it's best
to keep the `status="danger"` attribute for semantics and simply set the
color and fill on the validation icons.

closes #305
kevinbuhmann added a commit that referenced this issue Sep 13, 2022
The `status="danger"` attribute sets the icon color and fill to red 700,
but the form invalid color in ng-clarity is different. I think it's best
to keep the `status="danger"` attribute for semantics/intention simply
set the color and fill on the validation icons to the form invalid color.

closes #305
kevinbuhmann added a commit that referenced this issue Sep 13, 2022
The icon `status` attribute sets the icon color and fill which could
result in slightly different colors being used for the validation icon
and text. I think it's best to keep the icon `status` attributes for
semantics/intention simply set the color and fill on the validation
icons to the correct form color.

closes #305
kevinbuhmann added a commit that referenced this issue Sep 14, 2022
The icon `status` attribute sets the icon color and fill which could
result in slightly different colors being used for the validation icon
and text. I think it's best to keep the icon `status` attributes for
semantics/intention simply set the color and fill on the validation
icons to the correct form color.

closes #305
kevinbuhmann added a commit that referenced this issue Sep 14, 2022
The icon `status` attribute sets the icon color and fill which could
result in slightly different colors being used for the validation icon
and text. I think it's best to keep the icon `status` attributes for
semantics/intention simply set the color and fill on the validation
icons to the correct form color.

closes #305

This is a backport of 28bbc62.
kevinbuhmann added a commit that referenced this issue Sep 16, 2022
The icon `status` attribute sets the icon color and fill which could
result in slightly different colors being used for the validation icon
and text. I think it's best to keep the icon `status` attributes for
semantics/intention simply set the color and fill on the validation
icons to the correct form color.

closes #305

This is a backport of 28bbc62.
@github-actions
Copy link
Contributor

🎉 This issue has been resolved in version 13.8.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link
Contributor

🎉 This issue has been resolved in version 12.4.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link
Contributor

Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed issues after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.