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

fix(multiselect): close button for tags #4890

Merged
merged 3 commits into from
Dec 17, 2019
Merged

fix(multiselect): close button for tags #4890

merged 3 commits into from
Dec 17, 2019

Conversation

aledavila
Copy link
Contributor

Closes #4888

Close svg for tags was not aligned.

Changelog

Removed

  • absolute positioning for close svg

Testing / Reviewing

Make sure tag filter and multi select tags are working and close button is positioned correctly.

@netlify
Copy link

netlify bot commented Dec 16, 2019

Deploy preview for the-carbon-components ready!

Built with commit 6c8c0e0

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

@netlify
Copy link

netlify bot commented Dec 16, 2019

Deploy preview for carbon-elements ready!

Built with commit 6c8c0e0

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

@netlify
Copy link

netlify bot commented Dec 16, 2019

Deploy preview for carbon-components-react failed.

Built with commit 6c8c0e0

https://app.netlify.com/sites/carbon-components-react/deploys/5df7d9042f473a000aa603ca

@netlify
Copy link

netlify bot commented Dec 16, 2019

Deploy preview for the-carbon-components ready!

Built with commit 56d6ad8

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

@netlify
Copy link

netlify bot commented Dec 16, 2019

Deploy preview for carbon-elements ready!

Built with commit 56d6ad8

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

@netlify
Copy link

netlify bot commented Dec 16, 2019

Deploy preview for carbon-components-react failed.

Built with commit cd87efb

https://app.netlify.com/sites/carbon-components-react/deploys/5df7db782136c50008218298

@netlify
Copy link

netlify bot commented Dec 16, 2019

Deploy preview for carbon-components-react failed.

Built with commit 56d6ad8

https://app.netlify.com/sites/carbon-components-react/deploys/5df9029de3d7760007066db0

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.

This looks good to me as far as I can tell. Tag w/ filter looks the same and Multiselect is back to normal!

@abbeyhrt
Copy link
Contributor

Could someone check this on IE11? Every time i've seen

 top: 50%;	
 transform: translateY(-50%);

it's to address positioning there but I don't have a windows VM anymore to check.

@aledavila
Copy link
Contributor Author

@abbeyhrt just checked and it seems fine. There's an issue with the arrow on multiselect but it shouldn't be connected as this is styling for svg of tag.

Screen Shot 2019-12-16 at 3 51 51 PM

Screen Shot 2019-12-16 at 3 50 58 PM

@joshblack joshblack requested a review from a team December 16, 2019 22:01
@ghost ghost requested review from designertyler and removed request for a team December 16, 2019 22:01
@joshblack
Copy link
Contributor

For design reviewers, it seems like our MultiSelect had a visual regression when making a selection (the X icon was misaligned). This PR should address it in all browsers.

@abbeyhrt
Copy link
Contributor

Thank you for checking! @aledavila

Copy link
Contributor

@designertyler designertyler left a comment

Choose a reason for hiding this comment

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

LGTM

@joshblack joshblack merged commit 704de07 into carbon-design-system:master Dec 17, 2019
joshblack pushed a commit to joshblack/carbon that referenced this pull request Dec 17, 2019
* fix(multiselect): close button for tags

* chore: remove relative positioning
joshblack pushed a commit to joshblack/carbon that referenced this pull request Jan 6, 2020
* fix(multiselect): close button for tags

* chore: remove relative positioning
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[MultiSelect]: the X icon is in the wrong position
5 participants