Skip to content

docs: fix props table inference and show redeclared native props - #62

Open
aresnik11 wants to merge 7 commits into
mainfrom
ajr-props-table-fix
Open

aresnik11 wants to merge 7 commits into
mainfrom
ajr-props-table-fix

Conversation

@aresnik11

@aresnik11 aresnik11 commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Storybook's props tables had three gaps:

  1. Union props typed through a named alias showed an object control instead of a radio or select.
  2. Native props that Gamut redeclares on purpose were hidden.
  3. className was 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 shouldExtractLiteralValuesFromEnum

Root cause: Storybook merges preset config one level deep. Because main.ts sets its own reactDocgenTypescriptOptions, that object replaced Storybook's defaults entirely and dropped shouldExtractLiteralValuesFromEnum: 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 like AlertPlacements or PopoverContainerAlignment didn't split, so they became object controls.

I checked the other Storybook defaults. shouldRemoveUndefinedFromOptional is already set, our propFilter already includes the node_modules check, and savePropValueAsString is always forced on. So this was the only default being lost.

Impact: I ran docgen across packages/gamut/src with and without the option. About 12 props change, all from object to radio or select:

  • placement on Alert, AlertBanner and AlertBox
  • PopoverContainer alignment
  • DatePickerCalendar and CalendarBody weekStartsOn
  • DatePickerInputSegment field
  • animation on Popover, PopoverPortal and FloatingTipBody
  • CTAButton variant

I removed PopoverContainer's hand-built alignment argType, since an 8-option select is now inferred.

I didn't use shouldExtractValuesFromUnion instead, because it expands every union. That would turn booleans, ReactNode and mixed unions into enums.

2. Filter className in propFilter

className is now dropped centrally, next to mode and theme. I removed the two story argTypes that hid it by hand, in PopoverContainer and SkipToContent. No styleguide stories or .mdx pages hid it, so nothing else needed cleaning up.

3. Show native props that a Gamut type redeclares

Root cause: propFilter dropped any prop whose parent was in node_modules. Docgen sets parent to 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, parent can 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. A node_modules prop is kept when a Gamut file also declares it. I kept the original parent check as the first condition, so nothing that shows today gets hidden. A version that relied only on declarations hid about 149 as and children props, because those come from Emotion with no parent set.

Impact: 74 unique component props start showing, and none disappear. They include:

  • onClick on every button, already redeclared in Button/shared/types.ts.
  • aria-label on IconButton, DismissButton and the pagination ellipsis buttons.
  • The form input props from Form/types.ts and each input's own type: name, required, id, value, placeholder, type, checked, disabled, readOnly, onChange and tabIndex.
  • Tag's href and onClick.
  • children on Card, Modal and RadioGroup.
  • selectProps on SelectDropdown's internal parts, which have no stories.

Native props that only @types/react declares stay hidden, so tables don't fill up with every DOM attribute.

4. Declare href on ButtonBaseProps

href changes 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 on ButtonBaseProps as HTMLProps<HTMLAnchorElement>['href'], with a JSDoc description, and removed those five argTypes.

This changes types only. ButtonProps is ButtonBaseProps & ComponentProps<typeof ButtonBase>, and both sides now declare the same string | undefined, so the type consumers can pass doesn't change. The existing href?: never declarations in Tag and BarChart still narrow href as before, and tsc passes. The redeclaration follows the onClick one 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:

    • onChange and checked on ConnectedRadio, and value on ConnectedTextArea, override react-hook-form.
    • required does nothing useful on any of them, because Form sets noValidate and required state comes from validationRules.

    These props were already in the types; this PR only makes them visible. The real fix is to Omit them 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' disabled and type (default 'button') also change behavior and could be redeclared the same way as href. I left them for a follow-up.

  • Dialog's confirmCta.onClick still has no control. Deep controls doesn't build controls for function args.

Notes for reviewers

  • propFilter runs at docgen time, so any change here needs a Storybook restart before it shows up.
  • The new filter can only add props compared with the old one. I checked this by running both filters across every gamut component and diffing the output.

PR Checklist

  • Related to designs:
  • Related to JIRA ticket: GMT-1446
  • Changeset added (or empty changeset if no release)
  • I have run this code to verify it works
  • This PR includes unit tests for the code change
  • This PR includes testing instructions tests for the code change
  • Package previews from this PR are passing end-to-end tests in all relevant consuming repositories

Testing Instructions

  1. Run yarn start and open Storybook.
  2. Go to Overlays/PopoverContainer. In Controls, alignment should be a select with 8 options. The props table shouldn't list className.
  3. Go to Molecules/Alert. placement should be a radio with floating and inline, not an object control.
  4. Go to Navigation/SkipToContent. The props table shouldn't list className.
  5. Go to Actions/Buttons/FillButton:
    • The props table should list href, described as "If defined, component will use an anchor tag", with a text control.
    • Set href to any URL. The button should render as an <a>. Then set disabled, and it should render as a <button> again.
    • onClick should also be listed.
  6. Repeat step 5 on StrokeButton, TextButton, IconButton and CTAButton. IconButton should also list aria-label.
  7. Go to Inputs & forms/Radio. The props table should now list name, required, checked, disabled, value, id, tabIndex, readOnly and onChange.
  8. Spot-check that as still appears on Box or Text, and that native-only attributes like aria-describedby still don't appear on the buttons.
  9. Run yarn test:stories and check it passes.

🤖 Generated with Claude Code

aresnik11 and others added 5 commits November 3, 2026 10:22
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>
@pkg-pr-new

pkg-pr-new Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

@skillsoft/eslint-plugin-gamut

npm i https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/eslint-plugin-gamut@62
yarn add https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/eslint-plugin-gamut@62.tgz

@skillsoft/gamut

npm i https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut@62
yarn add https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut@62.tgz

@skillsoft/gamut-agent-tools

npm i https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut-agent-tools@62
yarn add https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut-agent-tools@62.tgz

@skillsoft/gamut-codemods

npm i https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut-codemods@62
yarn add https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut-codemods@62.tgz

@skillsoft/gamut-icons

npm i https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut-icons@62
yarn add https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut-icons@62.tgz

@skillsoft/gamut-illustrations

npm i https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut-illustrations@62
yarn add https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut-illustrations@62.tgz

@skillsoft/gamut-patterns

npm i https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut-patterns@62
yarn add https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut-patterns@62.tgz

@skillsoft/gamut-styles

npm i https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut-styles@62
yarn add https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut-styles@62.tgz

@skillsoft/gamut-tests

npm i https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut-tests@62
yarn add https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/gamut-tests@62.tgz

@skillsoft/variance

npm i https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/variance@62
yarn add https://pkg.pr.new/Codecademy/skillsoft-gamut/@skillsoft/variance@62.tgz

commit: 1e79302

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@aresnik11 aresnik11 changed the title Ajr props table fix docs: fix props table inference and show redeclared native props Nov 4, 2026
@codecov

codecov Bot commented Nov 4, 2026

Copy link
Copy Markdown

⚠️ JUnit XML file not found

The CLI was unable to find any JUnit XML files to upload.
For more help, visit our troubleshooting guide.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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 className centrally.
  • Declares and documents href on ButtonBaseProps without 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

@aresnik11
aresnik11 marked this pull request as ready for review November 4, 2026 20:33
@aresnik11
aresnik11 requested a review from a team as a code owner November 4, 2026 20:33

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants