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(list-box): reset multiselect selection count position #5405

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Feb 20, 2020

Closes #5404

This PR resets the positioning of the selection counter for multiselects only

Testing / Reviewing

Ensure the selection counter is properly aligned for comboboxes and multiselects

@emyarod emyarod requested a review from a team as a code owner February 20, 2020 12:40
@ghost ghost requested review from joshblack and tw15egan February 20, 2020 12:40
@asudoh asudoh requested review from a team and aagonzales and removed request for a team February 20, 2020 12:41
Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

Thank you for your quick fix @emyarod!

@netlify
Copy link

netlify bot commented Feb 20, 2020

Deploy preview for carbon-elements ready!

Built with commit 599a1f8

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

@netlify
Copy link

netlify bot commented Feb 20, 2020

Deploy preview for carbon-components-react ready!

Built with commit 599a1f8

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

@emyarod
Copy link
Member Author

emyarod commented Feb 20, 2020

looks like a11y tests are timing out?

@emyarod emyarod force-pushed the 5404-center-multiselect-selection-count branch from 49fa401 to 4bdee12 Compare February 20, 2020 14:08
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 👍 ✅

Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

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

This is unrelated but can we have the title not wrap in the example.
image

@emyarod
Copy link
Member Author

emyarod commented Feb 20, 2020

@aagonzales that's because in storybook we're setting an arbitrary 300px width on the component container. if I remove the rule entirely we would end up with something like this for the non-inline multiselect (similar to the text input story) if that's alright

image

otherwise I can just bump up the container width but depending on your screen size you would still see the title wrap behavior

@aagonzales
Copy link
Member

aagonzales commented Feb 20, 2020

Ok i guess if its just the react storybook where that's happening then leave it (still not an ideal example), I'll just make sure the demo on the website doesn't do that.

@asudoh
Copy link
Contributor

asudoh commented Feb 22, 2020

Got a green light to move this to "ready to merge" state - Thanks @emyarod!

@asudoh asudoh merged commit a0bc366 into carbon-design-system:master Feb 24, 2020
@emyarod emyarod deleted the 5404-center-multiselect-selection-count branch February 25, 2020 00:27
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 selection count not centered
5 participants