From bdfa4596ef738aca998a8bac5a3af30fe94e9f62 Mon Sep 17 00:00:00 2001 From: Lawrence Win Date: Wed, 29 Nov 2023 10:33:14 -0800 Subject: [PATCH] Fix disabled, selected Tab in TabList from impeding keyboarding navigation when trying to tab in (#3235) * Fix disabled, selected tab impeding keyboard nav If a selected tab manages to be disabled, the user cannot change keyboard focus to the tablist or any element beyond that. This commit fixes the issue. * Change files * Update snapshot tests * A11y state disabled prop is reflected correctly * Commit for testing narrator bug, PLEASE REVERT * Revert "Commit for testing narrator bug, PLEASE REVERT" This reverts commit 98a9c7e9e531848ae10282067ff8a6842d6f188e. * Address nits * Add comments to clairfy exhaustives-deps disabled warnings --- ...-c6f6ed00-2198-41ae-9b2f-c9a7aec340d7.json | 7 +++ .../experimental/TabList/src/Tab/useTab.ts | 8 +-- .../TabList/src/Tab/useTab.win32.ts | 41 ++++++++++--- .../TabList/src/TabList/TabList.tsx | 4 +- .../TabList/src/TabList/TabList.types.ts | 14 ++++- .../TabList/src/TabList/TabListContext.ts | 4 +- .../__snapshots__/TabList.test.tsx.snap | 5 ++ .../TabList/src/TabList/useTabList.ts | 61 ++++++++++++++++--- 8 files changed, 118 insertions(+), 26 deletions(-) create mode 100644 change/@fluentui-react-native-tablist-c6f6ed00-2198-41ae-9b2f-c9a7aec340d7.json diff --git a/change/@fluentui-react-native-tablist-c6f6ed00-2198-41ae-9b2f-c9a7aec340d7.json b/change/@fluentui-react-native-tablist-c6f6ed00-2198-41ae-9b2f-c9a7aec340d7.json new file mode 100644 index 0000000000..697198642e --- /dev/null +++ b/change/@fluentui-react-native-tablist-c6f6ed00-2198-41ae-9b2f-c9a7aec340d7.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Fix disabled selected tab impeding keyboard navigation", + "packageName": "@fluentui-react-native/tablist", + "email": "winlarry@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/experimental/TabList/src/Tab/useTab.ts b/packages/experimental/TabList/src/Tab/useTab.ts index 52e93d3768..56dd05557b 100644 --- a/packages/experimental/TabList/src/Tab/useTab.ts +++ b/packages/experimental/TabList/src/Tab/useTab.ts @@ -33,7 +33,7 @@ export const useTab = (props: TabProps): TabInfo => { ...rest } = props; // Grabs the context information from Tabs (currently selected Tab and client's onTabSelect callback). - const { addTabKey, invoked, onTabSelect, removeTabKey, setInvoked, setSelectedTabRef, selectedKey, tabKeys, ...tablist } = + const { addTabKey, invoked, onTabSelect, removeTabKey, setInvoked, setFocusedTabRef, selectedKey, tabKeys, ...tablist } = React.useContext(TabListContext); const isDisabled = disabled || tablist.disabled; @@ -41,9 +41,9 @@ export const useTab = (props: TabProps): TabInfo => { const changeSelection = React.useCallback(() => { if (tabKey !== selectedKey) { onTabSelect(tabKey); - componentRef && setSelectedTabRef(componentRef); + componentRef && setFocusedTabRef(componentRef); } - }, [componentRef, setSelectedTabRef, onTabSelect, selectedKey, tabKey]); + }, [componentRef, setFocusedTabRef, onTabSelect, selectedKey, tabKey]); const changeSelectionWithFocus = useOnPressWithFocus(componentRef, changeSelection); @@ -65,7 +65,7 @@ export const useTab = (props: TabProps): TabInfo => { addTabKey(tabKey); // Set a defaultTabbableElement if we're the initial selectedKey. if (selectedKey === tabKey) { - componentRef && setSelectedTabRef(componentRef); + componentRef && setFocusedTabRef(componentRef); } return () => removeTabKey(tabKey); // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/packages/experimental/TabList/src/Tab/useTab.win32.ts b/packages/experimental/TabList/src/Tab/useTab.win32.ts index 7b1ffb8a82..cd07bb1e1b 100644 --- a/packages/experimental/TabList/src/Tab/useTab.win32.ts +++ b/packages/experimental/TabList/src/Tab/useTab.win32.ts @@ -33,17 +33,27 @@ export const useTab = (props: TabProps): TabInfo => { ...rest } = props; // Grabs the context information from Tabs (currently selected Tab and client's onTabSelect callback). - const { addTabKey, invoked, onTabSelect, removeTabKey, setInvoked, setSelectedTabRef, selectedKey, tabKeys, vertical, ...tablist } = - React.useContext(TabListContext); + const { + addTabKey, + invoked, + onTabSelect, + removeTabKey, + setInvoked, + setFocusedTabRef, + selectedKey, + tabKeys, + vertical, + updateDisabledTabs, + updateTabRef, + ...tablist + } = React.useContext(TabListContext); const isDisabled = disabled || tablist.disabled; const changeSelection = React.useCallback(() => { - if (tabKey !== selectedKey) { - onTabSelect(tabKey); - componentRef && setSelectedTabRef(componentRef); - } - }, [componentRef, setSelectedTabRef, onTabSelect, selectedKey, tabKey]); + onTabSelect(tabKey); + componentRef && setFocusedTabRef(componentRef); + }, [componentRef, setFocusedTabRef, onTabSelect, tabKey]); /** * In the main useTab file, our onPress callback we pass to usePressableState below is generated by the useOnPressWithFocus hook. @@ -74,22 +84,35 @@ export const useTab = (props: TabProps): TabInfo => { addTabKey(tabKey); // Set a defaultTabbableElement if we're the initial selectedKey. if (selectedKey === tabKey) { - componentRef && setSelectedTabRef(componentRef); + componentRef && setFocusedTabRef(componentRef); } return () => removeTabKey(tabKey); // eslint-disable-next-line react-hooks/exhaustive-deps }, []); + React.useEffect(() => { + updateTabRef(tabKey, componentRef); + // Disable exhaustive-deps warning because the hook shouldn't run whenever the excluded dependency, updateTabRef, changes. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [tabKey, componentRef]); + + React.useEffect(() => { + updateDisabledTabs(tabKey, disabled); + // Disable exhaustive-deps warning because the hook shouldn't run whenever the excluded dependency, updateDisabledTabs, change. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [tabKey, disabled]); + /** * Continuing from `handlePressAndDeferFocus`, once we have updated the selection state, we can safely set focus to the correct tab so * the narrator will only run once. */ React.useEffect(() => { if (invoked && setInvoked && tabKey === selectedKey && !isDisabled) { - componentRef && setSelectedTabRef(componentRef); + componentRef && setFocusedTabRef(componentRef); componentRef?.current?.focus(); setInvoked(false); } + // Disable exhaustive-deps warning because hook should only run whenever 'invoked' and its setter are updated. // eslint-disable-next-line react-hooks/exhaustive-deps }, [invoked, setInvoked]); diff --git a/packages/experimental/TabList/src/TabList/TabList.tsx b/packages/experimental/TabList/src/TabList/TabList.tsx index c6e73081dc..22eda5c15e 100644 --- a/packages/experimental/TabList/src/TabList/TabList.tsx +++ b/packages/experimental/TabList/src/TabList/TabList.tsx @@ -36,7 +36,7 @@ export const TabList = compose({ const { disabled, defaultTabbableElement, isCircularNavigation, vertical, ...mergedProps } = mergeProps(tablist.props, final); - const { animatedIndicatorStyles, canShowAnimatedIndicator, layout, selectedKey } = tablist.state; + const { animatedIndicatorStyles, canShowAnimatedIndicator, disabled: tablistDisabledState, layout, selectedKey } = tablist.state; return ( ({ value={tablist.state} > void; /** - * Setter for the selected Tab's ref to set as the default tabbable element in the FocusZone + * Setter for the focused Tab's ref to set as the default tabbable element in the FocusZone */ - setSelectedTabRef: (ref: React.RefObject) => void; + setFocusedTabRef: (ref: React.RefObject) => void; /** * TabList's `size` prop. @@ -106,6 +106,16 @@ export interface TabListState { */ updateAnimatedIndicatorStyles?: (updates: AnimatedIndicatorStylesUpdate) => void; + /** + * Updates internal map that keeps track of each of this tablist's tabs disabled state + */ + updateDisabledTabs: (tabKey: string, isDisabled: boolean) => void; + + /** + * Updates internal map that keeps track of each of this tablist's tabs' refs. + */ + updateTabRef: (tabKey: string, ref: React.RefObject) => void; + /** * TabList's `vertical` prop. */ diff --git a/packages/experimental/TabList/src/TabList/TabListContext.ts b/packages/experimental/TabList/src/TabList/TabListContext.ts index 606ddd1ff3..470fc0ca27 100644 --- a/packages/experimental/TabList/src/TabList/TabListContext.ts +++ b/packages/experimental/TabList/src/TabList/TabListContext.ts @@ -12,8 +12,10 @@ export const TabListContext = React.createContext({ onTabSelect: nullFunction, removeTabKey: nullFunction, selectedKey: '', - setSelectedTabRef: nullFunction, + setFocusedTabRef: nullFunction, size: 'small', tabKeys: [], vertical: false, + updateDisabledTabs: nullFunction, + updateTabRef: nullFunction, }); diff --git a/packages/experimental/TabList/src/TabList/__tests__/__snapshots__/TabList.test.tsx.snap b/packages/experimental/TabList/src/TabList/__tests__/__snapshots__/TabList.test.tsx.snap index b792e5a9f5..c587687a6b 100644 --- a/packages/experimental/TabList/src/TabList/__tests__/__snapshots__/TabList.test.tsx.snap +++ b/packages/experimental/TabList/src/TabList/__tests__/__snapshots__/TabList.test.tsx.snap @@ -2,6 +2,7 @@ exports[`TabList component tests TabList appearance 1`] = ` @@ -432,6 +433,7 @@ exports[`TabList component tests TabList appearance 1`] = ` exports[`TabList component tests TabList default props 1`] = ` @@ -1293,6 +1295,7 @@ exports[`TabList component tests TabList disabled list 1`] = ` exports[`TabList component tests TabList orientation 1`] = ` @@ -1720,6 +1723,7 @@ exports[`TabList component tests TabList orientation 1`] = ` exports[`TabList component tests TabList selected key 1`] = ` @@ -2151,6 +2155,7 @@ exports[`TabList component tests TabList selected key 1`] = ` exports[`TabList component tests TabList size 1`] = ` diff --git a/packages/experimental/TabList/src/TabList/useTabList.ts b/packages/experimental/TabList/src/TabList/useTabList.ts index 8e072c9027..8d447a1f0a 100644 --- a/packages/experimental/TabList/src/TabList/useTabList.ts +++ b/packages/experimental/TabList/src/TabList/useTabList.ts @@ -36,11 +36,28 @@ export const useTabList = (props: TabListProps): TabListInfo => { } = props; const data = useSelectedKey(selectedKey || defaultSelectedKey || null, onTabSelect); + const selectedTabKey = selectedKey ?? data.selectedKey; - // selectedTabRef should be set to default tabbable element. - const [selectedTabRef, setSelectedTabRef] = React.useState(React.useRef(null)); + // focusedTabRef should be set to default tabbable element. + const [focusedTabRef, setFocusedTabRef] = React.useState(React.useRef(null)); const [invoked, setInvoked] = React.useState(false); const [tabKeys, setTabKeys] = React.useState([]); + const [allTabsDisabled, setAllTabsDisabled] = React.useState(false); + + // These maps are used to switch tab focus in the event the selected tab is disabled. React refs are used as storage because updating the maps shouldn't trigger a re-render. + const tabRefMap = React.useRef<{ [key: string]: React.RefObject }>({}).current; + const disabledStateMap = React.useRef<{ [key: string]: boolean }>({}).current; + + const updateTabRef = React.useCallback((key: string, ref: React.RefObject) => (tabRefMap[key] = ref), [tabRefMap]); + const updateDisabledTabs = React.useCallback( + (key: string, isDisabled: boolean) => { + disabledStateMap[key] = isDisabled; + if (allTabsDisabled && !isDisabled) { + setAllTabsDisabled(false); + } + }, + [allTabsDisabled, disabledStateMap], + ); const addTabKey = React.useCallback( (tabKey: string) => { @@ -94,15 +111,41 @@ export const useTabList = (props: TabListProps): TabListInfo => { } }; + // If the current selected tab becomes disabled, the following useEffect sets the default focused element to the next non-disabled tab key. + // Without this, keyboard navigation gets stuck when attempting to tab towards the tablist and every following element after + const isSelectedTabDisabled = disabledStateMap[selectedTabKey]; + + React.useEffect(() => { + if (isSelectedTabDisabled) { + // switch focus to the next available tab key + let tabIndex = tabKeys.indexOf(selectedTabKey); + for (let i = 0; i < tabKeys.length; i++) { + tabIndex = (tabIndex + 1) % tabKeys.length; + if (!disabledStateMap[tabKeys[tabIndex]]) { + break; + } + } + if (tabKeys[tabIndex] === selectedTabKey) { + // In the very rare edge case of all tabs somehow being disabled, we need to set this tablist to become disabled to prevent users from keyboarding in + setAllTabsDisabled(true); + } else { + const ref = tabRefMap[tabKeys[tabIndex]]; + setFocusedTabRef(ref); + } + } + // Disable exhaustive-deps warning because this hook should only run once 'isSelectedTabDisabled' dependency changes. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [isSelectedTabDisabled]); + return { props: { ...props, accessible: accessible ?? true, - accessibilityState: getAccessibilityState(disabled, accessibilityState), + accessibilityState: getAccessibilityState(disabled || allTabsDisabled, accessibilityState), accessibilityRole: 'tablist', appearance: appearance, componentRef: componentRef, - defaultTabbableElement: selectedTabRef, + defaultTabbableElement: focusedTabRef, isCircularNavigation: isCircularNavigation ?? false, onLayout: onTabListLayout, size: size, @@ -113,8 +156,8 @@ export const useTabList = (props: TabListProps): TabListInfo => { addTabLayout: addTabLayout, animatedIndicatorStyles: userDefinedAnimatedIndicatorStyles, appearance: appearance, - canShowAnimatedIndicator: !!(userDefinedAnimatedIndicatorStyles && listLayoutMap && listLayoutMap[selectedKey ?? data.selectedKey]), - disabled: disabled, + canShowAnimatedIndicator: !!(userDefinedAnimatedIndicatorStyles && listLayoutMap && listLayoutMap[selectedTabKey]), + disabled: disabled || allTabsDisabled, invoked: invoked, layout: { tablist: tabListLayout, @@ -122,13 +165,15 @@ export const useTabList = (props: TabListProps): TabListInfo => { }, onTabSelect: data.onKeySelect, removeTabKey: removeTabKey, - selectedKey: selectedKey ?? data.selectedKey, - setSelectedTabRef: setSelectedTabRef, + selectedKey: selectedTabKey, + setFocusedTabRef: setFocusedTabRef, setInvoked: setInvoked, size: size, tabKeys: tabKeys, vertical: vertical, updateAnimatedIndicatorStyles: updateStyles, + updateDisabledTabs, + updateTabRef, }, }; };