vmware-clarity / core

Clarity is a scalable, accessible, customizable, open-source design system built with web components. Works with any JavaScript framework, created for enterprises, and designed to be inclusive.
https://clarity.design
MIT License
164 stars 42 forks source link

fix(icons): use the non-shade alias color for info status icons #226

Closed astorije closed 1 year ago

astorije commented 1 year ago

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

What is the current behavior?

The current info status icons are colored with the shade variant, which gives the icon a very low contrast in dark theme.

Screen Shot 2023-04-11 at 11 36 50 AM Screen Shot 2023-04-11 at 11 37 09 AM

This is particularly true for regular-sized icons (i.e. smaller than lg):

Screen Shot 2023-04-11 at 11 40 29 AM

Issue Number: N/A

What is the new behavior?

This change aligns the info icons more closely with, well, most everything else such as cds-buttons which are displayed using the non-shade version, as well as non-info icons. As you can tell below, the impact on light theme is negligible, but in dark theme the contrast seems much better:

Screen Shot 2023-04-11 at 11 37 59 AM Screen Shot 2023-04-11 at 11 38 11 AM

And on smaller icons:

Screen Shot 2023-04-11 at 11 42 30 AM

Does this PR introduce a breaking change?

vmwclabot commented 1 year ago

@astorije, VMware has rejected your signed contributor license agreement. The merge can not proceed until the agreement has been resigned. Click here to resign the agreement. Reject reason:

Please provide city, state (or province), and country of residence or mailing address.

github-actions[bot] commented 1 year ago

👋 @astorije,

Thank you,

🤖 Clarity Release Bot

vmwclabot commented 1 year ago

@astorije, we have received your signed contributor license agreement. The review is usually completed within a week, but may take longer under certain circumstances. Another comment will be added to the pull request to notify you when the merge can proceed.

vmwclabot commented 1 year ago

@astorije, VMware has rejected your signed contributor license agreement. The merge can not proceed until the agreement has been resigned. Click here to resign the agreement. Reject reason:

Please provide a telephone number and full address. Please provide the name of the organization you are affiliated with or indicate none. Note: If you are a VMware employee please use your VMware email and you are then not required to execute the VMware CLA.

vmwclabot commented 1 year ago

@astorije, we have received your signed contributor license agreement. The review is usually completed within a week, but may take longer under certain circumstances. Another comment will be added to the pull request to notify you when the merge can proceed.

vmwclabot commented 1 year ago

@astorije, VMware has approved your signed contributor license agreement.

astorije commented 1 year ago

@kevinbuhmann and @colinreedmiller, friendly ping? :)

kevinbuhmann commented 1 year ago

@kevinbuhmann and @colinreedmiller, friendly ping? :)

I posted in our internal Slack channel for someone on our design team to review this.

colinreedmiller commented 1 year ago

@kevinbuhmann and @colinreedmiller, friendly ping? :)

I posted in our internal Slack channel for someone on our design team to review this.

We need to complete the token mapping evaluation across the themes to determine what the design guidance will be. The current use of --cds-alias-status-warning is problematic in dark theme barely meets contrast and the suggested fails to leverage the change to yellow for warning status - the info status is correct.

astorije commented 1 year ago

Thanks all!

github-actions[bot] commented 1 year ago

:tada: This PR is included in version 6.4.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] commented 1 year ago

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