Addon Docs: Ensure unique control IDs with multiple Controls blocks#33763
Addon Docs: Ensure unique control IDs with multiple Controls blocks#33763kaigritun wants to merge 1 commit into
Conversation
…cks (storybookjs#26144) When multiple <Controls> blocks are rendered on the same MDX page, they previously generated duplicate ID attributes (e.g., 'control-propName') because getControlId only used the arg name. This caused accessibility issues where clicking a label would focus the wrong control. This fix: - Updates getControlId and getControlSetterButtonId helpers to accept an optional idPrefix parameter - Adds idPrefix prop to ControlProps interface and all control components - Passes idPrefix through ArgsTable → ArgRow → ArgControl → controls - Uses story.id as idPrefix in the Controls block The generated IDs are now unique per story: 'control-story-id-propName' Fixes storybookjs#26144
📝 WalkthroughWalkthroughThis PR adds an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs✨ Finishing touches
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 183 | 183 | 0 |
| Self size | 779 KB | 776 KB | 🎉 -3 KB 🎉 |
| Dependency size | 67.61 MB | 67.57 MB | 🎉 -47 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 50 | 50 | 0 |
| Self size | 1.04 MB | 1000 KB | 🎉 -44 KB 🎉 |
| Dependency size | 36.93 MB | 36.93 MB | 🎉 -3 KB 🎉 |
| Bundle Size Analyzer | node | node |
Sidnioulz
left a comment
There was a problem hiding this comment.
Thanks for the PR @kaigritun!
Overall the idea behind your change seems good to me, and the code looks good, but I cannot find a way to manually test your change on your branch.
Could you please
- Add a story that specifically tests for this (if you could find a way to add a TabbedArgsTable story with editable controls, this would directly match the use case reported in the bug)
- rewrite your PR description to match our PR template, with "Manual testing instructions"? it might sound trivial but when the team QAs dozens of PRs, it really helps us to have everything laid out in the same way, and to know which story to navigate to to verify your bug fix
Thanks 🙏 I look forward to approving and merging your PR once I'm able to test it manually
|
Closing due to inactivity. Please feel free to re-open a new PR and address the above comment. Thank you so much for your contributions so far! |
What this does
When multiple
<Controls>blocks are rendered on the same MDX page, they previously generated duplicate ID attributes (e.g.,control-propName) becausegetControlIdonly used the arg name. This caused accessibility issues where clicking a label would focus the wrong control.Before: Clicking a label in the second Controls block would change the first control.
After: Each Controls block has unique IDs prefixed with the story ID.
Changes
getControlIdandgetControlSetterButtonIdhelpers to accept an optionalidPrefixparameteridPrefixprop toControlPropsinterface and all control componentsidPrefixthrough the component chain: Controls → ArgsTable → ArgRow → ArgControl → individual controlsstory.idas theidPrefixin the Controls blockGenerated IDs
Before:
control-disabled,control-sizeAfter:
control-component--story-a-disabled,control-component--story-b-disabledTesting
idPrefixparameter in helpers.test.tsFixes #26144
Checklist
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests