From c57d490060ca6f449a2480dfe507717157a90e2f Mon Sep 17 00:00:00 2001 From: Florent Mathieu Date: Fri, 30 Aug 2024 10:26:48 -1000 Subject: [PATCH 1/8] fix(container-menu): improves logic for returning focus to trigger --- packages/menu/src/useMenu.ts | 90 ++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 39 deletions(-) diff --git a/packages/menu/src/useMenu.ts b/packages/menu/src/useMenu.ts index bd1e170a4..892757449 100644 --- a/packages/menu/src/useMenu.ts +++ b/packages/menu/src/useMenu.ts @@ -12,7 +12,6 @@ import React, { useEffect, useMemo, useReducer, - useRef, useState } from 'react'; import { useSelection } from '@zendeskgarden/container-selection'; @@ -94,8 +93,6 @@ export const useMenu = (false); - const focusTriggerRef = useRef(false); - const [state, dispatch] = useReducer(stateReducer, { focusedValue: focusedValue || defaultFocusedValue, isExpanded: isExpanded || defaultExpanded, @@ -135,6 +132,15 @@ export const useMenu = { + if (!skip && triggerRef.current) { + triggerRef.current.focus(); + } + }, + [triggerRef] + ); + const closeMenu = useCallback( (changeType: string) => { dispatch({ @@ -272,22 +278,32 @@ export const useMenu = { @@ -395,6 +420,8 @@ export const useMenu = { if (state.focusOnOpen && menuVisible && controlledFocusedValue && controlledIsExpanded) { @@ -614,13 +632,7 @@ export const useMenu = Date: Wed, 11 Sep 2024 09:58:38 -1000 Subject: [PATCH 2/8] fix: improve focus handling for controlled menu --- packages/menu/src/useMenu.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/menu/src/useMenu.ts b/packages/menu/src/useMenu.ts index 892757449..593e491fc 100644 --- a/packages/menu/src/useMenu.ts +++ b/packages/menu/src/useMenu.ts @@ -289,7 +289,7 @@ export const useMenu = { + if (isExpandedControlled && isExpanded === false) { + returnFocusToTrigger(); + } + }, [isExpandedControlled, isExpanded, returnFocusToTrigger]); + /** * Respond to clicks outside the open menu */ From 10f54ca45bee5c673ac272287b416435f5e4dc92 Mon Sep 17 00:00:00 2001 From: Florent Mathieu Date: Wed, 11 Sep 2024 12:26:30 -1000 Subject: [PATCH 3/8] refactor: PR feedback --- packages/menu/src/useMenu.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/menu/src/useMenu.ts b/packages/menu/src/useMenu.ts index 593e491fc..5e6f73d34 100644 --- a/packages/menu/src/useMenu.ts +++ b/packages/menu/src/useMenu.ts @@ -278,18 +278,17 @@ export const useMenu = Date: Wed, 11 Sep 2024 17:59:53 -1000 Subject: [PATCH 4/8] feat(menu): add props.restoreFocus --- packages/menu/src/types.ts | 2 ++ packages/menu/src/useMenu.ts | 17 ++++++----------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/menu/src/types.ts b/packages/menu/src/types.ts index 201de2e1d..7e70a6570 100644 --- a/packages/menu/src/types.ts +++ b/packages/menu/src/types.ts @@ -66,6 +66,8 @@ export interface IUseMenuProps { focusedValue?: string | null; /** Determines focused value on menu initialization */ defaultFocusedValue?: string; + /** Returns focus to the trigger when the menu is collapsed */ + restoreFocus?: boolean; /** Sets the selected values in a controlled menu */ selectedItems?: ISelectedItem[]; /** diff --git a/packages/menu/src/useMenu.ts b/packages/menu/src/useMenu.ts index 5e6f73d34..95bc9157e 100644 --- a/packages/menu/src/useMenu.ts +++ b/packages/menu/src/useMenu.ts @@ -48,6 +48,7 @@ export const useMenu = undefined, isExpanded, defaultExpanded = false, + restoreFocus = true, selectedItems, focusedValue, defaultFocusedValue @@ -134,11 +135,11 @@ export const useMenu = { - if (!skip && triggerRef.current) { + if (!skip && restoreFocus && triggerRef.current) { triggerRef.current.focus(); } }, - [triggerRef] + [triggerRef, restoreFocus] ); const closeMenu = useCallback( @@ -288,7 +289,7 @@ export const useMenu = { - if (isExpandedControlled && isExpanded === false) { - returnFocusToTrigger(); - } - }, [isExpandedControlled, isExpanded, returnFocusToTrigger]); - /** * Respond to clicks outside the open menu */ From 21d41f2681c454e9a14a947efba402164b2b10b8 Mon Sep 17 00:00:00 2001 From: Florent Mathieu Date: Wed, 11 Sep 2024 18:03:29 -1000 Subject: [PATCH 5/8] docs: document restoreFocus prop + set default props --- packages/menu/src/MenuContainer.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/menu/src/MenuContainer.tsx b/packages/menu/src/MenuContainer.tsx index e002548e7..600876d14 100644 --- a/packages/menu/src/MenuContainer.tsx +++ b/packages/menu/src/MenuContainer.tsx @@ -29,5 +29,12 @@ MenuContainer.propTypes = { defaultExpanded: PropTypes.bool, selectedItems: PropTypes.arrayOf(PropTypes.any), focusedValue: PropTypes.oneOfType([PropTypes.string]), - defaultFocusedValue: PropTypes.oneOfType([PropTypes.string]) + defaultFocusedValue: PropTypes.oneOfType([PropTypes.string]), + restoreFocus: PropTypes.bool +}; + +MenuContainer.defaultProps = { + defaultExpanded: false, + restoreFocus: true, + rtl: false }; From 9e19d82dd29477b6052c69d78f8aa046076d75a3 Mon Sep 17 00:00:00 2001 From: Florent Mathieu Date: Wed, 11 Sep 2024 18:09:23 -1000 Subject: [PATCH 6/8] docs: add managed focus story --- packages/menu/demo/menu.stories.tsx | 48 +++++++++ packages/menu/demo/stories/MenuStory.tsx | 125 ++++++++++++----------- 2 files changed, 116 insertions(+), 57 deletions(-) diff --git a/packages/menu/demo/menu.stories.tsx b/packages/menu/demo/menu.stories.tsx index 76723ddab..d71770285 100644 --- a/packages/menu/demo/menu.stories.tsx +++ b/packages/menu/demo/menu.stories.tsx @@ -63,6 +63,54 @@ export const Controlled: Story = { }, args: { isExpanded: false, + restoreFocus: true, + focusedValue: 'plant-01', + selectedItems: [{ value: 'Cherry', type: 'checkbox' }] + } +}; + +export const ControlledManagedFocus: Story = { + render: function Render(args) { + const updateArgs = useArgs()[1]; + const triggerRef = React.useRef(null); + + return ( + { + // eslint-disable-next-line no-console + console.log('onChange:', _args); + + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { type, isExpanded, ...rest } = _args; + const { selectedItems } = rest; + let nextArgs: any = rest; + + const lastItem = selectedItems?.[selectedItems.length - 1]; + const isNonCheckboxItem = !selectedItems || lastItem?.type !== 'checkbox'; + + if (isExpanded !== undefined && isNonCheckboxItem) { + nextArgs = { ...nextArgs, isExpanded }; + } + + if (!args.restoreFocus && isExpanded === false && triggerRef.current) { + triggerRef.current.focus(); + } + updateArgs(nextArgs); + }} + /> + ); + }, + name: 'Controlled + Managed Focus', + argTypes: { + defaultFocusedValue: { control: false }, + defaultExpanded: { control: false }, + focusedValue: { control: { type: 'text' } } + }, + args: { + isExpanded: false, + restoreFocus: false, focusedValue: 'plant-01', selectedItems: [{ value: 'Cherry', type: 'checkbox' }] } diff --git a/packages/menu/demo/stories/MenuStory.tsx b/packages/menu/demo/stories/MenuStory.tsx index a60b8ecf8..788782ee1 100644 --- a/packages/menu/demo/stories/MenuStory.tsx +++ b/packages/menu/demo/stories/MenuStory.tsx @@ -91,60 +91,71 @@ const Component = ({ const selectedValues = selection.map(item => item.value); return ( -
- - -
    - {items.map((item: MenuItem) => { - if ('items' in item) { - return ( -
  • - - -
      - {item.items.map(groupItem => ( - +
      +
      + + +
        + {items.map((item: MenuItem) => { + if ('items' in item) { + return ( +
      • + + - ))} -
      - - ); - } - - if ('separator' in item) { - return ( -
    • - ); - } - - return ( - - ); - })} -
    +
      + {item.items.map(groupItem => ( + + ))} +
    +
    • + ); + } + + if ('separator' in item) { + return ( +
  • + ); + } + + return ( + + ); + })} +
+
+
+ +
+ ); }; @@ -167,16 +178,16 @@ interface IArgs extends MenuContainerProps { as: 'hook' | 'container'; } -export const MenuStory: StoryFn = ({ as, ...props }) => { - const triggerRef = useRef(null); +export const MenuStory: StoryFn = ({ as, triggerRef, ...props }) => { + const _triggerRef = useRef(null); const menuRef = useRef(null); switch (as) { case 'container': - return ; + return ; case 'hook': default: - return ; + return ; } }; From 5c70b5221b95fcb9fea76a407f5e6a0a7dca6396 Mon Sep 17 00:00:00 2001 From: Florent Mathieu Date: Thu, 12 Sep 2024 20:02:01 -1000 Subject: [PATCH 7/8] docs: fix story --- packages/menu/demo/menu.stories.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/menu/demo/menu.stories.tsx b/packages/menu/demo/menu.stories.tsx index d71770285..df42391c0 100644 --- a/packages/menu/demo/menu.stories.tsx +++ b/packages/menu/demo/menu.stories.tsx @@ -85,16 +85,16 @@ export const ControlledManagedFocus: Story = { // eslint-disable-next-line @typescript-eslint/no-unused-vars const { type, isExpanded, ...rest } = _args; const { selectedItems } = rest; - let nextArgs: any = rest; + const nextArgs: typeof rest & { isExpanded?: boolean } = rest; const lastItem = selectedItems?.[selectedItems.length - 1]; const isNonCheckboxItem = !selectedItems || lastItem?.type !== 'checkbox'; if (isExpanded !== undefined && isNonCheckboxItem) { - nextArgs = { ...nextArgs, isExpanded }; + nextArgs.isExpanded = isExpanded; } - if (!args.restoreFocus && isExpanded === false && triggerRef.current) { + if (!args.restoreFocus && nextArgs.isExpanded === false && triggerRef.current) { triggerRef.current.focus(); } updateArgs(nextArgs); From f43e450d0c6de8d0911496b209de3417dcc297f2 Mon Sep 17 00:00:00 2001 From: Florent Mathieu Date: Fri, 13 Sep 2024 09:50:11 -1000 Subject: [PATCH 8/8] fix: remove unnecessary skip param --- packages/menu/src/useMenu.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/menu/src/useMenu.ts b/packages/menu/src/useMenu.ts index 95bc9157e..913025554 100644 --- a/packages/menu/src/useMenu.ts +++ b/packages/menu/src/useMenu.ts @@ -337,7 +337,7 @@ export const useMenu =