Skip to content

Commit bf8fac4

Browse files
snowystingersnowystinger
authored
Rename MenuDialogTrigger (#4700)
1 parent fe63070 commit bf8fac4

File tree

6 files changed

+104
-34
lines changed

6 files changed

+104
-34
lines changed

‎packages/@react-aria/menu/src/useMenuItem.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re
103103
} = props;
104104
let {direction} = useLocale();
105105

106-
let isMenuDialogTrigger = state.collection.getItem(key).hasChildNodes;
106+
let isTrigger = !!hasPopup;
107107
let isOpen = state.expandedKeys.has(key);
108108

109109
let isDisabled = props.isDisabled ?? state.disabledKeys.has(key);
@@ -133,7 +133,7 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re
133133
// will need to disable this lint rule when using useEffectEvent https://react.dev/learn/separating-events-from-effects#logic-inside-effects-is-reactive
134134
// eslint-disable-next-line react-hooks/exhaustive-deps
135135
}, []);
136-
let onAction = isMenuDialogTrigger ? onActionMenuDialogTrigger : props.onAction || data.onAction;
136+
let onAction = isTrigger ? onActionMenuDialogTrigger : props.onAction || data.onAction;
137137

138138
let role = 'menuitem';
139139
if (state.selectionManager.selectionMode === 'single') {
@@ -182,7 +182,7 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re
182182

183183
// Pressing a menu item should close by default in single selection mode but not multiple
184184
// selection mode, except if overridden by the closeOnSelect prop.
185-
if (!isMenuDialogTrigger && onClose && (closeOnSelect ?? state.selectionManager.selectionMode !== 'multiple')) {
185+
if (!isTrigger && onClose && (closeOnSelect ?? state.selectionManager.selectionMode !== 'multiple')) {
186186
onClose();
187187
}
188188
}
@@ -196,11 +196,11 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re
196196
allowsDifferentPressOrigin: true
197197
});
198198

