Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion __mocks__/platform-bible-react.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -775,6 +777,9 @@ export function PopoverContent({
children,
className,
'data-testid': testId = 'popover-content',
id,
role = 'dialog',
style,
onEscapeKeyDown,
onPointerDownOutside,
onOpenAutoFocus,
Expand All @@ -786,6 +791,9 @@ export function PopoverContent({
children?: ReactNode;
className?: string;
'data-testid'?: string;
id?: string;
role?: string;
style?: CSSProperties;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
align?: 'start' | 'center' | 'end';
sideOffset?: number;
onEscapeKeyDown?: (event: KeyboardEvent) => void;
Expand Down Expand Up @@ -822,7 +830,9 @@ export function PopoverContent({
aria-label={ariaLabel}
className={className}
data-testid={testId}
role="dialog"
id={id}
role={role}
style={style}
onClick={onClick}
onKeyDown={(e) => {
if (e.key === 'Escape') onEscapeKeyDown?.(e.nativeEvent);
Expand Down
104 changes: 45 additions & 59 deletions src/__tests__/components/TokenChip.suggestions.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -654,78 +654,64 @@ 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);

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%)',
});
expect(input).toHaveAttribute('role', 'combobox');
expect(input).toHaveAttribute('aria-expanded', 'true');
expect(input).toHaveAttribute('aria-controls', screen.getByRole('listbox').id);
});

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('collapses the input to the closed combobox state when the panel closes', 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');
await userEvent.type(input, 'mine');

expect(screen.queryByTestId('suggestion-accept')).not.toBeInTheDocument();
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 below 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') });

const input = await focusGloss('bank');

// A far user scroll pushes the anchor below the bottom edge, abandoning this token.
stubGlossRect(window.innerHeight + 50);
fireEvent.scroll(window);
expect(input).not.toHaveAttribute('aria-activedescendant');
});

expect(screen.queryByTestId('suggestion-accept')).not.toBeInTheDocument();
it('names the keyboard-highlighted row as the active descendant', async () => {
renderChip(wordToken('tok-new', 'bank'), { initialAnalysis: homographBankPool('finance') });

const input = await focusGloss('bank');
await userEvent.keyboard('{ArrowDown}');

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.
fireEvent.click(screen.getByTestId('popover-close'));

expect(input).toHaveFocus();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});
});
125 changes: 34 additions & 91 deletions src/components/SuggestionDropdown.tsx
Original file line number Diff line number Diff line change
@@ -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;
/**
Expand All @@ -27,111 +22,57 @@ 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.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
*/
export default function SuggestionDropdown({
anchorRef,
listboxId,
optionId,
entries,
activeIndex,
surfaceText,
onActiveIndexChange,
onSelect,
onRequestClose,
}: SuggestionDropdownProps) {
const listRef = useRef<HTMLUListElement | undefined>(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.
useLayoutEffect(() => {
Comment thread
imnasnainaec marked this conversation as resolved.
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(
<ul
ref={setListRef}
className="tw:fixed tw:z-30 tw:max-h-48 tw:overflow-y-auto tw:rounded-md tw:border tw:border-border tw:bg-popover tw:py-1 tw:shadow-md"
return (
Comment thread
imnasnainaec marked this conversation as resolved.
// `role` and `id` are spread onto the panel after the popover's own dialog role and id, so they
// win and the gloss input's aria-controls resolves to a real listbox. The class overrides shed
// the menu-sized content defaults this row list does not want.
<PopoverContent
Comment thread
imnasnainaec marked this conversation as resolved.
className="tw:max-h-48 tw:w-auto tw:gap-0 tw:overflow-y-auto tw:p-0 tw:py-1"
Comment thread
imnasnainaec marked this conversation as resolved.
Outdated
id={listboxId}
role="listbox"
// `left` is the input's center; translateX(-50%) keeps the panel centered on the input, and
// minWidth pins it to at least the input's width.
style={{
top: position.top,
left: position.left,
minWidth: position.width,
transform: 'translateX(-50%)',
}}
// The popover publishes its anchor's width on the panel; using it as the floor keeps the
Comment thread
imnasnainaec marked this conversation as resolved.
Outdated
// panel from rendering narrower than the gloss field.
style={{ minWidth: 'var(--radix-popover-trigger-width)' }}
// Both focus events are suppressed so the panel never moves DOM focus, which the combobox
// depends on: focus belongs to the gloss input the whole time the panel is open, and the
// panel closes on that input's blur — by which point focus is wherever the user aimed it.
onCloseAutoFocus={(e) => e.preventDefault()}
onOpenAutoFocus={(e) => e.preventDefault()}
>
{entries.map((entry, index) => (
<li
<div
key={entry.id}
aria-label={
entry.status === 'suggested'
Expand All @@ -143,6 +84,9 @@ export default function SuggestionDropdown({
data-testid={entry.status === 'suggested' ? 'suggestion-accept' : 'suggestion-candidate'}
id={optionId(index)}
role="option"
// An option role must be focusable, though nothing focuses a row: the gloss input holds
// focus and marks the active row with aria-activedescendant.
tabIndex={-1}
// Select on mouse-down, suppressing its default focus shift, so choosing a row never blurs
// the gloss input (the input keeps focus for the combobox) and the keyboard path stays the
// only place Enter/arrow handling lives.
Expand All @@ -153,9 +97,8 @@ export default function SuggestionDropdown({
onMouseEnter={() => onActiveIndexChange(index)}
>
{entry.gloss}
</li>
</div>
))}
</ul>,
document.body,
</PopoverContent>
);
}
Loading
Loading