Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Menu] Fixed Menu item not toggling highlighted prop when submenu popup is opened using keyboard #1310

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
+133 −6
Open

[Menu] Fixed Menu item not toggling highlighted prop when submenu popup is opened using keyboard #1310

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0419c12
Added setActiveIndex to the MenuRoot and MenuSubmenuTrigger component…
onehanddev Jan 5, 2025
eb47bc2
fix: ts errors
onehanddev Jan 5, 2025
c4aa02c
Handled click event as well for setting active item for menu, added m…
onehanddev Jan 6, 2025
1700a03
addressed PR comments
onehanddev Jan 6, 2025
03c6581
added keyboard support for rtl direction for submenu toggling for Men…
onehanddev Jan 8, 2025
661c502
Merge branch 'master' into menu/submenu-highlight-keyboard-focus-updated
onehanddev Jan 8, 2025
eecafbd
Merge branch 'master' into menu/submenu-highlight-keyboard-focus-updated
onehanddev Jan 10, 2025
9ac8981
Added test cases to verify that the parent menu item remains inactive…
onehanddev Jan 13, 2025
08a8d3e
Merge branch 'master' into menu/submenu-highlight-keyboard-focus-updated
onehanddev Jan 14, 2025
7f69c60
fix: replaced tabindex with data-highlighted for <Menu.SubmenuTrigger…
onehanddev Jan 14, 2025
6281f1b
Merge branch 'menu/submenu-highlight-keyboard-focus-updated' of https…
onehanddev Jan 14, 2025
79cc42c
fix: retained tabindex of the submenutrigger once the submenu opens
onehanddev Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/react/src/menu/root/MenuRoot.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const MenuRoot: React.FC<MenuRoot.Props> = function MenuRoot(props) {
...menuRoot,
nested,
parentContext,
setActiveIndex: menuRoot.setActiveIndex,
disabled,
allowMouseUpTriggerRef:
parentContext?.allowMouseUpTriggerRef ?? menuRoot.allowMouseUpTriggerRef,
Expand Down
3 changes: 3 additions & 0 deletions packages/react/src/menu/root/useMenuRoot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ export function useMenuRoot(parameters: useMenuRoot.Parameters): useMenuRoot.Ret
() => ({
activeIndex,
allowMouseUpTriggerRef,
setActiveIndex,
floatingRootContext,
getItemProps,
getPopupProps,
Expand All @@ -269,6 +270,7 @@ export function useMenuRoot(parameters: useMenuRoot.Parameters): useMenuRoot.Ret
}),
[
activeIndex,
setActiveIndex,
floatingRootContext,
getItemProps,
getPopupProps,
Expand Down Expand Up @@ -351,6 +353,7 @@ export namespace useMenuRoot {

export interface ReturnValue {
activeIndex: number | null;
setActiveIndex: (index: number | null) => void;
floatingRootContext: FloatingRootContext;
getItemProps: (externalProps?: GenericHTMLProps) => GenericHTMLProps;
getPopupProps: (externalProps?: GenericHTMLProps) => GenericHTMLProps;
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/menu/submenu-trigger/MenuSubmenuTrigger.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,14 @@ const MenuSubmenuTrigger = React.forwardRef(function SubmenuTriggerComponent(
throw new Error('Base UI: ItemTrigger must be placed in a nested Menu.');
}

const { activeIndex, getItemProps } = parentContext;
const { activeIndex, getItemProps, setActiveIndex } = parentContext;
const item = useCompositeListItem();

const highlighted = activeIndex === item.index;

const mergedRef = useForkRef(forwardedRef, item.ref);

const { events: menuEvents } = useFloatingTree()!;

const { getRootProps } = useMenuSubmenuTrigger({
id,
highlighted,
Expand All @@ -55,6 +54,7 @@ const MenuSubmenuTrigger = React.forwardRef(function SubmenuTriggerComponent(
setTriggerElement,
allowMouseUpTriggerRef,
typingRef,
setActiveIndex,
});

const state: MenuSubmenuTrigger.State = React.useMemo(
Expand Down
31 changes: 27 additions & 4 deletions packages/react/src/menu/submenu-trigger/useMenuSubmenuTrigger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ import { FloatingEvents } from '@floating-ui/react';
import { useMenuItem } from '../item/useMenuItem';
import { useForkRef } from '../../utils/useForkRef';
import { GenericHTMLProps } from '../../utils/types';
import { mergeReactProps } from '../../utils/mergeReactProps';
import { useDirection } from '../../direction-provider/DirectionContext';

type MenuKeyboardEvent = {
key: 'ArrowRight' | 'ArrowLeft' | 'ArrowUp' | 'ArrowDown';
} & React.KeyboardEvent;

export function useMenuSubmenuTrigger(
parameters: useSubmenuTrigger.Parameters,
Expand All @@ -17,6 +23,7 @@ export function useMenuSubmenuTrigger(
setTriggerElement,
allowMouseUpTriggerRef,
typingRef,
setActiveIndex,
} = parameters;

const { getRootProps: getMenuItemProps, rootRef: menuItemRef } = useMenuItem({
Expand All @@ -32,17 +39,28 @@ export function useMenuSubmenuTrigger(

const menuTriggerRef = useForkRef(menuItemRef, setTriggerElement);

const direction = useDirection();

const getRootProps = React.useCallback(
(externalProps?: GenericHTMLProps) => {
return {
const openKey = direction === 'rtl' ? 'ArrowLeft' : 'ArrowRight';

return mergeReactProps(externalProps, {
Copy link
Contributor

Choose a reason for hiding this comment

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

mergeReactProps<'div'>(...) will be able to type the onKeyDown event automatically

...getMenuItemProps({
'aria-haspopup': 'menu' as const,
...externalProps,
onKeyDown: (event: MenuKeyboardEvent) => {
if (event.key === openKey && highlighted) {
// Clear parent menu's highlight state when entering submenu
// This prevents multiple highlighted items across menu levels
setActiveIndex(null);
}
},
onClick: () => highlighted && setActiveIndex(null),
}),
ref: menuTriggerRef,
};
});
},
[getMenuItemProps, menuTriggerRef],
[getMenuItemProps, menuTriggerRef, highlighted, setActiveIndex, direction],
);

return React.useMemo(
Expand Down Expand Up @@ -82,6 +100,11 @@ export namespace useSubmenuTrigger {
* A ref that is set to `true` when the user is using the typeahead feature.
*/
typingRef: React.RefObject<boolean>;
/**
* Callback to update the active (highlighted) item index.
* Set to null to remove highlighting from all items.
*/
setActiveIndex: (index: number | null) => void;
}

export interface ReturnValue {
Expand Down
Loading