199-
let {pressProps, isPressed} = usePress({onPressStart, onPressUp, isDisabled: isDisabled || (isMenuDialogTrigger && state.expandedKeys.has(key))});
199+
let {pressProps, isPressed} = usePress({onPressStart, onPressUp, isDisabled: isDisabled || (isTrigger && state.expandedKeys.has(key))});
200200
let {hoverProps} = useHover({
201201
isDisabled,
202202
onHoverStart() {
203-
if (!isFocusVisible() && !(isMenuDialogTrigger && state.expandedKeys.has(key))) {
203+
if (!isFocusVisible() && !(isTrigger && state.expandedKeys.has(key))) {
204204
state.selectionManager.setFocused(true);
205205
state.selectionManager.setFocusedKey(key);
206206
// focus immediately so that a focus scope opened on hover has the correct restore node
@@ -211,7 +211,7 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re
211211
}
212212
},
213213
onHoverChange: isHovered => {
214-
if (isHovered && isMenuDialogTrigger && !state.expandedKeys.has(key)) {
214+
if (isHovered && isTrigger && !state.expandedKeys.has(key)) {
215215
if (!openTimeout.current) {
216216
openTimeout.current = setTimeout(() => {
217217
onSubmenuOpen();
@@ -234,25 +234,25 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re
234234

235235
switch (e.key) {
236236
case ' ':
237-
if (!isDisabled && state.selectionManager.selectionMode === 'none' && !isMenuDialogTrigger && closeOnSelect !== false && onClose) {
237+
if (!isDisabled && state.selectionManager.selectionMode === 'none' && !isTrigger && closeOnSelect !== false && onClose) {
238238
onClose();
239239
}
240240
break;
241241
case 'Enter':
242242
// The Enter key should always close on select, except if overridden.
243-
if (!isDisabled && closeOnSelect !== false && !isMenuDialogTrigger && onClose) {
243+
if (!isDisabled && closeOnSelect !== false && !isTrigger && onClose) {
244244
onClose();
245245
}
246246
break;
247247
case 'ArrowRight':
248-
if (isMenuDialogTrigger && direction === 'ltr') {
248+
if (isTrigger && direction === 'ltr') {
249249
onSubmenuOpen();
250250
} else {
251251
e.continuePropagation();
252252
}
253253
break;
254254
case 'ArrowLeft':
255-
if (isMenuDialogTrigger && direction === 'rtl') {
255+
if (isTrigger && direction === 'rtl') {
256256
onSubmenuOpen();
257257
} else {
258258
e.continuePropagation();

‎packages/@react-spectrum/menu/src/MenuDialogTrigger.tsx renamed to ‎packages/@react-spectrum/menu/src/ContextualHelpTrigger.tsx

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export interface SpectrumMenuDialogTriggerProps<T> extends ItemProps<T> {
2424
targetKey: Key
2525
}
2626

27-
function MenuDialogTrigger<T>(props: SpectrumMenuDialogTriggerProps<T>): ReactElement {
27+
function ContextualHelpTrigger<T>(props: SpectrumMenuDialogTriggerProps<T>): ReactElement {
2828
let {isUnavailable} = props;
2929

3030
let triggerRef = useRef<HTMLLIElement>(null);
@@ -99,20 +99,20 @@ function MenuDialogTrigger<T>(props: SpectrumMenuDialogTriggerProps<T>): ReactEl
9999
);
100100
}
101101

102-
MenuDialogTrigger.getCollectionNode = function* getCollectionNode<T>(props: ItemProps<T>) {
102+
ContextualHelpTrigger.getCollectionNode = function* getCollectionNode<T>(props: ItemProps<T>) {
103103
let [trigger] = React.Children.toArray(props.children) as ReactElement[];
104104
let [, content] = props.children as [ReactElement, ReactElement];
105105

106106
yield {
107107
element: React.cloneElement(trigger, {...trigger.props, hasChildItems: true}),
108108
wrapper: (element) => (
109-
<MenuDialogTrigger key={element.key} targetKey={element.key} {...props}>
109+
<ContextualHelpTrigger key={element.key} targetKey={element.key} {...props}>
110110
{element}
111111
{content}
112-
</MenuDialogTrigger>
112+
</ContextualHelpTrigger>
113113
)
114114
};
115115
};
116116

117-
let _Item = MenuDialogTrigger as <T>(props: ItemProps<T> & {isUnavailable?: boolean}) => JSX.Element;
118-
export {_Item as MenuDialogTrigger};
117+
let _Item = ContextualHelpTrigger as <T>(props: ItemProps<T> & {isUnavailable?: boolean}) => JSX.Element;
118+
export {_Item as ContextualHelpTrigger};

‎packages/@react-spectrum/menu/src/MenuItem.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export function MenuItem<T>(props: MenuItemProps<T>) {
8585
closeOnSelect,
8686
isVirtualized,
8787
onAction,
88-
'aria-haspopup': isMenuDialogTrigger ? 'dialog' : undefined
88+
'aria-haspopup': isMenuDialogTrigger && isUnavailable ? 'dialog' : undefined
8989
},
9090
state,
9191
ref

‎packages/@react-spectrum/menu/src/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
export {MenuTrigger} from './MenuTrigger';
1616
export {Menu} from './Menu';
1717
export {ActionMenu} from './ActionMenu';
18-
export {MenuDialogTrigger} from './MenuDialogTrigger';
18+
export {ContextualHelpTrigger} from './ContextualHelpTrigger';
1919
export {Item, Section} from '@react-stately/collections';
2020
export type {SpectrumActionMenuProps, SpectrumMenuProps, SpectrumMenuTriggerProps} from '@react-types/menu';
21-
export type {SpectrumMenuDialogTriggerProps} from './MenuDialogTrigger';
21+
export type {SpectrumMenuDialogTriggerProps} from './ContextualHelpTrigger';

‎packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,15 @@ import AlignRight from '@spectrum-icons/workflow/AlignRight';
1818
import Blower from '@spectrum-icons/workflow/Blower';
1919
import Book from '@spectrum-icons/workflow/Book';
2020
import {Content, Footer} from '@react-spectrum/view';
21+
import {ContextualHelpTrigger, Item, Menu, MenuTrigger, Section} from '../';
2122
import Copy from '@spectrum-icons/workflow/Copy';
2223
import Cut from '@spectrum-icons/workflow/Cut';
2324
import {Dialog} from '@react-spectrum/dialog';
2425
import {Heading, Keyboard, Text} from '@react-spectrum/text';
25-
import {Item, Menu, MenuDialogTrigger, MenuTrigger, Section} from '../';
2626
import {Link} from '@react-spectrum/link';
2727
import Paste from '@spectrum-icons/workflow/Paste';
2828
import React, {useState} from 'react';
29+
import {ToggleButton} from '@adobe/react-spectrum';
2930
import {TranslateMenu} from './../chromatic/MenuTriggerLanguages.chromatic';
3031

3132
let iconMap = {
@@ -728,22 +729,22 @@ export let MenuItemUnavailable = {
728729
render: () => render(
729730
<Menu onAction={action('onAction')}>
730731
<Item key="1">One</Item>
731-
<MenuDialogTrigger isUnavailable>
732+
<ContextualHelpTrigger isUnavailable>
732733
<Item key="foo">Two</Item>
733734
<Dialog>
734735
<Heading>hello</Heading>
735736
<Content>Is it me you're looking for?</Content>
736737
</Dialog>
737-
</MenuDialogTrigger>
738-
<MenuDialogTrigger isUnavailable>
738+
</ContextualHelpTrigger>
739+
<ContextualHelpTrigger isUnavailable>
739740
<Item key="baz">Two point five</Item>
740741
<Dialog>
741742
<Heading>hello</Heading>
742743
<Content>Is it me you're looking for?</Content>
743744
</Dialog>
744-
</MenuDialogTrigger>
745+
</ContextualHelpTrigger>
745746
<Item key="3">Three</Item>
746-
<MenuDialogTrigger isUnavailable>
747+
<ContextualHelpTrigger isUnavailable>
747748
<Item key="bar">
748749
<Text>Four</Text>
749750
<Text slot={'description'}>Shut the door</Text>
@@ -753,7 +754,7 @@ export let MenuItemUnavailable = {
753754
<Content>Is it me you're looking for?</Content>
754755
<Footer><Link>Learn more</Link></Footer>
755756
</Dialog>
756-
</MenuDialogTrigger>
757+
</ContextualHelpTrigger>
757758
<Item key="5">Five</Item>
758759
</Menu>
759760
)
@@ -765,17 +766,67 @@ export let MenuItemUnavailableDynamic = {
765766
{(item) => {
766767
if (item.name === 'Kangaroo') {
767768
return (
768-
<MenuDialogTrigger isUnavailable>
769+
<ContextualHelpTrigger isUnavailable>
769770
<Item key={item.name}>{item.name}</Item>
770771
<Dialog>
771772
<Heading>hello</Heading>
772773
<Content>Is it me you're looking for?</Content>
773774
</Dialog>
774-
</MenuDialogTrigger>
775+
</ContextualHelpTrigger>
775776
);
776777
}
777778
return <Item key={item.name}>{item.name}</Item>;
778779
}}
779780
</Menu>
780781
)
781782
};
783+
784+
export let MenuItemUnavailableToggling = {
785+
render: () => <MenuWithUnavailableSometimes />
786+
};
787+
788+
function MenuWithUnavailableSometimes(props) {
789+
let [isUnavailable, setIsUnavailable] = useState(false);
790+
return (
791+
<>
792+
<ToggleButton isSelected={isUnavailable} onChange={setIsUnavailable}>Toggle item 2</ToggleButton>
793+
<div style={{display: 'flex', width: 'auto', margin: '250px 0'}}>
794+
<MenuTrigger onOpenChange={action('onOpenChange')} {...props}>
795+
<ActionButton>
796+
Menu Button
797+
</ActionButton>
798+
<Menu onAction={action('onAction')}>
799+
<Item key="1">One</Item>
800+
<ContextualHelpTrigger isUnavailable={isUnavailable}>
801+
<Item key="foo">Two</Item>
802+
<Dialog>
803+
<Heading>hello</Heading>
804+
<Content>Is it me you're looking for?</Content>
805+
</Dialog>
806+
</ContextualHelpTrigger>
807+
<ContextualHelpTrigger isUnavailable>
808+
<Item key="baz">Two point five</Item>
809+
<Dialog>
810+
<Heading>hello</Heading>
811+
<Content>Is it me you're looking for?</Content>
812+
</Dialog>
813+
</ContextualHelpTrigger>
814+
<Item key="3">Three</Item>
815+
<ContextualHelpTrigger isUnavailable>
816+
<Item key="bar">
817+
<Text>Four</Text>
818+
<Text slot={'description'}>Shut the door</Text>
819+
</Item>
820+
<Dialog>
821+
<Heading>hello</Heading>
822+
<Content>Is it me you're looking for?</Content>
823+
<Footer><Link>Learn more</Link></Footer>
824+
</Dialog>
825+
</ContextualHelpTrigger>
826+
<Item key="5">Five</Item>
827+
</Menu>
828+
</MenuTrigger>
829+
</div>
830+
</>
831+
);
832+
}

‎packages/@react-spectrum/menu/test/MenuTrigger.test.js

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ import {act, fireEvent, render, screen, within} from '@testing-library/react';
1414
import {action} from '@storybook/addon-actions';
1515
import {ActionButton, Button} from '@react-spectrum/button';
1616
import {Content, Footer} from '@react-spectrum/view';
17+
import {ContextualHelpTrigger, Item, Menu, MenuTrigger, Section} from '../';
1718
import {DEFAULT_LONG_PRESS_TIME, installPointerEvent, triggerLongPress, triggerPress, triggerTouch} from '@react-spectrum/test-utils';
1819
import {Dialog} from '@react-spectrum/dialog';
1920
import {Heading, Text} from '@react-spectrum/text';
20-
import {Item, Menu, MenuDialogTrigger, MenuTrigger, Section} from '../';
2121
import {Link} from '@react-spectrum/link';
2222
import {Provider} from '@react-spectrum/provider';
2323
import React from 'react';
@@ -982,15 +982,15 @@ describe('MenuTrigger', function () {
982982

983983
describe('unavailable item', function () {
984984
let renderTree = (options = {}) => {
985-
let {providerProps = {}} = options;
985+
let {providerProps = {}, isItem2Unavailable = true} = options;
986986
let {locale = 'en-US'} = providerProps;
987987
tree = render(
988988
<Provider theme={theme} locale={locale}>
989989
<MenuTrigger>
990990
<ActionButton>Menu</ActionButton>
991991
<Menu onAction={action('onAction')}>
992992
<Item key="1">One</Item>
993-
<MenuDialogTrigger isUnavailable>
993+
<ContextualHelpTrigger isUnavailable={isItem2Unavailable}>
994994
<Item key="foo" textValue="Hello">
995995
<Text>Hello</Text>
996996
<Text slot="description">Is it me you're looking for?</Text>
@@ -999,17 +999,17 @@ describe('MenuTrigger', function () {
999999
<Heading>Lionel Richie says:</Heading>
10001000
<Content>I can see it in your eyes</Content>
10011001
</Dialog>
1002-
</MenuDialogTrigger>
1002+
</ContextualHelpTrigger>
10031003
<Item key="3">Three</Item>
10041004
<Item key="5">Five</Item>
1005-
<MenuDialogTrigger isUnavailable>
1005+
<ContextualHelpTrigger isUnavailable>
10061006
<Item key="bar" textValue="Choose a college major">Choose a College Major</Item>
10071007
<Dialog>
10081008
<Heading>Choosing a College Major</Heading>
10091009
<Content>What factors should I consider when choosing a college major?</Content>
10101010
<Footer>Visit this link before choosing this action. <Link>Learn more</Link></Footer>
10111011
</Dialog>
1012-
</MenuDialogTrigger>
1012+
</ContextualHelpTrigger>
10131013
</Menu>
10141014
</MenuTrigger>
10151015
</Provider>
@@ -1054,6 +1054,25 @@ describe('MenuTrigger', function () {
10541054
expect(document.activeElement).toBe(menuItems[2]);
10551055
});
10561056

1057+
it('can not open a sub dialog with hover if isUnavailable is false', function () {
1058+
renderTree({isItem2Unavailable: false});
1059+
let menu = openMenu();
1060+
let menuItems = within(menu).getAllByRole('menuitem');
1061+
let availableItem = menuItems[1];
1062+
expect(availableItem).toBeVisible();
1063+
expect(within(availableItem).queryByRole('img', {hidden: true})).toBeNull();
1064+
expect(availableItem).not.toHaveAttribute('aria-haspopup', 'dialog');
1065+
1066+
fireEvent.mouseEnter(availableItem);
1067+
act(() => {jest.runAllTimers();});
1068+
expect(tree.queryByRole('dialog')).toBeNull();
1069+
1070+
expect(document.activeElement).toBe(availableItem);
1071+
fireEvent.keyDown(document.activeElement, {key: 'ArrowRight'});
1072+
fireEvent.keyUp(document.activeElement, {key: 'ArrowRight'});
1073+
expect(tree.queryByRole('dialog')).toBeNull();
1074+
});
1075+
10571076
it('can open a sub dialog with keyboard', function () {
10581077
renderTree();
10591078
let menu = openMenu();

0 commit comments

Comments
 (0)