fix(OverflowMenu): export v12 prop types and extend div props - #23148
fix(OverflowMenu): export v12 prop types and extend div props#23148RobbieTheWagner wants to merge 2 commits into
Conversation
|
All contributors have signed the DCO. |
The v12 OverflowMenu spreads its rest props onto the container div, but OverflowMenuProps did not extend the div's attributes, so DOM props such as onClick were rejected by TypeScript despite working at runtime. The interface was also unexported, leaving consumers unable to reference it without reaching into the compiled output. Match MenuButton, which exports its props and extends ComponentProps<'div'>. Signed-off-by: Robbie Wagner <2640861+RobbieTheWagner@users.noreply.github.com>
0c96e2e to
fa3d302
Compare
✅ Deploy Preview for v12-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for v12-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #23148 +/- ##
==========================================
- Coverage 89.66% 89.65% -0.01%
==========================================
Files 639 639
Lines 58865 58865
Branches 8267 8321 +54
==========================================
- Hits 52780 52777 -3
- Misses 5919 5922 +3
Partials 166 166
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I have read the DCO document and I hereby sign the DCO. |
Exporting OverflowMenuProps both inline and in the export statement is a duplicate export. Keep the inline form, matching MenuButton. Extending the div's props also surfaced an existing unsoundness in the barrel: it types props as the v11 shape, which is button-based, and spreads them into the v12 component, which is div-based, so the DOM event handlers are not assignable. Make the conversion explicit, as the v11 branch already does for its ref. Signed-off-by: Robbie Wagner <2640861+RobbieTheWagner@users.noreply.github.com>
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
Closes #16493
Two TypeScript gaps in the v12
OverflowMenu(OverflowMenu/next), found while adoptingenable-v12-overflowmenuin a design system that wraps it.1.
OverflowMenuPropswas not exported. Only the component was:Consumers wrapping the v12 component have no way to reference its props. The public
OverflowMenubarrel is typed against the v11 prop set (it re-exportsOverflowMenuPropsfrom./OverflowMenu), so anyone using the flag has to either restate the v12 props by hand or reach into the compiled output. Restating them means silent drift as the component evolves.2.
OverflowMenuPropsdid not extend the container's attributes. The implementation spreads rest props onto the containerdiv:…but the interface did not extend
HTMLAttributes/ComponentProps<'div'>, so DOM props were rejected by TypeScript even though they work at runtime:onClickon the container is a real use case — a menu inside a clickable row needs it tostopPropagationso opening the menu doesn't activate the row.Both changes follow
MenuButton, the closest sibling (same trigger +__containerdiv shape), which already does exactly this:Export style matches
ComboButton:export { ComboButton, type ComboButtonProps };Changelog
New
OverflowMenuProps(v12,OverflowMenu/next) is now exported alongside the componentChanged
OverflowMenuProps(v12) now extendsComponentProps<'div'>, matching the component's rest-prop spread onto the container elementRemoved
Testing / Reviewing
Types only — no runtime change. To verify:
enable-v12-overflowmenuenabled, passonClicktoOverflowMenu. It should now typecheck, and the handler fires on the container div.import type { OverflowMenuProps } from '@carbon/react/lib/components/OverflowMenu/next'should resolve.extends:menuAlignmentstill rejects anything outside'top-start' | 'top-end' | 'bottom-start' | 'bottom-end'.PR Checklist
As the author of this PR, before marking ready for review, confirm you:
Updated documentation and storybook examples— types-only change, no behavior or API surface to document beyond the exported typerequired v12 migration documentation
for any code change that affects v12, or struck through this item because
the PR does not affect v12
Wrote passing tests that cover this change— no runtime behavior changes; the guarantee is compile-time, covered by the repo's existing typecheckAddressed any impact on accessibility (a11y)— no rendered output changesTested for cross-browser consistency— no rendered output changesMore details can be found in the pull request guide