-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[Listbox, OptionList] Right align check icons on selected list items #11697
Conversation
47793ab
to
1ad488a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code and changed stories LGTM!
Also not totally related to your changes but I noticed that the option list item could use a little more padding at the end when not selected, its causing the popover to grow when selected right now.
d5bc4d8
to
0f0cbeb
Compare
@sophschneider after looking into that it's actually the character width changing from initial to I have a potential fix I'll ping you on in a separate PR |
/snapit |
🫰✨ Thanks @kyledurand! Your snapshots have been published to npm. Test the snapshots by updating your yarn add @shopify/[email protected] yarn add @shopify/[email protected] yarn add @shopify/[email protected] |
Fixes https://github.com/Shopify/polaris-internal/issues/1517 Open to other ideas on how to fix this but character width is the most performant I can think of https://github.com/Shopify/polaris/assets/6844391/268bc7c8-48ef-43ba-895f-b647a5a7b8c1
…hopify#11697) ### WHY are these changes introduced? Fixes https://github.com/Shopify/polaris-internal/issues/1503 ### WHAT is this pull request doing? Change CheckIcon placement from right to left alignment. Adds a placeholder space for non-selected options [Figma](https://www.figma.com/file/I3df2veCGRVUuxzpXVE71m/Pickers?type=design&node-id=1655%3A23178&mode=design&t=ojAxTxPUmN0SoZYW-1) <!-- Summary of the changes committed. Before / after screenshots are appreciated for UI changes. Make sure to include alt text that describes the screenshot. Include a video if your changes include interactive content. If you include an animated gif showing your change, wrapping it in a details tag is recommended. Gifs usually autoplay, which can cause accessibility issues for people reviewing your PR: <details> <summary>Summary of your gif(s)</summary> <img src="..." alt="Description of what the gif shows"> </details> --> ### How to 🎩 🖥 [Local development instructions](https://github.com/Shopify/polaris/blob/main/README.md#install-dependencies-and-build-workspaces) 🗒 [General tophatting guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md) 📄 [Changelog guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog) ### 🎩 checklist - [ ] Tested a [snapshot](https://github.com/Shopify/polaris/blob/main/documentation/Releasing.md#-snapshot-releases) - [ ] Tested on [mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing) - [ ] Tested on [multiple browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers) - [ ] Tested for [accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md) - [ ] Updated the component's `README.md` with documentation changes - [ ] [Tophatted documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md) changes in the style guide
WHY are these changes introduced?
Fixes https://github.com/Shopify/polaris-internal/issues/1503
WHAT is this pull request doing?
Change CheckIcon placement from right to left alignment.
Adds a placeholder space for non-selected options
Figma
How to 🎩
🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines
🎩 checklist
README.md
with documentation changes