Skip to content

Menu: fix TypeScript error - #759

Merged
mkernohanbc merged 2 commits into
mainfrom
bugfix/menu
Jul 14, 2026
Merged

Menu: fix TypeScript error#759
mkernohanbc merged 2 commits into
mainfrom
bugfix/menu

Conversation

@mkernohanbc

Copy link
Copy Markdown
Contributor

This PR fixes a TypeScript error in the Menu component. Previously, there was an inconsistency in how the items prop (used to pass an array of menu items) was typed in the MenuSectionProps and MenuProps interfaces. In MenuProps it was typed as items?: T[], which caused an incompatibility error with the onAction callback.

With this change, items is now typed consistently as items: MenuItemProps[] in both contexts. This resolves the error, and is consistent with how we've approached this same functionality in Select.

@mkernohanbc mkernohanbc added this to the Components v0.9.0 milestone Jun 25, 2026
@mkernohanbc mkernohanbc self-assigned this Jun 25, 2026
@mkernohanbc
mkernohanbc requested a review from a team as a code owner June 25, 2026 20:18
@mkernohanbc mkernohanbc added react-components Changes or issues affecting the design-system-react-components package bug labels Jun 25, 2026
@ty2k
ty2k requested review from Copilot and ty2k June 26, 2026 22:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Menu component’s TypeScript types to resolve an incompatibility stemming from inconsistent items typing between MenuSectionProps and MenuProps, aligning MenuProps.items to use MenuItemProps[].

Changes:

  • Remove the generic type parameter from MenuProps and the Menu component signature.
  • Type MenuProps as ReactAriaMenuProps<MenuItemProps> and items?: MenuItemProps[] to match MenuSectionProps.items.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/react-components/src/components/Menu/Menu.tsx

@ty2k ty2k left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good change, I think it's better to avoid the generic <T>.

On the itemsprop, I see right now it's required for MenuSectionProps but optional for MenuProps. If that's by design, can we add a comment to explain why? If it's not, can we use the same behaviour in both places?

@mkernohanbc

Copy link
Copy Markdown
Contributor Author

Good change, I think it's better to avoid the generic <T>.

On the itemsprop, I see right now it's required for MenuSectionProps but optional for MenuProps. If that's by design, can we add a comment to explain why? If it's not, can we use the same behaviour in both places?

Good spot — this was matching the structure from Select (where items is the only way to compose within sections), but doesn't fit the design here (where we're also exporting MenuSection and MenuItem to support another way to compose.)

6906096 makes items optional.

@mkernohanbc
mkernohanbc requested a review from ty2k July 10, 2026 22:46

@ty2k ty2k left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 👍

@mkernohanbc
mkernohanbc merged commit a9d610d into main Jul 14, 2026
5 checks passed
@mkernohanbc
mkernohanbc deleted the bugfix/menu branch July 14, 2026 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug react-components Changes or issues affecting the design-system-react-components package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants