feat(react-menu): replace keydown handlers by useARIAButtonShorthand on MenuItem

[bsunderhus]

Current Behavior

MenuItem currently provides its own custom keydown handlers to ensure button functionality.

New Behavior

1. Uses useARIAButtonShorthand instead of implementing it's own handlers
2. Stops spreading props over MenuItem, MenuItemCheckbox and MenuItemRadio state.

Related Issue(s)

While pressing Space the oficial functionality of a native button is to emit a click event on keyup. take the example bellow:

export const Default = () => (
  <Menu>
    <MenuTrigger>
      <Button>Toggle menu</Button>
    </MenuTrigger>

    <MenuPopover>
      <MenuList>
        <MenuItem>New </MenuItem>
        <MenuItem>New Window</MenuItem>
        <MenuItem disabled>Open File</MenuItem>
        <MenuItem>Open Folder</MenuItem>
      </MenuList>
    </MenuPopover>
  </Menu>
);

Meanwhile Button behaves as a proper native button and emits click on keyup, MenuItem will close on Space keydown, meaning that as soon as you press Space on MenuItem the Menu will close on keydown and immediately open up again as soon as Space is release emitting a keyup event.

Fixes #24680

[fabricteam]

📊 Bundle size report

Package & Exports	Baseline (minified/GZIP)	PR	Change
react-components react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	187.939 kB 52.05 kB	188.61 kB 52.323 kB	671 B 273 B
react-menu Menu (including children components)	115.735 kB 35.419 kB	116.529 kB 35.768 kB	794 B 349 B
react-menu Menu (including selectable components)	118.934 kB 35.916 kB	119.598 kB 36.287 kB	664 B 371 B

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
react-components react-components: FluentProvider & webLightTheme	33.359 kB 11.004 kB
react-overflow hooks only	10.685 kB 4.104 kB
react-portal-compat PortalCompatProvider	5.851 kB 1.964 kB

_{🤖 This report was generated against e0a0ab1481d0fb64f2a9804376ad17e4f4f8ea01}

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 44ab882:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration

[size-auditor]

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: e0a0ab1481d0fb64f2a9804376ad17e4f4f8ea01 (build)

[fabricteam]

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
Avatar	mount	1308	1303	5000
Button	mount	954	946	5000
FluentProvider	mount	1579	1573	5000
FluentProviderWithTheme	mount	641	640	10
FluentProviderWithTheme	virtual-rerender	591	592	10
FluentProviderWithTheme	virtual-rerender-with-unmount	633	639	10
MakeStyles	mount	1895	1892	50000
SpinButton	mount	2563	2526	5000

[bsunderhus]

Blocked by #24742

[ling1726]

does this mean that users that wrap the MenuItem component and spread props also need to cast, even if the type extends from MenuItemProps ?

[bsunderhus]

Yes! that is true for Button also, upper casting is required before spreading.

[ling1726]

This PR also fixes #24680 @bsunderhus can you add it to the description ?

[GeoffCoxMSFT]

I don't understand how the changes you made caused this cast to be required, but it won't hurt :)

[bsunderhus]

The casting is required since spreading of props can be conflicting when using discriminated unions. Basically there's no way to know if we're spreading button props or a props, so we need to explicitly upper cast to ensure it's a generic that represents both.