From 5fb9866a023f3854984afd914c8d34323d11a6e1 Mon Sep 17 00:00:00 2001 From: Andrew Prasetya Date: Wed, 2 Nov 2022 16:25:02 -0400 Subject: [PATCH 01/25] Progress on basic keynav. - RSC-989 --- .../NxDropdownMenu/NxDropdownMenu.tsx | 60 ++++++++++++++++++- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx b/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx index 34b67d81b4..8eb704b549 100644 --- a/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx +++ b/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx @@ -4,8 +4,9 @@ * the terms of the Eclipse Public License 2.0 which accompanies this * distribution and is available at https://www.eclipse.org/legal/epl-2.0/. */ -import React, { useLayoutEffect, forwardRef } from 'react'; +import React, { useLayoutEffect, forwardRef, useState, useEffect, useRef, KeyboardEventHandler } from 'react'; import classnames from 'classnames'; +import useMergedRef from '@react-hook/merged-ref'; import { Props, propTypes } from './types'; @@ -19,13 +20,66 @@ export { Props }; */ /* eslint-disable-next-line react/prop-types */ const NxDropdownMenu = forwardRef(function NxDropdownMenu(props, ref) { - const { onClosing, className: classNameProp, ...attrs } = props, + const menuRef = useRef(null); + const [focusedIndex, setFocusedIndex] = useState(0); + const mergedRef = useMergedRef(menuRef, ref); + + const { onClosing, className: classNameProp, children, ...attrs } = props, className = classnames('nx-dropdown-menu', classNameProp); // onClosing must execute when this element is being removed but BEFORE it actually gets removed from the DOM useLayoutEffect(() => onClosing, []); - return
; + useEffect(() => { + // or diff --git a/gallery/src/components/NxDropdown/NxDropdownCustomLabelExample.tsx b/gallery/src/components/NxDropdown/NxDropdownCustomLabelExample.tsx index 4d2191ec76..e801496b86 100644 --- a/gallery/src/components/NxDropdown/NxDropdownCustomLabelExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownCustomLabelExample.tsx @@ -24,7 +24,7 @@ function NxDropdownCustomLabelExample() { className="extra-class" isOpen={isOpen} onToggleCollapse={onToggleCollapse}> - diff --git a/gallery/src/components/NxDropdown/NxDropdownLinksExample.tsx b/gallery/src/components/NxDropdown/NxDropdownLinksExample.tsx index 633715bcb1..16614befad 100644 --- a/gallery/src/components/NxDropdown/NxDropdownLinksExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownLinksExample.tsx @@ -17,22 +17,30 @@ function NxDropdownActionsExample() { className="extra-class" isOpen={isOpen} onToggleCollapse={onToggleCollapse}> - + Go somewhere - + Go somewhere else - evt.preventDefault()}> + evt.preventDefault()}> Can't go here though - + Go to homepage - evt.preventDefault()}> + evt.preventDefault()} + role="menuitem"> Can't go here either diff --git a/gallery/src/components/NxDropdown/NxDropdownNavigationExample.tsx b/gallery/src/components/NxDropdown/NxDropdownNavigationExample.tsx index 3fc6ed3379..86ad1b4f8e 100644 --- a/gallery/src/components/NxDropdown/NxDropdownNavigationExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownNavigationExample.tsx @@ -14,26 +14,26 @@ function NxDropdownNavigationExample() { return ( - + Text link 1 - + Text link 2 - + Text link 3 - this link should trigger truncation - - - - diff --git a/gallery/src/components/NxDropdown/NxDropdownRightButtonsExample.tsx b/gallery/src/components/NxDropdown/NxDropdownRightButtonsExample.tsx index d963bf092e..819e138516 100644 --- a/gallery/src/components/NxDropdown/NxDropdownRightButtonsExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownRightButtonsExample.tsx @@ -14,10 +14,10 @@ function NxDropdownRightButtonsExample() { return ( - + Text Link - + Text Link - this link should trigger truncation @@ -25,19 +25,21 @@ function NxDropdownRightButtonsExample() { alert('icon click')} className="nx-dropdown-right-button" variant="icon-only" - title="Delete Button Link2"> + title="Delete Button Link2" + role="menuitem"> - + Button Link2 alert('icon click')} className="nx-dropdown-right-button" variant="icon-only" - title="Delete Text Link3"> + title="Delete Text Link3" + role="menuitem"> - + Text Link3 - this link should trigger truncation diff --git a/gallery/src/components/NxDropdown/NxDropdownScrollingExample.tsx b/gallery/src/components/NxDropdown/NxDropdownScrollingExample.tsx index 2f00fc9a03..4f75fcc7cf 100644 --- a/gallery/src/components/NxDropdown/NxDropdownScrollingExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownScrollingExample.tsx @@ -15,46 +15,47 @@ function NxDropdownNavigationExample() { - + Text Link 1 - + Text Link 2 - + Text Link 3 - + Text Link 4 - + Text Link 5 - + Text Link 6 { evt.preventDefault(); }} className="disabled nx-dropdown-button" - aria-disabled="true"> + aria-disabled="true" + role="menuitem"> Text Link 7 Disabled - + Text Link 8 - + Text Link 9 - + Text Link 10 - + Text Link 11 - + Text Link 12 - + Text Link 13 diff --git a/gallery/src/components/NxDropdown/NxDropdownShortExample.tsx b/gallery/src/components/NxDropdown/NxDropdownShortExample.tsx index 88c093ee8d..132a6294ce 100644 --- a/gallery/src/components/NxDropdown/NxDropdownShortExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownShortExample.tsx @@ -14,16 +14,16 @@ function NxDropdownNavigationExample() { return ( - - - - diff --git a/gallery/src/components/NxDropdown/NxDropdownWithNxThreatIndicatorExample.tsx b/gallery/src/components/NxDropdown/NxDropdownWithNxThreatIndicatorExample.tsx index 2f6af3ff19..003b9bb600 100644 --- a/gallery/src/components/NxDropdown/NxDropdownWithNxThreatIndicatorExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownWithNxThreatIndicatorExample.tsx @@ -22,27 +22,27 @@ function NxDropdownWithNxThreatIndicatorExample() { - - - - - - diff --git a/lib/src/components/NxCombobox/NxCombobox.tsx b/lib/src/components/NxCombobox/NxCombobox.tsx index 9ffc02ff62..065f135893 100644 --- a/lib/src/components/NxCombobox/NxCombobox.tsx +++ b/lib/src/components/NxCombobox/NxCombobox.tsx @@ -333,7 +333,8 @@ function NxComboboxRender( ref={dropdownRef} className='nx-combobox__menu' onClosing={() => {}} - aria-label="listbox of combobox"> + aria-label="listbox of combobox" + disableMenuKeyNav={true}> { matches.length && matches.map((match, i) => From b4a2f4c33134b48ff1e6c09b1917a33e22521f98 Mon Sep 17 00:00:00 2001 From: Andrew Prasetya Date: Mon, 7 Nov 2022 16:16:19 -0500 Subject: [PATCH 03/25] Small cleanup. - RSC-989 --- .../NxDropdownMenu/NxDropdownMenu.tsx | 49 +++++++++---------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx b/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx index c075b4ac34..7726ce0beb 100644 --- a/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx +++ b/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx @@ -20,17 +20,17 @@ export { Props }; */ /* eslint-disable-next-line react/prop-types */ const NxDropdownMenu = forwardRef(function NxDropdownMenu(props, ref) { + const { onClosing, className: classNameProp, children, disableMenuKeyNav, ...attrs } = props; + const menuRef = useRef(null); - const [focusedMenuItemIndex, setFocusedMenuItemIndex] = useState(0); const mergedRef = useMergedRef(menuRef, ref); + const [focusedMenuItemIndex, setFocusedMenuItemIndex] = useState(0); + const focusableItemsSelector = 'a:not([disabled]):not(.disabled), ' - + 'button:not([disabled]):not(disabled), ' + + 'button:not([disabled]):not(.disabled), ' + 'input:not([disabled]):not(.disabled)'; - const { onClosing, className: classNameProp, children, disableMenuKeyNav, ...attrs } = props, - className = classnames('nx-dropdown-menu', classNameProp); - // onClosing must execute when this element is being removed but BEFORE it actually gets removed from the DOM useLayoutEffect(() => onClosing, []); @@ -40,49 +40,42 @@ const NxDropdownMenu = forwardRef(function NxDropdownMenu } }, [disableMenuKeyNav]); - function setFocusToMenuItem(direction?: -1 | 0 | 1) { + function setFocusToMenuItem(direction: -1 | 1) { if (menuRef.current) { + const focusableEls = menuRef.current.querySelectorAll(focusableItemsSelector); - const elements = menuRef.current.querySelectorAll(focusableItemsSelector); + let newFocusedMenuItemIndex = 0; - let updatedFocusedIndex = 0; - - if (typeof direction !== 'number' && direction !== 0) { - if (!elements[focusedMenuItemIndex]) { - menuRef.current.focus(); - } - return; - } - else if (direction === -1) { - updatedFocusedIndex = (focusedMenuItemIndex - 1 >= 0) ? focusedMenuItemIndex - 1 : elements.length - 1; + if (direction === -1) { + newFocusedMenuItemIndex = (focusedMenuItemIndex - 1 >= 0) ? focusedMenuItemIndex - 1 : focusableEls.length - 1; if (document.activeElement === menuRef.current) { - updatedFocusedIndex = elements.length - 1; + newFocusedMenuItemIndex = focusableEls.length - 1; } } else if (direction === 1) { - updatedFocusedIndex = (focusedMenuItemIndex + 1 >= elements.length) ? 0 : focusedMenuItemIndex + 1; + newFocusedMenuItemIndex = (focusedMenuItemIndex + 1 >= focusableEls.length) ? 0 : focusedMenuItemIndex + 1; if (document.activeElement === menuRef.current) { - updatedFocusedIndex = 0; + newFocusedMenuItemIndex = 0; } } - setFocusedMenuItemIndex(updatedFocusedIndex); - elements[updatedFocusedIndex].focus(); + setFocusedMenuItemIndex(newFocusedMenuItemIndex); + focusableEls[newFocusedMenuItemIndex].focus(); } } function activateMenuItem(event: React.KeyboardEvent) { if (menuRef.current) { - const elements = menuRef.current.querySelectorAll(focusableItemsSelector); - const focusedElement = elements[focusedMenuItemIndex]; - if (focusedElement.matches('a, button')) { + const focusableEls = menuRef.current.querySelectorAll(focusableItemsSelector); + const currentFocusedEl = focusableEls[focusedMenuItemIndex]; + if (currentFocusedEl.matches('a, button')) { event.preventDefault(); - focusedElement.click(); + currentFocusedEl.click(); } } } - const handleKeyDown: KeyboardEventHandler = (event) => { + const handleKeyDown: KeyboardEventHandler = (event) => { if (!disableMenuKeyNav) { switch (event.key) { case 'ArrowUp': @@ -100,6 +93,8 @@ const NxDropdownMenu = forwardRef(function NxDropdownMenu } }; + const className = classnames('nx-dropdown-menu', classNameProp); + return (
Date: Tue, 8 Nov 2022 10:00:33 -0500 Subject: [PATCH 04/25] Added aria-controls to dropdown. - RSC-989 --- lib/src/components/NxDropdown/AbstractDropdown.tsx | 3 ++- lib/src/components/NxDropdown/NxDropdown.tsx | 8 +++++++- lib/src/components/NxDropdown/types.ts | 1 + 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/src/components/NxDropdown/AbstractDropdown.tsx b/lib/src/components/NxDropdown/AbstractDropdown.tsx index 7139e09f4d..30469ef58e 100644 --- a/lib/src/components/NxDropdown/AbstractDropdown.tsx +++ b/lib/src/components/NxDropdown/AbstractDropdown.tsx @@ -46,6 +46,7 @@ const AbstractDropdown = forwardRef((prop onCloseClick, onCloseKeyDown: onCloseKeyDownProp, menuRef: menuRefProp, + menuId, ...attrs } = props; @@ -142,7 +143,7 @@ const AbstractDropdown = forwardRef((prop
{ renderToggleElement(toggleRef, onToggleCollapse) } { isOpen && - + { children } } diff --git a/lib/src/components/NxDropdown/NxDropdown.tsx b/lib/src/components/NxDropdown/NxDropdown.tsx index 2b63cbbab5..6e7736f7b8 100644 --- a/lib/src/components/NxDropdown/NxDropdown.tsx +++ b/lib/src/components/NxDropdown/NxDropdown.tsx @@ -15,6 +15,7 @@ import NxFontAwesomeIcon from '../NxFontAwesomeIcon/NxFontAwesomeIcon'; import { wrapTooltipProps } from '../../util/tooltipUtils'; import './NxDropdown.scss'; import NxOverflowTooltip from '../NxTooltip/NxOverflowTooltip'; +import { useUniqueId } from '../../util/idUtil'; import AbstractDropdown, { AbstractDropdownRenderToggleElement } from './AbstractDropdown'; import withClass from '../../util/withClass'; @@ -28,6 +29,7 @@ const _NxDropdown = forwardRef(function NxDropdown(props, label, toggleTooltip, variant, + id: idProp, ...otherProps } = props; @@ -37,6 +39,8 @@ const _NxDropdown = forwardRef(function NxDropdown(props, const toggleTooltipProps = toggleTooltip && wrapTooltipProps(toggleTooltip); + const id = useUniqueId('nx-dropdown', idProp || undefined); + // Wrap .nx-dropdown-button and .nx-dropdown-link children in overflow tooltips const wrappedChildren = children && React.Children.map(children, child => ( /(\s|^)nx-dropdown-(button|link)(\s|$)/.test(child.props.className) ? @@ -52,7 +56,8 @@ const _NxDropdown = forwardRef(function NxDropdown(props, className={buttonClasses} onClick={!disabled && onToggleCollapse || undefined} aria-haspopup="true" - aria-expanded={isOpen}> + aria-expanded={isOpen} + aria-controls={id}> { label } @@ -67,6 +72,7 @@ const _NxDropdown = forwardRef(function NxDropdown(props, disabled={disabled} renderToggleElement={renderToggleElement} ref={ref} + menuId={id} { ...otherProps } > { wrappedChildren } diff --git a/lib/src/components/NxDropdown/types.ts b/lib/src/components/NxDropdown/types.ts index 74904b5400..c1ce179107 100644 --- a/lib/src/components/NxDropdown/types.ts +++ b/lib/src/components/NxDropdown/types.ts @@ -22,6 +22,7 @@ export interface AbstractDropdownProps extends HTMLAttributes { onCloseKeyDown?: KeyboardEventHandler | null; onCloseClick?: ((e: MouseEvent) => void) | null; menuRef?: Ref; + menuId?: string; } export type Props = Omit, 'className'> & { From e80780397a4e6b0ea6bc182e4f1b7894873859e6 Mon Sep 17 00:00:00 2001 From: Andrew Prasetya Date: Tue, 8 Nov 2022 13:06:30 -0500 Subject: [PATCH 05/25] Added dropdown menu test. - RSC-989 --- .../NxDropdownMenu/NxDropdownMenu.tsx | 14 ++- .../__tests__/NxDropdownMenu.test.tsx | 110 +++++++++++++++--- 2 files changed, 104 insertions(+), 20 deletions(-) diff --git a/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx b/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx index 7726ce0beb..b5f11d43f9 100644 --- a/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx +++ b/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx @@ -14,6 +14,10 @@ import './NxDropdownMenu.scss'; export { Props }; +const FOCUSABLE_MENU_ITEMS_SELECTOR = 'a:not([disabled]):not(.disabled), ' ++ 'button:not([disabled]):not(.disabled), ' ++ 'input:not([disabled]):not(.disabled)'; + /** * This component is not currently intended for public export. It is a helper for NxDropdown and NxSegmentedButton * so they can reset focus when they close @@ -27,10 +31,6 @@ const NxDropdownMenu = forwardRef(function NxDropdownMenu const [focusedMenuItemIndex, setFocusedMenuItemIndex] = useState(0); - const focusableItemsSelector = 'a:not([disabled]):not(.disabled), ' - + 'button:not([disabled]):not(.disabled), ' - + 'input:not([disabled]):not(.disabled)'; - // onClosing must execute when this element is being removed but BEFORE it actually gets removed from the DOM useLayoutEffect(() => onClosing, []); @@ -42,18 +42,20 @@ const NxDropdownMenu = forwardRef(function NxDropdownMenu function setFocusToMenuItem(direction: -1 | 1) { if (menuRef.current) { - const focusableEls = menuRef.current.querySelectorAll(focusableItemsSelector); + const focusableEls = menuRef.current.querySelectorAll(FOCUSABLE_MENU_ITEMS_SELECTOR); let newFocusedMenuItemIndex = 0; if (direction === -1) { newFocusedMenuItemIndex = (focusedMenuItemIndex - 1 >= 0) ? focusedMenuItemIndex - 1 : focusableEls.length - 1; + if (document.activeElement === menuRef.current) { newFocusedMenuItemIndex = focusableEls.length - 1; } } else if (direction === 1) { newFocusedMenuItemIndex = (focusedMenuItemIndex + 1 >= focusableEls.length) ? 0 : focusedMenuItemIndex + 1; + if (document.activeElement === menuRef.current) { newFocusedMenuItemIndex = 0; } @@ -66,7 +68,7 @@ const NxDropdownMenu = forwardRef(function NxDropdownMenu function activateMenuItem(event: React.KeyboardEvent) { if (menuRef.current) { - const focusableEls = menuRef.current.querySelectorAll(focusableItemsSelector); + const focusableEls = menuRef.current.querySelectorAll(FOCUSABLE_MENU_ITEMS_SELECTOR); const currentFocusedEl = focusableEls[focusedMenuItemIndex]; if (currentFocusedEl.matches('a, button')) { event.preventDefault(); diff --git a/lib/src/components/NxDropdownMenu/__tests__/NxDropdownMenu.test.tsx b/lib/src/components/NxDropdownMenu/__tests__/NxDropdownMenu.test.tsx index a146d9e409..a7e9140441 100644 --- a/lib/src/components/NxDropdownMenu/__tests__/NxDropdownMenu.test.tsx +++ b/lib/src/components/NxDropdownMenu/__tests__/NxDropdownMenu.test.tsx @@ -5,24 +5,105 @@ * distribution and is available at https://www.eclipse.org/legal/epl-2.0/. */ import React from 'react'; -import { mount } from 'enzyme'; - -import { getShallowComponent } from '../../../__testutils__/enzymeUtils'; +import { render, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { rtlRender, rtlRenderElement } from '../../../__testutils__/rtlUtils'; import NxDropdownMenu, { Props } from '../NxDropdownMenu'; describe('NxDropdownMenu', function() { const minimalProps = { onClosing: () => {} }, - getShallow = getShallowComponent(NxDropdownMenu, minimalProps); + quickRender = rtlRender(NxDropdownMenu, minimalProps), + renderEl = rtlRenderElement(NxDropdownMenu, minimalProps); - it('renders a div with the nx-dropdown-menu class name', function() { - expect(getShallow()).toMatchSelector('div.nx-dropdown-menu'); + it('renders its children into the menu', function() { + const dropdownMenu = renderEl({ children:
})!; + expect(within(dropdownMenu).getByTestId('foo')).toBeInTheDocument(); }); - it('renders its children into the div', function() { - const children =
; + describe('Enabled Keyboard Navigation', function() { + const menuItems = ( + <> + + link +
divider
+ + + ); + + it('focuses on the menu when first open', function() { + const dropdownMenu = quickRender().getByRole('menu'); + expect(document.activeElement).toBe(dropdownMenu); + }); + + it('does not focus on the menu when first open if disableKeyNav is true', function() { + const dropdownMenu = quickRender({ disableMenuKeyNav: true }).getByRole('menu'); + expect(document.activeElement).not.toBe(dropdownMenu); + }); + + it('does not focus on any menuitems when arrow keys are pressed if disableKeyNav is true', async function() { + const user = userEvent.setup(); + const { getByRole } = quickRender({ children: menuItems, disableMenuKeyNav: true }); + await user.keyboard('[ArrowDown]'); + await user.keyboard('[ArrowUp]'); + + const menu = getByRole('menu'); + expect(menu.contains(document.activeElement)).toBe(false); + }); + + it('focuses on first item when arrow down is pressed after first open', async function() { + const user = userEvent.setup(); + const { getByRole } = quickRender({ children: menuItems }); + await user.keyboard('[ArrowDown]'); + const firstItem = getByRole('menuitem', { name: 'button' }); + expect(document.activeElement).toBe(firstItem); + }); + + it('focuses on last item when arrow up is pressed after first open', async function() { + const user = userEvent.setup(); + const { getByRole } = quickRender({ children: menuItems }); + + await user.keyboard('[ArrowUp]'); + + const lastItem = getByRole('menuitemcheckbox'); + expect(document.activeElement).toBe(lastItem); + }); + + it('loops to the last item when arrow up is pressed at the top item', async function() { + const user = userEvent.setup(); + const { getByRole } = quickRender({ children: menuItems }); + + await user.keyboard('[ArrowDown]'); + await user.keyboard('[ArrowUp]'); + + const lastItem = getByRole('menuitemcheckbox'); + expect(document.activeElement).toBe(lastItem); + }); + + it('skip focus on element that is not focusable', async function() { + const user = userEvent.setup(); + const { getByRole } = quickRender({ children: menuItems }); + + await user.keyboard('[ArrowDown]'); // first + await user.keyboard('[ArrowDown]'); // second + await user.keyboard('[ArrowDown]'); // skiped divider and should select last + + const lastItem = getByRole('menuitemcheckbox'); + expect(document.activeElement).toBe(lastItem); + }); + + it('loops to the first item when arrow down is pressed at the last item', async function() { + const user = userEvent.setup(); + const { getByRole } = quickRender({ children: menuItems }); + + await user.keyboard('[ArrowDown]'); + await user.keyboard('[ArrowDown]'); + await user.keyboard('[ArrowDown]'); + await user.keyboard('[ArrowDown]'); - expect(getShallow({ children }).children()).toMatchSelector('div.foo'); + const firstItem = getByRole('menuitem', { name: 'button' }); + expect(document.activeElement).toBe(firstItem); + }); }); // NOTE: this test seems to be of limited usefulness because different behavior is observed here vs in @@ -34,21 +115,22 @@ describe('NxDropdownMenu', function() { const container = document.createElement('div'), onClosing = jest.fn(() => { - attachedWhenOnClosing = container.contains(component.getDOMNode()); + attachedWhenOnClosing = container.contains(component); }); function Fixture({ hasMenu }: { hasMenu: boolean }) { return hasMenu ? : null; } - const component = mount(, { attachTo: container }); + const { getByRole, rerender } = render(, { container: container }); + const component = getByRole('menu', { hidden: true }); - expect(component).toContainMatchingElement('.nx-dropdown-menu'); + expect(component).toHaveClass('nx-dropdown-menu'); expect(onClosing).not.toHaveBeenCalled(); - component.setProps({ hasMenu: false }); + rerender(); - expect(component).toBeEmptyRender(); + expect(component).not.toBeInTheDocument(); expect(onClosing).toHaveBeenCalledTimes(1); expect(attachedWhenOnClosing).toBe(true); }); From 47880d82d2993ad6cfb03c8f5f4588e7286e3fc9 Mon Sep 17 00:00:00 2001 From: Andrew Prasetya Date: Tue, 8 Nov 2022 13:47:10 -0500 Subject: [PATCH 06/25] Updated unit tests. - RSC-989 --- lib/src/components/NxDropdown/NxDropdown.tsx | 8 ++++---- .../__tests__/AbstractDropdown.test.tsx | 8 ++++++++ .../NxDropdown/__tests__/NxDropdown.test.tsx | 20 +++++++++++++++++++ lib/src/components/NxDropdown/types.ts | 1 + 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/lib/src/components/NxDropdown/NxDropdown.tsx b/lib/src/components/NxDropdown/NxDropdown.tsx index 6e7736f7b8..7a63c7532f 100644 --- a/lib/src/components/NxDropdown/NxDropdown.tsx +++ b/lib/src/components/NxDropdown/NxDropdown.tsx @@ -29,7 +29,7 @@ const _NxDropdown = forwardRef(function NxDropdown(props, label, toggleTooltip, variant, - id: idProp, + menuId: menuIdProp, ...otherProps } = props; @@ -39,7 +39,7 @@ const _NxDropdown = forwardRef(function NxDropdown(props, const toggleTooltipProps = toggleTooltip && wrapTooltipProps(toggleTooltip); - const id = useUniqueId('nx-dropdown', idProp || undefined); + const menuId = useUniqueId('nx-dropdown', menuIdProp || undefined); // Wrap .nx-dropdown-button and .nx-dropdown-link children in overflow tooltips const wrappedChildren = children && React.Children.map(children, child => ( @@ -57,7 +57,7 @@ const _NxDropdown = forwardRef(function NxDropdown(props, onClick={!disabled && onToggleCollapse || undefined} aria-haspopup="true" aria-expanded={isOpen} - aria-controls={id}> + aria-controls={menuId}> { label } @@ -72,7 +72,7 @@ const _NxDropdown = forwardRef(function NxDropdown(props, disabled={disabled} renderToggleElement={renderToggleElement} ref={ref} - menuId={id} + menuId={menuId} { ...otherProps } > { wrappedChildren } diff --git a/lib/src/components/NxDropdown/__tests__/AbstractDropdown.test.tsx b/lib/src/components/NxDropdown/__tests__/AbstractDropdown.test.tsx index 7270decaf9..2570ff585d 100644 --- a/lib/src/components/NxDropdown/__tests__/AbstractDropdown.test.tsx +++ b/lib/src/components/NxDropdown/__tests__/AbstractDropdown.test.tsx @@ -6,6 +6,7 @@ */ import React from 'react'; import * as enzymeUtils from '../../../__testutils__/enzymeUtils'; +import NxDropdownMenu from '../../NxDropdownMenu/NxDropdownMenu'; import AbstractDropdown, { AbstractDropdownProps, @@ -45,6 +46,13 @@ describe('AbstractDropdown', () => { ); + it('passes menuId to dropdown menu id', function() { + const menuId = 'foo'; + const component = getShallowComponent({ renderToggleElement, isOpen: true, menuId }); + const menu = component.find(NxDropdownMenu); + expect(menu).toHaveProp('id', menuId); + }); + it('renders toggleElement and calls onToggleCollapse when toggleElement is clicked', function() { const onToggleCollapse = jest.fn(); diff --git a/lib/src/components/NxDropdown/__tests__/NxDropdown.test.tsx b/lib/src/components/NxDropdown/__tests__/NxDropdown.test.tsx index 5ba5339902..f0aa64cbe5 100644 --- a/lib/src/components/NxDropdown/__tests__/NxDropdown.test.tsx +++ b/lib/src/components/NxDropdown/__tests__/NxDropdown.test.tsx @@ -56,6 +56,26 @@ describe('NxDropdown', () => { expect(icon).toHaveProp('icon', faCaretDown); }); + it('sets toggle button aria-controls and menu id with unique id if menuId is not specified', function () { + const component = getMountedComponent({ isOpen: true }); + const dropdown = component.find(AbstractDropdown); + const button = dropdown.find(NxButton); + + const menuId = component.find(NxDropdownMenu).prop('id'); + expect(button).toHaveProp('aria-controls', menuId); + }); + + it('sets toggle button aria-controls and menu id with specified menuId', function() { + const menuId = 'foo'; + const component = getMountedComponent({ isOpen: true, menuId }); + const dropdown = component.find(AbstractDropdown); + const button = dropdown.find(NxButton); + const menu = component.find(NxDropdownMenu); + + expect(button).toHaveProp('aria-controls', menuId); + expect(menu).toHaveProp('id', menuId); + }); + it('renders the button according to the supplied variant', function() { let component = getMountedComponent(); expect(component.find(NxButton)).toHaveProp('variant', 'tertiary'); diff --git a/lib/src/components/NxDropdown/types.ts b/lib/src/components/NxDropdown/types.ts index c1ce179107..a9b0ab0aed 100644 --- a/lib/src/components/NxDropdown/types.ts +++ b/lib/src/components/NxDropdown/types.ts @@ -37,6 +37,7 @@ export type Props = Omit, 'className'> & { onCloseClick?: ((e: MouseEvent) => void) | null; toggleTooltip?: TooltipConfigProps | string | null; menuRef?: Ref; + menuId?: string; }; export const propTypes: WeakValidationMap = { From fc40f5d2b2658edfe09a43d72edbc024bb41661f Mon Sep 17 00:00:00 2001 From: Andrew Prasetya Date: Tue, 8 Nov 2022 16:13:55 -0500 Subject: [PATCH 07/25] Fixed a11y tests. - RSC-989 --- .../src/components/NxDropdown/NxDropdownPage.tsx | 9 +++++++++ .../NxSegmentedButtonCloseHandlerExample.tsx | 4 ++-- .../NxSegmentedButtonPrimaryExample.tsx | 6 +++--- .../NxSegmentedButtonSecondaryExample.tsx | 6 +++--- .../NxSegmentedButtonTertiaryExample.tsx | 6 +++--- .../NxStatefulSegmentedButtonExample.tsx | 4 ++-- ...looks-right-when-focused-via-keyboard-1-snap.png | 3 +++ gallery/visualtests/nxDropdown.js | 13 ++++++++++++- lib/src/components/NxCheckbox/NxCheckbox.tsx | 2 ++ lib/src/components/NxCheckbox/types.ts | 1 + .../NxFilterDropdown/NxFilterDropdown.tsx | 2 +- .../NxSearchDropdown/NxSearchDropdown.tsx | 4 +++- 12 files changed, 44 insertions(+), 16 deletions(-) create mode 100644 gallery/visualtests/__image_snapshots__/nx-dropdown-js-nx-dropdown-nx-dropdown-links-looks-right-when-focused-via-keyboard-1-snap.png diff --git a/gallery/src/components/NxDropdown/NxDropdownPage.tsx b/gallery/src/components/NxDropdown/NxDropdownPage.tsx index c2e0c868a9..5534f76880 100644 --- a/gallery/src/components/NxDropdown/NxDropdownPage.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownPage.tsx @@ -150,6 +150,15 @@ const NxDropdownPage = () => dropdown not to close. + + menuId + string + No + + By default aria-controls and the menu is given a unique id.{' '} + This prop allows you to override that id. + + HTML <div> Attributes diff --git a/gallery/src/components/NxSegmentedButton/NxSegmentedButtonCloseHandlerExample.tsx b/gallery/src/components/NxSegmentedButton/NxSegmentedButtonCloseHandlerExample.tsx index 1f5cb70966..47f18aced3 100644 --- a/gallery/src/components/NxSegmentedButton/NxSegmentedButtonCloseHandlerExample.tsx +++ b/gallery/src/components/NxSegmentedButton/NxSegmentedButtonCloseHandlerExample.tsx @@ -24,10 +24,10 @@ export default function NxSegmentedButtonPrimaryExample() { buttonContent="Click Here" onCloseClick={(evt: MouseEvent) => evt.preventDefault()} onCloseKeyDown={(evt: KeyboardEvent) => evt.preventDefault()}> - - diff --git a/gallery/src/components/NxSegmentedButton/NxSegmentedButtonPrimaryExample.tsx b/gallery/src/components/NxSegmentedButton/NxSegmentedButtonPrimaryExample.tsx index daf7b33553..885b1e6682 100644 --- a/gallery/src/components/NxSegmentedButton/NxSegmentedButtonPrimaryExample.tsx +++ b/gallery/src/components/NxSegmentedButton/NxSegmentedButtonPrimaryExample.tsx @@ -22,10 +22,10 @@ export default function NxSegmentedButtonPrimaryExample() { onToggleOpen={onToggleOpen} onClick={onMainClick} buttonContent="Click Here"> - - @@ -36,7 +36,7 @@ export default function NxSegmentedButtonPrimaryExample() { onToggleOpen={onToggleOpen} onClick={onMainClick} buttonContent="Disabled Primary Button"> - diff --git a/gallery/src/components/NxSegmentedButton/NxSegmentedButtonSecondaryExample.tsx b/gallery/src/components/NxSegmentedButton/NxSegmentedButtonSecondaryExample.tsx index 5018f3c410..52c6dca1fd 100644 --- a/gallery/src/components/NxSegmentedButton/NxSegmentedButtonSecondaryExample.tsx +++ b/gallery/src/components/NxSegmentedButton/NxSegmentedButtonSecondaryExample.tsx @@ -22,10 +22,10 @@ export default function NxSegmentedButtonSecondaryExample() { onToggleOpen={onToggleOpen} onClick={onMainClick} buttonContent="Click Here"> - - @@ -36,7 +36,7 @@ export default function NxSegmentedButtonSecondaryExample() { onToggleOpen={onToggleOpen} onClick={onMainClick} buttonContent="Disabled Primary Button"> - diff --git a/gallery/src/components/NxSegmentedButton/NxSegmentedButtonTertiaryExample.tsx b/gallery/src/components/NxSegmentedButton/NxSegmentedButtonTertiaryExample.tsx index 2d2ae2b8c7..eaf8248c40 100644 --- a/gallery/src/components/NxSegmentedButton/NxSegmentedButtonTertiaryExample.tsx +++ b/gallery/src/components/NxSegmentedButton/NxSegmentedButtonTertiaryExample.tsx @@ -22,10 +22,10 @@ export default function NxSegmentedButtonTertiaryExample() { onToggleOpen={onToggleOpen} onClick={onMainClick} buttonContent="Click Here"> - - @@ -36,7 +36,7 @@ export default function NxSegmentedButtonTertiaryExample() { onToggleOpen={onToggleOpen} onClick={onMainClick} buttonContent="Disabled Primary Button"> - diff --git a/gallery/src/components/NxStatefulSegmentedButton/NxStatefulSegmentedButtonExample.tsx b/gallery/src/components/NxStatefulSegmentedButton/NxStatefulSegmentedButtonExample.tsx index be5c0c7129..bdb05f1f08 100644 --- a/gallery/src/components/NxStatefulSegmentedButton/NxStatefulSegmentedButtonExample.tsx +++ b/gallery/src/components/NxStatefulSegmentedButton/NxStatefulSegmentedButtonExample.tsx @@ -15,10 +15,10 @@ export default function NxStatefulSegmentedButtonPrimaryExample() { return ( - - diff --git a/gallery/visualtests/__image_snapshots__/nx-dropdown-js-nx-dropdown-nx-dropdown-links-looks-right-when-focused-via-keyboard-1-snap.png b/gallery/visualtests/__image_snapshots__/nx-dropdown-js-nx-dropdown-nx-dropdown-links-looks-right-when-focused-via-keyboard-1-snap.png new file mode 100644 index 0000000000..e017e4a43d --- /dev/null +++ b/gallery/visualtests/__image_snapshots__/nx-dropdown-js-nx-dropdown-nx-dropdown-links-looks-right-when-focused-via-keyboard-1-snap.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d688350386a120a3bc002f8c2775ebfb32f4b6f4e7e3bb045adabdaf4726ecca +size 13732 diff --git a/gallery/visualtests/nxDropdown.js b/gallery/visualtests/nxDropdown.js index 02c4e2e2a5..ad5852419b 100644 --- a/gallery/visualtests/nxDropdown.js +++ b/gallery/visualtests/nxDropdown.js @@ -16,7 +16,8 @@ describe('NxDropdown', function() { waitAndGetElements, moveMouseAway, checkScreenshot, - a11yTest + a11yTest, + getPage } = setupBrowser('#/pages/Dropdown'); const defaultSelector = '#nx-dropdown-scrolling-example .nx-dropdown', @@ -86,6 +87,16 @@ describe('NxDropdown', function() { await checkScreenshot(targetElement, 251, 218); }); + + it('looks right when focused via keyboard', async function() { + const [targetElement] = await waitAndGetElements(selector); + const page = getPage(); + + await moveMouseAway(); + await page.keyboard.press('ArrowLeft'); + + await checkScreenshot(targetElement, 251, 218); + }); }); describe('NxDropdown with icon', function() { diff --git a/lib/src/components/NxCheckbox/NxCheckbox.tsx b/lib/src/components/NxCheckbox/NxCheckbox.tsx index 15fc92f3d4..3667a24335 100644 --- a/lib/src/components/NxCheckbox/NxCheckbox.tsx +++ b/lib/src/components/NxCheckbox/NxCheckbox.tsx @@ -35,6 +35,7 @@ const NxCheckbox = forwardRef( checkboxId, overflowTooltip, children, + checkboxRole, inputAttributes = {}, ...otherProps } = props, @@ -63,6 +64,7 @@ const NxCheckbox = forwardRef( disabled={!!disabled} checked={isChecked} readOnly={!onChange} + role={checkboxRole} onChange={onChange || undefined} { ...otherInputAttributes } /> diff --git a/lib/src/components/NxCheckbox/types.ts b/lib/src/components/NxCheckbox/types.ts index 01b8094df3..3e9c9cb322 100644 --- a/lib/src/components/NxCheckbox/types.ts +++ b/lib/src/components/NxCheckbox/types.ts @@ -18,6 +18,7 @@ export type Props = Omit, 'onChange'> & { disabled?: boolean | null; overflowTooltip?: boolean | null; inputAttributes?: InputAttributesProp; + checkboxRole?: string; }; // In a strictly typescript environment, PropTypes are mostly redundant. However, they still provide safety when this diff --git a/lib/src/components/NxFilterDropdown/NxFilterDropdown.tsx b/lib/src/components/NxFilterDropdown/NxFilterDropdown.tsx index 5ddeb50c37..a310c188a1 100644 --- a/lib/src/components/NxFilterDropdown/NxFilterDropdown.tsx +++ b/lib/src/components/NxFilterDropdown/NxFilterDropdown.tsx @@ -77,7 +77,7 @@ function NxFilterDropdownRender(props: Props toggle(id)} isChecked={selectedIds.has(id)} - role="menuitemcheckbox"> + checkboxRole="menuitemcheckbox"> {displayName} ); diff --git a/lib/src/components/NxSearchDropdown/NxSearchDropdown.tsx b/lib/src/components/NxSearchDropdown/NxSearchDropdown.tsx index 76b72937eb..8b8d4663f9 100644 --- a/lib/src/components/NxSearchDropdown/NxSearchDropdown.tsx +++ b/lib/src/components/NxSearchDropdown/NxSearchDropdown.tsx @@ -192,9 +192,11 @@ function NxSearchDropdownRender( className={menuClassName} onClosing={() => {}} onKeyDown={handleButtonKeyDown} + disableMenuKeyNav={true} aria-busy={!!loading} aria-live="polite" - aria-hidden={!showDropdown}> + aria-hidden={!showDropdown} + tabIndex={-1}> doSearch(searchText)}> { matches.length ? matches.map((match, i) => From 3353a939799a2e11b4606976a08be82e902252d9 Mon Sep 17 00:00:00 2001 From: Andrew Prasetya Date: Thu, 10 Nov 2022 12:52:21 -0500 Subject: [PATCH 08/25] Added convinience components and withWrapper (in progress). - RSC-989 --- .../NxDropdownCloseHandlerExample.tsx | 8 +-- .../NxDropdownCustomLabelExample.tsx | 4 +- .../NxDropdown/NxDropdownLinksExample.tsx | 32 +++++----- .../NxDropdownNavigationExample.tsx | 28 ++++----- .../NxDropdownRightButtonsExample.tsx | 16 ++--- .../NxDropdown/NxDropdownScrollingExample.tsx | 59 +++++++++---------- .../NxDropdown/NxDropdownShortExample.tsx | 16 ++--- ...NxDropdownWithNxThreatIndicatorExample.tsx | 24 ++++---- lib/src/components/NxDropdown/NxDropdown.tsx | 5 +- lib/src/util/withClass.tsx | 8 ++- 10 files changed, 100 insertions(+), 100 deletions(-) diff --git a/gallery/src/components/NxDropdown/NxDropdownCloseHandlerExample.tsx b/gallery/src/components/NxDropdown/NxDropdownCloseHandlerExample.tsx index 9676fcc532..e84f02eb57 100644 --- a/gallery/src/components/NxDropdown/NxDropdownCloseHandlerExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownCloseHandlerExample.tsx @@ -18,12 +18,12 @@ function NxDropdownCloseHandlerExample() { onToggleCollapse={onToggleCollapse} onCloseClick={(evt: MouseEvent) => evt.preventDefault()} onCloseKeyDown={(evt: KeyboardEvent) => evt.preventDefault()}> - + Link - - + ); } diff --git a/gallery/src/components/NxDropdown/NxDropdownCustomLabelExample.tsx b/gallery/src/components/NxDropdown/NxDropdownCustomLabelExample.tsx index e801496b86..e506896a79 100644 --- a/gallery/src/components/NxDropdown/NxDropdownCustomLabelExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownCustomLabelExample.tsx @@ -24,10 +24,10 @@ function NxDropdownCustomLabelExample() { className="extra-class" isOpen={isOpen} onToggleCollapse={onToggleCollapse}> - + ); } diff --git a/gallery/src/components/NxDropdown/NxDropdownLinksExample.tsx b/gallery/src/components/NxDropdown/NxDropdownLinksExample.tsx index 16614befad..9aee799bd6 100644 --- a/gallery/src/components/NxDropdown/NxDropdownLinksExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownLinksExample.tsx @@ -17,32 +17,28 @@ function NxDropdownActionsExample() { className="extra-class" isOpen={isOpen} onToggleCollapse={onToggleCollapse}> - + Go somewhere - - + + Go somewhere else - - evt.preventDefault()}> + + evt.preventDefault()}> Can't go here though - - + + Go to homepage - - evt.preventDefault()} - role="menuitem"> + + evt.preventDefault()}> Can't go here either - + ); } diff --git a/gallery/src/components/NxDropdown/NxDropdownNavigationExample.tsx b/gallery/src/components/NxDropdown/NxDropdownNavigationExample.tsx index 86ad1b4f8e..8defb53b60 100644 --- a/gallery/src/components/NxDropdown/NxDropdownNavigationExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownNavigationExample.tsx @@ -14,28 +14,28 @@ function NxDropdownNavigationExample() { return ( - + Text link 1 - - + + Text link 2 - - + + Text link 3 - this link should trigger truncation - - - + - - + ); } diff --git a/gallery/src/components/NxDropdown/NxDropdownRightButtonsExample.tsx b/gallery/src/components/NxDropdown/NxDropdownRightButtonsExample.tsx index 819e138516..9867ba0657 100644 --- a/gallery/src/components/NxDropdown/NxDropdownRightButtonsExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownRightButtonsExample.tsx @@ -14,14 +14,14 @@ function NxDropdownRightButtonsExample() { return ( - + Text Link - - + + Text Link - this link should trigger truncation - + alert('icon click')} className="nx-dropdown-right-button" variant="icon-only" @@ -29,9 +29,9 @@ function NxDropdownRightButtonsExample() { role="menuitem"> - + Button Link2 - + alert('icon click')} className="nx-dropdown-right-button" variant="icon-only" @@ -39,9 +39,9 @@ function NxDropdownRightButtonsExample() { role="menuitem"> - + Text Link3 - this link should trigger truncation - + ); } diff --git a/gallery/src/components/NxDropdown/NxDropdownScrollingExample.tsx b/gallery/src/components/NxDropdown/NxDropdownScrollingExample.tsx index 4f75fcc7cf..7becbeaccf 100644 --- a/gallery/src/components/NxDropdown/NxDropdownScrollingExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownScrollingExample.tsx @@ -15,49 +15,48 @@ function NxDropdownNavigationExample() { - + Text Link 1 - - + + Text Link 2 - - + + Text Link 3 - - + + Text Link 4 - - + + Text Link 5 - - + + Text Link 6 - - { evt.preventDefault(); }} - className="disabled nx-dropdown-button" - aria-disabled="true" - role="menuitem"> + + { evt.preventDefault(); }} + className="disabled" + aria-disabled="true"> Text Link 7 Disabled - - + + Text Link 8 - - + + Text Link 9 - - + + Text Link 10 - - + + Text Link 11 - - + + Text Link 12 - - + + Text Link 13 - + ); } diff --git a/gallery/src/components/NxDropdown/NxDropdownShortExample.tsx b/gallery/src/components/NxDropdown/NxDropdownShortExample.tsx index 132a6294ce..a90d0ade7c 100644 --- a/gallery/src/components/NxDropdown/NxDropdownShortExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownShortExample.tsx @@ -14,18 +14,18 @@ function NxDropdownNavigationExample() { return ( - - - - + ); } diff --git a/gallery/src/components/NxDropdown/NxDropdownWithNxThreatIndicatorExample.tsx b/gallery/src/components/NxDropdown/NxDropdownWithNxThreatIndicatorExample.tsx index 003b9bb600..cc4f91d701 100644 --- a/gallery/src/components/NxDropdown/NxDropdownWithNxThreatIndicatorExample.tsx +++ b/gallery/src/components/NxDropdown/NxDropdownWithNxThreatIndicatorExample.tsx @@ -22,30 +22,30 @@ function NxDropdownWithNxThreatIndicatorExample() { - - - - - - + ); } diff --git a/lib/src/components/NxDropdown/NxDropdown.tsx b/lib/src/components/NxDropdown/NxDropdown.tsx index 7a63c7532f..34b7803881 100644 --- a/lib/src/components/NxDropdown/NxDropdown.tsx +++ b/lib/src/components/NxDropdown/NxDropdown.tsx @@ -83,7 +83,10 @@ const _NxDropdown = forwardRef(function NxDropdown(props, _NxDropdown.propTypes = propTypes; const NxDropdown = Object.assign(_NxDropdown, { - Divider: withClass('hr', 'nx-dropdown__divider') + Divider: withClass('hr', 'nx-dropdown__divider'), + Button: withClass('button', 'nx-dropdown-button', 'menuitem', NxOverflowTooltip), + Link: withClass('a', 'nx-dropdown-link', 'menuitem', NxOverflowTooltip), + LinkButton: withClass('a', 'nx-dropdown-button', 'menuitem', NxOverflowTooltip) }); export default NxDropdown; diff --git a/lib/src/util/withClass.tsx b/lib/src/util/withClass.tsx index 9ff48bab33..458369010c 100644 --- a/lib/src/util/withClass.tsx +++ b/lib/src/util/withClass.tsx @@ -4,7 +4,7 @@ * the terms of the Eclipse Public License 2.0 which accompanies this * distribution and is available at https://www.eclipse.org/legal/epl-2.0/. */ -import React, { forwardRef, DetailedHTMLProps, SVGProps } from 'react'; +import React, { forwardRef, DetailedHTMLProps, SVGProps, ElementType } from 'react'; import classnames from 'classnames'; type NativeElType = @@ -16,7 +16,8 @@ type NativeElType = export default function withClass( El: E, withClassName: string, - withRole?: string + withRole?: string, + Wrapper?: ElementType ) { return forwardRef, JSX.IntrinsicElements[E]>((props: JSX.IntrinsicElements[E], ref) => { const { @@ -24,6 +25,7 @@ export default function withClass( ...otherProps } = props; const classes = classnames(withClassName, className); - return React.createElement(El, { className: classes, role: withRole, ref, ...otherProps}); + const el = React.createElement(El, { className: classes, role: withRole, ref, ...otherProps}); + return Wrapper ? {el} : el; }); } From 1ee7a6f8f1584d3335d97084cd92a59e3e878d2a Mon Sep 17 00:00:00 2001 From: Andrew Prasetya Date: Thu, 10 Nov 2022 12:58:51 -0500 Subject: [PATCH 09/25] Use inputAttributes. - RSC-989 --- lib/src/components/NxCheckbox/NxCheckbox.tsx | 2 -- lib/src/components/NxCheckbox/types.ts | 1 - lib/src/components/NxFilterDropdown/NxFilterDropdown.tsx | 4 +++- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/src/components/NxCheckbox/NxCheckbox.tsx b/lib/src/components/NxCheckbox/NxCheckbox.tsx index 3667a24335..15fc92f3d4 100644 --- a/lib/src/components/NxCheckbox/NxCheckbox.tsx +++ b/lib/src/components/NxCheckbox/NxCheckbox.tsx @@ -35,7 +35,6 @@ const NxCheckbox = forwardRef( checkboxId, overflowTooltip, children, - checkboxRole, inputAttributes = {}, ...otherProps } = props, @@ -64,7 +63,6 @@ const NxCheckbox = forwardRef( disabled={!!disabled} checked={isChecked} readOnly={!onChange} - role={checkboxRole} onChange={onChange || undefined} { ...otherInputAttributes } /> diff --git a/lib/src/components/NxCheckbox/types.ts b/lib/src/components/NxCheckbox/types.ts index 3e9c9cb322..01b8094df3 100644 --- a/lib/src/components/NxCheckbox/types.ts +++ b/lib/src/components/NxCheckbox/types.ts @@ -18,7 +18,6 @@ export type Props = Omit, 'onChange'> & { disabled?: boolean | null; overflowTooltip?: boolean | null; inputAttributes?: InputAttributesProp; - checkboxRole?: string; }; // In a strictly typescript environment, PropTypes are mostly redundant. However, they still provide safety when this diff --git a/lib/src/components/NxFilterDropdown/NxFilterDropdown.tsx b/lib/src/components/NxFilterDropdown/NxFilterDropdown.tsx index a310c188a1..4455026361 100644 --- a/lib/src/components/NxFilterDropdown/NxFilterDropdown.tsx +++ b/lib/src/components/NxFilterDropdown/NxFilterDropdown.tsx @@ -77,7 +77,9 @@ function NxFilterDropdownRender(props: Props toggle(id)} isChecked={selectedIds.has(id)} - checkboxRole="menuitemcheckbox"> + inputAttributes={{ + role: 'menuitemcheckbox' + }}> {displayName} ); From 775db95bf7ab28a50885f34ce1cb434581a36390 Mon Sep 17 00:00:00 2001 From: Andrew Prasetya Date: Thu, 10 Nov 2022 13:00:58 -0500 Subject: [PATCH 10/25] Update lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx Co-authored-by: Ross Pokorny --- lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx b/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx index b5f11d43f9..a8a9997018 100644 --- a/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx +++ b/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx @@ -14,9 +14,7 @@ import './NxDropdownMenu.scss'; export { Props }; -const FOCUSABLE_MENU_ITEMS_SELECTOR = 'a:not([disabled]):not(.disabled), ' -+ 'button:not([disabled]):not(.disabled), ' -+ 'input:not([disabled]):not(.disabled)'; +const FOCUSABLE_MENU_ITEMS_SELECTOR = ':is(a, button, input):not([disabled], .disabled])'; /** * This component is not currently intended for public export. It is a helper for NxDropdown and NxSegmentedButton From d9fb143850b74df0bc73bc5fb767cc6b9f607bdd Mon Sep 17 00:00:00 2001 From: Andrew Prasetya Date: Thu, 10 Nov 2022 13:03:23 -0500 Subject: [PATCH 11/25] Allow null for menuId. - RSC-989 --- lib/src/components/NxDropdown/types.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/src/components/NxDropdown/types.ts b/lib/src/components/NxDropdown/types.ts index a9b0ab0aed..627ba5ce90 100644 --- a/lib/src/components/NxDropdown/types.ts +++ b/lib/src/components/NxDropdown/types.ts @@ -22,7 +22,7 @@ export interface AbstractDropdownProps extends HTMLAttributes { onCloseKeyDown?: KeyboardEventHandler | null; onCloseClick?: ((e: MouseEvent) => void) | null; menuRef?: Ref; - menuId?: string; + menuId?: string | null; } export type Props = Omit, 'className'> & { @@ -37,7 +37,7 @@ export type Props = Omit, 'className'> & { onCloseClick?: ((e: MouseEvent) => void) | null; toggleTooltip?: TooltipConfigProps | string | null; menuRef?: Ref; - menuId?: string; + menuId?: string | null; }; export const propTypes: WeakValidationMap = { @@ -56,5 +56,6 @@ export const propTypes: WeakValidationMap = { onToggleCollapse: PropTypes.func, onCloseKeyDown: PropTypes.func, onCloseClick: PropTypes.func, - toggleTooltip: PropTypes.oneOfType([tooltipPropTypesShape, PropTypes.string]) + toggleTooltip: PropTypes.oneOfType([tooltipPropTypesShape, PropTypes.string]), + menuId: PropTypes.string }; From a5b3689dc346ae89224a1a2a2ebff5674d127e19 Mon Sep 17 00:00:00 2001 From: Andrew Prasetya Date: Mon, 14 Nov 2022 09:31:18 -0500 Subject: [PATCH 12/25] Fixed menuId. - RSC-989 --- lib/src/components/NxDropdown/AbstractDropdown.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/components/NxDropdown/AbstractDropdown.tsx b/lib/src/components/NxDropdown/AbstractDropdown.tsx index 30469ef58e..39a5191fea 100644 --- a/lib/src/components/NxDropdown/AbstractDropdown.tsx +++ b/lib/src/components/NxDropdown/AbstractDropdown.tsx @@ -143,7 +143,7 @@ const AbstractDropdown = forwardRef((prop
{ renderToggleElement(toggleRef, onToggleCollapse) } { isOpen && - + { children } } From 10a903129a12e16a2689b12586b7ddb2fa05f4c3 Mon Sep 17 00:00:00 2001 From: Andrew Prasetya Date: Tue, 15 Nov 2022 16:34:06 -0500 Subject: [PATCH 13/25] Progress on moving keyboard nav logic to DropdownMenu. - RSC-989 --- lib/src/components/NxCombobox/NxCombobox.tsx | 37 +++--- .../NxDropdown/AbstractDropdown.tsx | 5 +- .../NxDropdownMenu/NxDropdownMenu.tsx | 111 +++++++++++++----- lib/src/components/NxDropdownMenu/types.tsx | 1 + .../NxSearchDropdown/NxSearchDropdown.tsx | 87 +++++++------- 5 files changed, 148 insertions(+), 93 deletions(-) diff --git a/lib/src/components/NxCombobox/NxCombobox.tsx b/lib/src/components/NxCombobox/NxCombobox.tsx index be551dc6e2..90b9444dd0 100644 --- a/lib/src/components/NxCombobox/NxCombobox.tsx +++ b/lib/src/components/NxCombobox/NxCombobox.tsx @@ -185,23 +185,23 @@ function NxComboboxRender {}} aria-label="listbox of combobox" - disableMenuKeyNav={true}> + > { matches.length && matches.map((match, i) => diff --git a/lib/src/components/NxDropdown/AbstractDropdown.tsx b/lib/src/components/NxDropdown/AbstractDropdown.tsx index 39a5191fea..c0dd58d891 100644 --- a/lib/src/components/NxDropdown/AbstractDropdown.tsx +++ b/lib/src/components/NxDropdown/AbstractDropdown.tsx @@ -143,7 +143,10 @@ const AbstractDropdown = forwardRef((prop
{ renderToggleElement(toggleRef, onToggleCollapse) } { isOpen && - + { children } } diff --git a/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx b/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx index a8a9997018..c1d67efd9c 100644 --- a/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx +++ b/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx @@ -7,6 +7,7 @@ import React, { useLayoutEffect, forwardRef, useState, useEffect, useRef, KeyboardEventHandler } from 'react'; import classnames from 'classnames'; import useMergedRef from '@react-hook/merged-ref'; +import { always, dec, inc } from 'ramda'; import { Props, propTypes } from './types'; @@ -14,7 +15,8 @@ import './NxDropdownMenu.scss'; export { Props }; -const FOCUSABLE_MENU_ITEMS_SELECTOR = ':is(a, button, input):not([disabled], .disabled])'; +const FOCUSABLE_MENU_ITEMS_SELECTOR = ':is(a, button, input)'; +const ACTIVE_FOCUSABLE_MENU_ITEMS_SELECTOR = `${FOCUSABLE_MENU_ITEMS_SELECTOR}:not([disabled], .disabled)`; /** * This component is not currently intended for public export. It is a helper for NxDropdown and NxSegmentedButton @@ -22,51 +24,86 @@ const FOCUSABLE_MENU_ITEMS_SELECTOR = ':is(a, button, input):not([disabled], .di */ /* eslint-disable-next-line react/prop-types */ const NxDropdownMenu = forwardRef(function NxDropdownMenu(props, ref) { - const { onClosing, className: classNameProp, children, disableMenuKeyNav, ...attrs } = props; + const { + onClosing, + onKeyDown: onKeyDownProp, + className: + classNameProp, + children, + disableMenuKeyNav, + toggleElement, + ...attrs + } = props; const menuRef = useRef(null); const mergedRef = useMergedRef(menuRef, ref); const [focusedMenuItemIndex, setFocusedMenuItemIndex] = useState(0); + const setMenuItemFocus = (adjust: (i: number) => number) => () => { + if (menuRef.current) { + const focusableElements = menuRef.current.querySelectorAll(ACTIVE_FOCUSABLE_MENU_ITEMS_SELECTOR); + const adjustedMenuItemIndex = adjust(focusedMenuItemIndex ?? 0); + + let newMenuItemIndex = adjustedMenuItemIndex; + + if (adjustedMenuItemIndex < 0) { + newMenuItemIndex = focusableElements.length - 1; + } + else if (adjustedMenuItemIndex >= focusableElements.length) { + newMenuItemIndex = 0; + } + + const elementToFocus = focusableElements[newMenuItemIndex]; + if (elementToFocus) { + elementToFocus.focus(); + setFocusedMenuItemIndex(newMenuItemIndex); + } + } + }, + focusNext = setMenuItemFocus(inc), + focusPrev = setMenuItemFocus(dec), + focusFirst = setMenuItemFocus(always(0)), + focusLast = setMenuItemFocus(always(-1)); + // onClosing must execute when this element is being removed but BEFORE it actually gets removed from the DOM useLayoutEffect(() => onClosing, []); useEffect(() => { - if (!disableMenuKeyNav && menuRef.current) { - menuRef.current.focus(); - } - }, [disableMenuKeyNav]); - - function setFocusToMenuItem(direction: -1 | 1) { if (menuRef.current) { - const focusableEls = menuRef.current.querySelectorAll(FOCUSABLE_MENU_ITEMS_SELECTOR); - - let newFocusedMenuItemIndex = 0; - - if (direction === -1) { - newFocusedMenuItemIndex = (focusedMenuItemIndex - 1 >= 0) ? focusedMenuItemIndex - 1 : focusableEls.length - 1; - - if (document.activeElement === menuRef.current) { - newFocusedMenuItemIndex = focusableEls.length - 1; - } - } - else if (direction === 1) { - newFocusedMenuItemIndex = (focusedMenuItemIndex + 1 >= focusableEls.length) ? 0 : focusedMenuItemIndex + 1; + const focusableElements = menuRef.current.querySelectorAll(FOCUSABLE_MENU_ITEMS_SELECTOR); + focusableElements.forEach(menuItem => menuItem.tabIndex = -1); + } + }, [menuRef]); - if (document.activeElement === menuRef.current) { - newFocusedMenuItemIndex = 0; - } + useEffect(() => { + const handleKeyDown = (event: KeyboardEvent) => { + switch (event.key) { + case 'ArrowUp': + event.preventDefault(); + focusLast(); + break; + case 'ArrowDown': + event.preventDefault(); + focusFirst(); + break; } + }; - setFocusedMenuItemIndex(newFocusedMenuItemIndex); - focusableEls[newFocusedMenuItemIndex].focus(); + if (!disableMenuKeyNav && toggleElement) { + toggleElement.addEventListener('keydown', handleKeyDown); } - } + + return () => { + if (!disableMenuKeyNav && toggleElement) { + toggleElement.removeEventListener('keydown', handleKeyDown); + } + }; + }, [toggleElement, disableMenuKeyNav]); function activateMenuItem(event: React.KeyboardEvent) { if (menuRef.current) { - const focusableEls = menuRef.current.querySelectorAll(FOCUSABLE_MENU_ITEMS_SELECTOR); + const focusableEls = menuRef.current.querySelectorAll(ACTIVE_FOCUSABLE_MENU_ITEMS_SELECTOR); const currentFocusedEl = focusableEls[focusedMenuItemIndex]; if (currentFocusedEl.matches('a, button')) { event.preventDefault(); @@ -78,27 +115,39 @@ const NxDropdownMenu = forwardRef(function NxDropdownMenu const handleKeyDown: KeyboardEventHandler = (event) => { if (!disableMenuKeyNav) { switch (event.key) { + case 'Home': + focusFirst(); + event.preventDefault(); + break; + case 'End': + focusLast(); + event.preventDefault(); + break; case 'ArrowUp': + focusPrev(); event.preventDefault(); - setFocusToMenuItem(-1); break; case 'ArrowDown': + focusNext(); event.preventDefault(); - setFocusToMenuItem(1); break; case ' ': case 'Enter': activateMenuItem(event); break; } } + + if (onKeyDownProp) { + onKeyDownProp(event); + } }; const className = classnames('nx-dropdown-menu', classNameProp); return ( + // eslint-disable-next-line jsx-a11y/interactive-supports-focus
{ children } diff --git a/lib/src/components/NxDropdownMenu/types.tsx b/lib/src/components/NxDropdownMenu/types.tsx index 85b11b1d89..1f021a8fa4 100644 --- a/lib/src/components/NxDropdownMenu/types.tsx +++ b/lib/src/components/NxDropdownMenu/types.tsx @@ -10,6 +10,7 @@ import * as PropTypes from 'prop-types'; export interface Props extends HTMLAttributes { onClosing: () => void; disableMenuKeyNav?: boolean | null; + toggleElement?: HTMLElement | null; } export const propTypes: PropTypes.ValidationMap = { diff --git a/lib/src/components/NxSearchDropdown/NxSearchDropdown.tsx b/lib/src/components/NxSearchDropdown/NxSearchDropdown.tsx index 8b8d4663f9..76ee43c13a 100644 --- a/lib/src/components/NxSearchDropdown/NxSearchDropdown.tsx +++ b/lib/src/components/NxSearchDropdown/NxSearchDropdown.tsx @@ -7,7 +7,7 @@ import React, { FocusEvent, KeyboardEvent, Ref, useCallback, useEffect, useRef, useState } from 'react'; import useMergedRef from '@react-hook/merged-ref'; import classnames from 'classnames'; -import { always, any, clamp, dec, inc, partial, pipe, prop } from 'ramda'; +import { any, clamp, partial, pipe, prop } from 'ramda'; import './NxSearchDropdown.scss'; @@ -84,39 +84,39 @@ function NxSearchDropdownRender( } } - // helper for focusing different buttons in the dropdown menu - const adjustBtnFocus = (adjust: (i: number) => number) => () => { - const newFocusableBtnIndex = adjust(focusableBtnIndex ?? 0), - elToFocus = menuRef.current?.children[newFocusableBtnIndex] as HTMLElement | null; - - if (elToFocus) { - elToFocus.focus(); - setFocusableBtnIndex(newFocusableBtnIndex); - } - }, - focusNext = adjustBtnFocus(inc), - focusPrev = adjustBtnFocus(dec), - focusFirst = adjustBtnFocus(always(0)), - focusLast = adjustBtnFocus(always(matches.length - 1)); + // // helper for focusing different buttons in the dropdown menu + // const adjustBtnFocus = (adjust: (i: number) => number) => () => { + // const newFocusableBtnIndex = adjust(focusableBtnIndex ?? 0), + // elToFocus = menuRef.current?.children[newFocusableBtnIndex] as HTMLElement | null; + + // if (elToFocus) { + // elToFocus.focus(); + // setFocusableBtnIndex(newFocusableBtnIndex); + // } + // }, + // focusNext = adjustBtnFocus(inc), + // focusPrev = adjustBtnFocus(dec), + // focusFirst = adjustBtnFocus(always(0)), + // focusLast = adjustBtnFocus(always(matches.length - 1)); function handleButtonKeyDown(evt: KeyboardEvent) { switch (evt.key) { - case 'Home': - focusFirst(); - evt.preventDefault(); - break; - case 'End': - focusLast(); - evt.preventDefault(); - break; - case 'ArrowDown': - focusNext(); - evt.preventDefault(); - break; - case 'ArrowUp': - focusPrev(); - evt.preventDefault(); - break; + // case 'Home': + // focusFirst(); + // evt.preventDefault(); + // break; + // case 'End': + // focusLast(); + // evt.preventDefault(); + // break; + // case 'ArrowDown': + // focusNext(); + // evt.preventDefault(); + // break; + // case 'ArrowUp': + // focusPrev(); + // evt.preventDefault(); + // break; case 'Escape': focusTextInput(); handleFilterChange(''); @@ -138,15 +138,15 @@ function NxSearchDropdownRender( filterRef.current?.querySelector('input')?.focus(); } - // Clamp or nullify focusableBtnIndex whenever the number of matches changes - useEffect(function() { - if (matches.length) { - setFocusableBtnIndex(clamp(0, matches.length - 1, focusableBtnIndex ?? 0)); - } - else { - setFocusableBtnIndex(null); - } - }, [matches]); + // // Clamp or nullify focusableBtnIndex whenever the number of matches changes + // useEffect(function() { + // if (matches.length) { + // setFocusableBtnIndex(clamp(0, matches.length - 1, focusableBtnIndex ?? 0)); + // } + // else { + // setFocusableBtnIndex(null); + // } + // }, [matches]); /* * Horrible Hack: When an element within the dropdown is removed from the DOM while it is focused, we want @@ -192,7 +192,7 @@ function NxSearchDropdownRender( className={menuClassName} onClosing={() => {}} onKeyDown={handleButtonKeyDown} - disableMenuKeyNav={true} + toggleElement={filterRef.current} aria-busy={!!loading} aria-live="polite" aria-hidden={!showDropdown} @@ -205,9 +205,10 @@ function NxSearchDropdownRender( className="nx-dropdown-button" disabled={disabled || undefined} key={match.id} - tabIndex={i === focusableBtnIndex ? 0 : -1} + // tabIndex={i === focusableBtnIndex ? 0 : -1} onClick={partial(onSelect, [match])} - onFocus={() => setFocusableBtnIndex(i)}> + // onFocus={() => setFocusableBtnIndex(i)}> + > {match.displayName} ) : From b5267a592166846d69488f9b42ca76bd56d552c7 Mon Sep 17 00:00:00 2001 From: Andrew Prasetya Date: Thu, 17 Nov 2022 09:54:34 -0500 Subject: [PATCH 14/25] Progress on DropdownMenu. - RSC-989 --- .../NxDropdown/AbstractDropdown.tsx | 17 ++++--- .../NxDropdownMenu/NxDropdownMenu.tsx | 46 +++++++++++++++---- lib/src/components/NxDropdownMenu/types.tsx | 3 ++ .../NxSearchDropdown/NxSearchDropdown.tsx | 8 ++-- 4 files changed, 52 insertions(+), 22 deletions(-) diff --git a/lib/src/components/NxDropdown/AbstractDropdown.tsx b/lib/src/components/NxDropdown/AbstractDropdown.tsx index c0dd58d891..f3a11432ee 100644 --- a/lib/src/components/NxDropdown/AbstractDropdown.tsx +++ b/lib/src/components/NxDropdown/AbstractDropdown.tsx @@ -21,7 +21,6 @@ export { * @param isOpen - a boolean to indicate if the dropdown is open or not. * @param className - a classname string for the div containing the toggle and dropdown. * @param disabled - If true disables toggling of dropdown. - * @param onToggleCollapse - The onToggleCollapse function that was provided from outside of the component * @param onCloseClick - A callback that should fire when a click * is detected that would result in the dropdown getting closed. The Mouse event is passed to this handler, * and if the handler calls `preventDefault` on it, the close is cancelled @@ -142,14 +141,14 @@ const AbstractDropdown = forwardRef((prop /* eslint-disable-next-line jsx-a11y/no-static-element-interactions */
{ renderToggleElement(toggleRef, onToggleCollapse) } - { isOpen && - - { children } - - } + + { children } +
); }); diff --git a/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx b/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx index c1d67efd9c..4e6c9e8773 100644 --- a/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx +++ b/lib/src/components/NxDropdownMenu/NxDropdownMenu.tsx @@ -7,7 +7,7 @@ import React, { useLayoutEffect, forwardRef, useState, useEffect, useRef, KeyboardEventHandler } from 'react'; import classnames from 'classnames'; import useMergedRef from '@react-hook/merged-ref'; -import { always, dec, inc } from 'ramda'; +import { always, dec, inc, move, reduce } from 'ramda'; import { Props, propTypes } from './types'; @@ -15,8 +15,9 @@ import './NxDropdownMenu.scss'; export { Props }; -const FOCUSABLE_MENU_ITEMS_SELECTOR = ':is(a, button, input)'; -const ACTIVE_FOCUSABLE_MENU_ITEMS_SELECTOR = `${FOCUSABLE_MENU_ITEMS_SELECTOR}:not([disabled], .disabled)`; +const FOCUSABLE_ELEMENTS = ['a', 'button', 'input']; +const FOCUSABLE_MENU_ITEMS_SELECTOR = FOCUSABLE_ELEMENTS.join(', '); +const ACTIVE_FOCUSABLE_MENU_ITEMS_SELECTOR = FOCUSABLE_ELEMENTS.join(':not([disabled], .disabled), '); /** * This component is not currently intended for public export. It is a helper for NxDropdown and NxSegmentedButton @@ -32,6 +33,9 @@ const NxDropdownMenu = forwardRef(function NxDropdownMenu children, disableMenuKeyNav, toggleElement, + isOpen, + onToggleCollapse, + onMenuItemFocus, ...attrs } = props; @@ -40,9 +44,21 @@ const NxDropdownMenu = forwardRef(function NxDropdownMenu const [focusedMenuItemIndex, setFocusedMenuItemIndex] = useState(0); + // Move this to NxDropdown... + const getFocusableMenuItems = (selector: string) => { + if (menuRef.current) { + const focusableElements = Array.from(menuRef.current.querySelectorAll(selector)); + const isRightButton = (element: Element) => element.classList.contains('nx-dropdown-right-button'); + const indicesToSwap = focusableElements.reduce((acc, element, i) => + isRightButton(element) ? [...acc, i] : acc, []); + return reduce((els, i) => move(i, i + 1, els), focusableElements, indicesToSwap); + } + return []; + }; + const setMenuItemFocus = (adjust: (i: number) => number) => () => { if (menuRef.current) { - const focusableElements = menuRef.current.querySelectorAll(ACTIVE_FOCUSABLE_MENU_ITEMS_SELECTOR); + const focusableElements = getFocusableMenuItems(ACTIVE_FOCUSABLE_MENU_ITEMS_SELECTOR); const adjustedMenuItemIndex = adjust(focusedMenuItemIndex ?? 0); let newMenuItemIndex = adjustedMenuItemIndex; @@ -56,8 +72,12 @@ const NxDropdownMenu = forwardRef(function NxDropdownMenu const elementToFocus = focusableElements[newMenuItemIndex]; if (elementToFocus) { + if (onMenuItemFocus) { + onMenuItemFocus(newMenuItemIndex, focusableElements); + } elementToFocus.focus(); setFocusedMenuItemIndex(newMenuItemIndex); + elementToFocus.scrollIntoView({ block: 'nearest' }); } } }, @@ -71,20 +91,28 @@ const NxDropdownMenu = forwardRef(function NxDropdownMenu useEffect(() => { if (menuRef.current) { - const focusableElements = menuRef.current.querySelectorAll(FOCUSABLE_MENU_ITEMS_SELECTOR); + const focusableElements = getFocusableMenuItems(FOCUSABLE_MENU_ITEMS_SELECTOR); focusableElements.forEach(menuItem => menuItem.tabIndex = -1); } - }, [menuRef]); + }, [menuRef, children]); useEffect(() => { const handleKeyDown = (event: KeyboardEvent) => { + const toggleOpen = () => { + if (!isOpen && onToggleCollapse) { + onToggleCollapse(); + } + }; + switch (event.key) { case 'ArrowUp': event.preventDefault(); + toggleOpen(); focusLast(); break; case 'ArrowDown': event.preventDefault(); + toggleOpen(); focusFirst(); break; } @@ -99,7 +127,7 @@ const NxDropdownMenu = forwardRef(function NxDropdownMenu toggleElement.removeEventListener('keydown', handleKeyDown); } }; - }, [toggleElement, disableMenuKeyNav]); + }, [toggleElement, disableMenuKeyNav, isOpen, onToggleCollapse]); function activateMenuItem(event: React.KeyboardEvent) { if (menuRef.current) { @@ -144,7 +172,7 @@ const NxDropdownMenu = forwardRef(function NxDropdownMenu const className = classnames('nx-dropdown-menu', classNameProp); - return ( + return isOpen ? ( // eslint-disable-next-line jsx-a11y/interactive-supports-focus
(function NxDropdownMenu { ...{ className, ...attrs } }> { children }
- ); + ) : <>; }); NxDropdownMenu.propTypes = propTypes; diff --git a/lib/src/components/NxDropdownMenu/types.tsx b/lib/src/components/NxDropdownMenu/types.tsx index 1f021a8fa4..dd53682474 100644 --- a/lib/src/components/NxDropdownMenu/types.tsx +++ b/lib/src/components/NxDropdownMenu/types.tsx @@ -11,6 +11,9 @@ export interface Props extends HTMLAttributes { onClosing: () => void; disableMenuKeyNav?: boolean | null; toggleElement?: HTMLElement | null; + onMenuItemFocus?: (index: number, focusableElements: HTMLElement[]) => void | null; + isOpen?: boolean; + onToggleCollapse?: () => void | null; } export const propTypes: PropTypes.ValidationMap = { diff --git a/lib/src/components/NxSearchDropdown/NxSearchDropdown.tsx b/lib/src/components/NxSearchDropdown/NxSearchDropdown.tsx index 76ee43c13a..d00150d858 100644 --- a/lib/src/components/NxSearchDropdown/NxSearchDropdown.tsx +++ b/lib/src/components/NxSearchDropdown/NxSearchDropdown.tsx @@ -4,10 +4,10 @@ * the terms of the Eclipse Public License 2.0 which accompanies this * distribution and is available at https://www.eclipse.org/legal/epl-2.0/. */ -import React, { FocusEvent, KeyboardEvent, Ref, useCallback, useEffect, useRef, useState } from 'react'; +import React, { FocusEvent, KeyboardEvent, Ref, useCallback, useRef } from 'react'; import useMergedRef from '@react-hook/merged-ref'; import classnames from 'classnames'; -import { any, clamp, partial, pipe, prop } from 'ramda'; +import { any, partial, pipe, prop } from 'ramda'; import './NxSearchDropdown.scss'; @@ -50,7 +50,7 @@ function NxSearchDropdownRender( filterRef = useRef(null), elFocusedOnMostRecentRender = useRef(null), - [focusableBtnIndex, setFocusableBtnIndex] = useState(null), + // [focusableBtnIndex, setFocusableBtnIndex] = useState(null), dropdownMenuId = useUniqueId('nx-search-dropdown-menu'), dropdownMenuRole = error || loading || isEmpty ? 'alert' : 'menu', @@ -199,7 +199,7 @@ function NxSearchDropdownRender( tabIndex={-1}> doSearch(searchText)}> { - matches.length ? matches.map((match, i) => + matches.length ? matches.map((match) =>