fix: prevent standalone MenuItem context errors

[miacycle]

Summary

• replace the base MenuItem wrapper with a standalone-compatible implementation that no longer relies on MUI's hidden menu context
• preserve the expected menu-item styling, states, and focus handling for standalone, MenuList, and Select usage
• add regression coverage for those three render paths

Notes

• focused validation run: npx eslint src/base/MenuItem/MenuItem.tsx src/testing/MenuItem.test.tsx
• focused validation run: npm test -- src/testing/MenuItem.test.tsx --runInBand
• repo-wide make tests and make build are currently failing on missing @rjsf/* modules in this checkout, unrelated to this change

[Copilot]

Pull request overview

This PR updates Sistent’s base/MenuItem wrapper to avoid MUI menu-context assumptions so MenuItem can be rendered safely as a standalone element, while still behaving correctly when used under MenuList and Select. It also adds regression tests to cover the key rendering paths.

Changes:

• Replaced the MenuItem wrapper (previously a direct @mui/material/MenuItem pass-through) with a standalone-compatible implementation built on ListItemButton while retaining MUI MenuItem utility classes and states.
• Preserved styling and interaction states (hover/selected/focusVisible/disabled, gutters/dense/divider) via a styled() root aligned to MuiMenuItem.
• Added a Jest regression test covering standalone usage, MenuList usage, and Select usage.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/base/MenuItem/MenuItem.tsx	Implements a standalone-safe MenuItem that still uses MUI MenuItem classes/states and theme overrides.
src/testing/MenuItem.test.tsx	Adds regression coverage for standalone, MenuList, and Select render paths.

[gemini-code-assist]

Code Review

This pull request replaces the basic MUI MenuItem wrapper with a custom implementation using ListItemButton and styled components, providing more granular control over styling for states such as selected, hover, and dense. A new test suite was also introduced to verify the component's behavior. Review feedback suggests improving the Select integration test by asserting on the 'option' role while the dropdown is open and recommends using the useForkRef hook from @mui/utils to simplify ref handling and align with MUI best practices.

[gemini-code-assist]

This test checks that the selected value of the Select is rendered, but it doesn't verify that the MenuItem renders correctly as an option inside the dropdown. To make the test more robust, you can use the open prop on the Select component to render it in an open state and then assert that the MenuItem is rendered as an option.

Suggested change

	it('renders as a Select option without throwing', () => {
	renderWithTheme(
	<Select value="standalone">
	<MenuItem value="standalone">Standalone option</MenuItem>
	</Select>
	);
	expect(screen.getByText('Standalone option')).toBeTruthy();
	});
	it('renders as a Select option without throwing', () => {
	renderWithTheme(
	<Select value="standalone" open>
	<MenuItem value="standalone">Standalone option</MenuItem>
	</Select>
	);
	expect(screen.getByRole('option', { name: 'Standalone option' })).toBeTruthy();
	});

[gemini-code-assist]

You can simplify the ref handling by using the useForkRef hook from @mui/utils, which is designed for this purpose. This will make the code more concise and idiomatic within the MUI ecosystem.

This change would involve:

1. Importing useForkRef from @mui/utils (it's in the same package as composeClasses which you already use).
2. Removing the assignRef helper function (lines 135-144).
3. Replacing the useCallback for handleRef with useForkRef.

    const handleRef = useForkRef(itemRef, ref);

[gemini-code-assist]

Code Review

This pull request refactors the MenuItem component from a simple Material UI wrapper to a custom implementation using ListItemButton, incorporating comprehensive styling for various states and adding a new test suite. A review comment identifies a potential accessibility regression where changing the default tabIndex to 0 for non-disabled items could break standard keyboard navigation patterns in menu lists.

[gemini-code-assist]

The default tabIndex for MenuItem in Material UI is -1, as it is typically used within a Menu or MenuList where focus is managed via roving tabindex. Changing the default to 0 for non-disabled items makes every item in a list tabbable by default. This can break standard keyboard navigation patterns (where Tab should skip the list and Arrow keys should navigate within it). While this change supports standalone usage, it might introduce accessibility regressions for existing MenuList implementations that do not use a Menu wrapper.

Suggested change

	tabIndex={tabIndexProp ?? (disabled ? -1 : 0)}
	tabIndex={tabIndexProp ?? (disabled ? -1 : -1)}