From c79d553f70e9d4f7c04938703727d5c948d05dff Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 12:20:08 -0600 Subject: [PATCH 1/5] Add tooltips to icon-only buttons Every control that showed only an icon had its wording reachable by screen readers alone; each now names its action on hover. The mock `Tooltip` gained the provider requirement the real one enforces, which caught the view-options dropdown rendering outside every provider. --- __mocks__/platform-bible-react.tsx | 37 +++- contributions/localizedStrings.json | 3 +- src/__tests__/components/ArcOverlay.test.tsx | 87 +++++--- .../components/Interlinearizer.test.tsx | 23 ++ src/__tests__/components/PhraseBox.test.tsx | 134 +++++++++++- .../components/PhraseStripParts.test.tsx | 31 ++- src/__tests__/components/TokenChip.test.tsx | 61 +++++- .../components/TokenLinkIcon.test.tsx | 103 ++++++++- .../controls/ViewOptionsDropdown.test.tsx | 28 +++ src/__tests__/components/test-helpers.tsx | 22 +- src/__tests__/test-helpers.ts | 1 + src/__tests__/utils/phrase-arc.test.ts | 19 ++ src/components/ArcOverlay.tsx | 82 ++++---- src/components/ContinuousView.tsx | 2 + src/components/Interlinearizer.tsx | 3 +- src/components/PhraseBox.tsx | 197 ++++++++++++------ src/components/PhraseStripContext.tsx | 5 + src/components/SegmentListView.tsx | 28 ++- src/components/SegmentView.tsx | 2 + src/components/TokenChip.tsx | 55 +++-- src/components/TokenLinkIcon.tsx | 29 ++- .../controls/ViewOptionsDropdown.tsx | 148 +++++++------ src/components/tooltip-delay.ts | 6 + src/hooks/usePhraseStripSetup.ts | 5 + src/utils/phrase-arc.ts | 18 +- 25 files changed, 867 insertions(+), 262 deletions(-) create mode 100644 src/components/tooltip-delay.ts diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index f62ecdd6..08618739 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -736,13 +736,19 @@ export function Popover({ * Stub popover trigger. With `asChild` (the only mode the extension uses) the real component merges * its trigger behavior onto the single child element rather than rendering a wrapper, so this stub * clones the child with the open-state attributes and the toggle handler Radix would supply. + * + * Props cloned onto the trigger itself pass through to that same child, so an outer `asChild` + * trigger — a {@link Tooltip} wrapping a popover-triggering button — reaches the button rather than + * stopping here. */ export function PopoverTrigger({ children, -}: Readonly<{ children?: ReactNode; asChild?: boolean }>): ReactNode { + ...forwarded +}: Readonly<{ children?: ReactNode; asChild?: boolean }> & Record): ReactNode { const { onOpenChange, open = false } = useContext(PopoverContext); if (!isValidElement(children)) return <>{children}; return cloneElement(children, { + ...forwarded, 'aria-expanded': open, 'aria-haspopup': 'dialog', onClick: () => onOpenChange?.(!open), @@ -951,6 +957,9 @@ function tooltipContentText(node: ReactNode): string { return ''; } +/** Marks that a {@link TooltipProvider} is in scope, so {@link Tooltip} can require one. */ +const TooltipProviderContext = createContext(false); + /** * Stub tooltip root. The real component shows {@link TooltipContent} in a portaled popover on hover; * because native and Radix tooltips are both invisible in jsdom, this stub instead reads the @@ -960,8 +969,22 @@ function tooltipContentText(node: ReactNode): string { * tooltip in production. * * A tooltip whose content contributes no text gets no `title` at all, rather than an empty one. + * + * Props cloned onto the tooltip itself pass through to that same trigger child, so an outer + * `asChild` trigger — a {@link PopoverTrigger} wrapping a tooltipped button — reaches the button + * rather than stopping here, matching how the real components compose. + * + * @throws If rendered outside a {@link TooltipProvider}, as the real component does. A stub that + * rendered anywhere would let a tooltip placed outside every provider pass its tests and throw + * only once the extension ran. */ -export function Tooltip({ children }: Readonly<{ children?: ReactNode }>): ReactNode { +export function Tooltip({ + children, + ...forwarded +}: Readonly<{ children?: ReactNode }> & Record): ReactNode { + if (!useContext(TooltipProviderContext)) { + throw new Error('`Tooltip` must be used within `TooltipProvider`'); + } let tooltipText: ReactNode; let triggerChild: ReactNode; Children.forEach(children, (child) => { @@ -971,15 +994,19 @@ export function Tooltip({ children }: Readonly<{ children?: ReactNode }>): React }); if (!isValidElement(triggerChild)) return <>{children}; const text = tooltipContentText(tooltipText); - return cloneElement(triggerChild, { title: text === '' ? undefined : text }); + return cloneElement(triggerChild, { + ...forwarded, + title: text === '' ? undefined : text, + }); } /** * Stub tooltip provider that shares hover-delay config across nested tooltips. The stub renders its - * children unchanged; the delay has no effect in tests. + * children unchanged; the delay has no effect in tests. Its one modeled behavior is satisfying the + * provider requirement {@link Tooltip} enforces. */ export function TooltipProvider({ children, }: Readonly<{ children?: ReactNode; delayDuration?: number }>): ReactElement { - return <>{children}; + return {children}; } diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index dd7c949c..c711627b 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -49,7 +49,8 @@ "%interlinearizer_tokenChip_defineMorphemes%": "Define morpheme breakdown for {token}", "%interlinearizer_tokenChip_glossLabel%": "Gloss for {token}", "%interlinearizer_tokenChip_showSuggestions%": "Show suggestions for {token}", - "%interlinearizer_tokenChip_removeFromPhrase%": "Remove {token} from phrase", + "%interlinearizer_tokenChip_removeFromPhrase%": "Remove \"{token}\" from phrase", + "%interlinearizer_tokenChip_addToPhrase%": "Add \"{token}\" to phrase", "%interlinearizer_suggestion_accept%": "Accept suggestion {gloss} for {token}", "%interlinearizer_suggestion_promote%": "Promote {gloss} for {token}", "%interlinearizer_linkButton_crossSegmentDisabledTooltip%": "Tokens in different segments can't be linked. Join the segments first to link across this boundary.", diff --git a/src/__tests__/components/ArcOverlay.test.tsx b/src/__tests__/components/ArcOverlay.test.tsx index 21132a69..d18f34b7 100644 --- a/src/__tests__/components/ArcOverlay.test.tsx +++ b/src/__tests__/components/ArcOverlay.test.tsx @@ -3,9 +3,11 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import type { ReactElement } from 'react'; import { ArcOverlay } from '../../components/ArcOverlay'; import type { ArcPath } from '../../utils/phrase-arc'; import { makePhraseLink } from '../test-helpers'; +import { withTooltipProvider } from './test-helpers'; /** Builds a minimal `ArcPath` fixture. */ function makeArcPath(phraseId: string, splitAfterTokenRef = 'tok-a'): ArcPath { @@ -39,19 +41,24 @@ function requiredProps(): Parameters[0] { }; } +/** Renders an `ArcOverlay` inside the `TooltipProvider` its split buttons require. */ +function renderOverlay(ui: ReactElement) { + return render(withTooltipProvider(ui)); +} + describe('ArcOverlay', () => { it('returns undefined when arcPaths is empty', () => { - const { container } = render(); + const { container } = renderOverlay(); expect(container.firstChild).toBeNull(); }); it('renders an SVG when arcPaths has entries', () => { - render(); + renderOverlay(); expect(document.querySelector('svg')).toBeInTheDocument(); }); it('renders one SVG path element per arc', () => { - render( + renderOverlay( { it('does not render split buttons in edit mode', () => { const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b', 'tok-c']); - render( + renderOverlay( { it('renders a split button in view mode even when the arc phrase is neither hovered nor focused', () => { const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); - render( + renderOverlay( { it('labels the split button with the strip-supplied label', () => { const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); - render( + renderOverlay( { expect(screen.getByRole('button', { name: 'label-from-strip' })).toBeInTheDocument(); }); + it('names the split action on hover over the arc button', () => { + // The tooltip text rides the Tooltip component; the mock projects it onto the trigger as `title`. + const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); + renderOverlay( + , + ); + expect(screen.getByTestId('split-arc-btn')).toHaveAttribute('title', 'Split phrase here'); + }); + + it('shows no split tooltip while its localized string is still an unresolved key', () => { + // A `%…%` key straight from PAPI's async localization window would be visible hover text; the + // default fixture label is still an unresolved key. + const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); + renderOverlay( + , + ); + expect(screen.getByTestId('split-arc-btn')).not.toHaveAttribute('title'); + }); + it('with simplifyPhrases on, hides the split button for a non-focused phrase but keeps its arc', () => { const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); - render( + renderOverlay( { it('with simplifyPhrases on, keeps the split button for the focused phrase', () => { const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); - render( + renderOverlay( { it('renders a split button in view mode when the arc phrase is hovered', () => { const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); - render( + renderOverlay( { it('renders a split button in view mode when the arc phrase is focused', () => { const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); - render( + renderOverlay( { const onArcSplit = jest.fn(); const onSplitHoverChange = jest.fn(); const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b', 'tok-c']); - render( + renderOverlay( { const onSplitHoverChange = jest.fn(); // Two-token phrase: splitting after tok-a frees both halves (each length 1). const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); - render( + renderOverlay( { it('calls onSplitHoverChange with undefined on mouse leave', async () => { const onSplitHoverChange = jest.fn(); const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); - render( + renderOverlay( { const onSplitHoverChange = jest.fn(); // Four-token phrase: splitting after tok-b gives before=[tok-a,tok-b] and after=[tok-c,tok-d], both ≥ 2. const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b', 'tok-c', 'tok-d']); - render( + renderOverlay( { const onSplitHoverChange = jest.fn(); // Arc refers to 'tok-stale' which is not in the phrase token list. const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); - render( + renderOverlay( { it('renders the arc path when a phrase is highlighted only via candidatePhraseIds', () => { const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); - render( + renderOverlay( { it('renders a split button when a phrase is highlighted only via candidatePhraseIds', () => { const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); - render( + renderOverlay( { // splitHoveredArc.phraseId === 'p1' but splitHoveredArc.splitAfterTokenRef !== 'tok-b', so // tok-b's arc falls through to the focusedPhraseId branch. const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b', 'tok-c']); - render( + renderOverlay( { // candidatePhraseIds includes 'p1'. When tok-a's split button is hovered, tok-b's arc falls // through to the candidate highlight. const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b', 'tok-c']); - render( + renderOverlay( { const onSplitHoverChange = jest.fn(); // Four-token phrase: splitting after tok-b leaves both halves ≥ 2, so no token is freed. const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b', 'tok-c', 'tok-d']); - render( + renderOverlay( { it('clears the phrase highlight via onHoverPhrase on leave for a non-freeing split', async () => { const onHoverPhrase = jest.fn(); const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b', 'tok-c', 'tok-d']); - render( + renderOverlay( { const onHoverPhrase = jest.fn(); const onArcSplit = jest.fn(); const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b', 'tok-c', 'tok-d']); - render( + renderOverlay( { it('does not call onHoverPhrase on enter when the split would free a token', async () => { const onHoverPhrase = jest.fn(); const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); - render( + renderOverlay( { it('renders destructive stroke style when the split button is hovered', async () => { const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); - render( + renderOverlay( { // Four-token phrase: splitting after tok-b leaves both halves ≥ 2, so it is a reshape, not a // free. The hovered segment dims rather than turning destructive red. const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b', 'tok-c', 'tok-d']); - render( + renderOverlay( { it('restores the standard stroke after a non-freeing split hover leaves', async () => { const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b', 'tok-c', 'tok-d']); - render( + renderOverlay( { ).not.toBeInTheDocument(); }); + it('names the snap-to-active-verse action on hover', () => { + // The tooltip text rides the Tooltip component; the mock projects it onto the trigger as `title`. + mockKeyAsValueLocalizedStrings({ + '%interlinearizer_segmentList_scrollToActiveVerse%': 'Scroll to current reference', + }); + renderInterlinearizer({ book: GEN_1_MULTI_BOOK }); + + expect(screen.getByRole('button', { name: 'Scroll to current reference' })).toHaveAttribute( + 'title', + 'Scroll to current reference', + ); + }); + + it('shows no snap-to-active-verse tooltip while its localized string is an unresolved key', () => { + // A `%…%` key straight from PAPI's async localization window would be visible hover text; the + // suite-wide key-as-value stub already leaves this key unresolved. + renderInterlinearizer({ book: GEN_1_MULTI_BOOK }); + + expect( + screen.getByRole('button', { name: '%interlinearizer_segmentList_scrollToActiveVerse%' }), + ).not.toHaveAttribute('title'); + }); + it('snap button fades, recenters, then scrolls the active segment to the top', () => { jest.useFakeTimers(); try { diff --git a/src/__tests__/components/PhraseBox.test.tsx b/src/__tests__/components/PhraseBox.test.tsx index a466113f..57596c13 100644 --- a/src/__tests__/components/PhraseBox.test.tsx +++ b/src/__tests__/components/PhraseBox.test.tsx @@ -17,7 +17,7 @@ import { makePunctToken, makeWordToken, } from '../test-helpers'; -import { mockKeyAsValueLocalizedStrings } from './test-helpers'; +import { mockKeyAsValueLocalizedStrings, withTooltipProvider } from './test-helpers'; /** Stable mock fns for AnalysisStore hooks. */ const mockUseGloss = jest.fn().mockReturnValue(''); @@ -178,7 +178,9 @@ function requiredProps(): PhraseBoxTestProps { function renderBox(ui: ReactElement, context: Partial = {}) { return render( - {ui} + + {withTooltipProvider(ui)} + , ); } @@ -450,6 +452,43 @@ describe('PhraseBox', () => { expect(screen.queryByTestId('unlink-phrase-btn')).not.toBeInTheDocument(); }); + it('names the edit and unlink actions on hover', () => { + // The tooltip text rides the Tooltip component; the mock projects it onto the trigger as `title`. + mockUsePhraseLinkForToken.mockReturnValue(TEST_PHRASE_LINK); + renderBox(, { + phraseEditLabel: 'Edit phrase', + phraseUnlinkLabel: 'Unlink phrase', + }); + + expect(screen.getByTestId('edit-phrase-btn')).toHaveAttribute('title', 'Edit phrase'); + expect(screen.getByTestId('unlink-phrase-btn')).toHaveAttribute('title', 'Unlink phrase'); + }); + + it('shows no edit tooltip while its localized string is still an unresolved key', () => { + // A `%…%` key straight from PAPI's async localization window would be visible hover text. + mockUsePhraseLinkForToken.mockReturnValue(TEST_PHRASE_LINK); + renderBox(, { + phraseEditLabel: '%interlinearizer_phraseBox_edit%', + phraseUnlinkLabel: 'Unlink phrase', + }); + + const button = screen.getByTestId('edit-phrase-btn'); + expect(button).toHaveAttribute('aria-label', '%interlinearizer_phraseBox_edit%'); + expect(button).not.toHaveAttribute('title'); + }); + + it('shows no unlink tooltip while its localized string is still an unresolved key', () => { + mockUsePhraseLinkForToken.mockReturnValue(TEST_PHRASE_LINK); + renderBox(, { + phraseEditLabel: 'Edit phrase', + phraseUnlinkLabel: '%interlinearizer_phraseBox_unlink%', + }); + + const button = screen.getByTestId('unlink-phrase-btn'); + expect(button).toHaveAttribute('aria-label', '%interlinearizer_phraseBox_unlink%'); + expect(button).not.toHaveAttribute('title'); + }); + it('clicking edit sets phraseMode to edit for this phrase', async () => { mockUsePhraseLinkForToken.mockReturnValue(TEST_PHRASE_LINK); const setPhraseMode = jest.fn(); @@ -507,6 +546,97 @@ describe('PhraseBox', () => { expect(screen.getByTestId('inert-punct-1')).toBeInTheDocument(); }); + it('names the removal on hover over a chip of the phrase being edited', () => { + mockUsePhraseLinkForToken.mockReturnValue(TEST_PHRASE_LINK); + renderBox( + , + { + phraseMode: { kind: 'edit', phraseId: 'phrase-1', originalTokens: TEST_PHRASE_LINK.tokens }, + removeTokenFromPhraseTemplate: 'Remove {token} from phrase', + }, + ); + + expect(screen.getByRole('button', { name: 'Remove Hello from phrase' })).toHaveAttribute( + 'title', + 'Remove Hello from phrase', + ); + }); + + it('shows no removal tooltip while its localized string is still an unresolved key', () => { + mockUsePhraseLinkForToken.mockReturnValue(TEST_PHRASE_LINK); + renderBox( + , + { + phraseMode: { kind: 'edit', phraseId: 'phrase-1', originalTokens: TEST_PHRASE_LINK.tokens }, + removeTokenFromPhraseTemplate: '%interlinearizer_tokenChip_removeFromPhrase%', + }, + ); + + expect( + screen.getByRole('button', { name: '%interlinearizer_tokenChip_removeFromPhrase%' }), + ).not.toHaveAttribute('title'); + }); + + it('names the addition on hover over a free token during edit mode', () => { + renderBox(, { + // Edit mode is active for a different phrase, so this free box is one the edit can absorb. + phraseMode: { kind: 'edit', phraseId: 'other-phrase', originalTokens: [] }, + addTokenToPhraseTemplate: 'Add {token} to phrase', + }); + + expect(document.querySelector('[data-phrase-box="true"]')).toHaveAttribute( + 'title', + 'Add Hello to phrase', + ); + }); + + it('marks an addable box with a dashed border during edit mode', () => { + // The dimmed solid border says only "not part of this phrase", which an addable box also is, so + // the dash is what distinguishes a box the edit can absorb. + renderBox(, { + phraseMode: { kind: 'edit', phraseId: 'other-phrase', originalTokens: [] }, + }); + + expect(document.querySelector('[data-phrase-box="true"]')?.className).toContain( + 'tw:border-dashed', + ); + }); + + it('leaves a box the edit cannot absorb without the addable dash', () => { + mockUsePhraseLinkForToken.mockReturnValue(TEST_PHRASE_LINK); + renderBox( + , + { phraseMode: { kind: 'edit', phraseId: 'other-phrase', originalTokens: [] } }, + ); + + expect(document.querySelector('[data-phrase-box="true"]')?.className).not.toContain( + 'tw:border-dashed', + ); + }); + + it('names no addition on a box already belonging to a phrase', () => { + // A dimmed box the click cannot absorb promises nothing, so it names nothing. + mockUsePhraseLinkForToken.mockReturnValue(TEST_PHRASE_LINK); + renderBox( + , + { + phraseMode: { kind: 'edit', phraseId: 'other-phrase', originalTokens: [] }, + addTokenToPhraseTemplate: 'Add {token} to phrase', + }, + ); + + expect(document.querySelector('[data-phrase-box="true"]')).not.toHaveAttribute('title'); + }); + + it('shows no addition tooltip while its localized string is still an unresolved key', () => { + renderBox(, { + phraseMode: { kind: 'edit', phraseId: 'other-phrase', originalTokens: [] }, + addTokenToPhraseTemplate: '%interlinearizer_tokenChip_addToPhrase%', + }); + + expect(document.querySelector('[data-phrase-box="true"]')).not.toHaveAttribute('title'); + }); + it('renders punctuation between tokens for a non-edit-target box during edit mode', () => { renderBox( = {}, ): ReactElement { - return {ui}; + return ( + + {withTooltipProvider(ui)} + + ); } /** A `tokenDocOrder` standing the given refs up as the book's word tokens, in the order listed. */ @@ -401,7 +406,7 @@ describe('PhraseSlot boundary controls', () => { ...options.stripContext, })} > - + {withTooltipProvider()} , @@ -671,7 +676,13 @@ describe('PhraseSlot boundary controls', () => { > - + {withTooltipProvider( + , + )} , @@ -946,12 +957,14 @@ describe('PhraseStrip', () => { const items = [groupItem(link, ['tok-a'])]; render( - + {withTooltipProvider( + , + )} , ); // The phrase is hovered but not focused (focus is NO_FOCUS), so its controls are suppressed. diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index e124d7a1..419c197e 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -4,6 +4,7 @@ import { fireEvent, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { AssignmentStatus, Token, TokenSnapshot } from 'interlinearizer'; +import { TooltipProvider } from 'platform-bible-react'; import * as AnalysisStore from '../../components/AnalysisStore'; import { AnalysisStoreProvider } from '../../components/AnalysisStore'; import { InertTokenChip, TokenChip } from '../../components/TokenChip'; @@ -319,7 +320,9 @@ describe('TokenChip', () => { it('renders remove button when onRemove is provided', () => { render( - + + + , ); expect( @@ -327,6 +330,42 @@ describe('TokenChip', () => { ).toBeInTheDocument(); }); + it('names the removal on hover over the remove button', () => { + // The tooltip text rides the Tooltip component; the mock projects it onto the trigger as + // `title`. The template quotes the token so the word reads as distinct from the surrounding + // wording. + render( + + + + + , + ); + expect(screen.getByRole('button', { name: 'Remove "hello" from phrase' })).toHaveAttribute( + 'title', + 'Remove "hello" from phrase', + ); + }); + + it('shows no removal tooltip while its localized string is still an unresolved key', () => { + // A `%…%` key straight from PAPI's async localization window would be visible hover text; the + // suite-wide key-as-value stub already leaves this template unresolved. + render( + + + + + , + ); + expect( + screen.getByRole('button', { name: '%interlinearizer_tokenChip_removeFromPhrase%' }), + ).not.toHaveAttribute('title'); + }); + it('does not render remove button when onRemove is not provided', () => { render( @@ -342,7 +381,9 @@ describe('TokenChip', () => { const onRemove = jest.fn(); render( - + + + , ); await userEvent.click( @@ -354,7 +395,9 @@ describe('TokenChip', () => { it('applies destructive border on the remove button when hovered', async () => { render( - + + + , ); const removeBtn = screen.getByRole('button', { @@ -367,7 +410,9 @@ describe('TokenChip', () => { it('removes destructive border when pointer leaves the remove button', async () => { render( - + + + , ); const removeBtn = screen.getByRole('button', { @@ -382,7 +427,9 @@ describe('TokenChip', () => { const onRemove = jest.fn(); const { rerender } = render( - + + + , ); await userEvent.hover( @@ -390,7 +437,9 @@ describe('TokenChip', () => { ); rerender( - + + + , ); const label = screen.getByText('hello').closest('label'); diff --git a/src/__tests__/components/TokenLinkIcon.test.tsx b/src/__tests__/components/TokenLinkIcon.test.tsx index 1e1d9848..f5c283d2 100644 --- a/src/__tests__/components/TokenLinkIcon.test.tsx +++ b/src/__tests__/components/TokenLinkIcon.test.tsx @@ -11,6 +11,7 @@ import { } from '../../components/PhraseStripContext'; import type { SlotFocusInfo } from '../../types/token-layout'; import { makePhraseLink, makePhraseStripContext, makeWordToken } from '../test-helpers'; +import { withTooltipProvider } from './test-helpers'; const mockCreatePhrase = jest.fn(); const mockUpdatePhrase = jest.fn(); @@ -56,7 +57,9 @@ function requiredProps(): ComponentProps { /** Renders a `TokenLinkIcon` inside a strip provider carrying the given context overrides. */ function renderIcon(ui: ReactElement, context: Partial = {}) { return render( - {ui}, + + {withTooltipProvider(ui)} + , ); } @@ -97,6 +100,40 @@ describe('TokenLinkIcon', () => { expect(screen.getByTestId('token-unlink-btn')).toBeInTheDocument(); }); + it('names the unlink action on hover', () => { + // The unlink button is disabled in some modes, and a disabled button can't be the hover trigger, + // so the tooltip rides the wrapper span (the mock projects TooltipContent's text onto it). + const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); + renderIcon( + , + { unlinkTokensLabel: 'Unlink tokens' }, + ); + expect(screen.getByTestId('token-unlink-btn').parentElement).toHaveAttribute( + 'title', + 'Unlink tokens', + ); + }); + + it('shows no unlink tooltip while its localized string is still an unresolved key', () => { + // A `%…%` key straight from PAPI's async localization window would be visible hover text. + const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); + renderIcon( + , + { unlinkTokensLabel: '%interlinearizer_linkButton_unlink%' }, + ); + const button = screen.getByTestId('token-unlink-btn'); + expect(button).toHaveAttribute('aria-label', '%interlinearizer_linkButton_unlink%'); + expect(button.parentElement).not.toHaveAttribute('title'); + }); + it('clicking unlink calls splitPhraseAtBoundary (deletePhrase for 2-token phrase)', async () => { const phraseLink = makePhraseLink('p1', ['tok-a', 'tok-b']); renderIcon( @@ -250,6 +287,52 @@ describe('TokenLinkIcon', () => { expect(screen.getByTestId('token-link-btn')).toBeDisabled(); }); + it('names the link action on hover while the link is actionable', () => { + renderIcon( + , + { linkTokensLabel: 'Link tokens' }, + ); + expect(screen.getByTestId('token-link-btn').parentElement).toHaveAttribute( + 'title', + 'Link tokens', + ); + }); + + it('shows no link tooltip while its localized string is still an unresolved key', () => { + renderIcon( + , + { linkTokensLabel: '%interlinearizer_linkButton_link%' }, + ); + const button = screen.getByTestId('token-link-btn'); + expect(button).toHaveAttribute('aria-label', '%interlinearizer_linkButton_link%'); + expect(button.parentElement).not.toHaveAttribute('title'); + }); + + it('names no link action while the link is inert for a reason already visible in the UI', () => { + // Confirm-unlink mode shows its own prompt, so the button explains nothing on hover — unlike the + // cross-segment case below, whose cause is not otherwise on screen. + renderIcon(, { + linkTokensLabel: 'Link tokens', + phraseMode: { kind: 'confirm-unlink', phraseId: 'p1' }, + }); + const button = screen.getByTestId('token-link-btn'); + expect(button).toBeDisabled(); + expect(button).not.toHaveAttribute('title'); + expect(button.parentElement).not.toHaveAttribute('title'); + }); + it('creates a phrase when clicking link with two free tokens', async () => { const focusedFreeToken = makeWordToken('tok-a'); renderIcon( @@ -505,14 +588,16 @@ describe('TokenLinkIcon', () => { function renderCrossSegment(focusedSideIsPrev: boolean) { return render( - + {withTooltipProvider( + , + )} , ); } diff --git a/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx b/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx index 9ea061cd..e1741f19 100644 --- a/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx +++ b/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx @@ -37,6 +37,34 @@ describe('ViewOptionsDropdown', () => { expect(screen.queryByTestId('view-options-panel')).not.toBeInTheDocument(); }); + it('names the gear button on hover while the panel is closed', () => { + // The tooltip text rides the Tooltip component; the mock projects it onto the trigger as `title`. + mockKeyAsValueLocalizedStrings({ '%interlinearizer_viewOptions_label%': 'View options' }); + render(); + + expect(screen.getByTestId('view-options-button')).toHaveAttribute('title', 'View options'); + }); + + it('drops the gear tooltip while the panel is open', async () => { + // The panel it opened is already on screen, so a tooltip naming it would only overlap that. + mockKeyAsValueLocalizedStrings({ '%interlinearizer_viewOptions_label%': 'View options' }); + render(); + + await userEvent.click(screen.getByTestId('view-options-button')); + + expect(screen.getByTestId('view-options-button')).not.toHaveAttribute('title'); + }); + + it('shows no gear tooltip while its localized string is still an unresolved key', () => { + // A `%…%` key straight from PAPI's async localization window would be visible hover text; the + // suite-wide key-as-value stub already leaves this key unresolved. + render(); + + const button = screen.getByTestId('view-options-button'); + expect(button).toHaveAttribute('aria-label', '%interlinearizer_viewOptions_label%'); + expect(button).not.toHaveAttribute('title'); + }); + it('opens the panel when the gear button is clicked', async () => { render(); diff --git a/src/__tests__/components/test-helpers.tsx b/src/__tests__/components/test-helpers.tsx index 5a644efc..b797a410 100644 --- a/src/__tests__/components/test-helpers.tsx +++ b/src/__tests__/components/test-helpers.tsx @@ -1,5 +1,6 @@ import { useLocalizedStrings } from '@papi/frontend/react'; -import type { ReactNode } from 'react'; +import { TooltipProvider } from 'platform-bible-react'; +import type { ReactElement, ReactNode } from 'react'; import { AnalysisStoreProvider } from '../../components/AnalysisStore'; import { ViewOptions } from '../../types/view-options'; @@ -34,14 +35,29 @@ export function mockKeyAsValueLocalizedStrings(overrides: Record /** * Testing Library render options that wrap a subject in `AnalysisStoreProvider` with the default - * analysis language ("und") used across component tests. + * analysis language ("und") used across component tests, and in the `TooltipProvider` the platform + * `Tooltip` requires — standing in for the one the interlinear view supplies around the whole + * tree. */ export const withAnalysisStore = { wrapper({ children }: Readonly<{ children: ReactNode }>) { - return {children}; + return ( + + {children} + + ); }, }; +/** + * Wraps a subject in the `TooltipProvider` the platform `Tooltip` requires, standing in for the one + * the interlinear view supplies around the whole tree. A suite rendering a tooltipped component in + * isolation needs it; without one the component throws exactly as it would in the app. + */ +export function withTooltipProvider(children: ReactNode): ReactElement { + return {children}; +} + /** A {@link ViewOptions} object with every toggle set to `false`, for use as a test baseline. */ export const allFalseViewOptions: ViewOptions = { hideInactiveLinkButtons: false, diff --git a/src/__tests__/test-helpers.ts b/src/__tests__/test-helpers.ts index 02fa2191..642d891c 100644 --- a/src/__tests__/test-helpers.ts +++ b/src/__tests__/test-helpers.ts @@ -88,6 +88,7 @@ export function makePhraseStripContext( phraseEditLabel: '', phraseUnlinkLabel: '', removeTokenFromPhraseTemplate: '', + addTokenToPhraseTemplate: '', boundaryMergeLabel: '', boundaryMergeAltHint: '', boundarySplitLabel: '', diff --git a/src/__tests__/utils/phrase-arc.test.ts b/src/__tests__/utils/phrase-arc.test.ts index 5ca1c748..06f5ee66 100644 --- a/src/__tests__/utils/phrase-arc.test.ts +++ b/src/__tests__/utils/phrase-arc.test.ts @@ -10,6 +10,7 @@ import { buildCrossRowArcPath, computeAllArcPaths, computeStripRowGap, + computeStripTopPadding, deconflictSplitButtons, getArcStrokeProps, roundedPolyline, @@ -72,6 +73,24 @@ function buildContainer(boxes: { phraseId: string; r: DOMRect }[]): Element { return container; } +describe('computeStripTopPadding', () => { + it('reserves the whole controls pill for a phrase drawing no arc', () => { + // A contiguous phrase rides the box top rather than an arc, so the entire pill sits above that + // line — reserving only half of it clips the pill's top edge against the strip's top. + expect(computeStripTopPadding(false, 0, true)).toBe(28); + }); + + it('clears the highest arc stem plus the whole controls pill when both are present', () => { + const maxArcLevel = 3; + const arcClearance = ARC_BASE_STEM + 5 + 4 + maxArcLevel * ARC_LEVEL_STEP; + expect(computeStripTopPadding(true, maxArcLevel, true)).toBe(arcClearance + 28); + }); + + it('falls back to the verse-superscript headroom with no arcs and no phrase', () => { + expect(computeStripTopPadding(false, 0, false)).toBe(14); + }); +}); + describe('computeStripRowGap', () => { it('returns the base gap when there are no arcs', () => { expect(computeStripRowGap(false, 0, false)).toBe(BASE_ROW_GAP_PX); diff --git a/src/components/ArcOverlay.tsx b/src/components/ArcOverlay.tsx index 9451c45c..4c9e2099 100644 --- a/src/components/ArcOverlay.tsx +++ b/src/components/ArcOverlay.tsx @@ -1,8 +1,9 @@ import type { PhraseAnalysisLink } from 'interlinearizer'; import { Link2Off } from 'lucide-react'; -import { Button } from 'platform-bible-react'; +import { Button, Tooltip, TooltipContent, TooltipTrigger } from 'platform-bible-react'; import { memo, useState, useCallback } from 'react'; import type { PhraseMode } from '../types/phrase-mode'; +import { resolvedOrEmpty, tooltipContentOrUndefined } from '../utils/localized-strings'; import { computeSplitFreeRefs, getArcStrokeProps, type ArcPath } from '../utils/phrase-arc'; /** @@ -130,6 +131,8 @@ export function ArcOverlay({ }: ArcOverlayProps) { const [splitHoveredArc, setSplitHoveredArc] = useState(); + const splitTooltip = tooltipContentOrUndefined(resolvedOrEmpty(splitHereLabel)); + /** * Marks a free-split arc segment as hovered and notifies the parent of the token refs that would * become free if the split were confirmed. @@ -303,40 +306,49 @@ export function ArcOverlay({ ); const willCreateFreeTokens = arcSplitFreeRefs !== undefined; return ( - + + + + + {splitTooltip !== undefined && {splitTooltip}} + ); })} diff --git a/src/components/ContinuousView.tsx b/src/components/ContinuousView.tsx index b429a189..4488732a 100644 --- a/src/components/ContinuousView.tsx +++ b/src/components/ContinuousView.tsx @@ -70,6 +70,7 @@ const STRING_KEYS = [ '%interlinearizer_phraseBox_unlink%', '%interlinearizer_phraseBox_splitHere%', '%interlinearizer_tokenChip_removeFromPhrase%', + '%interlinearizer_tokenChip_addToPhrase%', '%interlinearizer_glossInput_placeholder%', '%interlinearizer_continuousView_previousToken%', '%interlinearizer_continuousView_nextToken%', @@ -969,6 +970,7 @@ export default function ContinuousView({ phraseEditLabel: localizedStrings['%interlinearizer_phraseBox_edit%'], phraseUnlinkLabel: localizedStrings['%interlinearizer_phraseBox_unlink%'], removeTokenFromPhraseTemplate: localizedStrings['%interlinearizer_tokenChip_removeFromPhrase%'], + addTokenToPhraseTemplate: localizedStrings['%interlinearizer_tokenChip_addToPhrase%'], glossPlaceholder: resolvedOrEmpty(localizedStrings['%interlinearizer_glossInput_placeholder%']), skipLinkTransition: !isVisible || skipSlotTransitionForJump, showMorphology, diff --git a/src/components/Interlinearizer.tsx b/src/components/Interlinearizer.tsx index 14978f53..f44d7541 100644 --- a/src/components/Interlinearizer.tsx +++ b/src/components/Interlinearizer.tsx @@ -16,6 +16,7 @@ import EditPhraseControls from './controls/EditPhraseControls'; import useBookIndexes from '../hooks/useBookIndexes'; import { useAltHeld } from '../hooks/useAltHeld'; import type { PhraseMode } from '../types/phrase-mode'; +import { TOOLTIP_DELAY_MS } from './tooltip-delay'; import type { ViewOptions } from '../types/view-options'; import { phrasesStraddlingBoundary, splitPhraseAtBoundary } from '../utils/phrase-arc'; import SegmentListView from './SegmentListView'; @@ -222,7 +223,7 @@ export default function Interlinearizer({ return ( - +
{(phraseMode.kind === 'confirm-unlink' || phraseMode.kind === 'edit') && ( diff --git a/src/components/PhraseBox.tsx b/src/components/PhraseBox.tsx index 79fd69ff..4f65893a 100644 --- a/src/components/PhraseBox.tsx +++ b/src/components/PhraseBox.tsx @@ -1,9 +1,10 @@ import type { PhraseAnalysisLink, Token } from 'interlinearizer'; import { Trash2 } from 'lucide-react'; -import { Button } from 'platform-bible-react'; +import { Button, Tooltip, TooltipContent, TooltipTrigger } from 'platform-bible-react'; import { formatReplacementString } from 'platform-bible-utils'; import { memo, useCallback, useEffect, useState } from 'react'; -import type { KeyboardEvent, MouseEvent as ReactMouseEvent } from 'react'; +import type { KeyboardEvent, MouseEvent as ReactMouseEvent, ReactNode } from 'react'; +import { resolvedOrEmpty, tooltipContentOrUndefined } from '../utils/localized-strings'; import { sortByDocOrder } from '../utils/phrase-arc'; import { NO_SLOT_FOCUS } from '../utils/token-layout'; import { @@ -66,6 +67,45 @@ function PhraseGlossInput({ ); } +/** + * Wraps one token chip of the phrase being edited in the clickable target that removes it, naming + * that outcome on hover so the chips of a phrase under edit read as the controls they are. + * + * Stays a `span` with a supplied key handler rather than a `Button`, because the chip it wraps + * carries a gloss input whose focus behavior a nested button would capture. + */ +function RemoveFromPhraseChip({ + label, + onRemove, + onKeyDown, + children, +}: Readonly<{ + /** Accessible label and hover text, with the token's surface text already substituted. */ + label: string; + onRemove: () => void; + onKeyDown: (e: KeyboardEvent) => void; + children: ReactNode; +}>) { + const tooltip = tooltipContentOrUndefined(resolvedOrEmpty(label)); + return ( + + + + {children} + + + {tooltip !== undefined && {tooltip}} + + ); +} + /** Props for {@link PhraseBox}. */ type PhraseBoxProps = Readonly<{ /** Whether this phrase is the current navigation focus. */ @@ -168,7 +208,10 @@ export function PhraseBox({ phraseEditLabel, phraseUnlinkLabel, removeTokenFromPhraseTemplate, + addTokenToPhraseTemplate, } = usePhraseStripContext(); + const editTooltip = tooltipContentOrUndefined(resolvedOrEmpty(phraseEditLabel)); + const unlinkTooltip = tooltipContentOrUndefined(resolvedOrEmpty(phraseUnlinkLabel)); // When simplifyPhrases is on, a phrase exposes its interactive controls only while focused. // Intra-phrase unlink icons are hidden via opacity/pointer-events (not unmounted) to preserve the // layout gap they occupy; the remove-token ✕ is omitted from onRemove instead (it's a prop-driven @@ -321,30 +364,40 @@ export function PhraseBox({ className="tw:absolute tw:top-0 tw:z-1 tw:left-1/2 tw:-translate-x-1/2 tw:-translate-y-full tw:inline-flex tw:gap-0.5 tw:rounded tw:border tw:phrase-hovered tw:bg-background tw:px-0.5 tw:py-px" data-phrase-controls="true" > - - + + + + + {editTooltip !== undefined && {editTooltip}} + + + + + + {unlinkTooltip !== undefined && {unlinkTooltip}} + )}
{ if (isDisabled) return 'tw:phrase-box-base tw:phrase-dimmed tw:opacity-40'; if (isSelected) return 'tw:phrase-box-base tw:border-ring tw:bg-muted/30'; - return 'tw:phrase-box-base tw:phrase-dimmed tw:cursor-pointer'; + // A dashed border reads as a slot to fill, marking which boxes the edit can absorb — the dimmed + // solid border above says only "not part of this phrase", which an addable box also is. + return 'tw:phrase-box-base tw:phrase-dimmed tw:cursor-pointer tw:border-dashed tw:hover:border-ring'; })(); if (isInEditTarget) { @@ -521,14 +576,11 @@ export function PhraseBox({ {i > 0 && punctuationBetween?.[i - 1]?.map((p) => )} - handleEditRemove(token.ref)} + onRemove={() => handleEditRemove(token.ref)} onKeyDown={handlePerTokenKeyDown(token.ref)} > - + ))} @@ -571,38 +623,55 @@ export function PhraseBox({ } }; + // Named on exactly the condition the click acts on, so the hover text promises an outcome the + // click delivers: a dimmed box belonging to another phrase or another segment offers nothing, and + // names nothing. + const addTooltip = + isDisabled || isInAnyPhrase + ? undefined + : tooltipContentOrUndefined( + resolvedOrEmpty( + formatReplacementString(addTokenToPhraseTemplate, { token: tokens[0].surfaceText }), + ), + ); + return ( - - - {tokens.map((token, i) => ( - - {i > 0 && - punctuationBetween?.[i - 1]?.map((p) => )} - + + + + + {tokens.map((token, i) => ( + + {i > 0 && + punctuationBetween?.[i - 1]?.map((p) => )} + + + ))} - ))} - - {isRealPhrase && showGlossInput && ( - - )} - + {isRealPhrase && showGlossInput && ( + + )} + + + {addTooltip !== undefined && {addTooltip}} + ); } diff --git a/src/components/PhraseStripContext.tsx b/src/components/PhraseStripContext.tsx index 60a6d175..7fd6a03d 100644 --- a/src/components/PhraseStripContext.tsx +++ b/src/components/PhraseStripContext.tsx @@ -126,6 +126,11 @@ export type PhraseStripContextValue = Readonly<{ * cannot drift between them. */ removeTokenFromPhraseTemplate: string; + /** + * Accessible label for adding a free token to the phrase being edited, with `{token}` still to be + * substituted for the token's surface text. + */ + addTokenToPhraseTemplate: string; /** * Label and concise tooltip for the merge boundary button, fetched once per strip rather than per * slot (every between-group slot renders its own boundary control). diff --git a/src/components/SegmentListView.tsx b/src/components/SegmentListView.tsx index b03c88ec..435e4a45 100644 --- a/src/components/SegmentListView.tsx +++ b/src/components/SegmentListView.tsx @@ -195,6 +195,9 @@ export default function SegmentListView({ const { selectSegment } = useFocusActions(); const [localizedStrings] = useLocalizedStrings(HEADER_STRING_KEYS); + const recenterTooltip = tooltipContentOrUndefined( + resolvedOrEmpty(localizedStrings['%interlinearizer_segmentList_scrollToActiveVerse%']), + ); /** * Inline verse-superscript labels for every segment (chapter-qualified where a verse start opens * a new chapter), keyed by segment id. Computed over the whole `book.segments` list (not just the @@ -366,16 +369,21 @@ export default function SegmentListView({ {pinnedChapter !== undefined ? `${bookName} ${pinnedChapter}` : ''} - + + + + + {recenterTooltip !== undefined && {recenterTooltip}} +
)} diff --git a/src/components/SegmentView.tsx b/src/components/SegmentView.tsx index 6344fa7e..4ad333fd 100644 --- a/src/components/SegmentView.tsx +++ b/src/components/SegmentView.tsx @@ -54,6 +54,7 @@ const STRING_KEYS = [ '%interlinearizer_phraseBox_unlink%', '%interlinearizer_phraseBox_splitHere%', '%interlinearizer_tokenChip_removeFromPhrase%', + '%interlinearizer_tokenChip_addToPhrase%', '%interlinearizer_glossInput_placeholder%', ] as const satisfies `%${string}%`[]; @@ -586,6 +587,7 @@ export function SegmentView({ phraseEditLabel: localizedStrings['%interlinearizer_phraseBox_edit%'], phraseUnlinkLabel: localizedStrings['%interlinearizer_phraseBox_unlink%'], removeTokenFromPhraseTemplate: localizedStrings['%interlinearizer_tokenChip_removeFromPhrase%'], + addTokenToPhraseTemplate: localizedStrings['%interlinearizer_tokenChip_addToPhrase%'], glossPlaceholder: resolvedOrEmpty(localizedStrings['%interlinearizer_glossInput_placeholder%']), skipLinkTransition: !hasMounted, showMorphology, diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index 1de1eb73..8eb72d3a 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -1,6 +1,13 @@ import type { Token } from 'interlinearizer'; import { Plus, X } from 'lucide-react'; -import { Button, Popover, PopoverAnchor } from 'platform-bible-react'; +import { + Button, + Popover, + PopoverAnchor, + Tooltip, + TooltipContent, + TooltipTrigger, +} from 'platform-bible-react'; import { formatReplacementString } from 'platform-bible-utils'; import { type KeyboardEvent, @@ -13,6 +20,7 @@ import { useRef, useState, } from 'react'; +import { resolvedOrEmpty, tooltipContentOrUndefined } from '../utils/localized-strings'; import { glossedSuggestionEntries } from '../utils/suggestion-engine'; import { useAnalysisLanguage, @@ -362,6 +370,9 @@ export function TokenChip({ // always present, so this governs only opacity/interactivity, never layout. const addVisible = inputFocused || chipHovered; + const removeLabel = formatReplacementString(removeLabelTemplate, { token: token.surfaceText }); + const removeTooltip = tooltipContentOrUndefined(resolvedOrEmpty(removeLabel)); + // The label is bound to the gloss input with an explicit htmlFor so clicking the chip body always // focuses the gloss input. Without it, the label's implicit control would be its first labelable // descendant — the morpheme trigger button (or a morpheme gloss input) when showMorphology is on — @@ -369,23 +380,31 @@ export function TokenChip({ return ( {onRemove && ( - + + + + + {removeTooltip !== undefined && {removeTooltip}} + )}