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.

Sign up for GitHub

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

Jump to bottom

fix(overflow): fix styling color for danger overflow to use danger to… #7463

Merged
merged 3 commits into from
Dec 17, 2020
Merged

fix(overflow): fix styling color for danger overflow to use danger to… #7463

merged 3 commits into from
Dec 17, 2020

Conversation

DusanMilko
Copy link
Contributor

@DusanMilko DusanMilko commented Dec 15, 2020 •
edited

…ken. Harness the power of currentColor

fixes: #7467

  • fix overflow danger class to use danger token. (this effects theming, its referenced as danger so lets use the correct token)
  • Also harness the all powerful currentColor to get fill color - this is a more dynamic and cross browser supported way that simplifies logic

@DusanMilko DusanMilko requested a review from a team as a code owner December 15, 2020 22:07
@netlify
Copy link

netlify bot commented Dec 15, 2020 •
edited

✔️ Deploy preview for carbon-elements ready!

🔨 Explore the source changes: 07fcbf1

🔍 Inspect the deploy logs: https://app.netlify.com/sites/carbon-elements/deploys/5fda36bcc85a1c0007ff8a51

😎 Browse the preview: https://deploy-preview-7463--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Dec 15, 2020 •
edited

✔️ Deploy preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: 07fcbf1

🔍 Inspect the deploy logs: https://app.netlify.com/sites/carbon-components-react/deploys/5fda36bc12dca700083eb18c

😎 Browse the preview: https://deploy-preview-7463--carbon-components-react.netlify.app

@tw15egan
Copy link
Member

danger was deprecated in a recent release (#7179) and the website specifies as support-01.

Screen Shot 2020-12-16 at 10 02 12 AM

Is there an issue associated with these changes?

@DusanMilko
Copy link
Contributor Author

DusanMilko commented Dec 16, 2020 •
edited

@tw15egan added issue: #7467
Noticed when doing theming the danger and support-01 can have different colors. And the danger class in overflow menu should inherit danger color instead of support-01, no?

tw15egan
tw15egan approved these changes Dec 16, 2020
View reviewed changes
Copy link
Member

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

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

LGTM 👍 ✅

laurenmrice
laurenmrice reviewed Dec 16, 2020
View reviewed changes
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.

Looks good! Thank you 🙌🏻

andreancardona
andreancardona approved these changes Dec 17, 2020
View reviewed changes
Copy link
Contributor

@andreancardona andreancardona left a comment

Choose a reason for hiding this comment

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

@DusanMilko looks good!

@kodiakhq kodiakhq bot merged commit 83fd4b2 into carbon-design-system:master Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tw15egan tw15egan tw15egan approved these changes

@andreancardona andreancardona andreancardona approved these changes

@laurenmrice laurenmrice laurenmrice approved these changes

Assignees
No one assigned
Labels
status: ready to merge 🎉
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Overflow menu danger class gets support token
4 participants
@DusanMilko @tw15egan @andreancardona @laurenmrice