RadioGroup: migrate from reakit to ariakit
#55580
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: -1.01 kB (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
9 tasks
|
Flaky tests detected in 9f5a388. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6798666693
|
ciampo
commented
Oct 25, 2023
flootr
changed the title
RadioGroup: migrate from reakit to ariakit
flootr
added
[Type] Code Quality
flootr
force-pushed
the
feat/radio-group-ariakit
branch
2 times, most recently
from
1a91478
to
8ab0956
Compare
|
@flootr is this PR ready for another round of review? |
flootr
force-pushed
the
feat/radio-group-ariakit
branch
from
8ab0956
to
5f630e2
Compare
It is! 👍🏻 |
ciampo
commented
Nov 7, 2023
There was a problem hiding this comment.
I know this component is deprecated, but we could still make changes to keep our Storybook files up to date with the standards we've been using across the codebase.
That implies things like:
- using correct types
- using the template syntax and the
argsobject - tweaking controls when possible to avoid the default "Set object" button (eg. setting
checkedto be atextcontrol) - disabling controls that would be too complex for users to enter (eg.
children) - disabling controls when they don't make sense (ie.
checkedfor the controlled example of the component)
I gave it a quick spin on my local machine, here are the changes that I ended up applying:
diff --git a/packages/components/src/radio-group/stories/index.story.tsx b/packages/components/src/radio-group/stories/index.story.tsx
index 732e1859df..cfa0a253fb 100644
--- a/packages/components/src/radio-group/stories/index.story.tsx
+++ b/packages/components/src/radio-group/stories/index.story.tsx
@@ -21,6 +21,8 @@ const meta: Meta< typeof RadioGroup > = {
subcomponents: { Radio },
argTypes: {
onChange: { control: { type: null } },
+ children: { control: { type: null } },
+ checked: { control: { type: 'text' } },
},
parameters: {
actions: { argTypesRegex: '^on.*' },
@@ -30,61 +32,59 @@ const meta: Meta< typeof RadioGroup > = {
};
export default meta;
-export const Default: StoryFn< typeof RadioGroup > = () => {
- return (
- <RadioGroup
- // id is required for server side rendering
- // eslint-disable-next-line no-restricted-syntax
- id="default-radiogroup"
- label="options"
- defaultChecked="option2"
- >
- <Radio value="option1">Option 1</Radio>
- <Radio value="option2">Option 2</Radio>
- <Radio value="option3">Option 3</Radio>
- </RadioGroup>
- );
+const Template: StoryFn< typeof RadioGroup > = ( props ) => {
+ return <RadioGroup { ...props } />;
};
-export const Disabled = () => {
- /* eslint-disable no-restricted-syntax */
- return (
- <RadioGroup
- // id is required for server side rendering
- id="disabled-radiogroup"
- disabled
- label="options"
- defaultChecked="option2"
- >
+export const Default: StoryFn< typeof RadioGroup > = Template.bind( {} );
+Default.args = {
+ id: 'default-radiogroup',
+ label: 'options',
+ defaultChecked: 'option2',
+ children: (
+ <>
<Radio value="option1">Option 1</Radio>
<Radio value="option2">Option 2</Radio>
<Radio value="option3">Option 3</Radio>
- </RadioGroup>
- );
- /* eslint-enable no-restricted-syntax */
+ </>
+ ),
+};
+
+export const Disabled: StoryFn< typeof RadioGroup > = Template.bind( {} );
+Disabled.args = {
+ ...Default.args,
+ id: 'disabled-radiogroup',
+ disabled: true,
};
-const ControlledRadioGroupWithState = () => {
+const ControlledTemplate: StoryFn< typeof RadioGroup > = ( {
+ checked: checkedProp,
+ onChange: onChangeProp,
+ ...props
+} ) => {
const [ checked, setChecked ] =
- useState< React.ComponentProps< typeof RadioGroup >[ 'checked' ] >( 1 );
+ useState< React.ComponentProps< typeof RadioGroup >[ 'checked' ] >(
+ checkedProp
+ );
+
+ const onChange: typeof onChangeProp = ( value ) => {
+ setChecked( value );
+ onChangeProp?.( value );
+ };
- /* eslint-disable no-restricted-syntax */
return (
- <RadioGroup
- // id is required for server side rendering
- id="controlled-radiogroup"
- label="options"
- checked={ checked }
- onChange={ setChecked }
- >
- <Radio value={ 0 }>Option 1</Radio>
- <Radio value={ 1 }>Option 2</Radio>
- <Radio value={ 2 }>Option 3</Radio>
- </RadioGroup>
+ <RadioGroup { ...props } onChange={ onChange } checked={ checked } />
);
- /* eslint-enable no-restricted-syntax */
};
-export const Controlled = () => {
- return <ControlledRadioGroupWithState />;
+export const Controlled: StoryFn< typeof RadioGroup > = ControlledTemplate.bind(
+ {}
+);
+Controlled.args = {
+ ...Default.args,
+ checked: 'option2',
+ id: 'controlled-radiogroup',
+};
+Controlled.argTypes = {
+ checked: { control: { type: null } },
};
chad1008
approved these changes
Nov 7, 2023
flootr
force-pushed
the
feat/radio-group-ariakit
branch
from
910e1c4
to
86d73f2
Compare
Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com>
Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>
flootr
force-pushed
the
feat/radio-group-ariakit
branch
from
86d73f2
to
9f5a388
Compare
cbravobernal
pushed a commit
that referenced
this pull request
Nov 14, 2023
* add all the things * polish components & introduce types * types: add property descriptions * re-add deprecated flag * improve story according to feedback/best practices * `Radio`: re-add `@deprecated` flag * fix story options * radio: pass on `props` to `Button` explicitly * add changelog entry * types: fix typo Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com> * types: fix wording for `label` Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com> * apply current practices to story --------- Co-authored-by: Markus Dobmann <m.do@posteo.net> Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
RadioGroupfromreakittoariakitWhy?
Part of #53278
As stated in the linked issue Reakit does not explicitly support React 18 (causing peer dependency warnings) and Reakit has already been succeeded by Ariakit, so it will not receive any more updates.
How?
We switch out the Reakit dependency for Ariakit.
Testing Instructions
Please note, this component is deprecated and the sole purpose for this PR is migrating from
reakittoAriakit. Besides that we maintain the existing API to prevent breaking changes.