Skip to content

fix(Menu): update drilldown transition event handler #10839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tlabaj merged 4 commits into from
Aug 12, 2024
Merged

fix(Menu): update drilldown transition event handler #10839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a63a1d9
fix(Menu): update drilldown transition event handler
kmcfaul Aug 6, 2024
2a47488
fix part1
kmcfaul Aug 6, 2024
2e79eeb
update first item focus logic
kmcfaul Aug 8, 2024
6180bf5
add tick to some test to wait for menus
kmcfaul Aug 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions packages/react-core/src/components/Dropdown/Dropdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,22 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
? localToggleRef
: (toggle?.toggleRef as React.RefObject<HTMLButtonElement>);

const prevIsOpen = React.useRef<boolean>(isOpen);
React.useEffect(() => {
// menu was opened, focus on first menu item
if (prevIsOpen.current === false && isOpen === true && shouldFocusFirstItemOnOpen) {
setTimeout(() => {
const firstElement = menuRef?.current?.querySelector(
'li button:not(:disabled),li input:not(:disabled),li a:not([aria-disabled="true"])'
);
firstElement && (firstElement as HTMLElement).focus();
}, 0);
}

prevIsOpen.current = isOpen;
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isOpen]);

React.useEffect(() => {
const handleMenuKeys = (event: KeyboardEvent) => {
// Close the menu on tab or escape if onOpenChange is provided
Expand All @@ -120,16 +136,6 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
};

const handleClick = (event: MouseEvent) => {
// toggle was opened, focus on first menu item
if (isOpen && shouldFocusFirstItemOnOpen && toggleRef.current?.contains(event.target as Node)) {
setTimeout(() => {
const firstElement = menuRef?.current?.querySelector(
'li button:not(:disabled),li input:not(:disabled),li a:not([aria-disabled="true"])'
);
firstElement && (firstElement as HTMLElement).focus();
}, 0);
}

// If the event is not on the toggle and onOpenChange callback is provided, close the menu
if (isOpen && onOpenChange && !toggleRef?.current?.contains(event.target as Node)) {
if (isOpen && !menuRef.current?.contains(event.target as Node)) {
Expand Down
4 changes: 2 additions & 2 deletions packages/react-core/src/components/Menu/Menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,15 @@ class MenuBase extends React.Component<MenuProps, MenuState> {
if (this.context) {
this.setState({ disableHover: this.context.disableHover });
}
if (canUseDOM) {
if (canUseDOM && this.props.containsDrilldown) {
window.addEventListener('transitionend', this.props.isRootMenu ? this.handleDrilldownTransition : null);
}

this.allowTabFirstItem();
}

componentWillUnmount() {
if (canUseDOM) {
if (canUseDOM && this.props.containsDrilldown) {
window.removeEventListener('transitionend', this.handleDrilldownTransition);
}
}
Expand Down
31 changes: 20 additions & 11 deletions packages/react-core/src/components/Menu/MenuContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ export interface MenuContainerProps {
zIndex?: number;
/** Additional properties to pass to the Popper */
popperProps?: MenuPopperProps;
/** @beta Flag indicating the first menu item should be focused after opening the dropdown. */
shouldFocusFirstItemOnOpen?: boolean;
}

/**
Expand All @@ -51,8 +53,25 @@ export const MenuContainer: React.FunctionComponent<MenuContainerProps> = ({
onOpenChange,
zIndex = 9999,
popperProps,
onOpenChangeKeys = ['Escape', 'Tab']
onOpenChangeKeys = ['Escape', 'Tab'],
shouldFocusFirstItemOnOpen = true
}: MenuContainerProps) => {
const prevIsOpen = React.useRef<boolean>(isOpen);
React.useEffect(() => {
// menu was opened, focus on first menu item
if (prevIsOpen.current === false && isOpen === true && shouldFocusFirstItemOnOpen) {
setTimeout(() => {
const firstElement = menuRef?.current?.querySelector(
'li button:not(:disabled),li input:not(:disabled),li a:not([aria-disabled="true"])'
);
firstElement && (firstElement as HTMLElement).focus();
}, 0);
}

prevIsOpen.current = isOpen;
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isOpen]);

React.useEffect(() => {
const handleMenuKeys = (event: KeyboardEvent) => {
// Close the menu on tab or escape if onOpenChange is provided
Expand All @@ -68,16 +87,6 @@ export const MenuContainer: React.FunctionComponent<MenuContainerProps> = ({
};

const handleClick = (event: MouseEvent) => {
// toggle was opened, focus on first menu item
if (isOpen && toggleRef.current?.contains(event.target as Node)) {
setTimeout(() => {
const firstElement = menuRef?.current?.querySelector(
'li button:not(:disabled),li input:not(:disabled),li a:not([aria-disabled="true"])'
);
firstElement && (firstElement as HTMLElement).focus();
}, 0);
}

// If the event is not on the toggle and onOpenChange callback is provided, close the menu
if (isOpen && onOpenChange && !toggleRef?.current?.contains(event.target as Node)) {
if (isOpen && !menuRef.current?.contains(event.target as Node)) {
Expand Down
22 changes: 14 additions & 8 deletions packages/react-core/src/components/Select/Select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,20 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
? localToggleRef
: (toggle?.toggleRef as React.RefObject<HTMLButtonElement>);

const prevIsOpen = React.useRef<boolean>(isOpen);
React.useEffect(() => {
// menu was opened, focus on first menu item
if (prevIsOpen.current === false && isOpen === true && shouldFocusFirstItemOnOpen) {
setTimeout(() => {
const firstElement = menuRef?.current?.querySelector('li button:not(:disabled),li input:not(:disabled)');
firstElement && (firstElement as HTMLElement).focus();
}, 0);
}

prevIsOpen.current = isOpen;
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isOpen]);

React.useEffect(() => {
const handleMenuKeys = (event: KeyboardEvent) => {
// Close the menu on tab or escape if onOpenChange is provided
Expand All @@ -127,14 +141,6 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
};

const handleClick = (event: MouseEvent) => {
// toggle was opened, focus on first menu item
if (isOpen && shouldFocusFirstItemOnOpen && toggleRef.current?.contains(event.target as Node)) {
setTimeout(() => {
const firstElement = menuRef?.current?.querySelector('li button:not(:disabled),li input:not(:disabled)');
firstElement && (firstElement as HTMLElement).focus();
}, 0);
}

// If the event is not on the toggle and onOpenChange callback is provided, close the menu
if (isOpen && onOpenChange && !toggleRef?.current?.contains(event.target as Node)) {
if (isOpen && !menuRef.current?.contains(event.target as Node)) {
Expand Down
6 changes: 6 additions & 0 deletions packages/react-integration/cypress/integration/notificationdrawerbasic.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,23 @@ describe('Notification Drawer Basic Demo Test', () => {
// press Enter on toggle button, check whether the dropdown menu exsit and whether it focuses on the first item
// then press Tab on toggle button, check whether the dropdown menu is closed
cy.get('#toggle-id-0').then((toggleButton: JQuery<HTMLButtonElement>) => {
cy.clock();
cy.wrap(toggleButton).type(' ', { waitForAnimations: true });
cy.tick(200);
cy.get('.notification-0.pf-v6-c-menu').should('exist');
cy.wrap(toggleButton).type('{esc}', { waitForAnimations: true });
cy.tick(200);
cy.get('.notification-0.pf-v6-c-menu').should('not.exist');
});
// Verify the list item header toggle button keyboard interactivity opens/closes dropdown menu
// the method is the same as above
cy.get('#toggle-id-1').then((toggleButton: JQuery<HTMLButtonElement>) => {
cy.clock();
cy.wrap(toggleButton).type(' ', { waitForAnimations: true });
cy.tick(200);
cy.get('.notification-1.pf-v6-c-menu').should('exist');
cy.wrap(toggleButton).type('{esc}', { waitForAnimations: true });
cy.tick(200);
cy.get('.notification-1.pf-v6-c-menu').should('not.exist');
});
});
Expand Down
6 changes: 6 additions & 0 deletions packages/react-integration/cypress/integration/notificationdrawergroups.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,12 @@ describe('Notification Drawer Groups Demo Test', () => {
// press Enter on toggle button, check whether the dropdown menu exsit and whether it focuses on the first item
// then press Tab on toggle button, check whether the dropdown menu is closed
cy.get('#toggle-id-0').then((toggleButton: JQuery<HTMLButtonElement>) => {
cy.clock();
cy.wrap(toggleButton).type('{enter}', { waitForAnimations: true });
cy.tick(200);
cy.get('.notification-0.pf-v6-c-menu').should('exist');
cy.wrap(toggleButton).type('{esc}', { waitForAnimations: true });
cy.tick(200);
cy.get('.notification-0.pf-v6-c-menu').should('not.exist');
});
// Verify the group header keyboard interactivity opens/closes the whole group
Expand All @@ -69,9 +72,12 @@ describe('Notification Drawer Groups Demo Test', () => {
cy.get('.pf-v6-c-notification-drawer__group').first().should('not.have.class', 'pf-m-expanded');
// Verify the list item header toggle button keyboard interactivity opens/closes dropdown menu
cy.get('#toggle-id-9').then((toggleButton: JQuery<HTMLButtonElement>) => {
cy.clock();
cy.wrap(toggleButton).type('{enter}', { waitForAnimations: true });
cy.tick(200);
cy.get('.notification-9.pf-v6-c-menu').should('exist');
cy.wrap(toggleButton).type('{esc}', { waitForAnimations: true });
cy.tick(200);
cy.get('.notification-9.pf-v6-c-menu').should('not.exist');
});
});
Expand Down
4 changes: 2 additions & 2 deletions packages/react-integration/demo-app-ts/src/components/demos/DropdownDemo/DropdownDemo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ export const DropdownDemo: React.FunctionComponent = () => {
setIsOpen(!isOpen);
};
const onNoAutofocusToggleClick = () => {
setIsOpen(!isOpen);
setIsNoAutofocusOpen(!isNoAutofocusOpen);
};

const onSelect = (_event: React.MouseEvent<Element, MouseEvent> | undefined) => {
setIsOpen(false);
};
const onNoAutofocusSelect = (_event: React.MouseEvent<Element, MouseEvent> | undefined) => {
setIsOpen(false);
setIsNoAutofocusOpen(false);
};

return (
Expand Down