[OptionList] Consolidate se23 logic and styles #9953
Assignees
Comments
|
As part of this work, we should leverage the change made in #10081 to replace the hard-coded See context in this comment: #10081 (review) |
5 tasks
5 tasks
gwyneplaine
added a commit
that referenced
this issue
Aug 24, 2023
<!-- ☝️How to write a good PR title: - Prefix it with [ComponentName] (if applicable), for example: [Button] - Start with a verb, for example: Add, Delete, Improve, Fix… - Give as much context as necessary and as little as possible - Prefix it with [WIP] while it’s a work in progress --> ### WHY are these changes introduced? Fixes #9953 Fixes #10197 ### WHAT is this pull request doing? - `OptionList` consolidate se23 logic - `OptionList` consolidate se23 styles - Remove custom `Checkbox` component in favour of the standard Polaris Checkbox - Removes the `optionRole` prop from the `OptionList` component. This is presently being used to toggle the `presentation` role on the custom checkbox input. This PR removes this prop for a few reasons: - Its no longer used, since the custom Checkbox has been deleted. - The functionality it was providing (toggling presentation role) doesn't seem to be [supported by browsers](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/presentation_role). The browser ignores the `presentation` role on all focusable elements (including inputs). See excerpt from MDN below - A cursory [Grokt](https://grokt.shopify.io/results?q=optionRole) search, rules out internal usage of this prop  ### How to 🎩 - Storybook - Prod storybook ### 🎩 checklist - [ ] 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
sophschneider
pushed a commit
that referenced
this issue
Sep 19, 2023
<!-- ☝️How to write a good PR title: - Prefix it with [ComponentName] (if applicable), for example: [Button] - Start with a verb, for example: Add, Delete, Improve, Fix… - Give as much context as necessary and as little as possible - Prefix it with [WIP] while it’s a work in progress --> ### WHY are these changes introduced? Fixes #9953 Fixes #10197 ### WHAT is this pull request doing? - `OptionList` consolidate se23 logic - `OptionList` consolidate se23 styles - Remove custom `Checkbox` component in favour of the standard Polaris Checkbox - Removes the `optionRole` prop from the `OptionList` component. This is presently being used to toggle the `presentation` role on the custom checkbox input. This PR removes this prop for a few reasons: - Its no longer used, since the custom Checkbox has been deleted. - The functionality it was providing (toggling presentation role) doesn't seem to be [supported by browsers](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/presentation_role). The browser ignores the `presentation` role on all focusable elements (including inputs). See excerpt from MDN below - A cursory [Grokt](https://grokt.shopify.io/results?q=optionRole) search, rules out internal usage of this prop  ### How to 🎩 - Storybook - Prod storybook ### 🎩 checklist - [ ] 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
AnnaCheba
pushed a commit
to AnnaCheba/polaris
that referenced
this issue
Apr 22, 2024
<!-- ☝️How to write a good PR title: - Prefix it with [ComponentName] (if applicable), for example: [Button] - Start with a verb, for example: Add, Delete, Improve, Fix… - Give as much context as necessary and as little as possible - Prefix it with [WIP] while it’s a work in progress --> ### WHY are these changes introduced? Fixes Shopify#9953 Fixes Shopify#10197 ### WHAT is this pull request doing? - `OptionList` consolidate se23 logic - `OptionList` consolidate se23 styles - Remove custom `Checkbox` component in favour of the standard Polaris Checkbox - Removes the `optionRole` prop from the `OptionList` component. This is presently being used to toggle the `presentation` role on the custom checkbox input. This PR removes this prop for a few reasons: - Its no longer used, since the custom Checkbox has been deleted. - The functionality it was providing (toggling presentation role) doesn't seem to be [supported by browsers](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/presentation_role). The browser ignores the `presentation` role on all focusable elements (including inputs). See excerpt from MDN below - A cursory [Grokt](https://grokt.shopify.io/results?q=optionRole) search, rules out internal usage of this prop  ### How to 🎩 - Storybook - Prod storybook ### 🎩 checklist - [ ] 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
No description provided.
The text was updated successfully, but these errors were encountered: