Skip to content

Commit b3acd4e

Browse files
ksilerksiler
authored
[Menu] Update tab navigation on win32 (#3860)
* Fix Menu tab navigation on win32 * Change files * update comment * revert changes to yarn.lock * update E2E test for win32 * Change files
1 parent 7490520 commit b3acd4e

File tree

6 files changed

+68
-36
lines changed

6 files changed

+68
-36
lines changed

‎apps/E2E/src/Menu/specs/Menu.spec.win.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,15 +160,15 @@ describe('Menu Functional Testing', () => {
160160
.toBeFalsy();
161161
});
162162

163-
it('Press "Tab" to navigate between MenuItems. Validate that focus switches correctly between MenuItems.', async () => {
163+
it('Press "Tab" to navigate between MenuGroups. Validate that focus switches correctly between MenuGroups.', async () => {
164164
await MenuPageObject.openMenu();
165165

166-
await MenuPageObject.sendKeys(MenuPageObject.getMenuItem('Third'), [Keys.TAB]);
166+
await MenuPageObject.sendKeys(MenuPageObject.getMenuItem('First'), [Keys.TAB]);
167167
expect(
168-
await MenuPageObject.compareAttribute(MenuPageObject.getMenuItem('Fourth'), Attribute.IsFocused, AttributeValue.true),
168+
await MenuPageObject.compareAttribute(MenuPageObject.getMenuItem('Third'), Attribute.IsFocused, AttributeValue.true),
169169
).toBeTruthy();
170170

171-
await MenuPageObject.sendKeys(MenuPageObject.getMenuItem('Fourth'), [Keys.TAB]);
171+
await MenuPageObject.sendKeys(MenuPageObject.getMenuItem('Third'), [Keys.TAB]);
172172
expect(
173173
await MenuPageObject.compareAttribute(MenuPageObject.getMenuItem('First'), Attribute.IsFocused, AttributeValue.true),
174174
).toBeTruthy();

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

Lines changed: 36 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as React from 'react';
22
import { View } from 'react-native';
33

44
import { ButtonV1 as Button } from '@fluentui-react-native/button';
5-
import { Menu, MenuItem, MenuList, MenuPopover, MenuTrigger } from '@fluentui-react-native/menu';
5+
import { Menu, MenuDivider, MenuGroup, MenuItem, MenuList, MenuPopover, MenuTrigger } from '@fluentui-react-native/menu';
66
import { Stack } from '@fluentui-react-native/stack';
77
import { TextV1 as Text } from '@fluentui-react-native/text';
88

@@ -76,36 +76,41 @@ export const E2EMenuTest: React.FunctionComponent = () => {
7676
{...testProps(MENUPOPOVER_TEST_COMPONENT)}
7777
>
7878
<MenuList>
79-
<MenuItem
80-
onClick={onItemClick}
81-
accessibilityLabel={MENUITEM_ACCESSIBILITY_LABEL}
82-
persistOnClick
83-
/* For Android E2E testing purposes, testProps must be passed in after accessibilityLabel. */
84-
{...testProps(MENUITEM_TEST_COMPONENT)}
85-
>
86-
A plain MenuItem
87-
</MenuItem>
88-
<MenuItem
89-
onClick={onItemClick}
90-
/* For Android E2E testing purposes, testProps must be passed in after accessibilityLabel. */
91-
{...testProps(MENUITEM_DISABLED_COMPONENT)}
92-
disabled
93-
persistOnClick
94-
>
95-
A second disabled plain MenuItem
96-
</MenuItem>
97-
<MenuItem
98-
/* For Android E2E testing purposes, testProps must be passed in after accessibilityLabel. */
99-
{...testProps(MENUITEM_NO_A11Y_LABEL_COMPONENT)}
100-
>
101-
{MENUITEM_TEST_LABEL}
102-
</MenuItem>
103-
<MenuItem
104-
/* For Android E2E testing purposes, testProps must be passed in after accessibilityLabel. */
105-
{...testProps(MENUITEM_FOURTH_COMPONENT)}
106-
>
107-
A fourth plain MenuItem
108-
</MenuItem>
79+
<MenuGroup>
80+
<MenuItem
81+
onClick={onItemClick}
82+
accessibilityLabel={MENUITEM_ACCESSIBILITY_LABEL}
83+
persistOnClick
84+
/* For Android E2E testing purposes, testProps must be passed in after accessibilityLabel. */
85+
{...testProps(MENUITEM_TEST_COMPONENT)}
86+
>
87+
A plain MenuItem
88+
</MenuItem>
89+
<MenuItem
90+
onClick={onItemClick}
91+
/* For Android E2E testing purposes, testProps must be passed in after accessibilityLabel. */
92+
{...testProps(MENUITEM_DISABLED_COMPONENT)}
93+
disabled
94+
persistOnClick
95+
>
96+
A second disabled plain MenuItem
97+
</MenuItem>
98+
</MenuGroup>
99+
<MenuDivider />
100+
<MenuGroup>
101+
<MenuItem
102+
/* For Android E2E testing purposes, testProps must be passed in after accessibilityLabel. */
103+
{...testProps(MENUITEM_NO_A11Y_LABEL_COMPONENT)}
104+
>
105+
{MENUITEM_TEST_LABEL}
106+
</MenuItem>
107+
<MenuItem
108+
/* For Android E2E testing purposes, testProps must be passed in after accessibilityLabel. */
109+
{...testProps(MENUITEM_FOURTH_COMPONENT)}
110+
>
111+
A fourth plain MenuItem
112+
</MenuItem>
113+
</MenuGroup>
109114
</MenuList>
110115
</MenuPopover>
111116
</Menu>

‎change/@fluentui-react-native-e2e-testing-32828e64-4b64-492f-81be-792e3251f3dc.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "update E2E test for win32",
4+
"packageName": "@fluentui-react-native/e2e-testing",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎change/@fluentui-react-native-menu-b17320f8-20c5-4a54-970d-631cdc6f9288.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Fix Menu tab navigation on win32",
4+
"packageName": "@fluentui-react-native/menu",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎change/@fluentui-react-native-tester-98ce1cf5-f9a2-4543-8967-482d23186b0e.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "update E2E test for win32",
4+
"packageName": "@fluentui-react-native/tester",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ MenuStack.displayName = 'MenuStack';
3737
const shouldHaveFocusZone = ['macos', 'win32'].includes(Platform.OS as string);
3838
const focusLandsOnContainer = Platform.OS === 'macos';
3939
const hasCircularNavigation = Platform.OS === ('win32' as any);
40-
const hasTabNavigation = Platform.OS === ('win32' as any);
4140

4241
export const menuListLookup = (layer: string, state: MenuListState, userProps: MenuListProps): boolean => {
4342
return state[layer] || userProps[layer] || layer === 'hasMaxHeight';
@@ -84,6 +83,13 @@ export const MenuList = compose<MenuListType>({
8483
const shouldHaveScrollView = Platform.OS === 'macos' || menuList.hasMaxHeight || menuList.hasMaxWidth;
8584
const ScrollViewWrapper = shouldHaveScrollView ? Slots.scrollView : React.Fragment;
8685

86+
const hasMenuGroupChild = React.Children.toArray(children).some(
87+
(child) => child && (child as any).type && (child as any).type.displayName === 'MenuGroup',
88+
);
89+
90+
// On win32, tab navigation should only be enabled when we have menu groups.
91+
const hasTabNavigation = Platform.OS === ('win32' as any) && hasMenuGroupChild;
92+
8793
const content = (
8894
<Slots.root>
8995
<ScrollViewWrapper

0 commit comments

Comments
 (0)