diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index 4c44d889..2ae317aa 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -757,6 +757,8 @@ export function PopoverAnchor({ * implements positioning, portaling, and dismissal internally; this stub exposes the dismissal * callbacks so tests can simulate them: * + * - `role`, `id`, and `style` land on the panel element, as they do in the real component, which + * spreads caller props after its own — so a caller can override the panel's dialog role and id. * - The panel's children render only from the second commit, mirroring Radix's portal (which renders * nothing until its own layout effect flips its `mounted` state). Consumers must therefore not * reach for a child element from their own mount effect — in the real app that element does not @@ -768,13 +770,24 @@ export function PopoverAnchor({ * - A sentinel `data-testid="popover-outside"` button invokes `onPointerDownOutside` on click, * simulating a pointer press outside the popover. * - A sentinel `data-testid="popover-close"` button invokes `onCloseAutoFocus` on click, - * simulating Radix's focus-restoration event fired as the popover closes. + * simulating Radix's focus-restoration event fired as the popover closes; unless it is prevented, + * the focused element is blurred. Which element Radix restores focus to — its trigger, or the + * element focused before the panel opened — is not modeled; only that an unprevented event lets + * the panel take focus away. + * - Both sentinels are siblings of the panel rather than children — scaffolding with no counterpart + * in the real component has no business among the children of a panel whose role a caller has + * overridden. + * - The layout props are accepted and ignored: they steer positioning the real component computes + * from measurements jsdom does not produce. */ export function PopoverContent({ 'aria-label': ariaLabel, children, className, 'data-testid': testId = 'popover-content', + id, + role = 'dialog', + style, onEscapeKeyDown, onPointerDownOutside, onOpenAutoFocus, @@ -786,8 +799,12 @@ export function PopoverContent({ children?: ReactNode; className?: string; 'data-testid'?: string; + id?: string; + role?: string; + style?: CSSProperties; align?: 'start' | 'center' | 'end'; sideOffset?: number; + hideWhenDetached?: boolean; onEscapeKeyDown?: (event: KeyboardEvent) => void; onPointerDownOutside?: (event: CustomEvent) => void; onOpenAutoFocus?: (event: Event) => void; @@ -807,29 +824,30 @@ export function PopoverContent({ openAutoFocusRef.current?.(event); if (event.defaultPrevented) return; const candidates = contentRef.current?.querySelectorAll('input, button') ?? []; - // The sentinels below are test scaffolding, not panel content, so they are never focus targets. - const first = Array.from(candidates).find( - (el) => !el.hasAttribute('disabled') && !el.dataset.testid?.startsWith('popover-'), - ); + const first = Array.from(candidates).find((el) => !el.hasAttribute('disabled')); first?.focus(); if (first instanceof HTMLInputElement) first.select(); }, [portalMounted]); if (!portalMounted) return
; return ( - // eslint-disable-next-line jsx-a11y/no-static-element-interactions -
{ - if (e.key === 'Escape') onEscapeKeyDown?.(e.nativeEvent); - }} - onMouseDown={onMouseDown} - > - {children} + <> + {/* eslint-disable-next-line jsx-a11y/no-static-element-interactions */} +
{ + if (e.key === 'Escape') onEscapeKeyDown?.(e.nativeEvent); + }} + onMouseDown={onMouseDown} + > + {children} +
{onPointerDownOutside && ( )} -
+ ); } diff --git a/src/__tests__/components/TokenChip.suggestions.test.tsx b/src/__tests__/components/TokenChip.suggestions.test.tsx index 5af09004..a6f858d6 100644 --- a/src/__tests__/components/TokenChip.suggestions.test.tsx +++ b/src/__tests__/components/TokenChip.suggestions.test.tsx @@ -654,78 +654,78 @@ describe('TokenChip suggestion + button', () => { }); }); -describe('TokenChip suggestion dropdown scrolling', () => { - /** - * Stubs the gloss input's `getBoundingClientRect` so the dropdown's scroll handler sees a - * definite on- or off-screen anchor (jsdom returns an all-zero rect by default, which reads as - * on-screen). - */ - function stubGlossRect(top: number): void { - jest.spyOn(HTMLInputElement.prototype, 'getBoundingClientRect').mockReturnValue({ - top, - bottom: top + 10, - left: 0, - right: 50, - x: 0, - y: top, - width: 50, - height: 10, - toJSON: () => ({}), - }); - } +describe('TokenChip suggestion combobox wiring', () => { + it('points the input at the open panel as its listbox', async () => { + renderChip(wordToken('tok-new', 'bank'), { initialAnalysis: homographBankPool('finance') }); - it('stays open and follows the anchor when the surrounding view scrolls it in view', async () => { - renderChip(wordToken('tok-2', 'logos'), { initialAnalysis: poolWithOneApproved('word') }); - await focusGloss('logos'); - expect(screen.getByTestId('suggestion-accept')).toBeInTheDocument(); + const input = await focusGloss('bank'); - // The token strip centers the focused phrase on focus: the anchor moves but stays in view, so - // the dropdown repositions under it rather than dismissing the panel that just opened. - stubGlossRect(100); - fireEvent.scroll(window); + expect(input).toHaveAttribute('role', 'combobox'); + expect(input).toHaveAttribute('aria-expanded', 'true'); + expect(input).toHaveAttribute('aria-controls', screen.getByRole('listbox').id); + }); - const listbox = screen.getByRole('listbox'); - expect(listbox).toBeInTheDocument(); - // `left` is the input's center (left 0 + width 50 / 2) and the panel is translated -50% so it - // stays centered on the input; `min-width` pins it to at least the input's width. - expect(listbox).toHaveStyle({ - top: '112px', - left: '25px', - minWidth: '50px', - transform: 'translateX(-50%)', - }); + it('collapses the input to the closed combobox state when the panel closes', async () => { + renderChip(wordToken('tok-new', 'bank'), { initialAnalysis: homographBankPool('finance') }); + + const input = await focusGloss('bank'); + await userEvent.type(input, 'mine'); + + expect(screen.queryByRole('listbox')).not.toBeInTheDocument(); + expect(input).toHaveAttribute('aria-expanded', 'false'); + expect(input).not.toHaveAttribute('aria-controls'); }); - it('closes when the surrounding view scrolls the anchor out of the viewport', async () => { - renderChip(wordToken('tok-2', 'logos'), { initialAnalysis: poolWithOneApproved('word') }); - await focusGloss('logos'); - expect(screen.getByTestId('suggestion-accept')).toBeInTheDocument(); + it('names no active descendant while no row is highlighted', async () => { + renderChip(wordToken('tok-new', 'bank'), { initialAnalysis: homographBankPool('finance') }); - // A far user scroll pushes the anchor above the top of the viewport, abandoning this token. - stubGlossRect(-50); - fireEvent.scroll(window); + const input = await focusGloss('bank'); - expect(screen.queryByTestId('suggestion-accept')).not.toBeInTheDocument(); + expect(input).not.toHaveAttribute('aria-activedescendant'); }); - it('closes when the surrounding view scrolls the anchor below the viewport', async () => { - renderChip(wordToken('tok-2', 'logos'), { initialAnalysis: poolWithOneApproved('word') }); - await focusGloss('logos'); - expect(screen.getByTestId('suggestion-accept')).toBeInTheDocument(); + it('names the keyboard-highlighted row as the active descendant', async () => { + renderChip(wordToken('tok-new', 'bank'), { initialAnalysis: homographBankPool('finance') }); - // A far user scroll pushes the anchor below the bottom edge, abandoning this token. - stubGlossRect(window.innerHeight + 50); - fireEvent.scroll(window); + const input = await focusGloss('bank'); + await userEvent.keyboard('{ArrowDown}'); - expect(screen.queryByTestId('suggestion-accept')).not.toBeInTheDocument(); + const [first] = screen.getAllByRole('option'); + expect(first).toHaveAttribute('aria-selected', 'true'); + expect(input).toHaveAttribute('aria-activedescendant', first.id); }); - it('stays open when the dropdown list itself is scrolled', async () => { - renderChip(wordToken('tok-2', 'logos'), { initialAnalysis: poolWithOneApproved('word') }); - await focusGloss('logos'); + it('keeps focus in the gloss input while the panel is open', async () => { + renderChip(wordToken('tok-new', 'bank'), { initialAnalysis: homographBankPool('finance') }); - fireEvent.scroll(screen.getByRole('listbox')); + const input = await focusGloss('bank'); - expect(screen.getByTestId('suggestion-accept')).toBeInTheDocument(); + expect(screen.getByRole('listbox')).toBeInTheDocument(); + expect(input).toHaveFocus(); + }); + + it('keeps focus in the gloss input when the panel restores focus as it closes', async () => { + renderChip(wordToken('tok-new', 'bank'), { initialAnalysis: homographBankPool('finance') }); + const input = await focusGloss('bank'); + + // fireEvent so the sentinel itself never takes focus, leaving the panel's own focus-restoration + // as the only thing that could move it — which, left unprevented, blurs the input. + fireEvent.click(screen.getByTestId('popover-close')); + + expect(input).toHaveFocus(); + }); +}); + +describe('TokenChip suggestion dropdown scrolling', () => { + // The other half of scrolling — the panel hiding itself once the anchor is clipped away — is the + // popover's own doing, off measurements jsdom does not produce, so it stays beyond reach here. + it('keeps the panel open when the surrounding view scrolls', async () => { + renderChip(wordToken('tok-new', 'bank'), { initialAnalysis: homographBankPool('finance') }); + const input = await focusGloss('bank'); + + fireEvent.scroll(window); + + expect(screen.getByRole('listbox')).toBeInTheDocument(); + expect(input).toHaveFocus(); }); }); diff --git a/src/components/SuggestionDropdown.tsx b/src/components/SuggestionDropdown.tsx index 80bcaff1..07642676 100644 --- a/src/components/SuggestionDropdown.tsx +++ b/src/components/SuggestionDropdown.tsx @@ -1,15 +1,10 @@ -import { useLayoutEffect, useRef, useState } from 'react'; -import { createPortal } from 'react-dom'; +import { PopoverContent } from 'platform-bible-react'; +import { useLayoutEffect } from 'react'; import { STATUS_TEXT_COLOR_CLASS } from '../types/status-colors'; import type { GlossedSuggestionEntry } from '../utils/suggestion-engine'; /** Props for {@link SuggestionDropdown}. */ type SuggestionDropdownProps = Readonly<{ - /** - * The gloss input this dropdown anchors under. Read on open (and window resize) to position the - * panel; never focused or mutated here so DOM focus stays in the input for the combobox. - */ - anchorRef: Readonly<{ current: HTMLInputElement | undefined }>; /** The listbox element id, matching the input's `aria-controls`. */ listboxId: string; /** @@ -27,22 +22,21 @@ type SuggestionDropdownProps = Readonly<{ onActiveIndexChange: (index: number) => void; /** Called with a payload id when a row is chosen (approve the suggested / promote a candidate). */ onSelect: (id: string) => void; - /** Called when the dropdown should close itself (the user scrolled the view away). */ - onRequestClose: () => void; }>; /** - * The pop-down listbox a token chip shows while its gloss input is the active combobox. Rendering - * and positioning live here; the combobox state — open, active row, keyboard — is owned by the - * chip, which drives this purely through props. Portaled to the document body so it escapes the - * clipping and stacking of the interlinear view's scroll viewports and token rows. + * The pop-down listbox a token chip shows while its gloss input is the active combobox. Every bit + * of combobox state — open, active row, keyboard — arrives as props; this holds none of it. + * + * Must be rendered inside a popover anchored on the gloss field: the panel stays with that field as + * the view scrolls it, and escapes the clipping and stacking of the interlinear view's scroll + * viewports and token rows. * * Each row is colored and labeled by its own `status` — `'suggested'` (blue, "accept") or - * `'candidate'` (grey, "promote") — carried on the entry rather than inferred from position, so a + * `'candidate'` (gray, "promote") — carried on the entry rather than inferred from position, so a * dropped blank-in-language pick can never leave a candidate masquerading as the accept row. */ export default function SuggestionDropdown({ - anchorRef, listboxId, optionId, entries, @@ -50,88 +44,50 @@ export default function SuggestionDropdown({ surfaceText, onActiveIndexChange, onSelect, - onRequestClose, }: SuggestionDropdownProps) { - const listRef = useRef(undefined); - // `left` is the anchor input's horizontal center (the panel is translated -50% so it stays - // centered on the input regardless of its own width); `width` is the input's width, applied as the - // panel's min-width so it never renders narrower than the input it belongs to. - const [position, setPosition] = useState<{ top: number; left: number; width: number }>({ - top: 0, - left: 0, - width: 0, - }); - - /** - * Ref callback that stores the list element, used to tell apart scrolling the panel's own - * overflow (ignored) from outer scrolling (closes). Normalizes React's `null` on unmount to - * `undefined`. - * - * @param el - The mounted list, or `null` on unmount. - */ - const setListRef = (el: HTMLUListElement | null) => { - listRef.current = el ?? undefined; - }; - - // Position the panel under the anchor and keep it glued there across resizes and outer scrolling. - // The continuous view smooth-scrolls the token strip on focus — the same focus that opens this - // dropdown — so we reposition as the anchor moves rather than close immediately; we close only - // once the anchor leaves the viewport (a far scroll that abandons this token). Layout effect so - // the first measurement runs before paint — otherwise the portaled panel flashes at the default - // top-left before snapping under the anchor. - useLayoutEffect(() => { - const anchor = anchorRef.current; - /* v8 ignore next -- the chip only mounts this while the input (the anchor) is rendered */ - if (!anchor) return undefined; - const updatePosition = () => { - const rect = anchor.getBoundingClientRect(); - setPosition({ top: rect.bottom + 2, left: rect.left + rect.width / 2, width: rect.width }); - }; - updatePosition(); - const handleScroll = (e: Event) => { - if (e.target instanceof Node && listRef.current?.contains(e.target)) return; - const rect = anchor.getBoundingClientRect(); - // The anchor has scrolled out of view: there is nothing to glue to, so dismiss the panel. - if (rect.bottom < 0 || rect.top > window.innerHeight) { - onRequestClose(); - return; - } - setPosition({ top: rect.bottom + 2, left: rect.left + rect.width / 2, width: rect.width }); - }; - window.addEventListener('resize', updatePosition); - window.addEventListener('scroll', handleScroll, true); - return () => { - window.removeEventListener('resize', updatePosition); - window.removeEventListener('scroll', handleScroll, true); - }; - }, [anchorRef, onRequestClose]); - - // Keep the keyboard-highlighted row inside the panel's scroll window. Arrow navigation moves - // activeIndex past the visible edge of the max-h-48 overflow without this; scrollIntoView with - // block: 'nearest' only scrolls when the row is actually clipped, so it leaves an in-view row put. + // Keep the keyboard-highlighted row inside the panel's scroll window, which arrow navigation + // otherwise walks straight past. block: 'nearest' only scrolls when the row is actually clipped, + // so an in-view row stays put. The row is found by id because the panel element belongs to the + // popover. + // + // On the first commit the popover has portaled nothing yet, so the lookup finds no row. That is + // harmless only because every path that opens the panel highlights no row, taking the early + // return; an open path arriving with a row already highlighted would silently fail to scroll. useLayoutEffect(() => { if (activeIndex < 0) return; - const active = listRef.current?.querySelector(`#${CSS.escape(optionId(activeIndex))}`); - active?.scrollIntoView({ block: 'nearest' }); + document.getElementById(optionId(activeIndex))?.scrollIntoView({ block: 'nearest' }); }, [activeIndex, optionId, entries]); - return createPortal( -
    e.preventDefault()} + onOpenAutoFocus={(e) => e.preventDefault()} > {entries.map((entry, index) => ( -
  • onActiveIndexChange(index)} > {entry.gloss} -
    • +
))} - , - document.body, + ); } diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index 258f183d..71281d80 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -340,9 +340,7 @@ export function TokenChip({ }; /** - * Ref callback that stores the gloss input element for focus control (from the "+" button) and as - * the dropdown's positioning anchor. Normalizes React's `null` on unmount to `undefined` to match - * the repo's ref-typing convention. + * Ref callback that stores the gloss input element for focus control from the "+" button. * * @param el - The mounted input, or `null` on unmount. */ @@ -447,100 +445,112 @@ export function TokenChip({ )} )} - {/* The gloss input acts as the combobox; the "+" button is a trailing in-field decoration - that summons the dropdown over typed text, shown only for a token with more than one - suggestion and fading in on focus/hover. The input reserves symmetric end-padding (sized - to clear the button) on EVERY chip regardless of suggestions or feature state, so the - gloss text stays centered, the chip never reflows as the button appears, and widths stay - uniform. The button stays out of the tab order so tabbing hits one stop per token. */} - - = 0 ? optionId(activeIndex) : undefined - } - aria-autocomplete={hasSuggestions ? 'none' : undefined} - aria-controls={dropdownShown ? listboxId : undefined} - aria-expanded={hasSuggestions ? dropdownShown : undefined} - aria-label={`Gloss for ${token.surfaceText}`} - // When the empty input shows a suggested gloss as its placeholder, color that ghost text - // via the same `gloss-suggested` utility the dropdown's accept row uses (one source of - // truth for the suggested blue) and italicize it at full opacity, so it reads as a - // suggestion rather than a faint generic hint. - className={`tw:gloss-input${showSuggestedPlaceholder ? ' tw:placeholder:gloss-suggested tw:placeholder:italic tw:placeholder:opacity-100' : ''}`} - disabled={disabled} - id={glossInputId} - placeholder={ - showSuggestedPlaceholder - ? `${suggestedGloss}${SUGGESTED_PLACEHOLDER_PAD}` - : glossPlaceholder - } - role={hasSuggestions ? 'combobox' : undefined} - // Inline padding overrides the `gloss-input` utility's default px to reserve room for the - // trailing "+" button symmetrically (keeping the gloss text centered) without a spacer - // element. The top margin is zeroed here and moved to the wrapping span so the span's box - // matches the input exactly, letting the absolutely-positioned button center on the input - // rather than on a box inflated at the top by the margin. - style={{ - fieldSizing: 'content', - marginTop: 0, - minWidth: '5ch', - paddingLeft: '0.75rem', - paddingRight: '0.75rem', - }} - value={draft} - onBlur={ - disabled - ? undefined - : () => { - setInputFocused(false); - closeSuggestions(); - commitDraft(); - } - } - onChange={(e) => handleDraftChange(e.target.value)} - onFocus={disabled ? undefined : handleFocus} - onKeyDown={disabled ? undefined : handleGlossKeyDown} - onMouseDown={disabled ? undefined : handleMouseDown} - type="text" - /> - {hasMultipleSuggestions && ( - + {/* A second popover root, independent of the morpheme editor's above. Anchoring the panel on + the gloss field keeps it with that field as the continuous view scrolls the focused token + into place. + + `onOpenChange` is intentionally omitted: this consumer owns every dismissal path (the + input's key, typing, and blur handling). Don't wire it without also removing those, or + closes would double-fire. */} + + {/* The gloss input acts as the combobox; the "+" button is a trailing in-field decoration + that summons the dropdown over typed text, shown only for a token with more than one + suggestion and fading in on focus/hover. The input reserves symmetric end-padding + (sized to clear the button) on EVERY chip regardless of suggestions or feature state, + so the gloss text stays centered, the chip never reflows as the button appears, and + widths stay uniform. The button stays out of the tab order so tabbing hits one stop per + token. */} + + + = 0 ? optionId(activeIndex) : undefined + } + aria-autocomplete={hasSuggestions ? 'none' : undefined} + aria-controls={dropdownShown ? listboxId : undefined} + aria-expanded={hasSuggestions ? dropdownShown : undefined} + aria-label={`Gloss for ${token.surfaceText}`} + // When the empty input shows a suggested gloss as its placeholder, color that ghost + // text via the same `gloss-suggested` utility the dropdown's accept row uses (one + // source of truth for the suggested blue) and italicize it at full opacity, so it + // reads as a suggestion rather than a faint generic hint. + className={`tw:gloss-input${showSuggestedPlaceholder ? ' tw:placeholder:gloss-suggested tw:placeholder:italic tw:placeholder:opacity-100' : ''}`} + disabled={disabled} + id={glossInputId} + placeholder={ + showSuggestedPlaceholder + ? `${suggestedGloss}${SUGGESTED_PLACEHOLDER_PAD}` + : glossPlaceholder + } + role={hasSuggestions ? 'combobox' : undefined} + // Inline padding overrides the `gloss-input` utility's default px to reserve room + // for the trailing "+" button symmetrically (keeping the gloss text centered) + // without a spacer element. The top margin is zeroed here and moved to the wrapping + // span so the span's box matches the input exactly, letting the absolutely- + // positioned button center on the input rather than on a box inflated at the top by + // the margin. + style={{ + fieldSizing: 'content', + marginTop: 0, + minWidth: '5ch', + paddingLeft: '0.75rem', + paddingRight: '0.75rem', + }} + value={draft} + onBlur={ + disabled + ? undefined + : () => { + setInputFocused(false); + closeSuggestions(); + commitDraft(); + } + } + onChange={(e) => handleDraftChange(e.target.value)} + onFocus={disabled ? undefined : handleFocus} + onKeyDown={disabled ? undefined : handleGlossKeyDown} + onMouseDown={disabled ? undefined : handleMouseDown} + type="text" + /> + {hasMultipleSuggestions && ( + + )} + + + {dropdownShown && ( + )} - - {dropdownShown && ( - - )} + );