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(tag): fix for dark theme #4119

Merged
merged 5 commits into from
Oct 11, 2019
Merged

fix(tag): fix for dark theme #4119

merged 5 commits into from
Oct 11, 2019

Conversation

asudoh
Copy link
Contributor

@asudoh asudoh commented Sep 27, 2019

Fixes #4022.

Changelog

Changed

  • Colors of filter version of tag.

Testing / Reviewing

Testing should make sure our tag component is not broken.

@asudoh asudoh requested a review from a team as a code owner September 27, 2019 07:26
@netlify
Copy link

netlify bot commented Sep 27, 2019 •
edited
Loading

Deploy preview for the-carbon-components ready!

Built with commit 22aad2f

https://deploy-preview-4119--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Sep 27, 2019 •
edited
Loading

Deploy preview for carbon-components-react ready!

Built with commit 22aad2f

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

@netlify
Copy link

netlify bot commented Sep 27, 2019 •
edited
Loading

Deploy preview for carbon-elements failed.

Built with commit 22aad2f

https://app.netlify.com/sites/carbon-elements/deploys/5d9fc6849fdf8e0008d09b4e

emyarod
emyarod approved these changes Sep 27, 2019
View reviewed changes
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

image

image

image

laurenmrice
laurenmrice requested changes Sep 27, 2019
View reviewed changes
Copy link
Member

@laurenmrice laurenmrice left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • The tag focus should get the inverse-focus-ui token

  • The close icon should not be getting a white hover color, it should stay icon-01.
    tag

@asudoh
Copy link
Contributor Author

asudoh commented Sep 30, 2019

Thanks @laurenmrice for reviewing. Made an update. Let me close this PR for now. Will reopen when the work of next release starts.

@asudoh asudoh closed this Sep 30, 2019
@aledavila
Copy link
Contributor

Reopening as a11y team has started work again

@aledavila aledavila reopened this Oct 8, 2019
@asudoh
Copy link
Contributor Author

asudoh commented Oct 8, 2019

Thanks @aledavila! @laurenmrice Could you re-review? Thanks!

@joshblack joshblack requested a review from laurenmrice October 8, 2019 20:35
abbeyhrt
abbeyhrt approved these changes Oct 9, 2019
View reviewed changes
Copy link
Contributor

@abbeyhrt abbeyhrt 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! Thanks @asudoh!

laurenmrice
laurenmrice requested changes Oct 9, 2019
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.

The colors look correct per theme and in vanilla and react but when clicking on the close icon, the close icon gets smaller. It should always be the same size

Gif for reference:
tag

@asudoh asudoh mentioned this pull request Oct 10, 2019
Closed
2 tasks
@asudoh
Copy link
Contributor Author

asudoh commented Oct 10, 2019

Thanks @laurenmrice for pointing this out! It seems an existing issue that seems not related to color contrast stuff this PR addresses. Reported at: #4289

laurenmrice
laurenmrice approved these changes Oct 10, 2019
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.

Alright then looks good to me 👍 . thanks for opening that other pr !

@asudoh asudoh merged commit abc2ee6 into carbon-design-system:master Oct 11, 2019
@asudoh asudoh deleted the tag-filter-color branch October 11, 2019 00:14
@abbeyhrt abbeyhrt mentioned this pull request Oct 14, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurenmrice laurenmrice laurenmrice approved these changes

@abbeyhrt abbeyhrt abbeyhrt approved these changes

@emyarod emyarod emyarod 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.

Tag (tag) component has insufficient contrast on Gray 100 theme
6 participants
@asudoh @aledavila @emyarod @abbeyhrt @laurenmrice @joshblack