From a63a1d94f47552e0f95635f80c3ac047394dac39 Mon Sep 17 00:00:00 2001 From: Katie McFaul Date: Tue, 6 Aug 2024 16:51:10 -0400 Subject: [PATCH 1/4] fix(Menu): update drilldown transition event handler --- packages/react-core/src/components/Menu/Menu.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-core/src/components/Menu/Menu.tsx b/packages/react-core/src/components/Menu/Menu.tsx index 8a16d83daaf..5c010bbb0b0 100644 --- a/packages/react-core/src/components/Menu/Menu.tsx +++ b/packages/react-core/src/components/Menu/Menu.tsx @@ -119,7 +119,7 @@ class MenuBase extends React.Component { 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); } @@ -127,7 +127,7 @@ class MenuBase extends React.Component { } componentWillUnmount() { - if (canUseDOM) { + if (canUseDOM && this.props.containsDrilldown) { window.removeEventListener('transitionend', this.handleDrilldownTransition); } } From 2a47488bc46800fa590ce003463fb91edb64c8a7 Mon Sep 17 00:00:00 2001 From: Katie McFaul Date: Tue, 6 Aug 2024 17:36:59 -0400 Subject: [PATCH 2/4] fix part1 --- .../src/components/demos/DropdownDemo/DropdownDemo.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-integration/demo-app-ts/src/components/demos/DropdownDemo/DropdownDemo.tsx b/packages/react-integration/demo-app-ts/src/components/demos/DropdownDemo/DropdownDemo.tsx index 5f7c6b5288d..a9f701b108d 100644 --- a/packages/react-integration/demo-app-ts/src/components/demos/DropdownDemo/DropdownDemo.tsx +++ b/packages/react-integration/demo-app-ts/src/components/demos/DropdownDemo/DropdownDemo.tsx @@ -42,14 +42,14 @@ export const DropdownDemo: React.FunctionComponent = () => { setIsOpen(!isOpen); }; const onNoAutofocusToggleClick = () => { - setIsOpen(!isOpen); + setIsNoAutofocusOpen(!isNoAutofocusOpen); }; const onSelect = (_event: React.MouseEvent | undefined) => { setIsOpen(false); }; const onNoAutofocusSelect = (_event: React.MouseEvent | undefined) => { - setIsOpen(false); + setIsNoAutofocusOpen(false); }; return ( From 2e79eebc8620fc77f689467fc54c87a81f79b380 Mon Sep 17 00:00:00 2001 From: Katie McFaul Date: Thu, 8 Aug 2024 11:06:04 -0400 Subject: [PATCH 3/4] update first item focus logic --- .../src/components/Dropdown/Dropdown.tsx | 26 ++++++++++------ .../src/components/Menu/MenuContainer.tsx | 31 ++++++++++++------- .../src/components/Select/Select.tsx | 22 ++++++++----- 3 files changed, 50 insertions(+), 29 deletions(-) diff --git a/packages/react-core/src/components/Dropdown/Dropdown.tsx b/packages/react-core/src/components/Dropdown/Dropdown.tsx index 2259969e265..30a54d545ce 100644 --- a/packages/react-core/src/components/Dropdown/Dropdown.tsx +++ b/packages/react-core/src/components/Dropdown/Dropdown.tsx @@ -104,6 +104,22 @@ const DropdownBase: React.FunctionComponent = ({ ? localToggleRef : (toggle?.toggleRef as React.RefObject); + const prevIsOpen = React.useRef(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 @@ -120,16 +136,6 @@ const DropdownBase: React.FunctionComponent = ({ }; 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)) { diff --git a/packages/react-core/src/components/Menu/MenuContainer.tsx b/packages/react-core/src/components/Menu/MenuContainer.tsx index ba80067f383..1901b104ce7 100644 --- a/packages/react-core/src/components/Menu/MenuContainer.tsx +++ b/packages/react-core/src/components/Menu/MenuContainer.tsx @@ -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; } /** @@ -51,8 +53,25 @@ export const MenuContainer: React.FunctionComponent = ({ onOpenChange, zIndex = 9999, popperProps, - onOpenChangeKeys = ['Escape', 'Tab'] + onOpenChangeKeys = ['Escape', 'Tab'], + shouldFocusFirstItemOnOpen = true }: MenuContainerProps) => { + const prevIsOpen = React.useRef(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 @@ -68,16 +87,6 @@ export const MenuContainer: React.FunctionComponent = ({ }; 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)) { diff --git a/packages/react-core/src/components/Select/Select.tsx b/packages/react-core/src/components/Select/Select.tsx index 0cdb199de3c..8440895bfb7 100644 --- a/packages/react-core/src/components/Select/Select.tsx +++ b/packages/react-core/src/components/Select/Select.tsx @@ -110,6 +110,20 @@ const SelectBase: React.FunctionComponent = ({ ? localToggleRef : (toggle?.toggleRef as React.RefObject); + const prevIsOpen = React.useRef(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 @@ -127,14 +141,6 @@ const SelectBase: React.FunctionComponent = ({ }; 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)) { From 6180bf5ca64537a8079c6fd611ab5e92afb8980c Mon Sep 17 00:00:00 2001 From: Katie McFaul Date: Thu, 8 Aug 2024 11:43:52 -0400 Subject: [PATCH 4/4] add tick to some test to wait for menus --- .../cypress/integration/notificationdrawerbasic.spec.ts | 6 ++++++ .../cypress/integration/notificationdrawergroups.spec.ts | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/packages/react-integration/cypress/integration/notificationdrawerbasic.spec.ts b/packages/react-integration/cypress/integration/notificationdrawerbasic.spec.ts index 26e573be657..248c0f9a2f4 100644 --- a/packages/react-integration/cypress/integration/notificationdrawerbasic.spec.ts +++ b/packages/react-integration/cypress/integration/notificationdrawerbasic.spec.ts @@ -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) => { + 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) => { + 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'); }); }); diff --git a/packages/react-integration/cypress/integration/notificationdrawergroups.spec.ts b/packages/react-integration/cypress/integration/notificationdrawergroups.spec.ts index 5d8bbd9670b..a9636e9dd47 100644 --- a/packages/react-integration/cypress/integration/notificationdrawergroups.spec.ts +++ b/packages/react-integration/cypress/integration/notificationdrawergroups.spec.ts @@ -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) => { + 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 @@ -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) => { + 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'); }); });