From 179cd7a523ebc5a4c96fcb0194e5c8873da6c367 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matuzal=C3=A9m=20Teles?= Date: Thu, 20 Oct 2022 13:31:02 -0500 Subject: [PATCH 1/4] fix(@clayui/shared): fix error of not finding focused elements when React.Portal is under a conditional --- packages/clay-shared/src/FocusScope.tsx | 17 ++++++++--------- packages/clay-shared/src/useFocusManagement.ts | 2 +- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/clay-shared/src/FocusScope.tsx b/packages/clay-shared/src/FocusScope.tsx index be8cc1f344..ecc1baf55f 100644 --- a/packages/clay-shared/src/FocusScope.tsx +++ b/packages/clay-shared/src/FocusScope.tsx @@ -112,15 +112,14 @@ export const FocusScope = ({ onKeyDown(event); }, ref: (r: HTMLElement) => { - if (r) { - elRef.current = r; - const {ref} = child; - if (ref) { - if (typeof ref === 'object') { - ref.current = r; - } else if (typeof ref === 'function') { - ref(r); - } + elRef.current = r; + const {ref} = child; + + if (ref) { + if (typeof ref === 'object') { + ref.current = r; + } else if (typeof ref === 'function') { + ref(r); } } }, diff --git a/packages/clay-shared/src/useFocusManagement.ts b/packages/clay-shared/src/useFocusManagement.ts index 0fb2315294..2f4275ff87 100644 --- a/packages/clay-shared/src/useFocusManagement.ts +++ b/packages/clay-shared/src/useFocusManagement.ts @@ -160,7 +160,7 @@ const getFiber = (scope: React.RefObject) => { const getFocusableElementsInScope = (fiberNode: any) => { const focusableElements: Array = []; - const {child} = fiberNode; + const {child} = fiberNode.alternate ?? fiberNode; if (child !== null) { collectFocusableElements(child, focusableElements); From a156294284a491eb86e7d682e4d0c23c132eddf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matuzal=C3=A9m=20Teles?= Date: Thu, 20 Oct 2022 15:24:25 -0500 Subject: [PATCH 2/4] chore(@clayui/drop-down): improves the Menu positioning system to not depend on the external reference For those who use the DropDown.Menu directly, needed to add the reference to the component so that the positioning works correctly, I'm adding an internal reference to prevent this from being mandatory. --- packages/clay-drop-down/src/Menu.tsx | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/clay-drop-down/src/Menu.tsx b/packages/clay-drop-down/src/Menu.tsx index 96fd1b67dd..45c524b70a 100644 --- a/packages/clay-drop-down/src/Menu.tsx +++ b/packages/clay-drop-down/src/Menu.tsx @@ -223,6 +223,7 @@ const ClayDropDownMenu = React.forwardRef( ) => { const setActive = onActiveChange ?? onSetActive; + const menuRef = useRef(null); const subPortalRef = useRef(null); useEffect(() => { @@ -286,7 +287,7 @@ const ClayDropDownMenu = React.forwardRef( ); } - if ((ref as React.RefObject).current) { + if (menuRef.current) { doAlign({ offset: offsetFn(points), overflow: { @@ -295,8 +296,7 @@ const ClayDropDownMenu = React.forwardRef( alwaysByViewport: alignmentByViewport, }, points, - sourceElement: (ref as React.RefObject) - .current!, + sourceElement: menuRef.current, targetElement: alignElementRef.current, }); } @@ -329,7 +329,16 @@ const ClayDropDownMenu = React.forwardRef( [`dropdown-menu-width-${width}`]: width, show: active, })} - ref={ref} + ref={(node) => { + menuRef.current = node; + + if (ref instanceof Function) { + ref(node); + } else if (ref instanceof Object) { + // @ts-ignore + ref.current = node; + } + }} role="presentation" > {children} From 3062d5ed09292e187529749607f39f965ed71ae2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matuzal=C3=A9m=20Teles?= Date: Thu, 20 Oct 2022 16:20:10 -0500 Subject: [PATCH 3/4] fix(@clayui/drop-down): fix focus order error --- packages/clay-drop-down/src/DropDown.tsx | 186 +++++++++++------------ 1 file changed, 91 insertions(+), 95 deletions(-) diff --git a/packages/clay-drop-down/src/DropDown.tsx b/packages/clay-drop-down/src/DropDown.tsx index 3190c8d72a..e56cc43da5 100644 --- a/packages/clay-drop-down/src/DropDown.tsx +++ b/packages/clay-drop-down/src/DropDown.tsx @@ -4,7 +4,6 @@ */ import { - FOCUSABLE_ELEMENTS, FocusScope, InternalDispatch, Keys, @@ -218,113 +217,110 @@ function ClayDropDown({ return ( - - {React.cloneElement(trigger, { - 'aria-controls': ariaControls, - 'aria-expanded': internalActive, - 'aria-haspopup': 'true', - className: classNames( - 'dropdown-toggle', - trigger.props.className - ), - onClick: (event: React.MouseEvent) => { - if (trigger.props.onClick) { - trigger.props.onClick(event); - } - - openMenu(!internalActive); - }, - onKeyDown: ( - event: React.KeyboardEvent - ) => { - if (trigger.props.onKeyDown) { - trigger.props.onKeyDown(event); - } - - if (event.key === Keys.Spacebar) { + {(focusManager) => ( + + {React.cloneElement(trigger, { + 'aria-controls': ariaControls, + 'aria-expanded': internalActive, + 'aria-haspopup': 'true', + className: classNames( + 'dropdown-toggle', + trigger.props.className + ), + onClick: ( + event: React.MouseEvent + ) => { + if (trigger.props.onClick) { + trigger.props.onClick(event); + } + openMenu(!internalActive); - } + }, + onKeyDown: ( + event: React.KeyboardEvent + ) => { + if (trigger.props.onKeyDown) { + trigger.props.onKeyDown(event); + } - if (event.key === Keys.Down) { - event.preventDefault(); - event.stopPropagation(); + if (event.key === Keys.Spacebar) { + openMenu(!internalActive); + } - openMenu(true); - } + if (event.key === Keys.Down) { + event.preventDefault(); + event.stopPropagation(); - if ([Keys.Spacebar, Keys.Down].includes(event.key)) { - event.preventDefault(); - } - }, - ref: (node: HTMLButtonElement) => { - if (node) { + openMenu(true); + } + + if ( + [Keys.Spacebar, Keys.Down].includes(event.key) + ) { + event.preventDefault(); + } + }, + ref: (node: HTMLButtonElement) => { triggerElementRef.current = node; // Call the original ref, if any. const {ref} = trigger; if (typeof ref === 'function') { ref(node); } - } - }, - })} - - {initialized && ( - - { - // After a few milliseconds querying the elements in the DOM - // inside the menu. This especially when the menu is not - // rendered yet only after the menu is opened, React needs - // to commit the changes to the DOM so that the elements are - // visible and we can move the focus. - setTimeout(() => { - const first = - menuElementRef.current?.querySelector( - // @ts-ignore - FOCUSABLE_ELEMENTS - ); - - if (first) { - first.focus(); - } - }, 10); - }} + }, + })} + + {initialized && ( + - { - setInternalActive(false); - triggerElementRef.current?.focus(); - }, - closeOnClick, + { + // After a few milliseconds querying the elements in the DOM + // inside the menu. This especially when the menu is not + // rendered yet only after the menu is opened, React needs + // to commit the changes to the DOM so that the elements are + // visible and we can move the focus. + setTimeout(() => { + focusManager.focusNext(); + }, 10); }} > - {children} - - - - )} - + { + setInternalActive(false); + triggerElementRef.current?.focus(); + }, + closeOnClick, + }} + > + {children} + + + + )} + + )} ); } From 9ecc8e4acdebaa60f72ca3f13c22e138518d4540 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matuzal=C3=A9m=20Teles?= Date: Thu, 20 Oct 2022 16:37:18 -0500 Subject: [PATCH 4/4] fix(@clayui/drop-down): fix error when esc open menu --- packages/clay-drop-down/src/DropDown.tsx | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/clay-drop-down/src/DropDown.tsx b/packages/clay-drop-down/src/DropDown.tsx index e56cc43da5..15a73a1520 100644 --- a/packages/clay-drop-down/src/DropDown.tsx +++ b/packages/clay-drop-down/src/DropDown.tsx @@ -164,12 +164,6 @@ function ClayDropDown({ value: active, }); - const handleKeyUp = (event: React.KeyboardEvent) => { - if (event.key === Keys.Esc) { - setInternalActive(!internalActive); - } - }; - useEffect(() => { if (internalActive) { const onFocus = (event: FocusEvent) => { @@ -221,7 +215,6 @@ function ClayDropDown({ {React.cloneElement(trigger, { 'aria-controls': ariaControls, @@ -288,7 +281,7 @@ function ClayDropDown({ height={menuHeight} id={ariaControls} offsetFn={offsetFn} - onSetActive={setInternalActive} + onActiveChange={setInternalActive} ref={menuElementRef} width={menuWidth} >