Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| <<<<<<< fix/rtc-inline-inserter | ||
| - `RadioControl`: Add disabled styles. [#77127](https://github.com/WordPress/gutenberg/pull/77127) | ||
| - `Autocomplete`: Fix value comparison to avoid resetting block inserter in RTC ([#76980](https://github.com/WordPress/gutenberg/pull/76980)). | ||
| ======= |
There was a problem hiding this comment.
I've noticed these git conflict marks and removed them at b9b22f1
I am not sure if the changelog needs to be tweaked?
There was a problem hiding this comment.
It's enough to remove the markers, they were introduced at https://github.com/WordPress/gutenberg/pull/76980/changes#r3050030572
There was a problem hiding this comment.
Pull request overview
This PR adds first-class disabled support to RadioControl in @wordpress/components, ensuring the component’s UI reflects the disabled state and can be toggled in Storybook.
Changes:
- Added
disabled?: booleantoRadioControlProps. - Applied
disabledto the<fieldset>and added an.is-disabledclass hook inRadioControl. - Introduced disabled styling updates in
radio-control/style.scssand added adisabledboolean control to Storybook argTypes.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/components/src/radio-control/types.ts |
Adds a typed disabled prop for RadioControl consumers. |
packages/components/src/radio-control/index.tsx |
Wires disabled into the rendered <fieldset> and adds .is-disabled for styling. |
packages/components/src/radio-control/style.scss |
Adds visual/cursor changes for disabled radios and labels. |
packages/components/src/radio-control/stories/index.story.tsx |
Exposes disabled in Storybook controls for easier manual verification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| &:not(:disabled) { | ||
| cursor: pointer; | ||
| } | ||
|
|
||
| &:disabled { | ||
| background: $gray-100; | ||
| border-color: $gray-300; | ||
| opacity: 1; | ||
| } |
There was a problem hiding this comment.
&:disabled is declared before &:checked, so a disabled-but-selected radio will still pick up the checked (accent) styling. This doesn’t match the CheckboxControl disabled pattern (where :disabled overrides checked styles) and makes the disabled state inconsistent across options. Consider making the disabled styles override checked styles (e.g., target :disabled and :disabled:checked, and ensure the border/background are set explicitly even if checked removes the border).
| return ( | ||
| <fieldset | ||
| id={ id } | ||
| role="radiogroup" | ||
| className={ clsx( className, 'components-radio-control' ) } | ||
| className={ clsx( className, 'components-radio-control', { | ||
| 'is-disabled': disabled, | ||
| } ) } | ||
| disabled={ disabled } | ||
| aria-describedby={ !! help ? generateHelpId( id ) : undefined } |
There was a problem hiding this comment.
The PR introduces a new disabled prop/state (fieldset disabled + .is-disabled styling), but the existing RadioControl test suite doesn’t cover disabled behavior. Please add tests to verify radios are disabled (and onChange isn’t called) when disabled is true, including mouse click and keyboard interaction paths.
|
Size Change: +223 B (0%) Total Size: 7.74 MB 📦 View Changed
ℹ️ View Unchanged
|
|
Flaky tests detected in 3d23e9f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24125070127
|
|
Unfortunately I don't think we want to update to these specific disabled styles at this point, because they don't match the new disabled styles we've defined in the design system. (See for example the new My rough plan here was to first update the Let's coordinate a bit here, since I'm noticing you're updating disabled styling for a bunch of components. Some may make sense to do right now, others may be better handled as part of the broader systematic updates. Do you have a list perhaps? |
Yeah, I do have a list. That's every fix we need in DataForm and it is already implemented (it just took half the morning):
This sounds like a good plan. Would you have a sense of timeline for it to be deployed in all components DataForm uses? Depending on how long it takes I can see two ways forward: land this PRs as an intermediate step, or just close them and migrate the component styles. |
Follow-up to #77090
What?
Add disabled styles and a Storybook control for the RadioControl component.
Why?
While the radiocontrol could be disabled, the styles didn't reflect that state. This was made evident at #77090 where DataForm controls gained support for being disabled.
The control with disabled state:
The control with disabled state in the DataForm story:
How?
disabled?: booleanprop to RadioControlProps in types.ts.Testing Instructions
The control story:
The DataForm story:
Use of AI Tools
This PR was authored with assistance from Claude Code (Claude Opus 4.6). All changes were reviewed and validated by the author.