Skip to content

Commit

Permalink
Rename MenuDialogTrigger (#4700)
Browse files Browse the repository at this point in the history
  • Loading branch information
@snowystinger
snowystinger committed Jun 23, 2023
1 parent fe63070 commit bf8fac4
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 34 deletions.
20 changes: 10 additions & 10 deletions packages/@react-aria/menu/src/useMenuItem.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re
} = props;
let {direction} = useLocale();

let isMenuDialogTrigger = state.collection.getItem(key).hasChildNodes;
let isTrigger = !!hasPopup;
let isOpen = state.expandedKeys.has(key);

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

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

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

let {pressProps, isPressed} = usePress({onPressStart, onPressUp, isDisabled: isDisabled || (isMenuDialogTrigger && state.expandedKeys.has(key))});
let {pressProps, isPressed} = usePress({onPressStart, onPressUp, isDisabled: isDisabled || (isTrigger && state.expandedKeys.has(key))});
let {hoverProps} = useHover({
isDisabled,
onHoverStart() {
if (!isFocusVisible() && !(isMenuDialogTrigger && state.expandedKeys.has(key))) {
if (!isFocusVisible() && !(isTrigger && state.expandedKeys.has(key))) {
state.selectionManager.setFocused(true);
state.selectionManager.setFocusedKey(key);
// focus immediately so that a focus scope opened on hover has the correct restore node
Expand All @@ -211,7 +211,7 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re
}
},
onHoverChange: isHovered => {
if (isHovered && isMenuDialogTrigger && !state.expandedKeys.has(key)) {
if (isHovered && isTrigger && !state.expandedKeys.has(key)) {
if (!openTimeout.current) {
openTimeout.current = setTimeout(() => {
onSubmenuOpen();
Expand All @@ -234,25 +234,25 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re

switch (e.key) {
case ' ':
if (!isDisabled && state.selectionManager.selectionMode === 'none' && !isMenuDialogTrigger && closeOnSelect !== false && onClose) {
if (!isDisabled && state.selectionManager.selectionMode === 'none' && !isTrigger && closeOnSelect !== false && onClose) {
onClose();
}
break;
case 'Enter':
// The Enter key should always close on select, except if overridden.
if (!isDisabled && closeOnSelect !== false && !isMenuDialogTrigger && onClose) {
if (!isDisabled && closeOnSelect !== false && !isTrigger && onClose) {
onClose();
}
break;
case 'ArrowRight':
if (isMenuDialogTrigger && direction === 'ltr') {
if (isTrigger && direction === 'ltr') {
onSubmenuOpen();
} else {
e.continuePropagation();
}
break;
case 'ArrowLeft':
if (isMenuDialogTrigger && direction === 'rtl') {
if (isTrigger && direction === 'rtl') {
onSubmenuOpen();
} else {
e.continuePropagation();
Expand Down
12 changes: 6 additions & 6 deletions ...t-spectrum/menu/src/MenuDialogTrigger.tsx → ...ectrum/menu/src/ContextualHelpTrigger.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export interface SpectrumMenuDialogTriggerProps<T> extends ItemProps<T> {
targetKey: Key
}

function MenuDialogTrigger<T>(props: SpectrumMenuDialogTriggerProps<T>): ReactElement {
function ContextualHelpTrigger<T>(props: SpectrumMenuDialogTriggerProps<T>): ReactElement {
let {isUnavailable} = props;

let triggerRef = useRef<HTMLLIElement>(null);
Expand Down Expand Up @@ -99,20 +99,20 @@ function MenuDialogTrigger<T>(props: SpectrumMenuDialogTriggerProps<T>): ReactEl
);
}

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

yield {
element: React.cloneElement(trigger, {...trigger.props, hasChildItems: true}),
wrapper: (element) => (
<MenuDialogTrigger key={element.key} targetKey={element.key} {...props}>
<ContextualHelpTrigger key={element.key} targetKey={element.key} {...props}>
{element}
{content}
</MenuDialogTrigger>
</ContextualHelpTrigger>
)
};
};

let _Item = MenuDialogTrigger as <T>(props: ItemProps<T> & {isUnavailable?: boolean}) => JSX.Element;
export {_Item as MenuDialogTrigger};
let _Item = ContextualHelpTrigger as <T>(props: ItemProps<T> & {isUnavailable?: boolean}) => JSX.Element;
export {_Item as ContextualHelpTrigger};
2 changes: 1 addition & 1 deletion packages/@react-spectrum/menu/src/MenuItem.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export function MenuItem<T>(props: MenuItemProps<T>) {
closeOnSelect,
isVirtualized,
onAction,
'aria-haspopup': isMenuDialogTrigger ? 'dialog' : undefined
'aria-haspopup': isMenuDialogTrigger && isUnavailable ? 'dialog' : undefined
},
state,
ref
Expand Down
4 changes: 2 additions & 2 deletions packages/@react-spectrum/menu/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
export {MenuTrigger} from './MenuTrigger';
export {Menu} from './Menu';
export {ActionMenu} from './ActionMenu';
export {MenuDialogTrigger} from './MenuDialogTrigger';
export {ContextualHelpTrigger} from './ContextualHelpTrigger';
export {Item, Section} from '@react-stately/collections';
export type {SpectrumActionMenuProps, SpectrumMenuProps, SpectrumMenuTriggerProps} from '@react-types/menu';
export type {SpectrumMenuDialogTriggerProps} from './MenuDialogTrigger';
export type {SpectrumMenuDialogTriggerProps} from './ContextualHelpTrigger';
69 changes: 60 additions & 9 deletions packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,15 @@ import AlignRight from '@spectrum-icons/workflow/AlignRight';
import Blower from '@spectrum-icons/workflow/Blower';
import Book from '@spectrum-icons/workflow/Book';
import {Content, Footer} from '@react-spectrum/view';
import {ContextualHelpTrigger, Item, Menu, MenuTrigger, Section} from '../';
import Copy from '@spectrum-icons/workflow/Copy';
import Cut from '@spectrum-icons/workflow/Cut';
import {Dialog} from '@react-spectrum/dialog';
import {Heading, Keyboard, Text} from '@react-spectrum/text';
import {Item, Menu, MenuDialogTrigger, MenuTrigger, Section} from '../';
import {Link} from '@react-spectrum/link';
import Paste from '@spectrum-icons/workflow/Paste';
import React, {useState} from 'react';
import {ToggleButton} from '@adobe/react-spectrum';
import {TranslateMenu} from './../chromatic/MenuTriggerLanguages.chromatic';

let iconMap = {
Expand Down Expand Up @@ -728,22 +729,22 @@ export let MenuItemUnavailable = {
render: () => render(
<Menu onAction={action('onAction')}>
<Item key="1">One</Item>
<MenuDialogTrigger isUnavailable>
<ContextualHelpTrigger isUnavailable>
<Item key="foo">Two</Item>
<Dialog>
<Heading>hello</Heading>
<Content>Is it me you're looking for?</Content>
</Dialog>
</MenuDialogTrigger>
<MenuDialogTrigger isUnavailable>
</ContextualHelpTrigger>
<ContextualHelpTrigger isUnavailable>
<Item key="baz">Two point five</Item>
<Dialog>
<Heading>hello</Heading>
<Content>Is it me you're looking for?</Content>
</Dialog>
</MenuDialogTrigger>
</ContextualHelpTrigger>
<Item key="3">Three</Item>
<MenuDialogTrigger isUnavailable>
<ContextualHelpTrigger isUnavailable>
<Item key="bar">
<Text>Four</Text>
<Text slot={'description'}>Shut the door</Text>
Expand All @@ -753,7 +754,7 @@ export let MenuItemUnavailable = {
<Content>Is it me you're looking for?</Content>
<Footer><Link>Learn more</Link></Footer>
</Dialog>
</MenuDialogTrigger>
</ContextualHelpTrigger>
<Item key="5">Five</Item>
</Menu>
)
Expand All @@ -765,17 +766,67 @@ export let MenuItemUnavailableDynamic = {
{(item) => {
if (item.name === 'Kangaroo') {
return (
<MenuDialogTrigger isUnavailable>
<ContextualHelpTrigger isUnavailable>
<Item key={item.name}>{item.name}</Item>
<Dialog>
<Heading>hello</Heading>
<Content>Is it me you're looking for?</Content>
</Dialog>
</MenuDialogTrigger>
</ContextualHelpTrigger>
);
}
return <Item key={item.name}>{item.name}</Item>;
}}
</Menu>
)
};

export let MenuItemUnavailableToggling = {
render: () => <MenuWithUnavailableSometimes />
};

function MenuWithUnavailableSometimes(props) {
let [isUnavailable, setIsUnavailable] = useState(false);
return (
<>
<ToggleButton isSelected={isUnavailable} onChange={setIsUnavailable}>Toggle item 2</ToggleButton>
<div style={{display: 'flex', width: 'auto', margin: '250px 0'}}>
<MenuTrigger onOpenChange={action('onOpenChange')} {...props}>
<ActionButton>
Menu Button
</ActionButton>
<Menu onAction={action('onAction')}>
<Item key="1">One</Item>
<ContextualHelpTrigger isUnavailable={isUnavailable}>
<Item key="foo">Two</Item>
<Dialog>
<Heading>hello</Heading>
<Content>Is it me you're looking for?</Content>
</Dialog>
</ContextualHelpTrigger>
<ContextualHelpTrigger isUnavailable>
<Item key="baz">Two point five</Item>
<Dialog>
<Heading>hello</Heading>
<Content>Is it me you're looking for?</Content>
</Dialog>
</ContextualHelpTrigger>
<Item key="3">Three</Item>
<ContextualHelpTrigger isUnavailable>
<Item key="bar">
<Text>Four</Text>
<Text slot={'description'}>Shut the door</Text>
</Item>
<Dialog>
<Heading>hello</Heading>
<Content>Is it me you're looking for?</Content>
<Footer><Link>Learn more</Link></Footer>
</Dialog>
</ContextualHelpTrigger>
<Item key="5">Five</Item>
</Menu>
</MenuTrigger>
</div>
</>
);
}
31 changes: 25 additions & 6 deletions packages/@react-spectrum/menu/test/MenuTrigger.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ import {act, fireEvent, render, screen, within} from '@testing-library/react';
import {action} from '@storybook/addon-actions';
import {ActionButton, Button} from '@react-spectrum/button';
import {Content, Footer} from '@react-spectrum/view';
import {ContextualHelpTrigger, Item, Menu, MenuTrigger, Section} from '../';
import {DEFAULT_LONG_PRESS_TIME, installPointerEvent, triggerLongPress, triggerPress, triggerTouch} from '@react-spectrum/test-utils';
import {Dialog} from '@react-spectrum/dialog';
import {Heading, Text} from '@react-spectrum/text';
import {Item, Menu, MenuDialogTrigger, MenuTrigger, Section} from '../';
import {Link} from '@react-spectrum/link';
import {Provider} from '@react-spectrum/provider';
import React from 'react';
Expand Down Expand Up @@ -982,15 +982,15 @@ describe('MenuTrigger', function () {

describe('unavailable item', function () {
let renderTree = (options = {}) => {
let {providerProps = {}} = options;
let {providerProps = {}, isItem2Unavailable = true} = options;
let {locale = 'en-US'} = providerProps;
tree = render(
<Provider theme={theme} locale={locale}>
<MenuTrigger>
<ActionButton>Menu</ActionButton>
<Menu onAction={action('onAction')}>
<Item key="1">One</Item>
<MenuDialogTrigger isUnavailable>
<ContextualHelpTrigger isUnavailable={isItem2Unavailable}>
<Item key="foo" textValue="Hello">
<Text>Hello</Text>
<Text slot="description">Is it me you're looking for?</Text>
Expand All @@ -999,17 +999,17 @@ describe('MenuTrigger', function () {
<Heading>Lionel Richie says:</Heading>
<Content>I can see it in your eyes</Content>
</Dialog>
</MenuDialogTrigger>
</ContextualHelpTrigger>
<Item key="3">Three</Item>
<Item key="5">Five</Item>
<MenuDialogTrigger isUnavailable>
<ContextualHelpTrigger isUnavailable>
<Item key="bar" textValue="Choose a college major">Choose a College Major</Item>
<Dialog>
<Heading>Choosing a College Major</Heading>
<Content>What factors should I consider when choosing a college major?</Content>
<Footer>Visit this link before choosing this action. <Link>Learn more</Link></Footer>
</Dialog>
</MenuDialogTrigger>
</ContextualHelpTrigger>
</Menu>
</MenuTrigger>
</Provider>
Expand Down Expand Up @@ -1054,6 +1054,25 @@ describe('MenuTrigger', function () {
expect(document.activeElement).toBe(menuItems[2]);
});

it('can not open a sub dialog with hover if isUnavailable is false', function () {
renderTree({isItem2Unavailable: false});
let menu = openMenu();
let menuItems = within(menu).getAllByRole('menuitem');
let availableItem = menuItems[1];
expect(availableItem).toBeVisible();
expect(within(availableItem).queryByRole('img', {hidden: true})).toBeNull();
expect(availableItem).not.toHaveAttribute('aria-haspopup', 'dialog');

fireEvent.mouseEnter(availableItem);
act(() => {jest.runAllTimers();});
expect(tree.queryByRole('dialog')).toBeNull();

expect(document.activeElement).toBe(availableItem);
fireEvent.keyDown(document.activeElement, {key: 'ArrowRight'});
fireEvent.keyUp(document.activeElement, {key: 'ArrowRight'});
expect(tree.queryByRole('dialog')).toBeNull();
});

it('can open a sub dialog with keyboard', function () {
renderTree();
let menu = openMenu();
Expand Down

1 comment on commit bf8fac4

@rspbot
Copy link

@rspbot rspbot commented on bf8fac4 Jun 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdaccio builds:
CRA Test App
NextJS Test App
RAC Tailwind Example
CRA Test App Size
NextJS App Size
Publish stats
Size diff since last release
Docs

Please sign in to comment.