From 0afc92e61a47ba2c07ec1477e1085de8cc070b05 Mon Sep 17 00:00:00 2001 From: cknabe Date: Fri, 10 Jan 2025 21:37:54 -0600 Subject: [PATCH 1/4] fix(Dropdown): 18277 - Disable arrow up on dropdown --- packages/react/src/components/Dropdown/Dropdown.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/react/src/components/Dropdown/Dropdown.tsx b/packages/react/src/components/Dropdown/Dropdown.tsx index 6f2970fe7c85..f0abc1157d2a 100644 --- a/packages/react/src/components/Dropdown/Dropdown.tsx +++ b/packages/react/src/components/Dropdown/Dropdown.tsx @@ -548,7 +548,13 @@ const Dropdown = React.forwardRef( }, 3000) ); } - if (toggleButtonProps.onKeyDown) { + + if ( + toggleButtonProps.onKeyDown && + (evt.key !== 'ArrowUp' || + items?.[0] !== selectedItem || + (evt.key === 'ArrowUp' && isOpen)) + ) { toggleButtonProps.onKeyDown(evt); } }, From 6489e5536283e535c3d163ac2b3e09ed6f1a62f2 Mon Sep 17 00:00:00 2001 From: cknabe Date: Fri, 10 Jan 2025 23:18:13 -0600 Subject: [PATCH 2/4] fix: refactor to make it easier to read and include last item --- packages/react/src/components/Dropdown/Dropdown.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/react/src/components/Dropdown/Dropdown.tsx b/packages/react/src/components/Dropdown/Dropdown.tsx index f0abc1157d2a..3504881df59d 100644 --- a/packages/react/src/components/Dropdown/Dropdown.tsx +++ b/packages/react/src/components/Dropdown/Dropdown.tsx @@ -549,11 +549,18 @@ const Dropdown = React.forwardRef( ); } + // Allow any key press that is not ArrowUp or ArrowDown + const isNotUpOrDown = evt.key !== 'ArrowUp' && evt.key !== 'ArrowDown'; + // Allow any key press other than ArrowUp if the first item is selected + const isNotTopAndUp = + evt.key === 'ArrowUp' && items?.[0] !== selectedItem; + // Allow any key press other than ArrowDown if the last item is selected + const isNotBottomAndDown = + evt.key === 'ArrowDown' && + items?.[items?.length - 1] !== selectedItem; if ( toggleButtonProps.onKeyDown && - (evt.key !== 'ArrowUp' || - items?.[0] !== selectedItem || - (evt.key === 'ArrowUp' && isOpen)) + (isNotUpOrDown || isOpen || isNotTopAndUp || isNotBottomAndDown) ) { toggleButtonProps.onKeyDown(evt); } From 17143597a2d20487e1d5b3b14040676263b30525 Mon Sep 17 00:00:00 2001 From: cknabe Date: Tue, 21 Jan 2025 14:42:29 -0600 Subject: [PATCH 3/4] fix: almost never allow ArrowUp for DropDown, always allow ArrowDown --- packages/react/src/components/Dropdown/Dropdown.tsx | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/packages/react/src/components/Dropdown/Dropdown.tsx b/packages/react/src/components/Dropdown/Dropdown.tsx index 3504881df59d..a4ef1dd11617 100644 --- a/packages/react/src/components/Dropdown/Dropdown.tsx +++ b/packages/react/src/components/Dropdown/Dropdown.tsx @@ -549,18 +549,9 @@ const Dropdown = React.forwardRef( ); } - // Allow any key press that is not ArrowUp or ArrowDown - const isNotUpOrDown = evt.key !== 'ArrowUp' && evt.key !== 'ArrowDown'; - // Allow any key press other than ArrowUp if the first item is selected - const isNotTopAndUp = - evt.key === 'ArrowUp' && items?.[0] !== selectedItem; - // Allow any key press other than ArrowDown if the last item is selected - const isNotBottomAndDown = - evt.key === 'ArrowDown' && - items?.[items?.length - 1] !== selectedItem; if ( toggleButtonProps.onKeyDown && - (isNotUpOrDown || isOpen || isNotTopAndUp || isNotBottomAndDown) + (evt.key !== 'ArrowUp' || (isOpen && evt.key === 'ArrowUp')) ) { toggleButtonProps.onKeyDown(evt); } From 026108085b4db37cb9540506ade3f10f7914fd99 Mon Sep 17 00:00:00 2001 From: cknabe Date: Tue, 21 Jan 2025 17:30:48 -0600 Subject: [PATCH 4/4] fix: add a simple test to verify arrow up and arrow down actions --- .../src/components/Dropdown/Dropdown.tsx | 1 + .../Dropdown/__tests__/Dropdown-test.js | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/packages/react/src/components/Dropdown/Dropdown.tsx b/packages/react/src/components/Dropdown/Dropdown.tsx index a4ef1dd11617..dfa27244a8b7 100644 --- a/packages/react/src/components/Dropdown/Dropdown.tsx +++ b/packages/react/src/components/Dropdown/Dropdown.tsx @@ -549,6 +549,7 @@ const Dropdown = React.forwardRef( ); } + // For Dropdowns the arrow up key is only allowed if the Dropdown is open if ( toggleButtonProps.onKeyDown && (evt.key !== 'ArrowUp' || (isOpen && evt.key === 'ArrowUp')) diff --git a/packages/react/src/components/Dropdown/__tests__/Dropdown-test.js b/packages/react/src/components/Dropdown/__tests__/Dropdown-test.js index c58e341e4347..7c374d4dfc85 100644 --- a/packages/react/src/components/Dropdown/__tests__/Dropdown-test.js +++ b/packages/react/src/components/Dropdown/__tests__/Dropdown-test.js @@ -180,6 +180,29 @@ describe('Dropdown', () => { }); }); + it('should not open on arrowUp', async () => { + let mockProps1 = { ...mockProps }; + const ref = React.createRef(); + render(); + + const button = screen.getByRole('combobox'); + + if (button) { + assertMenuClosed(); + // ArrowUp should not open the menu + fireEvent.keyDown(button, { key: 'ArrowUp' }); + assertMenuClosed(); + // ArrowDown should open the menu + fireEvent.keyDown(button, { key: 'ArrowDown' }); + assertMenuOpen(mockProps1); + // ArrowUp is allowed now that the menu is open + fireEvent.keyDown(button, { key: 'ArrowUp' }); + assertMenuOpen(mockProps1); + } + + mockProps.onChange.mockClear(); + }); + it('should respect readOnly prop', async () => { let onChange = jest.fn(); let onKeyDown = jest.fn();