Skip to content

Commit e66e88a

Browse files
rurikoarakirurikoaraki
authored
[win32] Fix keyboarding in MenuList (#3189)
* Update test * Add focus zone around menu groups * Address feedback * Focus zone in MenuLis for win32t * Add test and a tiny bit of cleanup * Refactor * More refactoring * One more edit * Address feedback * Fix e2e test * Change test title * Fix keyboarding behavior * Fix keyboarding behavior * Change files * Some code cleanup * Revert "Fix e2e test" This reverts commit db8d902. * Update rex-win32 * Change files * yarn.lock * Address feedback
1 parent ae65ca9 commit e66e88a

16 files changed

+154
-54
lines changed

‎apps/fluent-tester/src/TestComponents/Menu/MenuComponentOutsideMenuList.tsx

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import * as React from 'react';
2+
3+
import { ButtonV1 as Button } from '@fluentui/react-native';
4+
import { Menu, MenuItem, MenuTrigger, MenuPopover, MenuList } from '@fluentui-react-native/menu';
5+
import { Stack } from '@fluentui-react-native/stack';
6+
7+
import { stackStyle } from '../Common/styles';
8+
9+
export const MenuComponentOutsideMenuList: React.FunctionComponent = () => {
10+
return (
11+
<Stack style={stackStyle}>
12+
<Menu>
13+
<MenuTrigger>
14+
<Button>The Menu</Button>
15+
</MenuTrigger>
16+
<MenuPopover>
17+
<MenuList>
18+
<MenuItem>A plain MenuItem</MenuItem>
19+
<MenuItem>A plain MenuItem 2</MenuItem>
20+
</MenuList>
21+
<Button>Not in arrow loop</Button>
22+
</MenuPopover>
23+
</Menu>
24+
</Stack>
25+
);
26+
};

‎apps/fluent-tester/src/TestComponents/Menu/MenuTest.tsx

+10
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { Switch } from '@fluentui-react-native/switch';
2020
import { TextV1 as Text } from '@fluentui-react-native/text';
2121

2222
import { E2EMenuTest } from './E2EMenuTest';
23+
import { MenuComponentOutsideMenuList } from './MenuComponentOutsideMenuList';
2324
import { MenuIcons } from './MenuIcons';
2425
import { MenuTriggerChildRef } from './MenuRefs';
2526
import { MenuScrollView } from './MenuScrollView';
@@ -377,10 +378,12 @@ const MenuWithGroups: React.FunctionComponent = () => {
377378
<MenuGroup>
378379
<MenuGroupHeader>Section 1</MenuGroupHeader>
379380
<MenuItem>A plain MenuItem</MenuItem>
381+
<MenuItem>A plain MenuItem</MenuItem>
380382
</MenuGroup>
381383
<MenuGroup>
382384
<MenuGroupHeader>Section 2</MenuGroupHeader>
383385
<MenuItem>A plain MenuItem</MenuItem>
386+
<MenuItem>A plain MenuItem</MenuItem>
384387
</MenuGroup>
385388
</MenuList>
386389
</MenuPopover>
@@ -525,6 +528,13 @@ const menuSections: TestSection[] = [
525528
name: 'Menu with groups',
526529
component: MenuWithGroups,
527530
},
531+
Platform.select({
532+
android: null,
533+
default: {
534+
name: 'MenuList component outside menu list test',
535+
component: MenuComponentOutsideMenuList,
536+
},
537+
}),
528538
];
529539

530540
const e2eSections: TestSection[] = [

‎apps/win32/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
"@fluentui-react-native/eslint-config-rules": "^0.1.1",
4141
"@fluentui-react-native/scripts": "^0.1.1",
4242
"@office-iss/react-native-win32": "^0.71.0",
43-
"@office-iss/rex-win32": "0.71.15-devmain.16608.10000",
43+
"@office-iss/rex-win32": "0.71.41-devmain.17024.10000",
4444
"@rnx-kit/cli": "^0.16.2",
4545
"@rnx-kit/metro-config": "^1.3.1",
4646
"@types/react": "^18.2.0",

‎change/@fluentui-react-native-e2e-testing-98efa3f8-8a58-41d0-ae51-84ae00149104.json

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Add test and a tiny bit of cleanup",
4+
"packageName": "@fluentui-react-native/e2e-testing",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎change/@fluentui-react-native-focus-zone-ed378e33-55e4-48bc-822e-f31a921cfb9c.json

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Fix keyboarding behavior",
4+
"packageName": "@fluentui-react-native/focus-zone",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎change/@fluentui-react-native-menu-25201e2a-f966-483f-8135-4ac201ae8d9e.json

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Fix keyboarding behavior",
4+
"packageName": "@fluentui-react-native/menu",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎change/@fluentui-react-native-tester-9ed38b60-4594-4e77-bbcb-25f5ff58a66e.json

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Add test and a tiny bit of cleanup",
4+
"packageName": "@fluentui-react-native/tester",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎change/@fluentui-react-native-tester-win32-7cedee8a-a8cf-4294-b78f-4dd41170417c.json

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Update rex-win32",
4+
"packageName": "@fluentui-react-native/tester-win32",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎packages/components/FocusZone/src/FocusZone.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ export const FocusZone = composable<FocusZoneType>({
3131
return {
3232
slotProps: mergeSettings<FocusZoneSlotProps>(useStyling(userProps), {
3333
root: {
34+
navigateAtEnd: isCircularNavigation ? 'NavigateWrap' : 'NavigateStopAtEnds', // let rest override
3435
...rest,
3536
defaultTabbableElement: targetNativeTag,
3637
ref: ftzRef,
37-
navigateAtEnd: isCircularNavigation ? 'NavigateWrap' : 'NavigateStopAtEnds',
3838
},
3939
}),
4040
};

‎packages/components/Menu/src/MenuGroup/MenuGroup.tsx

+21-2
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,23 @@
11
/** @jsxRuntime classic */
22
/** @jsx withSlots */
33
import React from 'react';
4-
import { View } from 'react-native';
4+
import { Platform, View } from 'react-native';
55

6+
import { FocusZone } from '@fluentui-react-native/focus-zone';
67
import { compose, mergeProps, withSlots } from '@fluentui-react-native/framework';
78
import type { UseSlots } from '@fluentui-react-native/framework';
89

910
import type { MenuGroupProps, MenuGroupType } from './MenuGroup.types';
1011
import { menuGroupName } from './MenuGroup.types';
1112

13+
// Intentionally not enabled on macOS to match system context menus
14+
const hasFocusZone = ['win32'].includes(Platform.OS as string);
15+
1216
export const MenuGroup = compose<MenuGroupType>({
1317
displayName: menuGroupName,
1418
slots: {
1519
root: View,
20+
contentWrapper: hasFocusZone ? FocusZone : React.Fragment,
1621
},
1722
useRender: (userProps: MenuGroupProps, useSlots: UseSlots<MenuGroupType>) => {
1823
const Slots = useSlots(userProps);
@@ -39,7 +44,21 @@ export const MenuGroup = compose<MenuGroupType>({
3944
}
4045
return child;
4146
});
42-
return <Slots.root {...mergedProps}>{childrenWithSet}</Slots.root>;
47+
48+
return (
49+
<Slots.root {...mergedProps}>
50+
<Slots.contentWrapper
51+
// avoid error that fires when props are passed into React.fragment
52+
{...(hasFocusZone && {
53+
focusZoneDirection: 'vertical',
54+
enableFocusRing: false,
55+
navigateAtEnd: 'NavigateContinue',
56+
})}
57+
>
58+
{childrenWithSet}
59+
</Slots.contentWrapper>
60+
</Slots.root>
61+
);
4362
};
4463
},
4564
});

‎packages/components/Menu/src/MenuGroup/MenuGroup.types.ts

+2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { ViewProps } from 'react-native';
22

33
import type { IViewProps } from '@fluentui-react-native/adapters';
4+
import type { FocusZoneProps } from '@fluentui-react-native/focus-zone';
45
import type { LayoutTokens } from '@fluentui-react-native/tokens';
56

67
export const menuGroupName = 'MenuGroup';
@@ -11,6 +12,7 @@ export type MenuGroupProps = IViewProps;
1112

1213
export interface MenuGroupSlotProps {
1314
root: ViewProps;
15+
contentWrapper: FocusZoneProps;
1416
}
1517

1618
export interface MenuGroupType {

‎packages/components/Menu/src/MenuList/MenuList.tsx

+34-38
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import type { MenuListProps, MenuListState, MenuListType } from './MenuList.type
1313
import { menuListName } from './MenuList.types';
1414
import { useMenuList } from './useMenuList';
1515
import { useMenuListContextValue } from './useMenuListContextValue';
16-
import { useMenuContext } from '../context';
1716
import { MenuListProvider } from '../context/menuListContext';
1817

1918
const MenuStack = stagedComponent((props: React.PropsWithRef<IViewProps> & { gap?: number }) => {
@@ -35,32 +34,28 @@ const MenuStack = stagedComponent((props: React.PropsWithRef<IViewProps> & { gap
3534
});
3635
MenuStack.displayName = 'MenuStack';
3736

37+
const shouldHaveFocusZone = ['macos', 'win32'].includes(Platform.OS as string);
38+
const focusLandsOnContainer = Platform.OS === 'macos';
39+
const hasCircularNavigation = Platform.OS === ('win32' as any);
40+
const hasTabNavigation = Platform.OS === ('win32' as any);
41+
3842
export const menuListLookup = (layer: string, state: MenuListState, userProps: MenuListProps): boolean => {
3943
return state[layer] || userProps[layer] || layer === 'hasMaxHeight';
4044
};
45+
4146
export const MenuList = compose<MenuListType>({
4247
displayName: menuListName,
4348
...stylingSettings,
4449
slots: {
4550
root: MenuStack,
4651
scrollView: ScrollView,
47-
...(Platform.OS === 'macos' && { focusZone: FocusZone }),
52+
focusZone: shouldHaveFocusZone ? FocusZone : React.Fragment,
4853
},
4954
useRender: (userProps: MenuListProps, useSlots: UseSlots<MenuListType>) => {
5055
const menuList = useMenuList(userProps);
51-
const menuContext = useMenuContext();
5256
const menuListContextValue = useMenuListContextValue(menuList);
5357
const Slots = useSlots(menuList.props, (layer) => menuListLookup(layer, menuList, userProps));
5458

55-
const focusZoneRef = React.useRef<View>();
56-
const setFocusZoneFocus = () => {
57-
focusZoneRef?.current?.focus();
58-
};
59-
60-
React.useEffect(() => {
61-
setFocusZoneFocus();
62-
}, []);
63-
6459
return (_final: MenuListProps, children: React.ReactNode) => {
6560
const itemCount = React.Children.toArray(children).filter(
6661
(child) => React.isValidElement(child) && (child as any).type.displayName !== 'MenuDivider',
@@ -85,33 +80,34 @@ export const MenuList = compose<MenuListType>({
8580
return child;
8681
});
8782

88-
const content =
89-
Platform.OS === 'macos' ? (
90-
<Slots.root onMouseLeave={setFocusZoneFocus} onKeyDown={menuList.onListKeyDown}>
91-
<Slots.scrollView
92-
showsVerticalScrollIndicator={menuContext.hasMaxHeight}
93-
showsHorizontalScrollIndicator={menuContext.hasMaxWidth}
83+
const shouldHaveScrollView = Platform.OS === 'macos' || menuList.hasMaxHeight || menuList.hasMaxWidth;
84+
const ScrollViewWrapper = shouldHaveScrollView ? Slots.scrollView : React.Fragment;
85+
86+
const content = (
87+
<Slots.root>
88+
<ScrollViewWrapper
89+
// avoid error that fires when props are passed into React.fragment
90+
{...(shouldHaveScrollView && {
91+
showsVerticalScrollIndicator: menuList.hasMaxHeight,
92+
showsHorizontalScrollIndicator: menuList.hasMaxWidth,
93+
})}
94+
>
95+
<Slots.focusZone
96+
// avoid error that fires when props are passed into React.fragment
97+
{...(shouldHaveFocusZone && {
98+
componentRef: focusLandsOnContainer && menuList.focusZoneRef,
99+
focusZoneDirection: 'vertical',
100+
defaultTabbableElement: focusLandsOnContainer && menuList.focusZoneRef,
101+
enableFocusRing: false,
102+
isCircularNavigation: hasCircularNavigation,
103+
tabKeyNavigation: hasTabNavigation ? 'Normal' : 'None',
104+
})}
94105
>
95-
<Slots.focusZone
96-
componentRef={focusZoneRef}
97-
focusZoneDirection={'vertical'}
98-
defaultTabbableElement={focusZoneRef} // eslint-disable-next-line @typescript-eslint/ban-ts-comment
99-
// @ts-ignore FocusZone takes ViewProps, but that isn't defined on it's type.
100-
enableFocusRing={false}
101-
>
102-
{childrenWithSet}
103-
</Slots.focusZone>
104-
</Slots.scrollView>
105-
</Slots.root>
106-
) : menuContext.hasMaxHeight ? (
107-
<Slots.root onKeyDown={menuList.onListKeyDown} style={menuContext.minWidth ? { minWidth: menuContext.minWidth } : {}}>
108-
<Slots.scrollView>{childrenWithSet}</Slots.scrollView>
109-
</Slots.root>
110-
) : (
111-
<Slots.root onKeyDown={menuList.onListKeyDown} style={menuContext.minWidth ? { minWidth: menuContext.minWidth } : {}}>
112-
{childrenWithSet}
113-
</Slots.root>
114-
);
106+
{childrenWithSet}
107+
</Slots.focusZone>
108+
</ScrollViewWrapper>
109+
</Slots.root>
110+
);
115111

116112
return <MenuListProvider value={menuListContextValue}>{content}</MenuListProvider>;
117113
};

‎packages/components/Menu/src/MenuList/MenuList.types.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,14 @@ export interface MenuListState extends Omit<MenuListProps, 'checked' | 'onChecke
7979
removeRadioItem: (name: string) => void;
8080
trackMenuItem: (item: TrackedMenuItem) => void;
8181
untrackMenuItem: (item: TrackedMenuItem) => void;
82-
onListKeyDown: (e: InteractionEvent) => void;
83-
hasMaxHeight?: boolean;
84-
hasMaxWidth?: boolean;
82+
hasMaxHeight: boolean;
83+
hasMaxWidth: boolean;
84+
focusZoneRef?: React.RefObject<View>;
8585
}
8686

8787
export interface MenuListSlotProps {
8888
root: React.PropsWithRef<IViewProps> & { gap?: number };
89-
focusZone?: FocusZoneProps; // macOS only
89+
focusZone?: FocusZoneProps; // macOS and win32 only
9090
scrollView?: ScrollViewProps;
9191
}
9292

‎packages/components/Menu/src/MenuList/useMenuList.ts

+14-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ const removeRadioItem = (name: string) => {
2222

2323
const platformsWithoutFocusOnDisabled = ['ios', 'macos'];
2424
const handledKeys = ['Home', 'End'];
25+
const handleFocusOnMouseLevae = Platform.OS === 'macos';
2526

2627
export const useMenuList = (_props: MenuListProps): MenuListState => {
2728
const context = useMenuContext();
@@ -162,9 +163,21 @@ export const useMenuList = (_props: MenuListProps): MenuListState => {
162163
};
163164
});
164165

166+
// focus management
167+
const focusZoneRef = React.useRef<View>();
168+
const setFocusZoneFocus = () => {
169+
focusZoneRef?.current?.focus();
170+
};
171+
172+
React.useEffect(() => {
173+
setFocusZoneFocus();
174+
}, []);
175+
165176
return {
166177
props: {
167178
...context,
179+
onMouseLeave: handleFocusOnMouseLevae ? setFocusZoneFocus : context.onMouseLeave,
180+
onKeyDown: onListKeyDown,
168181
},
169182
isCheckedControlled,
170183
checked,
@@ -175,9 +188,9 @@ export const useMenuList = (_props: MenuListProps): MenuListState => {
175188
removeRadioItem,
176189
trackMenuItem,
177190
untrackMenuItem,
178-
onListKeyDown,
179191
hasMaxHeight: context.hasMaxHeight,
180192
hasMaxWidth: context.hasMaxWidth,
193+
focusZoneRef: focusZoneRef,
181194
};
182195
};
183196

‎packages/components/Menu/src/context/menuListContext.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { MenuListState } from '../MenuList/MenuList.types';
55
/**
66
* Context shared between Menu and its child components
77
*/
8-
export type MenuListContextValue = Omit<MenuListState, 'props'> & {
8+
export type MenuListContextValue = Omit<MenuListState, 'props' | 'focusZoneRef' | 'hasMaxHeight' | 'hasMaxWidth'> & {
99
hasCheckmarks: boolean;
1010
hasIcons: boolean;
1111
hasTooltips: boolean;
@@ -23,7 +23,6 @@ export const MenuListContext = React.createContext<MenuListContextValue>({
2323
removeRadioItem: () => false,
2424
trackMenuItem: () => false,
2525
untrackMenuItem: () => false,
26-
onListKeyDown: () => false,
2726
});
2827

2928
export const MenuListProvider = MenuListContext.Provider;

0 commit comments

Comments
 (0)