Repository navigation
Conversation
Setting reactDocgenTypescriptOptions replaced Storybook's defaults, so alias-typed unions showed object controls. Drop PopoverContainer's now-inferred alignment argType. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drop the per-story argTypes that hid it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
propFilter now keeps a node_modules prop when a Gamut type also declares it, e.g. button onClick and form input props. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replaces the per-story href argTypes on the button stories. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@skillsoft/eslint-plugin-gamut
@skillsoft/gamut
@skillsoft/gamut-agent-tools
@skillsoft/gamut-codemods
@skillsoft/gamut-icons
@skillsoft/gamut-illustrations
@skillsoft/gamut-patterns
@skillsoft/gamut-styles
@skillsoft/gamut-tests
@skillsoft/variance
commit: |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are recorded for the Storybook props-table improvements.
No open issues 🎉
What changed in this PR
Improves Gamut’s Storybook props tables by fixing union control inference and showing native props explicitly redeclared by Gamut, while removing redundant story-level overrides.
Changes:
- Restores literal-union extraction, preserves redeclared native props, and hides
classNamecentrally. - Declares and documents
hrefonButtonBasePropswithout changing accepted values or runtime behavior. - Removes redundant argTypes from seven stories and adds a patch changeset.
| File | Description |
|---|---|
packages/gamut/src/SkipToContent/SkipToContent.stories.tsx |
Removes local className filtering. |
packages/gamut/src/PopoverContainer/PopoverContainer.stories.tsx |
Removes manual alignment controls and local className filtering. |
packages/gamut/src/Button/TextButton.stories.tsx |
Removes the manual href argType. |
packages/gamut/src/Button/StrokeButton.stories.tsx |
Removes the manual href argType. |
packages/gamut/src/Button/shared/types.ts |
Redeclares href with shared documentation for props tables. |
packages/gamut/src/Button/IconButton.stories.tsx |
Removes the manual href argType. |
packages/gamut/src/Button/FillButton.stories.tsx |
Removes the manual href argType. |
packages/gamut/src/Button/CTAButton.stories.tsx |
Removes the manual href argType. |
packages/.storybook/main.ts |
Updates docgen inference and centralized prop filtering. |
.changeset/button-href-prop.md |
Records the type-only href documentation change. |
Review effort: Balanced
This branch has not been deployed
This file contains hidden or 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
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.
Overview
Storybook's props tables had three gaps:
classNamewas hidden by hand-written argTypes in individual stories, not in one central place.This PR fixes all three in the shared docgen config (
packages/.storybook/main.ts) and removes the per-story argTypes that were covering for them.1. Restore
shouldExtractLiteralValuesFromEnumRoot cause: Storybook merges preset config one level deep. Because
main.tssets its ownreactDocgenTypescriptOptions, that object replaced Storybook's defaults entirely and droppedshouldExtractLiteralValuesFromEnum: true. Docgen then returned unions as raw type strings. Inline literal unions still worked, because Storybook splits names on|as a fallback. Alias-typed unions likeAlertPlacementsorPopoverContainerAlignmentdidn't split, so they becameobjectcontrols.I checked the other Storybook defaults.
shouldRemoveUndefinedFromOptionalis already set, ourpropFilteralready includes thenode_modulescheck, andsavePropValueAsStringis always forced on. So this was the only default being lost.Impact: I ran docgen across
packages/gamut/srcwith and without the option. About 12 props change, all from object to radio or select:placementon Alert, AlertBanner and AlertBoxalignmentweekStartsOnfieldanimationon Popover, PopoverPortal and FloatingTipBodyvariantI removed PopoverContainer's hand-built
alignmentargType, since an 8-option select is now inferred.I didn't use
shouldExtractValuesFromUnioninstead, because it expands every union. That would turn booleans,ReactNodeand mixed unions into enums.2. Filter
classNameinpropFilterclassNameis now dropped centrally, next tomodeandtheme. I removed the two story argTypes that hid it by hand, in PopoverContainer and SkipToContent. No styleguide stories or.mdxpages hid it, so nothing else needed cleaning up.3. Show native props that a Gamut type redeclares
Root cause:
propFilterdropped any prop whoseparentwas innode_modules. Docgen setsparentto the prop's first declaration only. When a Gamut type redeclares a native prop and the result is combined with React's attribute types in an intersection,parentcan still point at@types/react. So Gamut's own redeclaration, and its JSDoc, were filtered out.Fix: the filter now also checks
prop.declarations, which lists every declaration. Anode_modulesprop is kept when a Gamut file also declares it. I kept the originalparentcheck as the first condition, so nothing that shows today gets hidden. A version that relied only ondeclarationshid about 149asandchildrenprops, because those come from Emotion with noparentset.Impact: 74 unique component props start showing, and none disappear. They include:
onClickon every button, already redeclared inButton/shared/types.ts.aria-labelon IconButton, DismissButton and the pagination ellipsis buttons.Form/types.tsand each input's own type:name,required,id,value,placeholder,type,checked,disabled,readOnly,onChangeandtabIndex.hrefandonClick.childrenon Card, Modal and RadioGroup.selectPropson SelectDropdown's internal parts, which have no stories.Native props that only
@types/reactdeclares stay hidden, so tables don't fill up with every DOM attribute.4. Declare
hrefonButtonBasePropshrefchanges what the button renders: ButtonBase renders an<a>when it's set, unless the button is disabled. Until now this was documented by an identical hand-written argType in five button stories. I've redeclared it onButtonBasePropsasHTMLProps<HTMLAnchorElement>['href'], with a JSDoc description, and removed those five argTypes.This changes types only.
ButtonPropsisButtonBaseProps & ComponentProps<typeof ButtonBase>, and both sides now declare the samestring | undefined, so the type consumers can pass doesn't change. The existinghref?: neverdeclarations in Tag and BarChart still narrowhrefas before, andtscpasses. The redeclaration follows theonClickone that's been on this interface since 2021. That one exists to fix a handler type, though; this one only exists for the docs.Known limitations and follow-ups
The Connected inputs now show props that are harmful to set. They spread react-hook-form's
register()result first and the consumer's props after, so the consumer's props win:onChangeandcheckedon ConnectedRadio, andvalueon ConnectedTextArea, override react-hook-form.requireddoes nothing useful on any of them, becauseFormsetsnoValidateand required state comes fromvalidationRules.These props were already in the types; this PR only makes them visible. The real fix is to
Omitthem from the Connected types. That's a breaking type change, so it's out of scope here. Their stories are still in styleguide, so I'll hide them with argTypes when those stories are migrated.Buttons'
disabledandtype(default'button') also change behavior and could be redeclared the same way ashref. I left them for a follow-up.Dialog's
confirmCta.onClickstill has no control. Deep controls doesn't build controls for function args.Notes for reviewers
propFilterruns at docgen time, so any change here needs a Storybook restart before it shows up.PR Checklist
Testing Instructions
yarn startand open Storybook.alignmentshould be a select with 8 options. The props table shouldn't listclassName.placementshould be a radio withfloatingandinline, not an object control.className.href, described as "If defined, component will use an anchor tag", with a text control.hrefto any URL. The button should render as an<a>. Then setdisabled, and it should render as a<button>again.onClickshould also be listed.aria-label.name,required,checked,disabled,value,id,tabIndex,readOnlyandonChange.asstill appears on Box or Text, and that native-only attributes likearia-describedbystill don't appear on the buttons.yarn test:storiesand check it passes.🤖 Generated with Claude Code