From 498f64c943dd3986354ddb6021acfd380d877bf3 Mon Sep 17 00:00:00 2001 From: Amy Resnik Date: Thu, 6 Nov 2025 10:01:49 -0500 Subject: [PATCH 01/14] add logs --- packages/gamut/src/Tip/InfoTip/index.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index 9a39bae3fc..b9636af63f 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -107,6 +107,7 @@ export const InfoTip: React.FC = ({ }; const handleFocusOut = (event: FocusEvent) => { + console.log('in handleFocusOut'); const popoverContent = popoverContentRef.current; const button = buttonRef.current; const wrapper = wrapperRef.current; @@ -117,14 +118,21 @@ export const InfoTip: React.FC = ({ // If focus is moving back to the button or wrapper, allow it const movingToButton = button?.contains(relatedTarget) || wrapper?.contains(relatedTarget); - if (movingToButton) return; + if (movingToButton) { + console.log('focus moving to button or wrapper'); + return; + } // If focus is staying within the popover content, allow it - if (popoverContent?.contains(relatedTarget)) return; + if (popoverContent?.contains(relatedTarget)) { + console.log('focus staying within popover content'); + return; + } } // Return focus to button to maintain logical tab order setTimeout(() => { + console.log('in setTimeout'); buttonRef.current?.focus(); }, 0); }; From b11d0d6ddcbb7a889d61ceadbe1177e626a630c1 Mon Sep 17 00:00:00 2001 From: Amy Resnik Date: Thu, 6 Nov 2025 10:09:16 -0500 Subject: [PATCH 02/14] up timeout --- packages/gamut/src/Tip/InfoTip/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index b9636af63f..3a23b13c0b 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -134,7 +134,7 @@ export const InfoTip: React.FC = ({ setTimeout(() => { console.log('in setTimeout'); buttonRef.current?.focus(); - }, 0); + }, 300); }; // Wait for the popover ref to be set before attaching the listener From bb9a2dd19e8cb8d98781efa618cde7290a7899f4 Mon Sep 17 00:00:00 2001 From: Amy Resnik Date: Thu, 6 Nov 2025 10:09:53 -0500 Subject: [PATCH 03/14] add another log --- packages/gamut/src/Tip/InfoTip/index.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index 3a23b13c0b..dec1101e93 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -114,6 +114,7 @@ export const InfoTip: React.FC = ({ const { relatedTarget } = event; + console.log('relatedTarget', relatedTarget); if (relatedTarget instanceof Node) { // If focus is moving back to the button or wrapper, allow it const movingToButton = From c81bae1e66321cd5d82e52edc0cd7fbf9e1f3fc7 Mon Sep 17 00:00:00 2001 From: Amy Resnik Date: Thu, 6 Nov 2025 10:25:58 -0500 Subject: [PATCH 04/14] try something --- packages/gamut/src/Tip/InfoTip/index.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index dec1101e93..a3b0fbe51b 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -132,10 +132,8 @@ export const InfoTip: React.FC = ({ } // Return focus to button to maintain logical tab order - setTimeout(() => { - console.log('in setTimeout'); - buttonRef.current?.focus(); - }, 300); + console.log('focusing on button'); + buttonRef.current?.focus(); }; // Wait for the popover ref to be set before attaching the listener From 39830e87aba8caf20abb682dd009c506688ba867 Mon Sep 17 00:00:00 2001 From: Amy Resnik Date: Thu, 6 Nov 2025 10:26:35 -0500 Subject: [PATCH 05/14] longer timeout --- packages/gamut/src/Tip/InfoTip/index.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index a3b0fbe51b..e5a5a1d64f 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -132,8 +132,10 @@ export const InfoTip: React.FC = ({ } // Return focus to button to maintain logical tab order - console.log('focusing on button'); - buttonRef.current?.focus(); + setTimeout(() => { + console.log('in setTimeout'); + buttonRef.current?.focus(); + }, 1000); }; // Wait for the popover ref to be set before attaching the listener From 3513a21c425815fe4e977673229604ae54823565 Mon Sep 17 00:00:00 2001 From: Amy Resnik Date: Thu, 6 Nov 2025 10:43:56 -0500 Subject: [PATCH 06/14] change back --- packages/gamut/src/Tip/InfoTip/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index e5a5a1d64f..229c260681 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -135,7 +135,7 @@ export const InfoTip: React.FC = ({ setTimeout(() => { console.log('in setTimeout'); buttonRef.current?.focus(); - }, 1000); + }, 0); }; // Wait for the popover ref to be set before attaching the listener From d0e9ab11edc0adbbcb6a47f2855e8c8656fc588e Mon Sep 17 00:00:00 2001 From: Amy Resnik Date: Thu, 6 Nov 2025 10:54:21 -0500 Subject: [PATCH 07/14] try this --- packages/gamut/src/Tip/InfoTip/index.tsx | 55 +++++++++++------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index 229c260681..143c999f1a 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -106,36 +106,33 @@ export const InfoTip: React.FC = ({ } }; - const handleFocusOut = (event: FocusEvent) => { - console.log('in handleFocusOut'); - const popoverContent = popoverContentRef.current; - const button = buttonRef.current; - const wrapper = wrapperRef.current; - - const { relatedTarget } = event; - - console.log('relatedTarget', relatedTarget); - 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) { - console.log('focus moving to button or wrapper'); - return; + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Tab') { + const popoverContent = popoverContentRef.current; + if (!popoverContent) return; + + const focusableElements = + popoverContent.querySelectorAll( + 'a[href], button, textarea, input, select, [tabindex]:not([tabindex="-1"])' + ); + + if (focusableElements.length === 0) return; + + const firstElement = focusableElements[0]; + const lastElement = focusableElements[focusableElements.length - 1]; + const { activeElement } = document; + + // If tabbing forward from the last element, prevent default and move to button + if (!event.shiftKey && activeElement === lastElement) { + event.preventDefault(); + buttonRef.current?.focus(); } - - // If focus is staying within the popover content, allow it - if (popoverContent?.contains(relatedTarget)) { - console.log('focus staying within popover content'); - return; + // If tabbing backward from the first element, prevent default and move to button + else if (event.shiftKey && activeElement === firstElement) { + event.preventDefault(); + buttonRef.current?.focus(); } } - - // Return focus to button to maintain logical tab order - setTimeout(() => { - console.log('in setTimeout'); - buttonRef.current?.focus(); - }, 0); }; // Wait for the popover ref to be set before attaching the listener @@ -143,7 +140,7 @@ export const InfoTip: React.FC = ({ const timeoutId = setTimeout(() => { popoverContent = popoverContentRef.current; if (popoverContent) { - popoverContent.addEventListener('focusout', handleFocusOut); + popoverContent.addEventListener('keydown', handleKeyDown); } }, 0); @@ -152,7 +149,7 @@ export const InfoTip: React.FC = ({ return () => { clearTimeout(timeoutId); if (popoverContent) { - popoverContent.removeEventListener('focusout', handleFocusOut); + popoverContent.removeEventListener('keydown', handleKeyDown); } document.removeEventListener('keydown', handleGlobalEscapeKey); }; From 658cc64a7ae582bcea9e1907842819deba488c2a Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Fri, 7 Nov 2025 10:45:55 -0500 Subject: [PATCH 08/14] fix(InfoTip): wrap focus when shift tabbing --- packages/gamut/src/Tip/InfoTip/index.tsx | 48 ++++----- .../gamut/src/Tip/__tests__/InfoTip.test.tsx | 101 ++++++++++++++++++ 2 files changed, 124 insertions(+), 25 deletions(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index 143c999f1a..2195f4c085 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -107,31 +107,29 @@ export const InfoTip: React.FC = ({ }; const handleKeyDown = (event: KeyboardEvent) => { - if (event.key === 'Tab') { - const popoverContent = popoverContentRef.current; - if (!popoverContent) return; - - const focusableElements = - popoverContent.querySelectorAll( - 'a[href], button, textarea, input, select, [tabindex]:not([tabindex="-1"])' - ); - - if (focusableElements.length === 0) return; - - const firstElement = focusableElements[0]; - const lastElement = focusableElements[focusableElements.length - 1]; - const { activeElement } = document; - - // If tabbing forward from the last element, prevent default and move to button - if (!event.shiftKey && activeElement === lastElement) { - event.preventDefault(); - buttonRef.current?.focus(); - } - // If tabbing backward from the first element, prevent default and move to button - else if (event.shiftKey && activeElement === firstElement) { - event.preventDefault(); - buttonRef.current?.focus(); - } + if (event.key !== 'Tab') return; + + const popoverContent = popoverContentRef.current; + if (!popoverContent) return; + + const focusableElements = popoverContent.querySelectorAll( + 'a[href], button, textarea, input, select, [tabindex]:not([tabindex="-1"])' + ); + + if (focusableElements.length === 0) return; + + const firstElement = focusableElements[0]; + const lastElement = focusableElements[focusableElements.length - 1]; + const { activeElement } = document; + + const isTabbingForwardFromLast = + !event.shiftKey && activeElement === lastElement; + const isTabbingBackwardFromFirst = + event.shiftKey && activeElement === firstElement; + + if (isTabbingForwardFromLast || isTabbingBackwardFromFirst) { + event.preventDefault(); + buttonRef.current?.focus(); } }; diff --git a/packages/gamut/src/Tip/__tests__/InfoTip.test.tsx b/packages/gamut/src/Tip/__tests__/InfoTip.test.tsx index e884f3ed9d..c076ef2321 100644 --- a/packages/gamut/src/Tip/__tests__/InfoTip.test.tsx +++ b/packages/gamut/src/Tip/__tests__/InfoTip.test.tsx @@ -109,5 +109,106 @@ describe('InfoTip', () => { }); expect(button).toHaveFocus(); }); + + it('wraps focus to button when tabbing forward from last focusable element', async () => { + const linkText = 'cool link'; + const { view } = renderView({ + placement: 'floating', + info: ( + + Hey! Here is a{' '} + {linkText} that is super + important. + + ), + }); + + const button = view.getByLabelText('Show information'); + await act(async () => { + await userEvent.click(button); + }); + + await waitFor(() => { + expect(view.queryAllByText(linkText).length).toBe(2); + }); + + const link = view.getAllByRole('link', { name: linkText })[1]; + link.focus(); + expect(link).toHaveFocus(); + + await act(async () => { + await userEvent.keyboard('{Tab}'); + }); + + expect(button).toHaveFocus(); + }); + + it('wraps focus to button when shift+tabbing backward from first focusable element', async () => { + const linkText = 'cool link'; + const { view } = renderView({ + placement: 'floating', + info: ( + + Hey! Here is a{' '} + {linkText} that is super + important. + + ), + }); + + const button = view.getByLabelText('Show information'); + await act(async () => { + await userEvent.click(button); + }); + + await waitFor(() => { + expect(view.queryAllByText(linkText).length).toBe(2); + }); + + const link = view.getAllByRole('link', { name: linkText })[1]; + link.focus(); + expect(link).toHaveFocus(); + + await act(async () => { + await userEvent.keyboard('{Shift>}{Tab}{/Shift}'); + }); + + expect(button).toHaveFocus(); + }); + + it('allows normal tabbing between focusable elements within popover', async () => { + const firstLinkText = 'first link'; + const secondLinkText = 'second link'; + const { view } = renderView({ + placement: 'floating', + info: ( + + {firstLinkText} and{' '} + {secondLinkText} + + ), + }); + + const button = view.getByLabelText('Show information'); + await act(async () => { + await userEvent.click(button); + }); + + await waitFor(() => { + expect(view.queryAllByText(firstLinkText).length).toBe(2); + }); + + const firstLink = view.getAllByRole('link', { name: firstLinkText })[1]; + firstLink.focus(); + expect(firstLink).toHaveFocus(); + + await act(async () => { + await userEvent.keyboard('{Tab}'); + }); + + const secondLink = view.getAllByRole('link', { name: secondLinkText })[1]; + expect(secondLink).toHaveFocus(); + expect(button).not.toHaveFocus(); + }); }); }); From 60c60ccdddc6cbe3d545a6ba42b74ab6f2b13d61 Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Fri, 7 Nov 2025 12:23:46 -0500 Subject: [PATCH 09/14] tests passing, need to dry up --- packages/gamut/src/Tip/InfoTip/index.tsx | 42 +++- .../gamut/src/Tip/__tests__/InfoTip.test.tsx | 202 ++++++++++++++++-- 2 files changed, 218 insertions(+), 26 deletions(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index 2195f4c085..009f27523e 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -1,4 +1,10 @@ -import { useCallback, useEffect, useRef, useState } from 'react'; +import { + isValidElement, + useCallback, + useEffect, + useRef, + useState, +} from 'react'; import { FloatingTip } from '../shared/FloatingTip'; import { InlineTip } from '../shared/InlineTip'; @@ -87,7 +93,11 @@ export const InfoTip: React.FC = ({ }, 0); } // we want to call the onClick handler after the tip has mounted - if (onClick) setTimeout(() => onClick({ isTipHidden: currentTipState }), 0); + // For floating placement, wait a bit longer to ensure refs are set + if (onClick) { + const delay = placement === 'floating' ? 10 : 0; + setTimeout(() => onClick({ isTipHidden: currentTipState }), delay); + } }; useEffect(() => { @@ -168,13 +178,39 @@ export const InfoTip: React.FC = ({ ...rest, }; + // Helper function to recursively extract text content from React elements + // Converts everything to plain text for screenreader announcements + const extractTextContent = (children: React.ReactNode): string => { + if (!children) return ''; + + if (typeof children === 'string' || typeof children === 'number') { + return String(children); + } + + if (Array.isArray(children)) { + return children.map((child) => extractTextContent(child)).join(' '); + } + + if (isValidElement(children)) { + const props = children.props as Record; + if (props.children) { + return extractTextContent(props.children as React.ReactNode); + } + } + + return ''; + }; + + const screenreaderInfo = + shouldAnnounce && !isTipHidden ? extractTextContent(info) : `\xa0`; + const text = ( - {shouldAnnounce && !isTipHidden ? info : `\xa0`} + {screenreaderInfo} ); diff --git a/packages/gamut/src/Tip/__tests__/InfoTip.test.tsx b/packages/gamut/src/Tip/__tests__/InfoTip.test.tsx index c076ef2321..b99061135d 100644 --- a/packages/gamut/src/Tip/__tests__/InfoTip.test.tsx +++ b/packages/gamut/src/Tip/__tests__/InfoTip.test.tsx @@ -32,6 +32,101 @@ describe('InfoTip', () => { expect(tip).toBeVisible(); }); + + it('closes the tip when Escape key is pressed', async () => { + const { view } = renderView({}); + + const button = view.getByLabelText('Show information'); + await act(async () => { + await userEvent.click(button); + }); + + const tips = view.getAllByText(info); + const tip = tips[0]; + expect(tip).toBeVisible(); + + await act(async () => { + await userEvent.keyboard('{Escape}'); + }); + + await waitFor(() => { + expect(tip).not.toBeVisible(); + }); + }); + + it('allows normal tabbing through focusable elements within tip', async () => { + const firstLinkText = 'first link'; + const secondLinkText = 'second link'; + const firstLinkRef = createRef(); + const { view } = renderView({ + info: ( + + + {firstLinkText} + {' '} + and {secondLinkText} + + ), + onClick: ({ isTipHidden }: { isTipHidden: boolean }) => { + if (!isTipHidden) { + firstLinkRef.current?.focus(); + } + }, + }); + + const button = view.getByLabelText('Show information'); + await act(async () => { + await userEvent.click(button); + }); + + await waitFor(() => { + expect(view.getAllByText(firstLinkText)[0]).toBeVisible(); + }); + + const firstLink = view.getAllByRole('link', { name: firstLinkText })[0]; + expect(firstLink).toHaveFocus(); + + await act(async () => { + await userEvent.keyboard('{Tab}'); + }); + + const secondLink = view.getAllByRole('link', { name: secondLinkText })[0]; + expect(secondLink).toHaveFocus(); + expect(firstLink).not.toHaveFocus(); + }); + + it('allows focus to move to links within the tip', async () => { + const linkText = 'cool link'; + const linkRef = createRef(); + const { view } = renderView({ + info: ( + + Hey! Here is a{' '} + + {linkText} + {' '} + that is super important. + + ), + onClick: ({ isTipHidden }: { isTipHidden: boolean }) => { + if (!isTipHidden) { + linkRef.current?.focus(); + } + }, + }); + + const button = view.getByLabelText('Show information'); + await act(async () => { + await userEvent.click(button); + }); + + await waitFor(() => { + expect(view.getAllByText(linkText)[0]).toBeVisible(); + }); + + const link = view.getAllByRole('link', { name: linkText })[0]; + expect(link).toHaveFocus(); + }); }); describe('floating placement', () => { @@ -98,7 +193,10 @@ describe('InfoTip', () => { await userEvent.click(button); }); - expect(view.queryAllByText(linkText).length).toBe(2); + await waitFor(() => { + const links = view.getAllByRole('link', { name: linkText }); + expect(links.length).toBe(1); + }); await act(async () => { await userEvent.keyboard('{Escape}'); @@ -112,15 +210,23 @@ describe('InfoTip', () => { it('wraps focus to button when tabbing forward from last focusable element', async () => { const linkText = 'cool link'; + const linkRef = createRef(); const { view } = renderView({ placement: 'floating', info: ( Hey! Here is a{' '} - {linkText} that is super - important. + + {linkText} + {' '} + that is super important. ), + onClick: ({ isTipHidden }: { isTipHidden: boolean }) => { + if (!isTipHidden) { + linkRef.current?.focus(); + } + }, }); const button = view.getByLabelText('Show information'); @@ -128,13 +234,26 @@ describe('InfoTip', () => { await userEvent.click(button); }); - await waitFor(() => { - expect(view.queryAllByText(linkText).length).toBe(2); + const link = await waitFor(() => { + const links = view.getAllByRole('link', { name: linkText }); + expect(links.length).toBe(1); + return links[0]; }); - const link = view.getAllByRole('link', { name: linkText })[1]; - link.focus(); - expect(link).toHaveFocus(); + await waitFor( + () => { + expect(linkRef.current).toBeTruthy(); + expect(linkRef.current).toBe(link); + }, + { timeout: 2000 } + ); + + await waitFor( + () => { + expect(link).toHaveFocus(); + }, + { timeout: 2000 } + ); await act(async () => { await userEvent.keyboard('{Tab}'); @@ -145,15 +264,23 @@ describe('InfoTip', () => { it('wraps focus to button when shift+tabbing backward from first focusable element', async () => { const linkText = 'cool link'; + const linkRef = createRef(); const { view } = renderView({ placement: 'floating', info: ( Hey! Here is a{' '} - {linkText} that is super - important. + + {linkText} + {' '} + that is super important. ), + onClick: ({ isTipHidden }: { isTipHidden: boolean }) => { + if (!isTipHidden) { + linkRef.current?.focus(); + } + }, }); const button = view.getByLabelText('Show information'); @@ -161,13 +288,27 @@ describe('InfoTip', () => { await userEvent.click(button); }); - await waitFor(() => { - expect(view.queryAllByText(linkText).length).toBe(2); + // Wait for popover content to be visible (screenreader text doesn't interfere in real component) + const link = await waitFor(() => { + const links = view.getAllByRole('link', { name: linkText }); + expect(links.length).toBe(1); + return links[0]; }); - const link = view.getAllByRole('link', { name: linkText })[1]; - link.focus(); - expect(link).toHaveFocus(); + await waitFor( + () => { + expect(linkRef.current).toBeTruthy(); + expect(linkRef.current).toBe(link); + }, + { timeout: 2000 } + ); + + await waitFor( + () => { + expect(link).toHaveFocus(); + }, + { timeout: 2000 } + ); await act(async () => { await userEvent.keyboard('{Shift>}{Tab}{/Shift}'); @@ -179,14 +320,22 @@ describe('InfoTip', () => { it('allows normal tabbing between focusable elements within popover', async () => { const firstLinkText = 'first link'; const secondLinkText = 'second link'; + const firstLinkRef = createRef(); const { view } = renderView({ placement: 'floating', info: ( - {firstLinkText} and{' '} - {secondLinkText} + + {firstLinkText} + {' '} + and {secondLinkText} ), + onClick: ({ isTipHidden }: { isTipHidden: boolean }) => { + if (!isTipHidden) { + firstLinkRef.current?.focus(); + } + }, }); const button = view.getByLabelText('Show information'); @@ -194,19 +343,26 @@ describe('InfoTip', () => { await userEvent.click(button); }); - await waitFor(() => { - expect(view.queryAllByText(firstLinkText).length).toBe(2); + const firstLink = await waitFor(() => { + const links = view.getAllByRole('link', { name: firstLinkText }); + expect(links.length).toBe(1); + return links[0]; }); - const firstLink = view.getAllByRole('link', { name: firstLinkText })[1]; - firstLink.focus(); - expect(firstLink).toHaveFocus(); + await waitFor( + () => { + expect(firstLinkRef.current).toBeTruthy(); + expect(firstLinkRef.current).toBe(firstLink); + expect(firstLink).toHaveFocus(); + }, + { timeout: 2000 } + ); await act(async () => { await userEvent.keyboard('{Tab}'); }); - const secondLink = view.getAllByRole('link', { name: secondLinkText })[1]; + const secondLink = view.getAllByRole('link', { name: secondLinkText })[0]; expect(secondLink).toHaveFocus(); expect(button).not.toHaveFocus(); }); From 25abb1073b29957663cb094cda4cfa1a545a72fa Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Fri, 7 Nov 2025 12:41:54 -0500 Subject: [PATCH 10/14] dry up tests --- .../gamut/src/Tip/__tests__/InfoTip.test.tsx | 338 ++++++++---------- 1 file changed, 154 insertions(+), 184 deletions(-) diff --git a/packages/gamut/src/Tip/__tests__/InfoTip.test.tsx b/packages/gamut/src/Tip/__tests__/InfoTip.test.tsx index b99061135d..01c47b99ba 100644 --- a/packages/gamut/src/Tip/__tests__/InfoTip.test.tsx +++ b/packages/gamut/src/Tip/__tests__/InfoTip.test.tsx @@ -1,7 +1,7 @@ import { setupRtl } from '@codecademy/gamut-tests'; import { act, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { createRef } from 'react'; +import { createRef, RefObject } from 'react'; import { Anchor } from '../../Anchor'; import { Text } from '../../Typography'; @@ -12,6 +12,85 @@ const renderView = setupRtl(InfoTip, { info, }); +const createFocusOnClick = (ref: RefObject) => { + return ({ isTipHidden }: { isTipHidden: boolean }) => { + if (!isTipHidden) { + ref.current?.focus(); + } + }; +}; + +const createLinkSetup = (linkText: string, href = 'https://example.com') => { + const linkRef = createRef(); + const info = ( + + Hey! Here is a{' '} + + {linkText} + {' '} + that is super important. + + ); + return { linkRef, info, onClick: createFocusOnClick(linkRef) }; +}; + +const createMultiLinkSetup = ( + firstLinkText: string, + secondLinkText: string, + firstHref = 'https://example.com/1', + secondHref = 'https://example.com/2' +) => { + const firstLinkRef = createRef(); + const info = ( + + + {firstLinkText} + {' '} + and {secondLinkText} + + ); + return { firstLinkRef, info, onClick: createFocusOnClick(firstLinkRef) }; +}; + +const clickButton = async (view: ReturnType['view']) => { + const button = view.getByLabelText('Show information'); + await act(async () => { + await userEvent.click(button); + }); + return button; +}; + +const waitForPopoverLink = async ( + view: ReturnType['view'], + linkText: string +) => { + return await waitFor(() => { + const links = view.getAllByRole('link', { name: linkText }); + expect(links.length).toBe(1); + return links[0]; + }); +}; + +const waitForLinkFocus = async ( + linkRef: RefObject, + link: HTMLElement +) => { + await waitFor( + () => { + expect(linkRef.current).toBeTruthy(); + expect(linkRef.current).toBe(link); + }, + { timeout: 2000 } + ); + + await waitFor( + () => { + expect(link).toHaveFocus(); + }, + { timeout: 2000 } + ); +}; + describe('InfoTip', () => { describe('inline placement', () => { it('shows the tip when it is clicked on', async () => { @@ -41,8 +120,12 @@ describe('InfoTip', () => { await userEvent.click(button); }); - const tips = view.getAllByText(info); - const tip = tips[0]; + // For inline placement, get the tip body (not the screenreader text) + const tip = + view + .getAllByText(info) + .find((el) => el.getAttribute('aria-live') !== 'assertive') || + view.getAllByText(info)[0]; expect(tip).toBeVisible(); await act(async () => { @@ -57,75 +140,49 @@ describe('InfoTip', () => { it('allows normal tabbing through focusable elements within tip', async () => { const firstLinkText = 'first link'; const secondLinkText = 'second link'; - const firstLinkRef = createRef(); - const { view } = renderView({ - info: ( - - - {firstLinkText} - {' '} - and {secondLinkText} - - ), - onClick: ({ isTipHidden }: { isTipHidden: boolean }) => { - if (!isTipHidden) { - firstLinkRef.current?.focus(); - } - }, - }); + const { info, onClick } = createMultiLinkSetup( + firstLinkText, + secondLinkText + ); + const { view } = renderView({ info, onClick }); - const button = view.getByLabelText('Show information'); - await act(async () => { - await userEvent.click(button); - }); + await clickButton(view); await waitFor(() => { - expect(view.getAllByText(firstLinkText)[0]).toBeVisible(); + expect(view.getByText(firstLinkText)).toBeVisible(); }); - const firstLink = view.getAllByRole('link', { name: firstLinkText })[0]; - expect(firstLink).toHaveFocus(); + const firstLink = view.getByRole('link', { name: firstLinkText }); + await waitFor(() => { + expect(firstLink).toHaveFocus(); + }); await act(async () => { await userEvent.keyboard('{Tab}'); }); - const secondLink = view.getAllByRole('link', { name: secondLinkText })[0]; - expect(secondLink).toHaveFocus(); + const secondLink = view.getByRole('link', { name: secondLinkText }); + await waitFor(() => { + expect(secondLink).toHaveFocus(); + }); expect(firstLink).not.toHaveFocus(); }); it('allows focus to move to links within the tip', async () => { const linkText = 'cool link'; - const linkRef = createRef(); - const { view } = renderView({ - info: ( - - Hey! Here is a{' '} - - {linkText} - {' '} - that is super important. - - ), - onClick: ({ isTipHidden }: { isTipHidden: boolean }) => { - if (!isTipHidden) { - linkRef.current?.focus(); - } - }, - }); + const { info, onClick } = createLinkSetup(linkText); + const { view } = renderView({ info, onClick }); - const button = view.getByLabelText('Show information'); - await act(async () => { - await userEvent.click(button); - }); + await clickButton(view); await waitFor(() => { - expect(view.getAllByText(linkText)[0]).toBeVisible(); + expect(view.getByText(linkText)).toBeVisible(); }); - const link = view.getAllByRole('link', { name: linkText })[0]; - expect(link).toHaveFocus(); + const link = view.getByRole('link', { name: linkText }); + await waitFor(() => { + expect(link).toHaveFocus(); + }); }); }); @@ -150,10 +207,7 @@ describe('InfoTip', () => { placement: 'floating', }); - const button = view.getByLabelText('Show information'); - await act(async () => { - await userEvent.click(button); - }); + const button = await clickButton(view); expect(view.queryAllByText(info).length).toBe(2); @@ -164,34 +218,24 @@ describe('InfoTip', () => { await waitFor(() => { expect(view.queryByText(info)).toBeNull(); }); - expect(button).toHaveFocus(); + await waitFor(() => { + expect(button).toHaveFocus(); + }); }); it('closes the tip with links when Escape key is pressed and returns focus to the button', async () => { const linkText = 'cool link'; - const linkRef = createRef(); + const { info, onClick } = createLinkSetup( + linkText, + 'https://giphy.com/search/nichijou' + ); const { view } = renderView({ placement: 'floating', - info: ( - - Hey! Here is a{' '} - - {linkText} - {' '} - that is super important. - - ), - onClick: ({ isTipHidden }: { isTipHidden: boolean }) => { - if (!isTipHidden) { - linkRef.current?.focus(); - } - }, + info, + onClick, }); - const button = view.getByLabelText('Show information'); - await act(async () => { - await userEvent.click(button); - }); + const button = await clickButton(view); await waitFor(() => { const links = view.getAllByRole('link', { name: linkText }); @@ -205,149 +249,73 @@ describe('InfoTip', () => { await waitFor(() => { expect(view.queryByText(linkText)).toBeNull(); }); - expect(button).toHaveFocus(); + await waitFor(() => { + expect(button).toHaveFocus(); + }); }); it('wraps focus to button when tabbing forward from last focusable element', async () => { const linkText = 'cool link'; - const linkRef = createRef(); + const { linkRef, info, onClick } = createLinkSetup(linkText); const { view } = renderView({ placement: 'floating', - info: ( - - Hey! Here is a{' '} - - {linkText} - {' '} - that is super important. - - ), - onClick: ({ isTipHidden }: { isTipHidden: boolean }) => { - if (!isTipHidden) { - linkRef.current?.focus(); - } - }, - }); - - const button = view.getByLabelText('Show information'); - await act(async () => { - await userEvent.click(button); - }); - - const link = await waitFor(() => { - const links = view.getAllByRole('link', { name: linkText }); - expect(links.length).toBe(1); - return links[0]; + info, + onClick, }); - await waitFor( - () => { - expect(linkRef.current).toBeTruthy(); - expect(linkRef.current).toBe(link); - }, - { timeout: 2000 } - ); + const button = await clickButton(view); - await waitFor( - () => { - expect(link).toHaveFocus(); - }, - { timeout: 2000 } - ); + const link = await waitForPopoverLink(view, linkText); + await waitForLinkFocus(linkRef, link); await act(async () => { await userEvent.keyboard('{Tab}'); }); - expect(button).toHaveFocus(); + await waitFor(() => { + expect(button).toHaveFocus(); + }); }); it('wraps focus to button when shift+tabbing backward from first focusable element', async () => { const linkText = 'cool link'; - const linkRef = createRef(); + const { linkRef, info, onClick } = createLinkSetup(linkText); const { view } = renderView({ placement: 'floating', - info: ( - - Hey! Here is a{' '} - - {linkText} - {' '} - that is super important. - - ), - onClick: ({ isTipHidden }: { isTipHidden: boolean }) => { - if (!isTipHidden) { - linkRef.current?.focus(); - } - }, + info, + onClick, }); - const button = view.getByLabelText('Show information'); - await act(async () => { - await userEvent.click(button); - }); + const button = await clickButton(view); - // Wait for popover content to be visible (screenreader text doesn't interfere in real component) - const link = await waitFor(() => { - const links = view.getAllByRole('link', { name: linkText }); - expect(links.length).toBe(1); - return links[0]; - }); - - await waitFor( - () => { - expect(linkRef.current).toBeTruthy(); - expect(linkRef.current).toBe(link); - }, - { timeout: 2000 } - ); - - await waitFor( - () => { - expect(link).toHaveFocus(); - }, - { timeout: 2000 } - ); + const link = await waitForPopoverLink(view, linkText); + await waitForLinkFocus(linkRef, link); await act(async () => { await userEvent.keyboard('{Shift>}{Tab}{/Shift}'); }); - expect(button).toHaveFocus(); + await waitFor(() => { + expect(button).toHaveFocus(); + }); }); it('allows normal tabbing between focusable elements within popover', async () => { const firstLinkText = 'first link'; const secondLinkText = 'second link'; - const firstLinkRef = createRef(); + const { firstLinkRef, info, onClick } = createMultiLinkSetup( + firstLinkText, + secondLinkText + ); const { view } = renderView({ placement: 'floating', - info: ( - - - {firstLinkText} - {' '} - and {secondLinkText} - - ), - onClick: ({ isTipHidden }: { isTipHidden: boolean }) => { - if (!isTipHidden) { - firstLinkRef.current?.focus(); - } - }, + info, + onClick, }); - const button = view.getByLabelText('Show information'); - await act(async () => { - await userEvent.click(button); - }); + await clickButton(view); - const firstLink = await waitFor(() => { - const links = view.getAllByRole('link', { name: firstLinkText }); - expect(links.length).toBe(1); - return links[0]; - }); + const firstLink = await waitForPopoverLink(view, firstLinkText); await waitFor( () => { @@ -362,9 +330,11 @@ describe('InfoTip', () => { await userEvent.keyboard('{Tab}'); }); - const secondLink = view.getAllByRole('link', { name: secondLinkText })[0]; - expect(secondLink).toHaveFocus(); - expect(button).not.toHaveFocus(); + const secondLink = view.getByRole('link', { name: secondLinkText }); + await waitFor(() => { + expect(secondLink).toHaveFocus(); + }); + expect(view.getByLabelText('Show information')).not.toHaveFocus(); }); }); }); From 8e49f83ffb9b238ea8a62c12cd1e6f3e4606f9df Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Fri, 7 Nov 2025 13:43:47 -0500 Subject: [PATCH 11/14] Children --- packages/gamut/src/Tip/InfoTip/index.tsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index 009f27523e..bebb08c4ad 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -1,4 +1,5 @@ import { + Children, isValidElement, useCallback, useEffect, @@ -187,18 +188,18 @@ export const InfoTip: React.FC = ({ return String(children); } - if (Array.isArray(children)) { - return children.map((child) => extractTextContent(child)).join(' '); - } - if (isValidElement(children)) { const props = children.props as Record; if (props.children) { return extractTextContent(props.children as React.ReactNode); } + return ''; } - return ''; + // Children.toArray normalizes arrays and fragments automatically + return Children.toArray(children) + .map((child) => extractTextContent(child)) + .join(' '); }; const screenreaderInfo = From c0bb30d53132047425ead7a63a9ed47fa892d043 Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Fri, 7 Nov 2025 14:10:24 -0500 Subject: [PATCH 12/14] fix --- packages/gamut/src/Tip/InfoTip/index.tsx | 32 +++++++++++++----------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index bebb08c4ad..48803534cc 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -3,6 +3,7 @@ import { isValidElement, useCallback, useEffect, + useMemo, useRef, useState, } from 'react'; @@ -179,31 +180,32 @@ export const InfoTip: React.FC = ({ ...rest, }; - // Helper function to recursively extract text content from React elements - // Converts everything to plain text for screenreader announcements const extractTextContent = (children: React.ReactNode): string => { - if (!children) return ''; - if (typeof children === 'string' || typeof children === 'number') { return String(children); } - if (isValidElement(children)) { - const props = children.props as Record; - if (props.children) { - return extractTextContent(props.children as React.ReactNode); - } - return ''; - } - - // Children.toArray normalizes arrays and fragments automatically return Children.toArray(children) - .map((child) => extractTextContent(child)) + .map((child) => { + if (typeof child === 'string' || typeof child === 'number') { + return String(child); + } + if (typeof child === 'boolean' || child == null) { + return ''; + } + if (isValidElement(child)) { + return extractTextContent(child.props.children); + } + return ''; + }) + .filter(Boolean) .join(' '); }; + const extractedTextContent = useMemo(() => extractTextContent(info), [info]); + const screenreaderInfo = - shouldAnnounce && !isTipHidden ? extractTextContent(info) : `\xa0`; + shouldAnnounce && !isTipHidden ? extractedTextContent : `\xa0`; const text = ( Date: Fri, 7 Nov 2025 15:06:35 -0500 Subject: [PATCH 13/14] lint --- packages/gamut/src/Tip/InfoTip/index.tsx | 25 +++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index 48803534cc..cb512599e7 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -74,16 +74,19 @@ export const InfoTip: React.FC = ({ } }; - const handleOutsideClick = (e: MouseEvent) => { - if ( - wrapperRef.current && - (e.target instanceof HTMLElement - ? !wrapperRef.current?.contains(e?.target) - : true) - ) { - setTipIsHidden(true); - } - }; + const handleOutsideClick = useCallback( + (e: MouseEvent) => { + if ( + wrapperRef.current && + (e.target instanceof HTMLElement + ? !wrapperRef.current?.contains(e?.target) + : true) + ) { + setTipIsHidden(true); + } + }, + [setTipIsHidden] + ); const clickHandler = () => { const currentTipState = !isTipHidden; @@ -107,7 +110,7 @@ export const InfoTip: React.FC = ({ return () => { document.removeEventListener('mousedown', handleOutsideClick); }; - }); + }, [handleOutsideClick]); useEffect(() => { if (!isTipHidden && placement === 'floating') { From 7de0791e80b87c53c8f99c076227ddeadcfcc611 Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Fri, 7 Nov 2025 15:15:42 -0500 Subject: [PATCH 14/14] lint fix --- packages/gamut/src/Tip/InfoTip/index.tsx | 46 ++++++++++++------------ 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index cb512599e7..1f4cf5ab04 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -27,6 +27,30 @@ export type InfoTipProps = TipBaseProps & { onClick?: (arg0: { isTipHidden: boolean }) => void; }; +// Helper function to recursively extract text content from React elements +// Converts everything to plain text for screenreader announcements +const extractTextContent = (children: React.ReactNode): string => { + if (typeof children === 'string' || typeof children === 'number') { + return String(children); + } + + return Children.toArray(children) + .map((child) => { + if (typeof child === 'string' || typeof child === 'number') { + return String(child); + } + if (typeof child === 'boolean' || child == null) { + return ''; + } + if (isValidElement(child)) { + return extractTextContent(child.props.children); + } + return ''; + }) + .filter(Boolean) + .join(' '); +}; + export const InfoTip: React.FC = ({ alignment = 'top-right', emphasis = 'low', @@ -183,28 +207,6 @@ export const InfoTip: React.FC = ({ ...rest, }; - const extractTextContent = (children: React.ReactNode): string => { - if (typeof children === 'string' || typeof children === 'number') { - return String(children); - } - - return Children.toArray(children) - .map((child) => { - if (typeof child === 'string' || typeof child === 'number') { - return String(child); - } - if (typeof child === 'boolean' || child == null) { - return ''; - } - if (isValidElement(child)) { - return extractTextContent(child.props.children); - } - return ''; - }) - .filter(Boolean) - .join(' '); - }; - const extractedTextContent = useMemo(() => extractTextContent(info), [info]); const screenreaderInfo =