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

feat(tokens): add decorative-01, update dropdown styles #5688

Merged
merged 4 commits into from Mar 25, 2020

Conversation

tw15egan
Copy link
Member

Closes #2439

Starts the work of fixing visual issues on dark themes when list menus are using field-02 as a background. field-02 and ui-03 are the same value in dark themes, which were causing problems specifically when the light prop was added to Dropdown. When the light prop was added, the horizontal rules would blend into the background.

@laurenmrice this PR is specifically addressing the Dropdown issue. Let me know if there are other components that have a light version that this would also affect.

Changelog

New

  • decorative-01 as a token for horizontal rules on components with a light variation.

Changed

-light Dropdown menu now has the correct background of field-02

Testing / Reviewing

Run the vanilla components locally, change themes on the Dropdown page, and ensure you can see the horizontal rules on all variations / themes.

Screen Shot 2020-03-24 at 10 43 54 AM

Screen Shot 2020-03-24 at 10 44 00 AM

Screen Shot 2020-03-24 at 10 44 16 AM

Screen Shot 2020-03-24 at 10 44 22 AM

@tw15egan tw15egan requested a review from a team as a code owner March 24, 2020 18:11
@ghost ghost requested review from aledavila and emyarod March 24, 2020 18:11
@netlify
Copy link

netlify bot commented Mar 24, 2020

Deploy preview for carbon-elements ready!

Built with commit 795ff41

https://deploy-preview-5688--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Mar 24, 2020

Deploy preview for carbon-components-react ready!

Built with commit 795ff41

https://deploy-preview-5688--carbon-components-react.netlify.com

Copy link
Contributor

@aledavila aledavila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good after design review @laurenmrice

Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me pending design review

Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vanilla looks correct, new token is correct in elements. I don't think it has been changed in react yet though.

Other components that need this token applied for light prop (can either do this in this pr or separate if you want):

  • Combobox
  • Multi-select
  • Overflow menu

@tw15egan
Copy link
Member Author

tw15egan commented Mar 25, 2020

@laurenmrice do you have specs for what everything should be? Multiselect seems to have some issues with hover colors, want to make sure they are correct.

Might be best to create a new issue and I can add them in that way 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem with dark themes' $ui-02 and '$ui-03` values are the same
6 participants