From 0d8ad73d77a340fee7be21b40314567fdee14109 Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Thu, 6 Nov 2025 16:32:19 -0500 Subject: [PATCH 1/5] fix(infotip): tab out of infotip --- packages/gamut/src/Popover/Popover.tsx | 2 + packages/gamut/src/Popover/types.tsx | 5 ++ packages/gamut/src/Tip/InfoTip/index.tsx | 78 +++++++++++++++---- packages/gamut/src/Tip/shared/FloatingTip.tsx | 13 +++- packages/gamut/src/Tip/shared/types.tsx | 2 + 5 files changed, 86 insertions(+), 14 deletions(-) diff --git a/packages/gamut/src/Popover/Popover.tsx b/packages/gamut/src/Popover/Popover.tsx index 9f3e4aba728..647c0843209 100755 --- a/packages/gamut/src/Popover/Popover.tsx +++ b/packages/gamut/src/Popover/Popover.tsx @@ -24,6 +24,7 @@ export const Popover: React.FC = ({ beak, children, className, + focusOnProps, isOpen, onRequestClose, outline = false, @@ -238,6 +239,7 @@ export const Popover: React.FC = ({ ) : ( diff --git a/packages/gamut/src/Popover/types.tsx b/packages/gamut/src/Popover/types.tsx index b85d4d4595c..2f2771a2256 100755 --- a/packages/gamut/src/Popover/types.tsx +++ b/packages/gamut/src/Popover/types.tsx @@ -113,4 +113,9 @@ export type PopoverProps = PopoverBaseProps & * Whether to add width restrictions to Popover. */ widthRestricted?: boolean; + + /** + * Props to pass through to FocusTrap's focusOnProps (only used when skipFocusTrap is false). + */ + focusOnProps?: import('../FocusTrap').FocusTrapProps['focusOnProps']; }; diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index 9a39bae3fc6..35941dc903d 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -106,35 +106,84 @@ export const InfoTip: React.FC = ({ } }; + // Define focusout handler outside timeout so we can clean it up const handleFocusOut = (event: FocusEvent) => { const popoverContent = popoverContentRef.current; const button = buttonRef.current; const wrapper = wrapperRef.current; - const { relatedTarget } = event; - if (relatedTarget instanceof Node) { - // If focus is moving back to the button or wrapper, allow it - const movingToButton = - button?.contains(relatedTarget) || wrapper?.contains(relatedTarget); - if (movingToButton) return; + if (!popoverContent || !button || isTipHidden) return; + + // If relatedTarget is null (common with portals), check activeElement after focus settles + if (!relatedTarget) { + setTimeout(() => { + if (isTipHidden) return; + const { activeElement } = document; + const currentPopoverContent = popoverContentRef.current; + const currentButton = buttonRef.current; + + if ( + activeElement && + activeElement !== button && + activeElement !== wrapper && + !currentPopoverContent?.contains(activeElement) && + !button.contains(activeElement) && + !wrapper?.contains(activeElement) && + currentButton && + currentButton.isConnected && + currentButton instanceof HTMLElement && + !currentButton.hasAttribute('disabled') && + currentButton.tabIndex !== -1 + ) { + currentButton.focus(); + } + }, 0); + return; + } + + // Type guard: relatedTarget must be a Node to use contains + if (!(relatedTarget instanceof Node)) { + return; + } - // If focus is staying within the popover content, allow it - if (popoverContent?.contains(relatedTarget)) return; + // If focus is moving to button or wrapper, allow it + if ( + button.contains(relatedTarget) || + wrapper?.contains(relatedTarget) + ) { + return; } - // Return focus to button to maintain logical tab order + // If focus is staying within the popover content, allow it + if (popoverContent.contains(relatedTarget)) { + return; + } + + // Focus is leaving the popover - return to button + // Use setTimeout to ensure this happens after the focus change setTimeout(() => { - buttonRef.current?.focus(); + if (isTipHidden) return; + const currentButton = buttonRef.current; + if ( + currentButton && + currentButton.isConnected && + currentButton instanceof HTMLElement && + !currentButton.hasAttribute('disabled') && + currentButton.tabIndex !== -1 + ) { + currentButton.focus(); + } }, 0); }; - // Wait for the popover ref to be set before attaching the listener + // Wait for popover ref to be set before attaching focusout listener let popoverContent: HTMLDivElement | null = null; const timeoutId = setTimeout(() => { popoverContent = popoverContentRef.current; if (popoverContent) { - popoverContent.addEventListener('focusout', handleFocusOut); + // Use capture phase to catch focusout events + popoverContent.addEventListener('focusout', handleFocusOut, true); } }, 0); @@ -143,7 +192,7 @@ export const InfoTip: React.FC = ({ return () => { clearTimeout(timeoutId); if (popoverContent) { - popoverContent.removeEventListener('focusout', handleFocusOut); + popoverContent.removeEventListener('focusout', handleFocusOut, true); } document.removeEventListener('keydown', handleGlobalEscapeKey); }; @@ -156,9 +205,12 @@ export const InfoTip: React.FC = ({ const tipProps = { alignment, + buttonRef, escapeKeyPressHandler, info, isTipHidden, + onRequestClose: + placement === 'floating' ? () => setTipIsHidden(true) : undefined, popoverContentRef, wrapperRef, ...rest, diff --git a/packages/gamut/src/Tip/shared/FloatingTip.tsx b/packages/gamut/src/Tip/shared/FloatingTip.tsx index c251d1570cf..1255fca5497 100644 --- a/packages/gamut/src/Tip/shared/FloatingTip.tsx +++ b/packages/gamut/src/Tip/shared/FloatingTip.tsx @@ -23,6 +23,7 @@ type FocusOrMouseEvent = export const FloatingTip: React.FC = ({ alignment, avatar, + buttonRef, children, escapeKeyPressHandler, inheritDims, @@ -30,6 +31,7 @@ export const FloatingTip: React.FC = ({ isTipHidden, loading, narrow, + onRequestClose, overline, popoverContentRef, truncateLines, @@ -137,6 +139,14 @@ export const FloatingTip: React.FC = ({ const isPopoverOpen = isHoverType ? isOpen : !isTipHidden; + const focusOnProps = + type === 'info' && buttonRef + ? { + // Disable focus trapping - we'll handle it manually + focusLock: false, + returnFocus: buttonRef, + } + : undefined; return ( = ({ {...commonPopoverProps} animation="fade" dims={dims} + focusOnProps={focusOnProps} horizontalOffset={offset} isOpen={isPopoverOpen} outline popoverContainerRef={popoverContentRef} - skipFocusTrap targetRef={ref} variant="secondary" widthRestricted={false} + onRequestClose={onRequestClose} > ) => void; id?: string; isTipHidden?: boolean; + onRequestClose?: () => void; popoverContentRef?: React.RefObject; type: 'info' | 'tool' | 'preview'; wrapperRef?: React.RefObject; zIndex?: number; + buttonRef?: React.RefObject; } & React.PropsWithChildren; From 5ab7effe7961bd7a0e4ebf97908154bb356615b9 Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Thu, 6 Nov 2025 17:08:27 -0500 Subject: [PATCH 2/5] test skip --- packages/gamut/src/Popover/Popover.tsx | 2 - packages/gamut/src/Popover/types.tsx | 5 -- packages/gamut/src/Tip/InfoTip/index.tsx | 56 ++++++++++--------- packages/gamut/src/Tip/shared/FloatingTip.tsx | 19 +++---- packages/gamut/src/Tip/shared/types.tsx | 1 - 5 files changed, 38 insertions(+), 45 deletions(-) diff --git a/packages/gamut/src/Popover/Popover.tsx b/packages/gamut/src/Popover/Popover.tsx index 647c0843209..9f3e4aba728 100755 --- a/packages/gamut/src/Popover/Popover.tsx +++ b/packages/gamut/src/Popover/Popover.tsx @@ -24,7 +24,6 @@ export const Popover: React.FC = ({ beak, children, className, - focusOnProps, isOpen, onRequestClose, outline = false, @@ -239,7 +238,6 @@ export const Popover: React.FC = ({ ) : ( diff --git a/packages/gamut/src/Popover/types.tsx b/packages/gamut/src/Popover/types.tsx index 2f2771a2256..b85d4d4595c 100755 --- a/packages/gamut/src/Popover/types.tsx +++ b/packages/gamut/src/Popover/types.tsx @@ -113,9 +113,4 @@ export type PopoverProps = PopoverBaseProps & * Whether to add width restrictions to Popover. */ widthRestricted?: boolean; - - /** - * Props to pass through to FocusTrap's focusOnProps (only used when skipFocusTrap is false). - */ - focusOnProps?: import('../FocusTrap').FocusTrapProps['focusOnProps']; }; diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index 35941dc903d..cccf01fb0ea 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -115,30 +115,47 @@ export const InfoTip: React.FC = ({ if (!popoverContent || !button || isTipHidden) return; - // If relatedTarget is null (common with portals), check activeElement after focus settles + // Helper function to return focus to button + const returnFocusToButton = () => { + // Use requestAnimationFrame to ensure this happens after the focus change + requestAnimationFrame(() => { + if (isTipHidden) return; + const currentButton = buttonRef.current; + if ( + currentButton?.isConnected && + currentButton instanceof HTMLElement && + !currentButton.hasAttribute('disabled') && + currentButton.tabIndex !== -1 + ) { + currentButton.focus(); + } + }); + }; + + // If relatedTarget is null (common with portals or when tabbing to browser UI), + // check activeElement after focus settles if (!relatedTarget) { + // Use a slightly longer delay to ensure focus has settled setTimeout(() => { if (isTipHidden) return; const { activeElement } = document; const currentPopoverContent = popoverContentRef.current; const currentButton = buttonRef.current; + const currentWrapper = wrapperRef.current; + // If activeElement is not within our component, return focus to button if ( activeElement && - activeElement !== button && - activeElement !== wrapper && - !currentPopoverContent?.contains(activeElement) && - !button.contains(activeElement) && - !wrapper?.contains(activeElement) && currentButton && - currentButton.isConnected && - currentButton instanceof HTMLElement && - !currentButton.hasAttribute('disabled') && - currentButton.tabIndex !== -1 + activeElement !== currentButton && + activeElement !== currentWrapper && + !currentPopoverContent?.contains(activeElement) && + !currentButton.contains(activeElement) && + !currentWrapper?.contains(activeElement) ) { - currentButton.focus(); + returnFocusToButton(); } - }, 0); + }, 10); return; } @@ -161,20 +178,7 @@ export const InfoTip: React.FC = ({ } // Focus is leaving the popover - return to button - // Use setTimeout to ensure this happens after the focus change - setTimeout(() => { - if (isTipHidden) return; - const currentButton = buttonRef.current; - if ( - currentButton && - currentButton.isConnected && - currentButton instanceof HTMLElement && - !currentButton.hasAttribute('disabled') && - currentButton.tabIndex !== -1 - ) { - currentButton.focus(); - } - }, 0); + returnFocusToButton(); }; // Wait for popover ref to be set before attaching focusout listener diff --git a/packages/gamut/src/Tip/shared/FloatingTip.tsx b/packages/gamut/src/Tip/shared/FloatingTip.tsx index 1255fca5497..c02a46a95dc 100644 --- a/packages/gamut/src/Tip/shared/FloatingTip.tsx +++ b/packages/gamut/src/Tip/shared/FloatingTip.tsx @@ -23,7 +23,6 @@ type FocusOrMouseEvent = export const FloatingTip: React.FC = ({ alignment, avatar, - buttonRef, children, escapeKeyPressHandler, inheritDims, @@ -139,14 +138,13 @@ export const FloatingTip: React.FC = ({ const isPopoverOpen = isHoverType ? isOpen : !isTipHidden; - const focusOnProps = - type === 'info' && buttonRef - ? { - // Disable focus trapping - we'll handle it manually - focusLock: false, - returnFocus: buttonRef, - } - : undefined; + // When type is 'info', skip focus trap and don't pass onRequestClose + // (InfoTip handles its own focus management and click-outside/escape handling) + const popoverFocusProps = + type === 'info' + ? ({ skipFocusTrap: true, onRequestClose: undefined } as const) + : ({ skipFocusTrap: undefined, onRequestClose } as const); + return ( = ({ = ({ targetRef={ref} variant="secondary" widthRestricted={false} - onRequestClose={onRequestClose} > ; zIndex?: number; - buttonRef?: React.RefObject; } & React.PropsWithChildren; From 0483c664b278970bef6bb2e6013c9b21546780d0 Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Thu, 6 Nov 2025 17:22:48 -0500 Subject: [PATCH 3/5] test microtask --- packages/gamut/src/Tip/InfoTip/index.tsx | 153 ++++++++++++++++------- 1 file changed, 110 insertions(+), 43 deletions(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index cccf01fb0ea..cba6f514f81 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -33,6 +33,7 @@ export const InfoTip: React.FC = ({ const wrapperRef = useRef(null); const buttonRef = useRef(null); const popoverContentRef = useRef(null); + const previousFocusRef = useRef(null); const [loaded, setLoaded] = useState(false); useEffect(() => { @@ -106,56 +107,125 @@ export const InfoTip: React.FC = ({ } }; - // Define focusout handler outside timeout so we can clean it up - const handleFocusOut = (event: FocusEvent) => { + // Helper function to check if an element is within our component + const isWithinComponent = (element: Node | null): boolean => { + if (!element) return false; const popoverContent = popoverContentRef.current; const button = buttonRef.current; const wrapper = wrapperRef.current; - const { relatedTarget } = event; + if (!popoverContent || !button) return false; + return ( + button.contains(element) || + wrapper?.contains(element) || + popoverContent.contains(element) + ); + }; - if (!popoverContent || !button || isTipHidden) return; + // Helper function to return focus to button + const returnFocusToButton = () => { + if (isTipHidden) return; + const currentButton = buttonRef.current; + if ( + currentButton?.isConnected && + currentButton instanceof HTMLElement && + !currentButton.hasAttribute('disabled') && + currentButton.tabIndex !== -1 + ) { + currentButton.focus(); + } + }; + + // Intercept Tab key presses when focus is in the popover + // This catches Tab before focus moves, allowing us to redirect it + const handleTabKey = (e: KeyboardEvent) => { + if (e.key !== 'Tab' || isTipHidden) return; + + const { activeElement } = document; + if (!activeElement) return; - // Helper function to return focus to button - const returnFocusToButton = () => { - // Use requestAnimationFrame to ensure this happens after the focus change - requestAnimationFrame(() => { + // Check if focus is currently within the popover content (not the button) + const popoverContent = popoverContentRef.current; + if ( + popoverContent?.contains(activeElement) && + activeElement !== buttonRef.current + ) { + // Focus is in the popover content - we'll check after Tab processes + // where focus ends up and redirect if needed + // Use a microtask to check immediately after Tab key processing + queueMicrotask(() => { if (isTipHidden) return; - const currentButton = buttonRef.current; + const newActiveElement = document.activeElement; if ( - currentButton?.isConnected && - currentButton instanceof HTMLElement && - !currentButton.hasAttribute('disabled') && - currentButton.tabIndex !== -1 + newActiveElement && + !isWithinComponent(newActiveElement) && + newActiveElement !== buttonRef.current ) { - currentButton.focus(); + // Focus moved outside - return to button immediately + returnFocusToButton(); } }); - }; + } + }; + + // Use focusin on document to catch when focus moves anywhere + // This catches focus changes earlier than focusout + const handleFocusIn = (event: FocusEvent) => { + if (isTipHidden) return; + const { target } = event; + + // Check if the previous focus was within our component + const wasPreviousFocusInComponent = previousFocusRef.current + ? isWithinComponent(previousFocusRef.current) + : false; + + // Update previous focus for next time + if (target instanceof HTMLElement) { + previousFocusRef.current = target; + } + + // Only act if previous focus was in our component and new focus is outside + if ( + wasPreviousFocusInComponent && + target && + !isWithinComponent(target as Node) + ) { + // Check if the target is actually focusable (not just any element) + const targetElement = target as HTMLElement; + if ( + targetElement && + (targetElement.tabIndex >= 0 || + targetElement instanceof HTMLAnchorElement || + targetElement instanceof HTMLButtonElement || + targetElement instanceof HTMLInputElement || + targetElement instanceof HTMLSelectElement || + targetElement instanceof HTMLTextAreaElement || + (targetElement instanceof HTMLElement && + targetElement.isContentEditable)) + ) { + // Focus moved outside - return to button immediately + returnFocusToButton(); + } + } + }; + + // Also handle focusout on the popover content as a backup + const handleFocusOut = (event: FocusEvent) => { + const popoverContent = popoverContentRef.current; + const button = buttonRef.current; + const { relatedTarget } = event; + + if (!popoverContent || !button || isTipHidden) return; // If relatedTarget is null (common with portals or when tabbing to browser UI), // check activeElement after focus settles if (!relatedTarget) { - // Use a slightly longer delay to ensure focus has settled setTimeout(() => { if (isTipHidden) return; const { activeElement } = document; - const currentPopoverContent = popoverContentRef.current; - const currentButton = buttonRef.current; - const currentWrapper = wrapperRef.current; - - // If activeElement is not within our component, return focus to button - if ( - activeElement && - currentButton && - activeElement !== currentButton && - activeElement !== currentWrapper && - !currentPopoverContent?.contains(activeElement) && - !currentButton.contains(activeElement) && - !currentWrapper?.contains(activeElement) - ) { + if (activeElement && !isWithinComponent(activeElement)) { returnFocusToButton(); } - }, 10); + }, 0); return; } @@ -164,16 +234,8 @@ export const InfoTip: React.FC = ({ return; } - // If focus is moving to button or wrapper, allow it - if ( - button.contains(relatedTarget) || - wrapper?.contains(relatedTarget) - ) { - return; - } - - // If focus is staying within the popover content, allow it - if (popoverContent.contains(relatedTarget)) { + // If focus is staying within our component, allow it + if (isWithinComponent(relatedTarget)) { return; } @@ -181,24 +243,29 @@ export const InfoTip: React.FC = ({ returnFocusToButton(); }; - // Wait for popover ref to be set before attaching focusout listener + // Wait for popover ref to be set before attaching listeners let popoverContent: HTMLDivElement | null = null; const timeoutId = setTimeout(() => { popoverContent = popoverContentRef.current; if (popoverContent) { - // Use capture phase to catch focusout events + // Use capture phase to catch focusout events early popoverContent.addEventListener('focusout', handleFocusOut, true); } }, 0); + // Use capture phase on document to catch focusin events early + document.addEventListener('focusin', handleFocusIn, true); document.addEventListener('keydown', handleGlobalEscapeKey); + document.addEventListener('keydown', handleTabKey, true); return () => { clearTimeout(timeoutId); if (popoverContent) { popoverContent.removeEventListener('focusout', handleFocusOut, true); } + document.removeEventListener('focusin', handleFocusIn, true); document.removeEventListener('keydown', handleGlobalEscapeKey); + document.removeEventListener('keydown', handleTabKey, true); }; } }, [isTipHidden, placement, setTipIsHidden]); From 5238aeefee8952011b26c4af21daad67a357f8e4 Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Thu, 6 Nov 2025 17:41:26 -0500 Subject: [PATCH 4/5] more test --- packages/gamut/src/Popover/Popover.tsx | 2 + packages/gamut/src/Popover/types.tsx | 5 + packages/gamut/src/Tip/InfoTip/index.tsx | 116 ++++-------------- packages/gamut/src/Tip/shared/FloatingTip.tsx | 5 +- packages/gamut/src/Tip/shared/types.tsx | 1 + .../Tips/InfoTip/InfoTip.stories.tsx | 3 + 6 files changed, 36 insertions(+), 96 deletions(-) diff --git a/packages/gamut/src/Popover/Popover.tsx b/packages/gamut/src/Popover/Popover.tsx index 9f3e4aba728..d5a0caff2d1 100755 --- a/packages/gamut/src/Popover/Popover.tsx +++ b/packages/gamut/src/Popover/Popover.tsx @@ -28,6 +28,7 @@ export const Popover: React.FC = ({ onRequestClose, outline = false, skipFocusTrap, + focusOnProps, pattern: Pattern, popoverContainerRef, position = 'below', @@ -240,6 +241,7 @@ export const Popover: React.FC = ({ allowPageInteraction onClickOutside={handleClickOutside} onEscapeKey={onRequestClose} + focusOnProps={focusOnProps} > {contents} diff --git a/packages/gamut/src/Popover/types.tsx b/packages/gamut/src/Popover/types.tsx index b85d4d4595c..a0d8a2f801b 100755 --- a/packages/gamut/src/Popover/types.tsx +++ b/packages/gamut/src/Popover/types.tsx @@ -1,5 +1,6 @@ import { PatternProps } from '@codecademy/gamut-patterns'; import { HTMLAttributes } from 'react'; +import { ReactFocusOnProps } from 'react-focus-on/dist/es5/types'; import { PopoverVariants } from './elements'; @@ -13,6 +14,10 @@ export type FocusTrapPopoverProps = { * Whether to include the focus trap - should only be skipped if parent of Popover is handling focus managment and accessibility (as is the case with FloatingToolTip). This also disables you from being to specify FocusTrap specific event handlers. */ skipFocusTrap?: never; + /** + * Props to pass through to the underlying FocusTrap component's react-focus-on instance. + */ + focusOnProps?: Partial>; }; export type SkippedFocusTrapPopoverProps = { diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index cba6f514f81..b8864ef1bca 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -33,7 +33,6 @@ export const InfoTip: React.FC = ({ const wrapperRef = useRef(null); const buttonRef = useRef(null); const popoverContentRef = useRef(null); - const previousFocusRef = useRef(null); const [loaded, setLoaded] = useState(false); useEffect(() => { @@ -107,18 +106,12 @@ export const InfoTip: React.FC = ({ } }; - // Helper function to check if an element is within our component - const isWithinComponent = (element: Node | null): boolean => { + // Helper function to check if an element is within the popover content (not the button) + const isWithinPopoverContent = (element: Node | null): boolean => { if (!element) return false; const popoverContent = popoverContentRef.current; - const button = buttonRef.current; - const wrapper = wrapperRef.current; - if (!popoverContent || !button) return false; - return ( - button.contains(element) || - wrapper?.contains(element) || - popoverContent.contains(element) - ); + if (!popoverContent) return false; + return popoverContent.contains(element); }; // Helper function to return focus to button @@ -135,80 +128,9 @@ export const InfoTip: React.FC = ({ } }; - // Intercept Tab key presses when focus is in the popover - // This catches Tab before focus moves, allowing us to redirect it - const handleTabKey = (e: KeyboardEvent) => { - if (e.key !== 'Tab' || isTipHidden) return; - - const { activeElement } = document; - if (!activeElement) return; - - // Check if focus is currently within the popover content (not the button) - const popoverContent = popoverContentRef.current; - if ( - popoverContent?.contains(activeElement) && - activeElement !== buttonRef.current - ) { - // Focus is in the popover content - we'll check after Tab processes - // where focus ends up and redirect if needed - // Use a microtask to check immediately after Tab key processing - queueMicrotask(() => { - if (isTipHidden) return; - const newActiveElement = document.activeElement; - if ( - newActiveElement && - !isWithinComponent(newActiveElement) && - newActiveElement !== buttonRef.current - ) { - // Focus moved outside - return to button immediately - returnFocusToButton(); - } - }); - } - }; - - // Use focusin on document to catch when focus moves anywhere - // This catches focus changes earlier than focusout - const handleFocusIn = (event: FocusEvent) => { - if (isTipHidden) return; - const { target } = event; - - // Check if the previous focus was within our component - const wasPreviousFocusInComponent = previousFocusRef.current - ? isWithinComponent(previousFocusRef.current) - : false; - - // Update previous focus for next time - if (target instanceof HTMLElement) { - previousFocusRef.current = target; - } - - // Only act if previous focus was in our component and new focus is outside - if ( - wasPreviousFocusInComponent && - target && - !isWithinComponent(target as Node) - ) { - // Check if the target is actually focusable (not just any element) - const targetElement = target as HTMLElement; - if ( - targetElement && - (targetElement.tabIndex >= 0 || - targetElement instanceof HTMLAnchorElement || - targetElement instanceof HTMLButtonElement || - targetElement instanceof HTMLInputElement || - targetElement instanceof HTMLSelectElement || - targetElement instanceof HTMLTextAreaElement || - (targetElement instanceof HTMLElement && - targetElement.isContentEditable)) - ) { - // Focus moved outside - return to button immediately - returnFocusToButton(); - } - } - }; - - // Also handle focusout on the popover content as a backup + // Handle focusout on the popover content only + // This catches when focus leaves the popover content and returns it to the button + // But allows focus to leave the button freely const handleFocusOut = (event: FocusEvent) => { const popoverContent = popoverContentRef.current; const button = buttonRef.current; @@ -222,7 +144,12 @@ export const InfoTip: React.FC = ({ setTimeout(() => { if (isTipHidden) return; const { activeElement } = document; - if (activeElement && !isWithinComponent(activeElement)) { + // Only return focus if it left the popover content and didn't go to the button + if ( + activeElement && + activeElement !== button && + !isWithinPopoverContent(activeElement) + ) { returnFocusToButton(); } }, 0); @@ -234,12 +161,18 @@ export const InfoTip: React.FC = ({ return; } - // If focus is staying within our component, allow it - if (isWithinComponent(relatedTarget)) { + // If focus is moving to the button, allow it + if (button.contains(relatedTarget)) { + return; + } + + // If focus is staying within the popover content, allow it + if (isWithinPopoverContent(relatedTarget)) { return; } - // Focus is leaving the popover - return to button + // Focus is leaving the popover content - return to button + // But don't trap it - user can tab away from button freely returnFocusToButton(); }; @@ -253,19 +186,14 @@ export const InfoTip: React.FC = ({ } }, 0); - // Use capture phase on document to catch focusin events early - document.addEventListener('focusin', handleFocusIn, true); document.addEventListener('keydown', handleGlobalEscapeKey); - document.addEventListener('keydown', handleTabKey, true); return () => { clearTimeout(timeoutId); if (popoverContent) { popoverContent.removeEventListener('focusout', handleFocusOut, true); } - document.removeEventListener('focusin', handleFocusIn, true); document.removeEventListener('keydown', handleGlobalEscapeKey); - document.removeEventListener('keydown', handleTabKey, true); }; } }, [isTipHidden, placement, setTipIsHidden]); diff --git a/packages/gamut/src/Tip/shared/FloatingTip.tsx b/packages/gamut/src/Tip/shared/FloatingTip.tsx index c02a46a95dc..91a4ced6d0d 100644 --- a/packages/gamut/src/Tip/shared/FloatingTip.tsx +++ b/packages/gamut/src/Tip/shared/FloatingTip.tsx @@ -138,8 +138,9 @@ export const FloatingTip: React.FC = ({ const isPopoverOpen = isHoverType ? isOpen : !isTipHidden; - // When type is 'info', skip focus trap and don't pass onRequestClose - // (InfoTip handles its own focus management and click-outside/escape handling) + // When type is 'info', skip focus trap entirely since we're handling focus management ourselves + // This allows focus to leave freely, and custom logic in InfoTip will catch when focus leaves + // and return it to the button const popoverFocusProps = type === 'info' ? ({ skipFocusTrap: true, onRequestClose: undefined } as const) diff --git a/packages/gamut/src/Tip/shared/types.tsx b/packages/gamut/src/Tip/shared/types.tsx index 7d5442353e3..62df2f2b2e4 100644 --- a/packages/gamut/src/Tip/shared/types.tsx +++ b/packages/gamut/src/Tip/shared/types.tsx @@ -75,6 +75,7 @@ export type TipPlacementComponentProps = Omit< 'placement' | 'emphasis' > & { alignment: TipStaticAlignment; + buttonRef?: React.RefObject; escapeKeyPressHandler?: (event: React.KeyboardEvent) => void; id?: string; isTipHidden?: boolean; diff --git a/packages/styleguide/src/lib/Molecules/Tips/InfoTip/InfoTip.stories.tsx b/packages/styleguide/src/lib/Molecules/Tips/InfoTip/InfoTip.stories.tsx index efad164ad61..6e661bf282f 100644 --- a/packages/styleguide/src/lib/Molecules/Tips/InfoTip/InfoTip.stories.tsx +++ b/packages/styleguide/src/lib/Molecules/Tips/InfoTip/InfoTip.stories.tsx @@ -101,6 +101,9 @@ export const WithLinksOrButtons: Story = { } onClick={onClick} /> + ); }, From e12cc30b415aa987993712d40ed3e31e9ee8c6f5 Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Thu, 6 Nov 2025 18:43:11 -0500 Subject: [PATCH 5/5] build --- packages/gamut/src/FocusTrap/index.tsx | 2 +- packages/gamut/src/Popover/Popover.tsx | 56 ++++++++++++++------------ 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/packages/gamut/src/FocusTrap/index.tsx b/packages/gamut/src/FocusTrap/index.tsx index ad8f7cb635c..03326cb30c6 100644 --- a/packages/gamut/src/FocusTrap/index.tsx +++ b/packages/gamut/src/FocusTrap/index.tsx @@ -35,7 +35,7 @@ export interface FocusTrapProps extends WithChildrenProp { /** * Passthrough for react-focus-on library props */ - focusOnProps?: ReactFocusOnProps; + focusOnProps?: Partial>; } export const FocusTrap: React.FC = ({ diff --git a/packages/gamut/src/Popover/Popover.tsx b/packages/gamut/src/Popover/Popover.tsx index d5a0caff2d1..c9ea59a5c72 100755 --- a/packages/gamut/src/Popover/Popover.tsx +++ b/packages/gamut/src/Popover/Popover.tsx @@ -18,32 +18,36 @@ import { getBeakVariant } from './styles/beak'; import { PopoverProps } from './types'; import { getDefaultOffset } from './utils'; -export const Popover: React.FC = ({ - animation, - align = 'left', - beak, - children, - className, - isOpen, - onRequestClose, - outline = false, - skipFocusTrap, - focusOnProps, - pattern: Pattern, - popoverContainerRef, - position = 'below', - role, - variant, - targetRef, - horizontalOffset = getDefaultOffset({ - axis: 'horizontal', - position, +export const Popover: React.FC = (props) => { + const { + animation, + align = 'left', + beak, + children, + className, + isOpen, + onRequestClose, + outline = false, + skipFocusTrap, + pattern: Pattern, + popoverContainerRef, + position = 'below', + role, variant, - }), - verticalOffset = getDefaultOffset({ axis: 'vertical', position, variant }), - - widthRestricted, -}) => { + targetRef, + horizontalOffset = getDefaultOffset({ + axis: 'horizontal', + position, + variant, + }), + verticalOffset = getDefaultOffset({ axis: 'vertical', position, variant }), + + widthRestricted, + } = props; + + // Type guard: focusOnProps is only available when skipFocusTrap is false + const focusOnProps = + 'focusOnProps' in props && !skipFocusTrap ? props.focusOnProps : undefined; const [popoverHeight, setPopoverHeight] = useState(0); const [popoverWidth, setPopoverWidth] = useState(0); const [targetRect, setTargetRect] = useState(); @@ -241,7 +245,7 @@ export const Popover: React.FC = ({ allowPageInteraction onClickOutside={handleClickOutside} onEscapeKey={onRequestClose} - focusOnProps={focusOnProps} + {...(focusOnProps ? { focusOnProps } : {})} > {contents}