From 7cf36c3f36be1d0bc042460e228227e4fc7ec660 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 14 Aug 2026 12:12:49 -0600 Subject: [PATCH 01/29] Add the read-only analysis catalog panel Hoists the analysis store above the cross-book fade curtain so a jump to a usage in another book cannot dim the panel. Search, sort, filter, and row windowing are deferred to #231. --- __mocks__/lucide-react.tsx | 14 + __mocks__/platform-bible-react.tsx | 26 + contributions/localizedStrings.json | 14 + contributions/menus.json | 16 + .../components/AnalysisCatalogPanel.test.tsx | 480 ++++++++++++++++++ .../components/InterlinearizerLoader.test.tsx | 83 ++- .../hooks/useInterlinearizerBookData.test.ts | 12 + src/__tests__/main.test.ts | 1 + src/__tests__/test-helpers.ts | 22 + src/components/AnalysisCatalogPanel.tsx | 179 +++++++ src/components/AnalysisStore.tsx | 19 + src/components/CatalogRowView.tsx | 193 +++++++ src/components/InterlinearizerLoader.tsx | 118 +++-- src/hooks/useInterlinearizerBookData.ts | 8 +- src/hooks/usePanelResize.ts | 114 +++++ src/main.ts | 14 + src/types/interlinearizer.d.ts | 7 + 17 files changed, 1282 insertions(+), 38 deletions(-) create mode 100644 src/__tests__/components/AnalysisCatalogPanel.test.tsx create mode 100644 src/components/AnalysisCatalogPanel.tsx create mode 100644 src/components/CatalogRowView.tsx create mode 100644 src/hooks/usePanelResize.ts diff --git a/__mocks__/lucide-react.tsx b/__mocks__/lucide-react.tsx index 12bd9903..38b2544b 100644 --- a/__mocks__/lucide-react.tsx +++ b/__mocks__/lucide-react.tsx @@ -84,3 +84,17 @@ export function Merge(props: Readonly<{ className?: string }>): ReactElement { export function Split(props: Readonly<{ size?: number; className?: string }>): ReactElement { return ; } + +/** + * Stub for the ChevronRight icon, marking a collapsed catalog row. + */ +export function ChevronRight(props: Readonly<{ className?: string }>): ReactElement { + return ; +} + +/** + * Stub for the ChevronDown icon, marking an expanded catalog row. + */ +export function ChevronDown(props: Readonly<{ className?: string }>): ReactElement { + return ; +} diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index f62ecdd6..714870f7 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -112,6 +112,15 @@ export const MOCK_WIPE_MENU_ITEM: MenuItemContainingCommand = { localizeNotes: '', }; +/** Sentinel menu item passed by the mock toolbar when the analysis-catalog button is clicked. */ +export const MOCK_OPEN_ANALYSIS_CATALOG_MENU_ITEM: MenuItemContainingCommand = { + label: '%interlinearizer_openAnalysisCatalog%', + command: 'interlinearizer.openAnalysisCatalog', + group: 'interlinearizer.viewActions', + order: 1, + localizeNotes: '', +}; + /** * Stub toolbar that renders project-menu and view-info buttons using sentinel menu items so tests @@ -195,6 +204,15 @@ export function TabToolbar({ Wipe )} + {onSelectProjectMenuItem && ( + + )} {onSelectViewInfoMenuItem && ( + + + {rows.length === 0 ? ( +

+ {localizedStrings['%interlinearizer_analysisCatalog_empty%']} +

+ ) : ( + + )} + + + ); +} diff --git a/src/components/AnalysisStore.tsx b/src/components/AnalysisStore.tsx index bc233e2a..53224897 100644 --- a/src/components/AnalysisStore.tsx +++ b/src/components/AnalysisStore.tsx @@ -17,6 +17,7 @@ import { selectAnalysis, selectAnalysisLanguage, selectApprovedGloss, + selectCatalogRows, selectApprovedMorphemes, selectMorphemeResetLosesGlosses, selectPhraseLinkByAnalysisId, @@ -33,6 +34,7 @@ import { writeSegmentFreeTranslation, } from '../store/analysisSlice'; import { emptyAnalysis } from '../types/empty-factories'; +import type { CatalogRow } from '../utils/analysis-query'; import { resolvedTokenAnalysisEqual, type ResolvedTokenAnalysis } from '../utils/suggestion-engine'; // #region Internal context @@ -364,6 +366,23 @@ export function useMorphemeResetLosesGlosses(tokenRef: string): boolean { ); } +/** + * Returns one row per distinct token analysis in the draft, each carrying the usage data the + * analysis catalog lists it by, in the analysis's own order. Narrowing and ordering are the + * caller's ({@link applyCatalogQuery}), so a keystroke re-runs only that pass. + * + * The result keeps its reference while the analyses and their links keep theirs, so an unrelated + * write — a free translation, a phrase link — leaves the list unrendered. It changes with + * `currentBook`, which the per-book usage count is taken against. + * + * @throws When called outside an {@link AnalysisStoreProvider}. + */ +export function useCatalogRows(currentBook: string): readonly CatalogRow[] { + useRequiredCallbacks('useCatalogRows'); + + return useSelector((state: AnalysisRootState) => selectCatalogRows(state.analysis, currentBook)); +} + /** * Returns the active BCP 47 analysis-language tag from the nearest {@link AnalysisStoreProvider}. * diff --git a/src/components/CatalogRowView.tsx b/src/components/CatalogRowView.tsx new file mode 100644 index 00000000..8cb54dff --- /dev/null +++ b/src/components/CatalogRowView.tsx @@ -0,0 +1,193 @@ +import { useLocalizedStrings } from '@papi/frontend/react'; +import { ChevronDown, ChevronRight } from 'lucide-react'; +import { Button } from 'platform-bible-react'; +import { formatReplacementString } from 'platform-bible-utils'; +import { useState } from 'react'; +import { useAnalysisLanguage } from './AnalysisStore'; +import type { CatalogRow, CatalogUsage } from '../utils/analysis-query'; + +/** + * Localized string keys a row needs. Hoisted to module scope so the reference passed to + * `useLocalizedStrings` is stable across renders; a fresh array literal each render makes the PAPI + * hook re-fetch and re-set state every render. + */ +const STRING_KEYS = [ + '%interlinearizer_analysisCatalog_noGloss%', + '%interlinearizer_analysisCatalog_usageCount%', + '%interlinearizer_analysisCatalog_usageCountInBook%', + '%interlinearizer_analysisCatalog_expandRow%', + '%interlinearizer_analysisCatalog_collapseRow%', + '%interlinearizer_analysisCatalog_noUsages%', + '%interlinearizer_analysisCatalog_showAllUsages%', +] as const satisfies `%${string}%`[]; + +/** + * How many usages an expanded row lists before the rest go behind an expander. An analysis applied + * across a whole book has hundreds; listing them all would bury every row beneath it. + */ +const INLINE_USAGE_LIMIT = 12; + +/** Props for {@link CatalogRowView}. */ +type CatalogRowViewProps = Readonly<{ + /** The analysis this row lists. */ + row: CatalogRow; + /** Book code `row.usageCountInBook` was taken against, named in that count's label. */ + currentBook: string; + /** Whether this is the row the view was last jumped from. */ + isSelected: boolean; + /** Jumps the interlinear view to one of this analysis's usages. */ + onUsageSelect: (analysisId: string, usage: CatalogUsage) => void; +}>; + +/** Renders a usage's location the way scripture references are written, e.g. `GEN 1:1`. */ +function usageLabel(usage: CatalogUsage): string { + return `${usage.book} ${usage.chapter}:${usage.verse}`; +} + +/** + * One analysis in the catalog: its surface form and gloss, and how much of the draft it accounts + * for — the whole draft's usage count beside the current book's. Expanding it reveals the morpheme + * breakdown and the places the analysis is applied. + * + * Each row owns its own layout so that its detail can be nested inside it. One element per analysis + * is what lets the list window and be walked by keyboard a row at a time. + */ +export default function CatalogRowView({ + row, + currentBook, + isSelected, + onUsageSelect, +}: CatalogRowViewProps) { + const [localizedStrings] = useLocalizedStrings(STRING_KEYS); + const analysisLanguage = useAnalysisLanguage(); + + const [isExpanded, setIsExpanded] = useState(false); + + /** Whether the usage list is showing every usage rather than the first {@link INLINE_USAGE_LIMIT}. */ + const [showsAllUsages, setShowsAllUsages] = useState(false); + + const visibleUsages = showsAllUsages ? row.usages : row.usages.slice(0, INLINE_USAGE_LIMIT); + const hiddenUsageCount = row.usages.length - visibleUsages.length; + + return ( +
  • + + + {isExpanded && ( +
    + {row.morphemes.length > 0 && ( +
    + {row.morphemes.map((morpheme) => ( + // Form above gloss, as the interlinear view arranges them, so a breakdown reads the + // same in both places. +
    + {morpheme.form} + + {morpheme.gloss?.[analysisLanguage] ?? ''} + +
    + ))} +
    + )} + + {row.usages.length === 0 ? ( +

    + {localizedStrings['%interlinearizer_analysisCatalog_noUsages%']} +

    + ) : ( +
    + {visibleUsages.map((usage) => ( + + ))} + {hiddenUsageCount > 0 && ( + + )} +
    + )} +
    + )} +
    • + ); +} diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 4a1de720..d8ca3d91 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -24,6 +24,7 @@ import type { SegmentationDispatch } from './SegmentationStore'; import type { InterlinearProjectSummary } from '../types/interlinear-project-summary'; import Interlinearizer from './Interlinearizer'; import { AnalysisStoreProvider } from './AnalysisStore'; +import AnalysisCatalogPanel from './AnalysisCatalogPanel'; import ViewOptionsDropdown from './controls/ViewOptionsDropdown'; import type { PhraseMode } from '../types/phrase-mode'; import ProjectModals, { type ModalState } from './modals/ProjectModals'; @@ -56,6 +57,9 @@ const BASE_TAB_TITLE = 'Interlinearizer'; /** Glyph appended to the tab title while the draft has unsaved changes. */ const UNSAVED_TAB_MARKER = ' ●'; +/** Width the analysis catalog panel opens at before the user has ever resized it, in pixels. */ +const DEFAULT_CATALOG_WIDTH_PX = 340; + /** * Localized string keys the load/error placeholder needs. Hoisted to module scope so the reference * passed to `useLocalizedStrings` is stable across renders; a fresh array literal each render makes @@ -246,6 +250,7 @@ function InterlinearizerLoaderInner({ isLoading, bookError, tokenizeError, + writingSystem, } = useInterlinearizerBookData({ projectId, scrRef, @@ -378,6 +383,19 @@ function InterlinearizerLoaderInner({ if (hasError) cancelFade(); }, [hasError, cancelFade]); + /** + * Whether the analysis catalog panel is showing. Tab-scoped rather than a project setting: two + * tabs on one project are routinely opened to look at different things, and a panel one of them + * opened has no business appearing in the other. + */ + const [catalogOpen, setCatalogOpen] = useWebViewState('analysisCatalogOpen', false); + + /** The catalog panel's width in pixels, tab-scoped for the same reason its open flag is. */ + const [catalogWidth, setCatalogWidth] = useWebViewState( + 'analysisCatalogWidth', + DEFAULT_CATALOG_WIDTH_PX, + ); + const [modal, setModal] = useState('none'); /** Whether the destructive wipe dialog (book / whole-draft scope picker) is open. */ @@ -466,6 +484,9 @@ function InterlinearizerLoaderInner({ /** Dismisses the wipe dialog, leaving the draft untouched. */ const handleWipeCancel = useCallback(() => setWipeModalOpen(false), []); + /** Dismisses the analysis catalog panel. */ + const handleCatalogClose = useCallback(() => setCatalogOpen(false), [setCatalogOpen]); + /** * Routes top-menu commands to the appropriate action. The project commands open their modals; the * file commands save (or open Save As); the draft command opens the wipe dialog. @@ -486,9 +507,11 @@ function InterlinearizerLoaderInner({ setModal('saveAs'); } else if (item.command === 'interlinearizer.wipe') { setWipeModalOpen(true); + } else if (item.command === 'interlinearizer.openAnalysisCatalog') { + setCatalogOpen(true); } }, - [activeProject, handleSave], + [activeProject, handleSave, setCatalogOpen], ); /** @@ -613,41 +636,64 @@ function InterlinearizerLoaderInner({ }} /> -
    - {isDraftLoading ? ( - // The store below waits for the draft: it seeds on mount alone, and the draft version - // that remounts it does not bump when the load completes. Nothing is lost by waiting — - // while the draft loads there is only ever a placeholder or an error panel to show. - loadingOrErrorPanel - ) : ( - // The store's lifetime is the draft's, not the loaded book's — it holds every book. Keyed - // on the draft version because the seed is not reactive, so a wholesale replacement (New - // / Open / Wipe) reseeds by remounting. Wrapping the loading and error branches too keeps - // it alive across the gap while the next book's USJ is in flight. - - {bookArea} - - )} -
    + {isDraftLoading ? ( + // The store below waits for the draft: it seeds on mount alone, and the draft version that + // remounts it does not bump when the load completes. Nothing is lost by waiting — while the + // draft loads there is only ever a placeholder or an error panel to show. +
    + {loadingOrErrorPanel} +
    + ) : ( + // The store's lifetime is the draft's, not the loaded book's — it holds every book. Keyed + // on the draft version because the seed is not reactive, so a wholesale replacement (New / + // Open / Wipe) reseeds by remounting. Wrapping the loading and error branches too keeps it + // alive across the gap while the next book's USJ is in flight. + // + // Declared above the cross-book curtain, not inside it, so the catalog panel can read the + // store without being dimmed by it: a jump to a usage in another book fades the view it + // navigates, and fading the list the jump was made from along with it would blank the panel + // at precisely the moment it is being used. + +
    +
    + {bookArea} +
    + + {catalogOpen && ( + + )} +
    +     + )} void; + /** Resizes by one step per arrow key. Attach to the resize handle. */ + onKeyDown: (event: ReactKeyboardEvent) => void; +} + +/** + * Drives a panel anchored to the container's end edge, resized in pixels by dragging a handle on + * its start edge or by arrowing that handle once focused. + * + * The committed width is the caller's to hold and persist, and a drag stays local until released, + * so a gesture crossing a hundred pixels reports once rather than once per frame — which matters + * when the caller's store is the host's. An arrow key reports on each press. + */ +export default function usePanelResize( + width: number, + onWidthChange: (width: number) => void, + bounds: PanelWidthBounds, +): PanelResize { + /** Width the panel is drawn at while a drag is in flight, or `undefined` when none is. */ + const [dragWidth, setDragWidth] = useState(undefined); + + /** Where the in-flight drag started, and the width it started from. */ + const dragOriginRef = useRef<{ clientX: number; width: number } | undefined>(undefined); + + const { min, max } = bounds; + + /** Holds a width within the range a drag may reach. */ + const clampWidth = useCallback( + (candidate: number) => Math.min(max, Math.max(min, candidate)), + [min, max], + ); + + const onMouseDown = useCallback( + (event: ReactMouseEvent) => { + dragOriginRef.current = { clientX: event.clientX, width }; + setDragWidth(width); + // Suppresses the text selection a drag across the panel would otherwise sweep up. + event.preventDefault(); + }, + [width], + ); + + // Runs the in-flight drag. Mounted only while one is in flight, so an idle panel listens to + // nothing. The listeners sit on the window rather than the handle because the pointer leaves the + // handle's box the moment the drag begins, and a release outside it must still end the drag. + useEffect(() => { + if (dragWidth === undefined) return undefined; + + const handleMouseMove = (event: MouseEvent) => { + const origin = dragOriginRef.current; + /* v8 ignore next -- the origin is set before this listener is ever mounted */ + if (!origin) return; + // The panel is anchored to the end edge, so the handle moving toward the start edge widens + // it: the delta is subtracted, not added. + setDragWidth(clampWidth(origin.width - (event.clientX - origin.clientX))); + }; + const handleMouseUp = () => { + setDragWidth((committed) => { + /* v8 ignore next -- a drag always has a width by the time it is released */ + if (committed !== undefined) onWidthChange(committed); + return undefined; + }); + dragOriginRef.current = undefined; + }; + + window.addEventListener('mousemove', handleMouseMove); + window.addEventListener('mouseup', handleMouseUp); + return () => { + window.removeEventListener('mousemove', handleMouseMove); + window.removeEventListener('mouseup', handleMouseUp); + }; + // `dragWidth` is read only as the in-flight flag; listing its value as a dep would tear down + // and remount both listeners on every frame of the drag. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [dragWidth === undefined, onWidthChange, clampWidth]); + + const onKeyDown = useCallback( + (event: ReactKeyboardEvent) => { + // Reading the visual direction rather than the writing direction: the key that moves the + // handle toward the start edge widens the panel, whichever edge the interface language puts + // that on. + // eslint-disable-next-line no-nested-ternary -- a two-key lookup reads worse as a map + const step = event.key === 'ArrowLeft' ? 1 : event.key === 'ArrowRight' ? -1 : 0; + if (step === 0) return; + event.preventDefault(); + onWidthChange(clampWidth(width + step * KEYBOARD_RESIZE_STEP_PX)); + }, + [width, onWidthChange, clampWidth], + ); + + return { displayWidth: dragWidth ?? width, onMouseDown, onKeyDown }; +} diff --git a/src/main.ts b/src/main.ts index 16fd3c8e..4a2f1e29 100644 --- a/src/main.ts +++ b/src/main.ts @@ -766,6 +766,19 @@ export async function activate(context: ExecutionActivationContext): Promise {}, + { + method: { + summary: 'Open the analysis catalog panel in the Interlinearizer WebView', + params: [], + result: { name: 'return value', summary: 'void', schema: { type: 'null' } }, + }, + }, + ); + const saveCommandRegistration = await papi.commands.registerCommand( 'interlinearizer.save', // Handled entirely in the WebView; backend registration makes the command known to the platform. @@ -838,6 +851,7 @@ export async function activate(context: ExecutionActivationContext): Promise Promise; + /** + * Opens the analysis catalog panel in the Interlinearizer WebView, listing every analysis the + * draft records with its usage counts and locations. The backend registers this command to make + * it visible to the platform menu system; all logic executes in the WebView. + */ + 'interlinearizer.openAnalysisCatalog': () => Promise; + /** * Loads the interlinearizer project with the given UUID, including its full `TextAnalysis`. The * WebView calls this when the active project changes to load the stored analysis. From 2699eced973d01b80f284b6cc63fd155a33186b4 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 14 Aug 2026 15:35:59 -0600 Subject: [PATCH 02/29] Adopt the platform Button for the catalog row toggle Also commits a released drag width from a ref rather than from inside the setDragWidth updater, which React may run more than once. --- .../components/InterlinearizerLoader.test.tsx | 12 +++++++++++ .../hooks/useInterlinearizerBookData.test.ts | 12 +++++++++++ src/components/CatalogRowView.tsx | 9 +++++--- src/hooks/usePanelResize.ts | 21 +++++++++++++------ 4 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 7debd161..0d6865d7 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -1796,6 +1796,18 @@ describe('InterlinearizerLoader', () => { expect(screen.getByTestId('interlinearizer')).toBeInTheDocument(); }); + it('keeps the catalog panel out of the wrapper the cross-book fade dims', async () => { + await act(async () => { + renderLoader(); + }); + + await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); + + expect(screen.getByTestId('book-fade-wrapper')).not.toContainElement( + screen.getByTestId('analysis-catalog-panel'), + ); + }); + it('closes the catalog panel from its own close control', async () => { await act(async () => { renderLoader(); diff --git a/src/__tests__/hooks/useInterlinearizerBookData.test.ts b/src/__tests__/hooks/useInterlinearizerBookData.test.ts index c0fb3778..dfec6117 100644 --- a/src/__tests__/hooks/useInterlinearizerBookData.test.ts +++ b/src/__tests__/hooks/useInterlinearizerBookData.test.ts @@ -190,6 +190,18 @@ describe('useInterlinearizerBookData', () => { expect(jest.mocked(extractBookFromUsj)).toHaveBeenCalledWith({ USJ: 'mock-usj' }, 'und'); }); + it('reports the "und" fallback before the book has loaded', () => { + jest.mocked(useProjectData).mockReturnValue({ BookUSJ: () => [undefined, jest.fn(), true] }); + mockUseProjectSettings(''); + + const { result } = renderHook(() => + useInterlinearizerBookData({ projectId: 'test-project', scrRef: { ...GEN_1_1_SRC_REF } }), + ); + + expect(result.current.book).toBeUndefined(); + expect(result.current.writingSystem).toBe('und'); + }); + it('logs tokenization error when hook has projectId and tokenizeError occurs', () => { jest.mocked(extractBookFromUsj).mockReturnValue(TEST_RAW_BOOK); diff --git a/src/components/CatalogRowView.tsx b/src/components/CatalogRowView.tsx index 8cb54dff..8cd43e28 100644 --- a/src/components/CatalogRowView.tsx +++ b/src/components/CatalogRowView.tsx @@ -78,7 +78,7 @@ export default function CatalogRowView({ data-selected={String(isSelected)} data-testid="catalog-row" > - + {isExpanded && (
    (undefined); + /** + * The in-flight drag's latest width, mirroring `dragWidth`. A release commits the width from here + * because a state updater has to stay free of side effects — React may run one more than once. + */ + const dragWidthRef = useRef(undefined); + const { min, max } = bounds; /** Holds a width within the range a drag may reach. */ @@ -55,6 +61,7 @@ export default function usePanelResize( const onMouseDown = useCallback( (event: ReactMouseEvent) => { dragOriginRef.current = { clientX: event.clientX, width }; + dragWidthRef.current = width; setDragWidth(width); // Suppresses the text selection a drag across the panel would otherwise sweep up. event.preventDefault(); @@ -74,14 +81,16 @@ export default function usePanelResize( if (!origin) return; // The panel is anchored to the end edge, so the handle moving toward the start edge widens // it: the delta is subtracted, not added. - setDragWidth(clampWidth(origin.width - (event.clientX - origin.clientX))); + const next = clampWidth(origin.width - (event.clientX - origin.clientX)); + dragWidthRef.current = next; + setDragWidth(next); }; const handleMouseUp = () => { - setDragWidth((committed) => { - /* v8 ignore next -- a drag always has a width by the time it is released */ - if (committed !== undefined) onWidthChange(committed); - return undefined; - }); + const committed = dragWidthRef.current; + /* v8 ignore next -- a drag always has a width by the time it is released */ + if (committed !== undefined) onWidthChange(committed); + setDragWidth(undefined); + dragWidthRef.current = undefined; dragOriginRef.current = undefined; }; From 528a73c9c5859fcd89544a1586cf9b4de290dea3 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 14 Aug 2026 16:04:11 -0600 Subject: [PATCH 03/29] Harden the analysis catalog against bad input, RTL, and screen readers An unparseable analysis-language tag threw out of Intl.Collator and blanked the whole view; the resize handle inverted in right-to-left interfaces; and the row toggle's aria-label suppressed the analysis it named. Rows now share one localization subscription instead of one apiece. --- REVIEW.md | 2 +- contributions/localizedStrings.json | 2 - .../components/AnalysisCatalogPanel.test.tsx | 108 ++++++++++++++++++ src/__tests__/utils/language-tags.test.ts | 17 +++ src/components/AnalysisCatalogPanel.tsx | 25 ++-- src/components/CatalogRowView.tsx | 54 +++++---- src/components/InterlinearizerLoader.tsx | 4 +- src/hooks/usePanelResize.ts | 45 +++++--- src/utils/language-tags.ts | 15 +++ 9 files changed, 216 insertions(+), 56 deletions(-) create mode 100644 src/__tests__/utils/language-tags.test.ts diff --git a/REVIEW.md b/REVIEW.md index 2db5e0f7..d23a8f87 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -62,7 +62,7 @@ The merge control's **tooltip** runs its localized strings through `resolvedOrEm ## A `title` in a test assertion does not mean the code renders a `title` attribute -Every tooltip in this extension goes through the platform `Tooltip`/`TooltipContent`; none sets an HTML `title` attribute. Tests nevertheless assert `toHaveAttribute('title', …)` because the `Tooltip` stub in [\_\_mocks\_\_/platform-bible-react.tsx](__mocks__/platform-bible-react.tsx) reads its `TooltipContent` child's text and clones the trigger with that text as a `title`, which keeps the tooltip assertable without simulating hover in jsdom. The `title` **prop** some components take (the boundary button's, for one) is likewise just a prop name; it is rendered as `TooltipContent` children. +Every tooltip in this extension goes through the platform `Tooltip`/`TooltipContent`, with one documented exception: the usage counts in [src/components/CatalogRowView.tsx](src/components/CatalogRowView.tsx) sit inside the row's own button, where a tooltip trigger would nest one interactive element in another, so they carry a native `title` and repeat their label in screen-reader-only text. Outside that exception, no component sets an HTML `title` attribute. Tests nevertheless assert `toHaveAttribute('title', …)` because the `Tooltip` stub in [\_\_mocks\_\_/platform-bible-react.tsx](__mocks__/platform-bible-react.tsx) reads its `TooltipContent` child's text and clones the trigger with that text as a `title`, which keeps the tooltip assertable without simulating hover in jsdom. The `title` **prop** some components take (the boundary button's, for one) is likewise just a prop name; it is rendered as `TooltipContent` children. So do not conclude from either signal that a control is limited to plain text — for instance, that it cannot hold a `Kbd` or any other element. Read the component's own JSX before claiming a render path is text-only. diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index f28c3310..2eb66e79 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -23,8 +23,6 @@ "%interlinearizer_analysisCatalog_noGloss%": "(no gloss)", "%interlinearizer_analysisCatalog_usageCount%": "Uses in the whole draft", "%interlinearizer_analysisCatalog_usageCountInBook%": "Uses in {book}", - "%interlinearizer_analysisCatalog_expandRow%": "Show breakdown and usages", - "%interlinearizer_analysisCatalog_collapseRow%": "Hide breakdown and usages", "%interlinearizer_analysisCatalog_noUsages%": "Not used anywhere", "%interlinearizer_analysisCatalog_showAllUsages%": "Show {count} more", diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index f3595811..a72607f2 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -179,6 +179,67 @@ describe('AnalysisCatalogPanel', () => { expect(onWidthChange).not.toHaveBeenCalled(); }); + + it('commits nothing when the handle is pressed without being moved', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); + fireEvent.mouseUp(window, { clientX: 500 }); + + // The width is persisted, so a stray click on the handle would otherwise put an unchanged + // width through the host. + expect(onWidthChange).not.toHaveBeenCalled(); + }); + + describe('in a right-to-left interface', () => { + beforeEach(() => { + document.documentElement.dir = 'rtl'; + }); + + // Plain assignment to the document, which `restoreMocks` cannot undo. + afterEach(() => { + document.documentElement.dir = ''; + }); + + it('widens the panel when the handle is dragged toward the start edge', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + // The end edge the panel is anchored to is the screen's left here, putting the handle on + // its right, so the travel that widens it is the mirror of the left-to-right one. + dragHandle(500, 540); + + expect(onWidthChange).toHaveBeenCalledWith(360); + }); + + it('narrows the panel when the handle is dragged toward the end edge', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + dragHandle(500, 460); + + expect(onWidthChange).toHaveBeenCalledWith(280); + }); + + it('widens the panel by one step on ArrowRight', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowRight' }); + + expect(onWidthChange).toHaveBeenCalledWith(336); + }); + + it('narrows the panel by one step on ArrowLeft', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowLeft' }); + + expect(onWidthChange).toHaveBeenCalledWith(304); + }); + }); }); describe('rows', () => { @@ -224,6 +285,53 @@ describe('AnalysisCatalogPanel', () => { ]); }); + it('lists the rows under an analysis language Intl cannot parse', () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [{ ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'λόγος' }], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0')], + }; + + // Analysis languages are free text from the project modals, so a hand-typed tag reaches the + // sort's collator unchecked. Throwing here would take the whole view down with it. + renderPanel({ analysis, analysisLanguage: 'en_US' }); + + expect(within(rowFor('ta-1')).getByTestId('catalog-row-surface')).toHaveTextContent('λόγος'); + }); + + it('leaves the row toggle unlabeled so that its own content names it', () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [{ ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'λόγος' }], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0')], + }; + renderPanel({ analysis }); + + // An accessible name on a button overrides everything inside it, so labeling the row would + // leave every row announced alike and the analysis itself unread. + expect(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')).not.toHaveAttribute( + 'aria-label', + ); + }); + + it('spells out what each usage count means for assistive tech', () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [{ ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'λόγος' }], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0')], + }; + renderPanel({ analysis, currentBook: 'GEN' }); + + // The counts render as bare numerals, and their `title` is not announced on a span. + const row = within(rowFor('ta-1')); + expect(row.getByTestId('catalog-row-usage-count')).toHaveTextContent( + '%interlinearizer_analysisCatalog_usageCount%', + ); + expect(row.getByTestId('catalog-row-usage-count-in-book')).toHaveTextContent( + '%interlinearizer_analysisCatalog_usageCountInBook%', + ); + }); + it('marks an analysis with no gloss in the active language rather than leaving the cell blank', () => { const analysis: TextAnalysis = { ...emptyAnalysis(), diff --git a/src/__tests__/utils/language-tags.test.ts b/src/__tests__/utils/language-tags.test.ts new file mode 100644 index 00000000..74608641 --- /dev/null +++ b/src/__tests__/utils/language-tags.test.ts @@ -0,0 +1,17 @@ +/// + +import { collatorForTag } from '../../utils/language-tags'; + +describe('collatorForTag', () => { + it('collates under the tag it is given', () => { + // Swedish sorts "ä" after "z", which the default locale does not, so the tag demonstrably + // reached the collator rather than being dropped. + expect(collatorForTag('sv').compare('ä', 'z')).toBeGreaterThan(0); + }); + + it('falls back to the default collation for a tag Intl rejects', () => { + // Underscores instead of hyphens is the classic hand-typed tag, and `Intl` throws on it. + expect(() => collatorForTag('en_US')).not.toThrow(); + expect(collatorForTag('en_US').compare('a', 'b')).toBeLessThan(0); + }); +}); diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index 3438b6f3..b9b96b46 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -3,21 +3,24 @@ import { X } from 'lucide-react'; import { Button } from 'platform-bible-react'; import { useCallback, useMemo, useState } from 'react'; import { useAnalysisLanguage, useCatalogRows } from './AnalysisStore'; -import CatalogRowView from './CatalogRowView'; +import CatalogRowView, { ROW_STRING_KEYS } from './CatalogRowView'; import { useInterlinearNav } from './InterlinearNavContext'; import usePanelResize, { type PanelWidthBounds } from '../hooks/usePanelResize'; import { applyCatalogQuery, type CatalogQuery, type CatalogUsage } from '../utils/analysis-query'; +import { collatorForTag } from '../utils/language-tags'; /** - * Localized string keys the panel needs. Hoisted to module scope so the reference passed to - * `useLocalizedStrings` is stable across renders; a fresh array literal each render makes the PAPI - * hook re-fetch and re-set state every render. + * Localized string keys the panel needs, the rows' among them so the list resolves once rather than + * once per analysis. Hoisted to module scope so the reference passed to `useLocalizedStrings` is + * stable across renders; a fresh array literal each render makes the PAPI hook re-fetch and re-set + * state every render. */ const STRING_KEYS = [ '%interlinearizer_analysisCatalog_title%', '%interlinearizer_analysisCatalog_close%', '%interlinearizer_analysisCatalog_resize%', '%interlinearizer_analysisCatalog_empty%', + ...ROW_STRING_KEYS, ] as const satisfies `%${string}%`[]; /** @@ -70,8 +73,8 @@ export default function AnalysisCatalogPanel({ search: '', sort: 'usageCount', filters: {}, - surfaceCollator: new Intl.Collator(sourceLanguageTag), - glossCollator: new Intl.Collator(analysisLanguage), + surfaceCollator: collatorForTag(sourceLanguageTag), + glossCollator: collatorForTag(analysisLanguage), }), [sourceLanguageTag, analysisLanguage], ); @@ -90,10 +93,10 @@ export default function AnalysisCatalogPanel({ /** * Moves the interlinear view to a usage: the verse it sits in, then the token itself. * - * The focus request is raised before the navigation, not after: a pending request is abandoned - * once navigation lands on a book other than the one it names, so one raised afterward would be - * discarded by its own navigation. A cross-book jump therefore leaves the request outstanding - * until that book's view mounts and claims it. + * The focus request is raised before the navigation so that it is already pending when the + * reference moves. A request is abandoned only once the reference names a book other than the one + * the request does, so a cross-book jump leaves it outstanding until that book's view mounts and + * claims it. * * The navigation is external — the default — because a usage may name any verse in the draft, so * the view has to recenter on it rather than track it in place. @@ -165,8 +168,10 @@ export default function AnalysisCatalogPanel({ {rows.map((row) => ( diff --git a/src/components/CatalogRowView.tsx b/src/components/CatalogRowView.tsx index 8cd43e28..5b44165e 100644 --- a/src/components/CatalogRowView.tsx +++ b/src/components/CatalogRowView.tsx @@ -1,22 +1,18 @@ -import { useLocalizedStrings } from '@papi/frontend/react'; import { ChevronDown, ChevronRight } from 'lucide-react'; import { Button } from 'platform-bible-react'; -import { formatReplacementString } from 'platform-bible-utils'; +import { formatReplacementString, type LanguageStrings } from 'platform-bible-utils'; import { useState } from 'react'; -import { useAnalysisLanguage } from './AnalysisStore'; import type { CatalogRow, CatalogUsage } from '../utils/analysis-query'; /** - * Localized string keys a row needs. Hoisted to module scope so the reference passed to - * `useLocalizedStrings` is stable across renders; a fresh array literal each render makes the PAPI - * hook re-fetch and re-set state every render. + * Localized string keys a row renders. Every row asks for the same strings, and subscribing per row + * would be a subscription per analysis in the draft, so they are resolved above the list and handed + * down. */ -const STRING_KEYS = [ +export const ROW_STRING_KEYS = [ '%interlinearizer_analysisCatalog_noGloss%', '%interlinearizer_analysisCatalog_usageCount%', '%interlinearizer_analysisCatalog_usageCountInBook%', - '%interlinearizer_analysisCatalog_expandRow%', - '%interlinearizer_analysisCatalog_collapseRow%', '%interlinearizer_analysisCatalog_noUsages%', '%interlinearizer_analysisCatalog_showAllUsages%', ] as const satisfies `%${string}%`[]; @@ -37,6 +33,10 @@ type CatalogRowViewProps = Readonly<{ isSelected: boolean; /** Jumps the interlinear view to one of this analysis's usages. */ onUsageSelect: (analysisId: string, usage: CatalogUsage) => void; + /** Resolved localizations covering at least {@link ROW_STRING_KEYS}, shared by the whole list. */ + localizedStrings: LanguageStrings; + /** BCP 47 tag the morpheme glosses are read under. */ + analysisLanguage: string; }>; /** Renders a usage's location the way scripture references are written, e.g. `GEN 1:1`. */ @@ -57,10 +57,9 @@ export default function CatalogRowView({ currentBook, isSelected, onUsageSelect, + localizedStrings, + analysisLanguage, }: CatalogRowViewProps) { - const [localizedStrings] = useLocalizedStrings(STRING_KEYS); - const analysisLanguage = useAnalysisLanguage(); - const [isExpanded, setIsExpanded] = useState(false); /** Whether the usage list is showing every usage rather than the first {@link INLINE_USAGE_LIMIT}. */ @@ -69,6 +68,12 @@ export default function CatalogRowView({ const visibleUsages = showsAllUsages ? row.usages : row.usages.slice(0, INLINE_USAGE_LIMIT); const hiddenUsageCount = row.usages.length - visibleUsages.length; + const usageCountLabel = localizedStrings['%interlinearizer_analysisCatalog_usageCount%']; + const usageCountInBookLabel = formatReplacementString( + localizedStrings['%interlinearizer_analysisCatalog_usageCountInBook%'], + { book: currentBook }, + ); + return (
  • + {/* + Carries no `aria-label`: a name on a button overrides its content, so one here would + announce every row alike and suppress the analysis each lists. + */} diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index d8ca3d91..cd564b01 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -640,9 +640,7 @@ function InterlinearizerLoaderInner({ // The store below waits for the draft: it seeds on mount alone, and the draft version that // remounts it does not bump when the load completes. Nothing is lost by waiting — while the // draft loads there is only ever a placeholder or an error panel to show. -
    - {loadingOrErrorPanel} -
    +
    {loadingOrErrorPanel}
    ) : ( // The store's lifetime is the draft's, not the loaded book's — it holds every book. Keyed // on the draft version because the seed is not reactive, so a wholesale replacement (New / diff --git a/src/hooks/usePanelResize.ts b/src/hooks/usePanelResize.ts index f4e7e9aa..5c3fc939 100644 --- a/src/hooks/usePanelResize.ts +++ b/src/hooks/usePanelResize.ts @@ -4,6 +4,18 @@ import type { KeyboardEvent as ReactKeyboardEvent, MouseEvent as ReactMouseEvent /** How far one arrow-key press resizes the panel, in pixels. */ const KEYBOARD_RESIZE_STEP_PX = 16; +/** + * Which way along the screen the handle has to travel to widen the panel: `-1` toward smaller + * `clientX`, `1` toward larger. + * + * The panel is anchored to the container's end edge, which the interface language decides the side + * of. A function rather than a constant, so a panel that outlives a language change still resizes + * the way it is pointing. + */ +function widenTravel(): number { + return document.documentElement.dir === 'rtl' ? 1 : -1; +} + /** How wide a panel may be dragged. */ export interface PanelWidthBounds { /** Narrowest the panel may be dragged, in pixels. */ @@ -31,7 +43,8 @@ export interface PanelResize { * * The committed width is the caller's to hold and persist, and a drag stays local until released, * so a gesture crossing a hundred pixels reports once rather than once per frame — which matters - * when the caller's store is the host's. An arrow key reports on each press. + * when the caller's store is the host's. A press that never moves reports nothing at all. An arrow + * key reports on each press. */ export default function usePanelResize( width: number, @@ -41,12 +54,15 @@ export default function usePanelResize( /** Width the panel is drawn at while a drag is in flight, or `undefined` when none is. */ const [dragWidth, setDragWidth] = useState(undefined); - /** Where the in-flight drag started, and the width it started from. */ - const dragOriginRef = useRef<{ clientX: number; width: number } | undefined>(undefined); + /** Where the in-flight drag started, the width it started from, and which way widens it. */ + const dragOriginRef = useRef<{ clientX: number; width: number; widenTravel: number } | undefined>( + undefined, + ); /** - * The in-flight drag's latest width, mirroring `dragWidth`. A release commits the width from here - * because a state updater has to stay free of side effects — React may run one more than once. + * The width the in-flight drag has reached, or `undefined` until it moves. A release commits from + * here because a state updater has to stay free of side effects — React may run one more than + * once. */ const dragWidthRef = useRef(undefined); @@ -60,8 +76,7 @@ export default function usePanelResize( const onMouseDown = useCallback( (event: ReactMouseEvent) => { - dragOriginRef.current = { clientX: event.clientX, width }; - dragWidthRef.current = width; + dragOriginRef.current = { clientX: event.clientX, width, widenTravel: widenTravel() }; setDragWidth(width); // Suppresses the text selection a drag across the panel would otherwise sweep up. event.preventDefault(); @@ -79,15 +94,13 @@ export default function usePanelResize( const origin = dragOriginRef.current; /* v8 ignore next -- the origin is set before this listener is ever mounted */ if (!origin) return; - // The panel is anchored to the end edge, so the handle moving toward the start edge widens - // it: the delta is subtracted, not added. - const next = clampWidth(origin.width - (event.clientX - origin.clientX)); + const next = clampWidth(origin.width + origin.widenTravel * (event.clientX - origin.clientX)); dragWidthRef.current = next; setDragWidth(next); }; const handleMouseUp = () => { const committed = dragWidthRef.current; - /* v8 ignore next -- a drag always has a width by the time it is released */ + // Writing an unchanged width back would put a stray click on the handle through the store. if (committed !== undefined) onWidthChange(committed); setDragWidth(undefined); dragWidthRef.current = undefined; @@ -107,13 +120,13 @@ export default function usePanelResize( const onKeyDown = useCallback( (event: ReactKeyboardEvent) => { - // Reading the visual direction rather than the writing direction: the key that moves the - // handle toward the start edge widens the panel, whichever edge the interface language puts - // that on. // eslint-disable-next-line no-nested-ternary -- a two-key lookup reads worse as a map - const step = event.key === 'ArrowLeft' ? 1 : event.key === 'ArrowRight' ? -1 : 0; - if (step === 0) return; + const travel = event.key === 'ArrowLeft' ? -1 : event.key === 'ArrowRight' ? 1 : 0; + if (travel === 0) return; event.preventDefault(); + // The arrow that moves the handle the way a drag would widen the panel widens it too, + // whichever side of the container the interface language anchors it to. + const step = travel === widenTravel() ? 1 : -1; onWidthChange(clampWidth(width + step * KEYBOARD_RESIZE_STEP_PX)); }, [width, onWidthChange, clampWidth], diff --git a/src/utils/language-tags.ts b/src/utils/language-tags.ts index 8d929ea9..b949426e 100644 --- a/src/utils/language-tags.ts +++ b/src/utils/language-tags.ts @@ -13,3 +13,18 @@ export function parseLanguageTags(input: string): string[] { .map((tag) => tag.trim()) .filter((tag) => tag.length > 0); } + +/** + * A collator for `tag`, falling back to the host's default collation when `Intl` rejects it. + * + * Language tags reach this as free text — nothing checks them for BCP 47 structure on the way in — + * and `Intl` throws on one it cannot parse, so an unusable tag has to degrade to some ordering + * rather than throw. + */ +export function collatorForTag(tag: string): Intl.Collator { + try { + return new Intl.Collator(tag); + } catch { + return new Intl.Collator(); + } +} From b6ad5ec5187feda98353962afe929627f838405a Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 14 Aug 2026 16:22:49 -0600 Subject: [PATCH 04/29] Stop a panel resize that outlives its release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pointer released where the window cannot see it — over a native menu, which takes the pointer with it — left the drag running, so the panel went on resizing under a button-less pointer and committed that width at the next click anywhere. A move reporting no button held now ends the drag at the width it had reached, since that move is the only word the window gets of such a release. Arrow keys stand aside while a drag is in flight. They stepped off the width the drag began at, reporting a width the panel was not showing only for the release to overwrite it; the pointer owns the width while it is held. The drag test helper now dispatches its moves with a held button, which a real one carries and jsdom does not. --- .../components/AnalysisCatalogPanel.test.tsx | 55 ++++++++++++++++++- src/hooks/usePanelResize.ts | 37 +++++++++---- 2 files changed, 78 insertions(+), 14 deletions(-) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index a72607f2..de93f700 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -89,10 +89,13 @@ function rowFor(analysisId: string): HTMLElement { * Drags the resize handle from `fromClientX` to `toClientX`. The handle listens on the window for * the move and release, so those are dispatched there rather than on the handle itself — a real * drag routinely leaves the handle's own box between frames. + * + * The move carries the button it holds — a real one does, jsdom does not unless told — because the + * panel reads that to tell a live drag from a release it never saw. */ function dragHandle(fromClientX: number, toClientX: number): void { fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: fromClientX }); - fireEvent.mouseMove(window, { clientX: toClientX }); + fireEvent.mouseMove(window, { clientX: toClientX, buttons: 1 }); fireEvent.mouseUp(window, { clientX: toClientX }); } @@ -108,7 +111,7 @@ describe('AnalysisCatalogPanel', () => { renderPanel({ width: 320, onWidthChange }); // The panel is anchored to the end edge, so the handle moving toward the start edge widens it - // by the distance travelled. + // by the distance traveled. dragHandle(500, 460); expect(onWidthChange).toHaveBeenCalledWith(360); @@ -128,7 +131,7 @@ describe('AnalysisCatalogPanel', () => { renderPanel({ width: 320, onWidthChange }); fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); - fireEvent.mouseMove(window, { clientX: 460 }); + fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); // The width is persisted, so a write per frame would put the whole gesture through the host. expect(onWidthChange).not.toHaveBeenCalled(); @@ -192,6 +195,52 @@ describe('AnalysisCatalogPanel', () => { expect(onWidthChange).not.toHaveBeenCalled(); }); + it('ends the drag at the width it reached when a move reports no button held', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); + fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); + // Stands in for a release the window never saw — one over a native menu, say. + fireEvent.mouseMove(window, { clientX: 400, buttons: 0 }); + + expect(onWidthChange).toHaveBeenCalledWith(360); + }); + + it('stops following the pointer once a release it never saw has ended the drag', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); + fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); + fireEvent.mouseMove(window, { clientX: 400, buttons: 0 }); + fireEvent.mouseMove(window, { clientX: 200, buttons: 0 }); + fireEvent.mouseUp(window, { clientX: 200 }); + + // Otherwise a pointer merely crossing the window keeps widening the panel, and the click that + // ends up committing reports wherever it got to. + expect(onWidthChange).toHaveBeenCalledTimes(1); + expect(onWidthChange).toHaveBeenLastCalledWith(360); + }); + + it('leaves an arrow key alone while a drag is in flight', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + const handle = screen.getByTestId('analysis-catalog-resize'); + fireEvent.mouseDown(handle, { clientX: 500 }); + fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); + fireEvent.keyDown(handle, { key: 'ArrowLeft' }); + + // Stepping off the width the drag began at would report a width the panel is not showing, + // which the release then overwrites. + expect(onWidthChange).not.toHaveBeenCalled(); + + fireEvent.mouseUp(window, { clientX: 460 }); + + expect(onWidthChange).toHaveBeenCalledWith(360); + }); + describe('in a right-to-left interface', () => { beforeEach(() => { document.documentElement.dir = 'rtl'; diff --git a/src/hooks/usePanelResize.ts b/src/hooks/usePanelResize.ts index 5c3fc939..a42384d1 100644 --- a/src/hooks/usePanelResize.ts +++ b/src/hooks/usePanelResize.ts @@ -44,7 +44,7 @@ export interface PanelResize { * The committed width is the caller's to hold and persist, and a drag stays local until released, * so a gesture crossing a hundred pixels reports once rather than once per frame — which matters * when the caller's store is the host's. A press that never moves reports nothing at all. An arrow - * key reports on each press. + * key reports on each press, save while a drag is holding the width. */ export default function usePanelResize( width: number, @@ -90,7 +90,24 @@ export default function usePanelResize( useEffect(() => { if (dragWidth === undefined) return undefined; + const endDrag = () => { + const committed = dragWidthRef.current; + // Writing an unchanged width back would put a stray click on the handle through the store. + if (committed !== undefined) onWidthChange(committed); + setDragWidth(undefined); + dragWidthRef.current = undefined; + dragOriginRef.current = undefined; + }; + const handleMouseMove = (event: MouseEvent) => { + // A release the window never saw — one over a native menu, which takes the pointer with it — + // would otherwise leave the drag running, resizing the panel under a pointer holding nothing + // and committing that width at the next click anywhere. A move reporting no button held is + // the only word the window gets of such a release. + if (event.buttons === 0) { + endDrag(); + return; + } const origin = dragOriginRef.current; /* v8 ignore next -- the origin is set before this listener is ever mounted */ if (!origin) return; @@ -98,20 +115,12 @@ export default function usePanelResize( dragWidthRef.current = next; setDragWidth(next); }; - const handleMouseUp = () => { - const committed = dragWidthRef.current; - // Writing an unchanged width back would put a stray click on the handle through the store. - if (committed !== undefined) onWidthChange(committed); - setDragWidth(undefined); - dragWidthRef.current = undefined; - dragOriginRef.current = undefined; - }; window.addEventListener('mousemove', handleMouseMove); - window.addEventListener('mouseup', handleMouseUp); + window.addEventListener('mouseup', endDrag); return () => { window.removeEventListener('mousemove', handleMouseMove); - window.removeEventListener('mouseup', handleMouseUp); + window.removeEventListener('mouseup', endDrag); }; // `dragWidth` is read only as the in-flight flag; listing its value as a dep would tear down // and remount both listeners on every frame of the drag. @@ -123,6 +132,12 @@ export default function usePanelResize( // eslint-disable-next-line no-nested-ternary -- a two-key lookup reads worse as a map const travel = event.key === 'ArrowLeft' ? -1 : event.key === 'ArrowRight' ? 1 : 0; if (travel === 0) return; + // The pointer owns the width while it is held: a key press mid-drag would step off the width + // the drag began at, which is neither what the panel is showing nor what the release is about + // to commit. Read off a ref, so the handler stays stable across a drag's frames. The case is + // unusual but reachable — the press that begins a drag suppresses the focus change, so the + // handle has to have been tabbed to first. + if (dragOriginRef.current) return; event.preventDefault(); // The arrow that moves the handle the way a drag would widen the panel widens it too, // whichever side of the container the interface language anchors it to. From a6c2e4a35ca11d12e480802e6eca9fc001028803 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 17 Aug 2026 08:45:40 -0600 Subject: [PATCH 05/29] Cover the catalog's empty state and cross-book jump faithfully The held-focus-request test left the reference on GEN while EXO's view mounted, a state the host never produces; move it to EXO with the jump. --- .../components/AnalysisCatalogPanel.test.tsx | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index de93f700..e759abda 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -292,6 +292,17 @@ describe('AnalysisCatalogPanel', () => { }); describe('rows', () => { + it('says the catalog is empty rather than leaving the panel blank', () => { + // A draft records nothing until the first gloss is entered, so the empty catalog is what most + // readers open the panel to. + renderPanel({ analysis: emptyAnalysis() }); + + expect(screen.queryAllByTestId('catalog-row')).toHaveLength(0); + expect(screen.getByTestId('analysis-catalog-panel')).toHaveTextContent( + '%interlinearizer_analysisCatalog_empty%', + ); + }); + it('renders an analysis with its gloss and its usage counts inside and outside the book', () => { const analysis: TextAnalysis = { ...emptyAnalysis(), @@ -589,9 +600,15 @@ describe('AnalysisCatalogPanel', () => { // mounted yet, and claiming would drop the request before it could be honored. expect(claimedFocusRequest).toBeUndefined(); - // EXO's USJ arrives and its view mounts. + // The host echoes the jump's reference, then EXO's USJ arrives and its view mounts. rerender( - + From fafcd77ec509c340afd9a2784ea0ef477f6f61ad Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 17 Aug 2026 09:26:34 -0600 Subject: [PATCH 06/29] Give the catalog width-restore test teeth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The simulated drag omitted `buttons`, so the move arrived reporting none — which the resize hook reads as a release it never saw, ending the drag before it recorded a width. The panel fell back to its default, and the remount assertion compared that default against itself. Dropping width persistence outright left the test green. Carry `buttons` on the move so the drag resizes, and name the expected width at both ends rather than checking the two renders agree, since the default is what a dead drag and a dropped write alike leave behind. --- .../components/InterlinearizerLoader.test.tsx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 0d6865d7..e63f6abf 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -1843,19 +1843,20 @@ describe('InterlinearizerLoader', () => { await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); - fireEvent.mouseMove(window, { clientX: 420 }); + // A move reporting no buttons reads as a release the window missed, ending the drag before + // it resizes anything. + fireEvent.mouseMove(window, { buttons: 1, clientX: 420 }); fireEvent.mouseUp(window, { clientX: 420 }); - const draggedWidth = screen.getByTestId('analysis-catalog-panel').style.width; + // The default is what both a dead drag and a dropped write leave behind, so each assertion + // names a width only a live drag reaches. + expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: '420px' }); cleanup(); await act(async () => { renderLoader({ useWebViewState }); }); - // The drag has to have moved the width, or a panel that ignored it entirely would still - // match its own restored default. - expect(draggedWidth).not.toBe(''); - expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: draggedWidth }); + expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: '420px' }); }); it('leaves the catalog panel closed on remount when it was never opened', async () => { From f5dc2695caf3cc95d95e395ca8ef1458dd2f8c99 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 17 Aug 2026 11:02:46 -0600 Subject: [PATCH 07/29] Confine the panel resize drag to the primary button Any button began a drag: the move handler asks only whether some button is held, so a middle-button press followed the pointer to its release and persisted the width it reached. A right-button press does the same wherever the context menu opens on release rather than on press. Also correct the loader's width-restore comment, which described a move reporting no buttons held while the move beneath it carries one. --- .../components/AnalysisCatalogPanel.test.tsx | 18 ++++++++++++++++++ .../components/InterlinearizerLoader.test.tsx | 4 ++-- src/hooks/usePanelResize.ts | 8 +++++++- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index e759abda..46f07a88 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -195,6 +195,24 @@ describe('AnalysisCatalogPanel', () => { expect(onWidthChange).not.toHaveBeenCalled(); }); + it('leaves the width alone when a button other than the primary one presses the handle', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { + button: 1, + buttons: 4, + clientX: 500, + }); + // The middle button reports itself held on every move that follows, so a drag begun from one + // would follow the pointer and persist wherever the release found it. + fireEvent.mouseMove(window, { buttons: 4, clientX: 460 }); + fireEvent.mouseUp(window, { clientX: 460 }); + + expect(onWidthChange).not.toHaveBeenCalled(); + expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: '320px' }); + }); + it('ends the drag at the width it reached when a move reports no button held', () => { const onWidthChange = jest.fn(); renderPanel({ width: 320, onWidthChange }); diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index e63f6abf..f37ef5c0 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -1843,8 +1843,8 @@ describe('InterlinearizerLoader', () => { await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); - // A move reporting no buttons reads as a release the window missed, ending the drag before - // it resizes anything. + // The move has to report a button held — jsdom sends none unless told — because one reporting + // none reads as a release the window missed, ending the drag before it resizes anything. fireEvent.mouseMove(window, { buttons: 1, clientX: 420 }); fireEvent.mouseUp(window, { clientX: 420 }); // The default is what both a dead drag and a dropped write leave behind, so each assertion diff --git a/src/hooks/usePanelResize.ts b/src/hooks/usePanelResize.ts index a42384d1..276fc567 100644 --- a/src/hooks/usePanelResize.ts +++ b/src/hooks/usePanelResize.ts @@ -4,6 +4,9 @@ import type { KeyboardEvent as ReactKeyboardEvent, MouseEvent as ReactMouseEvent /** How far one arrow-key press resizes the panel, in pixels. */ const KEYBOARD_RESIZE_STEP_PX = 16; +/** `MouseEvent.button` for the primary button, the only one that drags the handle. */ +const PRIMARY_MOUSE_BUTTON = 0; + /** * Which way along the screen the handle has to travel to widen the panel: `-1` toward smaller * `clientX`, `1` toward larger. @@ -31,7 +34,7 @@ export interface PanelResize { * running. */ displayWidth: number; - /** Begins a drag. Attach to the resize handle. */ + /** Begins a drag on a primary-button press, ignoring any other. Attach to the resize handle. */ onMouseDown: (event: ReactMouseEvent) => void; /** Resizes by one step per arrow key. Attach to the resize handle. */ onKeyDown: (event: ReactKeyboardEvent) => void; @@ -76,6 +79,9 @@ export default function usePanelResize( const onMouseDown = useCallback( (event: ReactMouseEvent) => { + // The drag below asks only whether some button is held, so one begun by any other button + // would follow the pointer to its release and commit the width it reached. + if (event.button !== PRIMARY_MOUSE_BUTTON) return; dragOriginRef.current = { clientX: event.clientX, width, widenTravel: widenTravel() }; setDragWidth(width); // Suppresses the text selection a drag across the panel would otherwise sweep up. From e117d307c22fffa07fa01ca1dad749dcc7fc0f60 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 17 Aug 2026 11:42:59 -0600 Subject: [PATCH 08/29] End the panel resize drag only on a primary release --- .../components/AnalysisCatalogPanel.test.tsx | 21 +++++++++++++++++++ src/hooks/usePanelResize.ts | 12 +++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 46f07a88..4cdae3ff 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -213,6 +213,27 @@ describe('AnalysisCatalogPanel', () => { expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: '320px' }); }); + it('keeps a drag running when a button other than the primary one is released', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); + fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); + // `button` is the one released, `buttons` the one still down: a middle-button click made + // without letting go of the primary. + fireEvent.mouseUp(window, { button: 1, buttons: 1, clientX: 460 }); + + expect(onWidthChange).not.toHaveBeenCalled(); + + fireEvent.mouseMove(window, { clientX: 440, buttons: 1 }); + fireEvent.mouseUp(window, { clientX: 440 }); + + // Otherwise the gesture ends at the stray click, committing the width it had reached there + // rather than the one it finished on. + expect(onWidthChange).toHaveBeenCalledTimes(1); + expect(onWidthChange).toHaveBeenCalledWith(380); + }); + it('ends the drag at the width it reached when a move reports no button held', () => { const onWidthChange = jest.fn(); renderPanel({ width: 320, onWidthChange }); diff --git a/src/hooks/usePanelResize.ts b/src/hooks/usePanelResize.ts index 276fc567..2f37c9c0 100644 --- a/src/hooks/usePanelResize.ts +++ b/src/hooks/usePanelResize.ts @@ -105,6 +105,14 @@ export default function usePanelResize( dragOriginRef.current = undefined; }; + const handleMouseUp = (event: MouseEvent) => { + // A middle- or right-button click made mid-drag raises `mouseup` too, and ending the drag + // there would commit the width it had reached and leave the pointer still holding a handle + // the panel does not follow. + if (event.button !== PRIMARY_MOUSE_BUTTON) return; + endDrag(); + }; + const handleMouseMove = (event: MouseEvent) => { // A release the window never saw — one over a native menu, which takes the pointer with it — // would otherwise leave the drag running, resizing the panel under a pointer holding nothing @@ -123,10 +131,10 @@ export default function usePanelResize( }; window.addEventListener('mousemove', handleMouseMove); - window.addEventListener('mouseup', endDrag); + window.addEventListener('mouseup', handleMouseUp); return () => { window.removeEventListener('mousemove', handleMouseMove); - window.removeEventListener('mouseup', endDrag); + window.removeEventListener('mouseup', handleMouseUp); }; // `dragWidth` is read only as the in-flight flag; listing its value as a dep would tear down // and remount both listeners on every frame of the drag. From a0a06dafc1f352698dc96fc85bded06e64e87a44 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 18 Aug 2026 15:31:15 -0600 Subject: [PATCH 09/29] Tighten the analysis catalog panel's resizing and rows The splitter gains Home/End and commits a drag the panel is unmounted holding; rows memoize and return to the inline usage cap when collapsed. --- .../components/AnalysisCatalogPanel.test.tsx | 92 +++++++++++++++ src/components/AnalysisStore.tsx | 4 +- src/components/CatalogRowView.tsx | 25 +++- src/components/InterlinearizerLoader.tsx | 108 +++++++++++------- src/hooks/usePanelResize.ts | 62 ++++++++-- 5 files changed, 233 insertions(+), 58 deletions(-) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 4cdae3ff..4338b02f 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -174,6 +174,69 @@ describe('AnalysisCatalogPanel', () => { expect(onWidthChange).toHaveBeenCalledWith(304); }); + it('jumps to the narrowest width on Home', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + // The ends of the range are the same widths whichever side the panel is anchored to, so + // unlike the arrow keys these need no right-to-left counterpart. + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'Home' }); + + expect(onWidthChange).toHaveBeenCalledWith(220); + }); + + it('jumps to the widest width on End', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'End' }); + + expect(onWidthChange).toHaveBeenCalledWith(800); + }); + + it('leaves the width alone on Home while a drag is in flight', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + const handle = screen.getByTestId('analysis-catalog-resize'); + fireEvent.mouseDown(handle, { clientX: 500 }); + fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); + fireEvent.keyDown(handle, { key: 'Home' }); + + expect(onWidthChange).not.toHaveBeenCalled(); + }); + + it('draws a committed width the bounds do not allow at the nearest width they do', () => { + // Only the gestures clamp, so a width arriving from the store is the one way the panel can be + // asked to draw itself outside the range it reports. + renderPanel({ width: 5000 }); + + const handle = screen.getByTestId('analysis-catalog-resize'); + expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: '800px' }); + expect(handle).toHaveAttribute('aria-valuenow', '800'); + }); + + it('commits the width a drag reached when the panel goes away under it', () => { + const onWidthChange = jest.fn(); + const { unmount } = renderPanel({ width: 320, onWidthChange }); + + fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); + fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); + // The panel is remounted by anything that replaces the draft, so a gesture can outlive it. + unmount(); + + expect(onWidthChange).toHaveBeenCalledWith(360); + }); + + it('commits nothing when the panel goes away with no drag in flight', () => { + const onWidthChange = jest.fn(); + const { unmount } = renderPanel({ width: 320, onWidthChange }); + + unmount(); + + expect(onWidthChange).not.toHaveBeenCalled(); + }); + it('leaves the width alone on a key that does not resize', () => { const onWidthChange = jest.fn(); renderPanel({ width: 320, onWidthChange }); @@ -431,6 +494,22 @@ describe('AnalysisCatalogPanel', () => { ); }); + it('names the book the per-book count is taken against rather than showing its code', () => { + mockKeyAsValueLocalizedStrings({ + '%interlinearizer_analysisCatalog_usageCountInBook%': 'Uses in {book}', + }); + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [{ ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'λόγος' }], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0')], + }; + renderPanel({ analysis, currentBook: 'GEN' }); + + expect( + within(rowFor('ta-1')).getByTestId('catalog-row-usage-count-in-book'), + ).toHaveTextContent('Uses in Genesis'); + }); + it('marks an analysis with no gloss in the active language rather than leaving the cell blank', () => { const analysis: TextAnalysis = { ...emptyAnalysis(), @@ -577,6 +656,19 @@ describe('AnalysisCatalogPanel', () => { within(rowFor('ta-1')).queryByTestId('catalog-usages-show-all'), ).not.toBeInTheDocument(); }); + + it('returns to the inline cap once the row is collapsed', async () => { + renderPanel({ analysis: MANY_USAGES }); + const toggle = within(rowFor('ta-1')).getByTestId('catalog-row-toggle'); + await userEvent.click(toggle); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-usages-show-all')); + + await userEvent.click(toggle); + await userEvent.click(toggle); + + // A row left showing everything buries the rows below it for the rest of the session. + expect(within(rowFor('ta-1')).getAllByTestId('catalog-usage')).toHaveLength(12); + }); }); }); diff --git a/src/components/AnalysisStore.tsx b/src/components/AnalysisStore.tsx index 53224897..eda99e79 100644 --- a/src/components/AnalysisStore.tsx +++ b/src/components/AnalysisStore.tsx @@ -17,8 +17,8 @@ import { selectAnalysis, selectAnalysisLanguage, selectApprovedGloss, - selectCatalogRows, selectApprovedMorphemes, + selectCatalogRows, selectMorphemeResetLosesGlosses, selectPhraseLinkByAnalysisId, selectPhraseLinkByTokenRef, @@ -369,7 +369,7 @@ export function useMorphemeResetLosesGlosses(tokenRef: string): boolean { /** * Returns one row per distinct token analysis in the draft, each carrying the usage data the * analysis catalog lists it by, in the analysis's own order. Narrowing and ordering are the - * caller's ({@link applyCatalogQuery}), so a keystroke re-runs only that pass. + * caller's, so a keystroke re-runs only that pass. * * The result keeps its reference while the analyses and their links keep theirs, so an unrelated * write — a free translation, a phrase link — leaves the list unrendered. It changes with diff --git a/src/components/CatalogRowView.tsx b/src/components/CatalogRowView.tsx index 5b44165e..acf8eae3 100644 --- a/src/components/CatalogRowView.tsx +++ b/src/components/CatalogRowView.tsx @@ -1,7 +1,8 @@ +import { Canon } from '@sillsdev/scripture'; import { ChevronDown, ChevronRight } from 'lucide-react'; import { Button } from 'platform-bible-react'; import { formatReplacementString, type LanguageStrings } from 'platform-bible-utils'; -import { useState } from 'react'; +import { memo, useCallback, useState } from 'react'; import type { CatalogRow, CatalogUsage } from '../utils/analysis-query'; /** @@ -27,7 +28,7 @@ const INLINE_USAGE_LIMIT = 12; type CatalogRowViewProps = Readonly<{ /** The analysis this row lists. */ row: CatalogRow; - /** Book code `row.usageCountInBook` was taken against, named in that count's label. */ + /** Book code `row.usageCountInBook` was taken against, whose name that count's label carries. */ currentBook: string; /** Whether this is the row the view was last jumped from. */ isSelected: boolean; @@ -52,7 +53,7 @@ function usageLabel(usage: CatalogUsage): string { * Each row owns its own layout so that its detail can be nested inside it. One element per analysis * is what lets the list window and be walked by keyboard a row at a time. */ -export default function CatalogRowView({ +function CatalogRowView({ row, currentBook, isSelected, @@ -65,13 +66,23 @@ export default function CatalogRowView({ /** Whether the usage list is showing every usage rather than the first {@link INLINE_USAGE_LIMIT}. */ const [showsAllUsages, setShowsAllUsages] = useState(false); + // Collapsing returns the row to the inline cap: without it a row once expanded to hundreds of + // usages has no way back, since the expander it was opened from is gone. + const handleToggle = useCallback(() => { + setIsExpanded((expanded) => !expanded); + setShowsAllUsages(false); + }, []); + const visibleUsages = showsAllUsages ? row.usages : row.usages.slice(0, INLINE_USAGE_LIMIT); const hiddenUsageCount = row.usages.length - visibleUsages.length; const usageCountLabel = localizedStrings['%interlinearizer_analysisCatalog_usageCount%']; const usageCountInBookLabel = formatReplacementString( localizedStrings['%interlinearizer_analysisCatalog_usageCountInBook%'], - { book: currentBook }, + // English name rather than the code, because this label reads as prose where the usage links + // below it read as references. A platform-localized name would need PAPI wiring this view does + // not yet have. + { book: Canon.bookIdToEnglishName(currentBook) }, ); return ( @@ -93,7 +104,7 @@ export default function CatalogRowView({ // sitting in one. className="tw:flex tw:h-auto tw:w-full tw:items-baseline tw:justify-start tw:gap-2 tw:rounded-none tw:px-3 tw:py-2 tw:text-start tw:font-normal" data-testid="catalog-row-toggle" - onClick={() => setIsExpanded((expanded) => !expanded)} + onClick={handleToggle} type="button" variant="ghost" > @@ -200,3 +211,7 @@ export default function CatalogRowView({
    • ); } + +/** Memoized version of {@link CatalogRowView}; use in render-stable row lists. */ +const MemoizedCatalogRowView = memo(CatalogRowView); +export default MemoizedCatalogRowView; diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index cd564b01..8ad21acb 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -9,6 +9,7 @@ import { TabToolbar } from 'platform-bible-react'; import type { SelectMenuItemHandler } from 'platform-bible-react'; import { isPlatformError } from 'platform-bible-utils'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import type { ReactNode } from 'react'; import { resegmentBook } from 'parsers/papi/resegmentBook'; import useDraftProject from '../hooks/useDraftProject'; import useInterlinearizerBookData from '../hooks/useInterlinearizerBookData'; @@ -30,7 +31,7 @@ import type { PhraseMode } from '../types/phrase-mode'; import ProjectModals, { type ModalState } from './modals/ProjectModals'; import { WipeModal, type WipeScope } from './modals/WipeModal'; import ScriptureNavControls from './controls/ScriptureNavControls'; -import { InterlinearNavProvider, useInterlinearNav } from './InterlinearNavContext'; +import { InterlinearNavProvider, useInterlinearNav, type FadePhase } from './InterlinearNavContext'; import { RECENTER_FADE_TRANSITION_STYLE } from './recenter-fade'; import { firstVerseNumber, segmentContainsVerse } from '../utils/verse-ref'; import { resolvedOrEmpty } from '../utils/localized-strings'; @@ -54,6 +55,40 @@ const DEFAULT_WEB_VIEW_MENU = { */ const BASE_TAB_TITLE = 'Interlinearizer'; +/** Props for {@link BookFadeWrapper}. */ +type BookFadeWrapperProps = Readonly<{ + /** How far through a cross-book fade the view is. */ + fadePhase: FadePhase; + /** The interlinear view, or the placeholder standing in for it. */ + children: ReactNode; +}>; + +/** + * The cross-book curtain: the column holding whatever the view is showing of the book, dimmed while + * a jump to another book is in flight. + * + * @returns An element carrying `data-testid="book-fade-wrapper"`. + */ +function BookFadeWrapper({ fadePhase, children }: BookFadeWrapperProps) { + return ( +
    + {children} +
    + ); +} + /** Glyph appended to the tab title while the draft has unsaved changes. */ const UNSAVED_TAB_MARKER = ' ●'; @@ -636,46 +671,31 @@ function InterlinearizerLoaderInner({ }} /> - {isDraftLoading ? ( - // The store below waits for the draft: it seeds on mount alone, and the draft version that - // remounts it does not bump when the load completes. Nothing is lost by waiting — while the - // draft loads there is only ever a placeholder or an error panel to show. -
    {loadingOrErrorPanel}
    - ) : ( - // The store's lifetime is the draft's, not the loaded book's — it holds every book. Keyed - // on the draft version because the seed is not reactive, so a wholesale replacement (New / - // Open / Wipe) reseeds by remounting. Wrapping the loading and error branches too keeps it - // alive across the gap while the next book's USJ is in flight. - // - // Declared above the cross-book curtain, not inside it, so the catalog panel can read the - // store without being dimmed by it: a jump to a usage in another book fades the view it - // navigates, and fading the list the jump was made from along with it would blank the panel - // at precisely the moment it is being used. - -
    -
    - {bookArea} -
    +
    + {isDraftLoading ? ( + // The store below waits for the draft: it seeds on mount alone, and the draft version + // that remounts it does not bump when the load completes. Nothing is lost by waiting — + // while the draft loads there is only ever a placeholder or an error panel to show. + {loadingOrErrorPanel} + ) : ( + // The store's lifetime is the draft's, not the loaded book's — it holds every book. + // Keyed on the draft version because the seed is not reactive, so a wholesale replacement + // (New / Open / Wipe) reseeds by remounting. Wrapping the loading and error branches too + // keeps it alive across the gap while the next book's USJ is in flight. + // + // Declared above the cross-book curtain, not inside it, so the catalog panel can read the + // store without being dimmed by it: a jump to a usage in another book fades the view it + // navigates, and fading the list the jump was made from along with it would blank the + // panel at precisely the moment it is being used. + + {bookArea} {catalogOpen && ( )} -
    - - )} + + )} +
    void; - /** Resizes by one step per arrow key. Attach to the resize handle. */ + /** + * Resizes by one step per arrow key, or to an end of the range on Home/End. Attach to the resize + * handle. + */ onKeyDown: (event: ReactKeyboardEvent) => void; } @@ -47,7 +68,8 @@ export interface PanelResize { * The committed width is the caller's to hold and persist, and a drag stays local until released, * so a gesture crossing a hundred pixels reports once rather than once per frame — which matters * when the caller's store is the host's. A press that never moves reports nothing at all. An arrow - * key reports on each press, save while a drag is holding the width. + * key reports on each press, save while a drag is holding the width. A width a drag reached and + * never released is committed if the panel goes away under it. */ export default function usePanelResize( width: number, @@ -141,11 +163,31 @@ export default function usePanelResize( // eslint-disable-next-line react-hooks/exhaustive-deps }, [dragWidth === undefined, onWidthChange, clampWidth]); + /** + * Latest `onWidthChange`, so the commit below can run for the hook's whole lifetime. An effect + * keyed on the caller's callback tears down whenever its identity changes, committing a width + * part-way through a gesture. + */ + const onWidthChangeRef = useRef(onWidthChange); + useEffect(() => { + onWidthChangeRef.current = onWidthChange; + }, [onWidthChange]); + + // A drag stays local until released, so a panel unmounted while one is in flight would otherwise + // discard the width the gesture reached and come back at the width it started from. + useEffect( + () => () => { + const reached = dragWidthRef.current; + if (reached !== undefined) onWidthChangeRef.current(reached); + }, + [], + ); + const onKeyDown = useCallback( (event: ReactKeyboardEvent) => { - // eslint-disable-next-line no-nested-ternary -- a two-key lookup reads worse as a map - const travel = event.key === 'ArrowLeft' ? -1 : event.key === 'ArrowRight' ? 1 : 0; - if (travel === 0) return; + const travel = keyTravel(event.key); + const jumpTarget = keyJumpTarget(event.key, min, max); + if (travel === 0 && jumpTarget === undefined) return; // The pointer owns the width while it is held: a key press mid-drag would step off the width // the drag began at, which is neither what the panel is showing nor what the release is about // to commit. Read off a ref, so the handler stays stable across a drag's frames. The case is @@ -153,13 +195,19 @@ export default function usePanelResize( // handle has to have been tabbed to first. if (dragOriginRef.current) return; event.preventDefault(); + if (jumpTarget !== undefined) { + onWidthChange(jumpTarget); + return; + } // The arrow that moves the handle the way a drag would widen the panel widens it too, // whichever side of the container the interface language anchors it to. const step = travel === widenTravel() ? 1 : -1; onWidthChange(clampWidth(width + step * KEYBOARD_RESIZE_STEP_PX)); }, - [width, onWidthChange, clampWidth], + [width, onWidthChange, clampWidth, min, max], ); - return { displayWidth: dragWidth ?? width, onMouseDown, onKeyDown }; + // Clamped rather than passed through, so the bounds govern what is drawn and reported even when + // a committed width arrives from outside them. + return { displayWidth: dragWidth ?? clampWidth(width), onMouseDown, onKeyDown }; } From ff10255b27939f1bd5ef94dbb6d9d099ef6a39b1 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 18 Aug 2026 17:12:32 -0600 Subject: [PATCH 10/29] Start a panel resize from the width on screen A drag seeded from a committed width outside the bounds drew the panel past its announced maximum on the press alone. The jump and arrow keys now skip writing a width the panel already holds. --- .../components/AnalysisCatalogPanel.test.tsx | 40 +++++++++++++++++++ src/hooks/usePanelResize.ts | 35 ++++++++++------ 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 4338b02f..5c0b12a6 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -216,6 +216,46 @@ describe('AnalysisCatalogPanel', () => { expect(handle).toHaveAttribute('aria-valuenow', '800'); }); + it('holds a committed width the bounds do not allow at that same width under a press', () => { + renderPanel({ width: 5000 }); + + fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); + + // A drag that began at the committed width would widen the panel to it on the press alone, + // past the maximum the handle announces. + expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: '800px' }); + }); + + it('drags from the width on screen when the committed width is one the bounds do not allow', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 5000, onWidthChange }); + + dragHandle(500, 540); + + // A drag measured from the committed width would sit at the maximum until the pointer had + // traveled the whole difference, rather than following it from the first pixel. + expect(onWidthChange).toHaveBeenCalledWith(760); + }); + + it('leaves the width alone on End when the panel is already at its widest', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 800, onWidthChange }); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'End' }); + + // The width is persisted through the host, so an identical write is not free. + expect(onWidthChange).not.toHaveBeenCalled(); + }); + + it('leaves the width alone on an arrow that would widen the panel past its widest', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 800, onWidthChange }); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowLeft' }); + + expect(onWidthChange).not.toHaveBeenCalled(); + }); + it('commits the width a drag reached when the panel goes away under it', () => { const onWidthChange = jest.fn(); const { unmount } = renderPanel({ width: 320, onWidthChange }); diff --git a/src/hooks/usePanelResize.ts b/src/hooks/usePanelResize.ts index d3d5a0c3..e6e097ba 100644 --- a/src/hooks/usePanelResize.ts +++ b/src/hooks/usePanelResize.ts @@ -67,9 +67,10 @@ export interface PanelResize { * * The committed width is the caller's to hold and persist, and a drag stays local until released, * so a gesture crossing a hundred pixels reports once rather than once per frame — which matters - * when the caller's store is the host's. A press that never moves reports nothing at all. An arrow - * key reports on each press, save while a drag is holding the width. A width a drag reached and - * never released is committed if the panel goes away under it. + * when the caller's store is the host's. A press that never moves reports nothing at all, and + * neither does a key that leaves the width where it already is. An arrow key otherwise reports on + * each press, save while a drag is holding the width. A width a drag reached and never released is + * committed if the panel goes away under it. */ export default function usePanelResize( width: number, @@ -104,12 +105,20 @@ export default function usePanelResize( // The drag below asks only whether some button is held, so one begun by any other button // would follow the pointer to its release and commit the width it reached. if (event.button !== PRIMARY_MOUSE_BUTTON) return; - dragOriginRef.current = { clientX: event.clientX, width, widenTravel: widenTravel() }; - setDragWidth(width); + // The width on screen rather than the committed one behind it: a drag starting from a + // committed width the bounds disallow would jump to it on the press and then hold still until + // the pointer had traveled the whole difference. + const startWidth = clampWidth(width); + dragOriginRef.current = { + clientX: event.clientX, + width: startWidth, + widenTravel: widenTravel(), + }; + setDragWidth(startWidth); // Suppresses the text selection a drag across the panel would otherwise sweep up. event.preventDefault(); }, - [width], + [width, clampWidth], ); // Runs the in-flight drag. Mounted only while one is in flight, so an idle panel listens to @@ -145,7 +154,10 @@ export default function usePanelResize( return; } const origin = dragOriginRef.current; - /* v8 ignore next -- the origin is set before this listener is ever mounted */ + // The listener is mounted only with an origin already set, and whatever clears the origin + // ends the drag too, so a move reaches this only in the window between that clear and this + // listener's removal, which the re-render carrying it has yet to reach. + /* v8 ignore next -- a test never interleaves a move into that window: the re-render lands first */ if (!origin) return; const next = clampWidth(origin.width + origin.widenTravel * (event.clientX - origin.clientX)); dragWidthRef.current = next; @@ -195,14 +207,13 @@ export default function usePanelResize( // handle has to have been tabbed to first. if (dragOriginRef.current) return; event.preventDefault(); - if (jumpTarget !== undefined) { - onWidthChange(jumpTarget); - return; - } // The arrow that moves the handle the way a drag would widen the panel widens it too, // whichever side of the container the interface language anchors it to. const step = travel === widenTravel() ? 1 : -1; - onWidthChange(clampWidth(width + step * KEYBOARD_RESIZE_STEP_PX)); + const next = jumpTarget ?? clampWidth(width + step * KEYBOARD_RESIZE_STEP_PX); + // An arrow held down at an end of the range repeats, and each repeat would otherwise put the + // same width through the store. + if (next !== width) onWidthChange(next); }, [width, onWidthChange, clampWidth, min, max], ); From 02b655a577d77310d3dbe08b800ecf2fe778ab8b Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 19 Aug 2026 09:44:37 -0600 Subject: [PATCH 11/29] Propagate a reset through the WebView state stub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only writes notified subscribers, so a component reading a key another component reset never re-rendered — a future test would have failed for a reason the production code has nothing to do with. A reset also now lands on the resetting caller's default rather than restoring the seed, matching what the real hook leaves behind. --- src/__tests__/test-helpers.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/__tests__/test-helpers.ts b/src/__tests__/test-helpers.ts index aeebb967..7d5b47cd 100644 --- a/src/__tests__/test-helpers.ts +++ b/src/__tests__/test-helpers.ts @@ -38,6 +38,9 @@ type StateSlot = { get: () => T; set: (v: T) => void }; * never seen, and its test would fail for a reason the production code has nothing to do with. * Notification is store-wide rather than per key: a test render tree is small enough that the extra * renders cost nothing, and keying it would be a second thing to keep right. + * + * A reset re-renders every reader as a write does, and lands on the resetting caller's default + * rather than on the `seed` this stub opened with, matching what a real reset leaves behind. */ export function makeWebViewState(seed: Record = {}) { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -74,9 +77,7 @@ export function makeWebViewState(seed: Record = {}) { return [ resolvedSlot.get(), (v: T) => resolvedSlot.set(v), - () => { - slots.delete(key); - }, + () => resolvedSlot.set(defaultValue), ]; }; } From 1e32f06aefde1272de0272f75aea6d24c2590040 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 19 Aug 2026 11:15:19 -0600 Subject: [PATCH 12/29] Hold the catalog panel's resize to the room on screen The widest width gives way to a container too narrow to hold the panel and the text both, an arrow key steps from the width the panel is drawn at rather than the committed one behind it, and a drag that ends where it began commits nothing. --- .../components/AnalysisCatalogPanel.test.tsx | 137 +++++++++++++++++- src/components/AnalysisCatalogPanel.tsx | 42 +++++- src/hooks/useContainerWidth.ts | 31 ++++ src/hooks/usePanelResize.ts | 42 +++--- 4 files changed, 226 insertions(+), 26 deletions(-) create mode 100644 src/hooks/useContainerWidth.ts diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 5c0b12a6..19d73f24 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -2,7 +2,7 @@ /// import type { SerializedVerseRef } from '@sillsdev/scripture'; -import { fireEvent, render, screen, within } from '@testing-library/react'; +import { act, fireEvent, render, screen, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { AssignmentStatus, TextAnalysis, TokenAnalysisLink } from 'interlinearizer'; import { useEffect } from 'react'; @@ -39,6 +39,39 @@ function FocusRequestProbe({ bookCode }: Readonly<{ bookCode: string }>) { return undefined; } +/** Every {@link TrackingResizeObserver} created since the last reset, newest last. */ +let resizeObserverInstances: TrackingResizeObserver[] = []; + +/** + * A ResizeObserver test double that records its callback and appends itself to + * {@link resizeObserverInstances}, so a test can fire the container resize jsdom never raises. + * Module-scoped (rather than an inline class per test) so the file stays under + * `max-classes-per-file`. + */ +class TrackingResizeObserver implements ResizeObserver { + constructor(public callback: ResizeObserverCallback) { + resizeObserverInstances.push(this); + } + + // eslint-disable-next-line @typescript-eslint/class-methods-use-this + observe() {} + + // eslint-disable-next-line @typescript-eslint/class-methods-use-this + unobserve() {} + + // eslint-disable-next-line @typescript-eslint/class-methods-use-this + disconnect() {} +} + +/** + * Reports `width` as the client width of every element, standing in for the layout jsdom does not + * do: the panel measures the container it is rendered into, which is otherwise zero-width. The spy + * it returns reports a different width on demand, for a test that fires a resize. + */ +function stubContainerWidth(width: number) { + return jest.spyOn(Element.prototype, 'clientWidth', 'get').mockReturnValue(width); +} + /** Options every `renderPanel` call may override. */ type PanelOptions = Partial<{ width: number; @@ -237,6 +270,17 @@ describe('AnalysisCatalogPanel', () => { expect(onWidthChange).toHaveBeenCalledWith(760); }); + it('steps from the width on screen when the committed width is one the bounds do not allow', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 5000, onWidthChange }); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowRight' }); + + // A step measured from the committed width would land on the maximum the panel is already + // drawn at, leaving the first press of the arrow to move nothing. + expect(onWidthChange).toHaveBeenCalledWith(784); + }); + it('leaves the width alone on End when the panel is already at its widest', () => { const onWidthChange = jest.fn(); renderPanel({ width: 800, onWidthChange }); @@ -298,6 +342,30 @@ describe('AnalysisCatalogPanel', () => { expect(onWidthChange).not.toHaveBeenCalled(); }); + it('commits nothing when a drag returns to the width it started from', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); + fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); + fireEvent.mouseMove(window, { clientX: 500, buttons: 1 }); + fireEvent.mouseUp(window, { clientX: 500 }); + + expect(onWidthChange).not.toHaveBeenCalled(); + }); + + it('commits nothing when the panel goes away with the drag back where it started', () => { + const onWidthChange = jest.fn(); + const { unmount } = renderPanel({ width: 320, onWidthChange }); + + fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); + fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); + fireEvent.mouseMove(window, { clientX: 500, buttons: 1 }); + unmount(); + + expect(onWidthChange).not.toHaveBeenCalled(); + }); + it('leaves the width alone when a button other than the primary one presses the handle', () => { const onWidthChange = jest.fn(); renderPanel({ width: 320, onWidthChange }); @@ -383,6 +451,73 @@ describe('AnalysisCatalogPanel', () => { expect(onWidthChange).toHaveBeenCalledWith(360); }); + describe('within its container', () => { + it('holds the widest width to the room the interlinear view is left', () => { + const onWidthChange = jest.fn(); + stubContainerWidth(600); + renderPanel({ width: 320, onWidthChange }); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'End' }); + + expect(onWidthChange).toHaveBeenCalledWith(360); + }); + + it('announces the width the container holds it to', () => { + stubContainerWidth(600); + renderPanel({ width: 320 }); + + expect(screen.getByTestId('analysis-catalog-resize')).toHaveAttribute( + 'aria-valuemax', + '360', + ); + }); + + it('holds to its own widest width in a container with room to spare', () => { + const onWidthChange = jest.fn(); + stubContainerWidth(2000); + renderPanel({ width: 320, onWidthChange }); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'End' }); + + expect(onWidthChange).toHaveBeenCalledWith(800); + }); + + it('stays at its narrowest in a container with no room for the view either', () => { + const onWidthChange = jest.fn(); + stubContainerWidth(300); + renderPanel({ width: 320, onWidthChange }); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'End' }); + + // Something has to give in a container this narrow, and a panel below its own minimum + // would be unreadable, so what is left of the view gives way instead. + expect(onWidthChange).toHaveBeenCalledWith(220); + }); + + it('narrows the panel as the container shrinks under it', () => { + const originalResizeObserver = global.ResizeObserver; + resizeObserverInstances = []; + global.ResizeObserver = TrackingResizeObserver; + const clientWidth = stubContainerWidth(2000); + + try { + renderPanel({ width: 320 }); + clientWidth.mockReturnValue(600); + act(() => { + resizeObserverInstances.forEach((observer) => observer.callback([], observer)); + }); + + // A tab redocked narrower carries the width it was given in the wide one. + expect(screen.getByTestId('analysis-catalog-resize')).toHaveAttribute( + 'aria-valuemax', + '360', + ); + } finally { + global.ResizeObserver = originalResizeObserver; + } + }); + }); + describe('in a right-to-left interface', () => { beforeEach(() => { document.documentElement.dir = 'rtl'; diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index b9b96b46..d4bc10b1 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -1,10 +1,11 @@ import { useLocalizedStrings } from '@papi/frontend/react'; import { X } from 'lucide-react'; import { Button } from 'platform-bible-react'; -import { useCallback, useMemo, useState } from 'react'; +import { useCallback, useMemo, useRef, useState } from 'react'; import { useAnalysisLanguage, useCatalogRows } from './AnalysisStore'; import CatalogRowView, { ROW_STRING_KEYS } from './CatalogRowView'; import { useInterlinearNav } from './InterlinearNavContext'; +import useContainerWidth from '../hooks/useContainerWidth'; import usePanelResize, { type PanelWidthBounds } from '../hooks/usePanelResize'; import { applyCatalogQuery, type CatalogQuery, type CatalogUsage } from '../utils/analysis-query'; import { collatorForTag } from '../utils/language-tags'; @@ -24,11 +25,33 @@ const STRING_KEYS = [ ] as const satisfies `%${string}%`[]; /** - * How far the panel may be resized: narrow enough that the usage counts still fit, wide enough that - * a drag can never squeeze the interlinear view out entirely. + * How far the panel may be resized on its own account: from narrow enough that the usage counts + * still fit to wide enough to read a gloss whole. A container with no room for the wide end gives + * the panel less than this — see {@link MIN_VIEW_WIDTH_PX}. */ const WIDTH_BOUNDS: PanelWidthBounds = { min: 220, max: 800 }; +/** + * How much of the container the interlinear view keeps whatever width the panel is asked for, in + * pixels. The panel sits beside the text rather than over it, so a container too narrow for both at + * their full width narrows the panel rather than pushing the text off the screen — unless it has no + * room for this and the narrowest panel together. + */ +const MIN_VIEW_WIDTH_PX = 240; + +/** + * How far a panel in a container this wide may be resized: {@link WIDTH_BOUNDS}, its wide end held + * to the room {@link MIN_VIEW_WIDTH_PX} leaves. A container of unknown width — one nothing has laid + * out yet — constrains nothing. + */ +function boundsWithin(containerWidth: number | undefined): PanelWidthBounds { + if (containerWidth === undefined) return WIDTH_BOUNDS; + return { + min: WIDTH_BOUNDS.min, + max: Math.max(WIDTH_BOUNDS.min, Math.min(WIDTH_BOUNDS.max, containerWidth - MIN_VIEW_WIDTH_PX)), + }; +} + /** Props for {@link AnalysisCatalogPanel}. */ type AnalysisCatalogPanelProps = Readonly<{ /** Dismisses the panel. */ @@ -110,14 +133,21 @@ export default function AnalysisCatalogPanel({ [navigate, requestFocusToken], ); + /** The panel itself, so the room its container leaves for the view can be measured. */ + // eslint-disable-next-line no-null/no-null + const panelRef = useRef(null); + const containerWidth = useContainerWidth(panelRef); + const bounds = boundsWithin(containerWidth); + const { displayWidth, onMouseDown: handleResizeMouseDown, onKeyDown: handleResizeKeyDown, - } = usePanelResize(width, onWidthChange, WIDTH_BOUNDS); + } = usePanelResize(width, onWidthChange, bounds); return (
    ): number | undefined { + const [containerWidth, setContainerWidth] = useState(undefined); + + useEffect(() => { + const container = ref.current?.parentElement; + /* v8 ignore next -- the ref is attached by the very render that mounts this effect */ + if (!container) return undefined; + + const measure = () => { + setContainerWidth(container.clientWidth > 0 ? container.clientWidth : undefined); + }; + measure(); + + const observer = new ResizeObserver(measure); + observer.observe(container); + return () => observer.disconnect(); + }, [ref]); + + return containerWidth; +} diff --git a/src/hooks/usePanelResize.ts b/src/hooks/usePanelResize.ts index e6e097ba..309c5873 100644 --- a/src/hooks/usePanelResize.ts +++ b/src/hooks/usePanelResize.ts @@ -1,5 +1,6 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import type { KeyboardEvent as ReactKeyboardEvent, MouseEvent as ReactMouseEvent } from 'react'; +import useLatestRef from './useLatestRef'; /** How far one arrow-key press resizes the panel, in pixels. */ const KEYBOARD_RESIZE_STEP_PX = 16; @@ -67,10 +68,10 @@ export interface PanelResize { * * The committed width is the caller's to hold and persist, and a drag stays local until released, * so a gesture crossing a hundred pixels reports once rather than once per frame — which matters - * when the caller's store is the host's. A press that never moves reports nothing at all, and - * neither does a key that leaves the width where it already is. An arrow key otherwise reports on - * each press, save while a drag is holding the width. A width a drag reached and never released is - * committed if the panel goes away under it. + * when the caller's store is the host's. A gesture that leaves the width where it found it reports + * nothing at all — a press that never moves, a drag that wanders out and returns, a key at an end + * of the range. An arrow key otherwise reports on each press, save while a drag is holding the + * width. A width a drag reached and never released is committed if the panel goes away under it. */ export default function usePanelResize( width: number, @@ -92,6 +93,14 @@ export default function usePanelResize( */ const dragWidthRef = useRef(undefined); + /** + * Latest committed width and `onWidthChange`, read through refs by the commits below. An effect + * listing either as a dependency tears down whenever it changes, committing a width part-way + * through a gesture. + */ + const widthRef = useLatestRef(width); + const onWidthChangeRef = useLatestRef(onWidthChange); + const { min, max } = bounds; /** Holds a width within the range a drag may reach. */ @@ -129,8 +138,9 @@ export default function usePanelResize( const endDrag = () => { const committed = dragWidthRef.current; - // Writing an unchanged width back would put a stray click on the handle through the store. - if (committed !== undefined) onWidthChange(committed); + // A gesture that ends where it began — a stray click on the handle, a drag that wanders out + // and returns — would otherwise put a width the store already holds back through it. + if (committed !== undefined && committed !== widthRef.current) onWidthChange(committed); setDragWidth(undefined); dragWidthRef.current = undefined; dragOriginRef.current = undefined; @@ -175,24 +185,14 @@ export default function usePanelResize( // eslint-disable-next-line react-hooks/exhaustive-deps }, [dragWidth === undefined, onWidthChange, clampWidth]); - /** - * Latest `onWidthChange`, so the commit below can run for the hook's whole lifetime. An effect - * keyed on the caller's callback tears down whenever its identity changes, committing a width - * part-way through a gesture. - */ - const onWidthChangeRef = useRef(onWidthChange); - useEffect(() => { - onWidthChangeRef.current = onWidthChange; - }, [onWidthChange]); - // A drag stays local until released, so a panel unmounted while one is in flight would otherwise // discard the width the gesture reached and come back at the width it started from. useEffect( () => () => { const reached = dragWidthRef.current; - if (reached !== undefined) onWidthChangeRef.current(reached); + if (reached !== undefined && reached !== widthRef.current) onWidthChangeRef.current(reached); }, - [], + [widthRef, onWidthChangeRef], ); const onKeyDown = useCallback( @@ -210,7 +210,11 @@ export default function usePanelResize( // The arrow that moves the handle the way a drag would widen the panel widens it too, // whichever side of the container the interface language anchors it to. const step = travel === widenTravel() ? 1 : -1; - const next = jumpTarget ?? clampWidth(width + step * KEYBOARD_RESIZE_STEP_PX); + // Stepping from the width on screen rather than the committed one behind it, as a press + // starts from: a step measured from a committed width the bounds disallow lands on the width + // the panel is already showing, so the first press of an arrow would move nothing. + const onScreen = clampWidth(width); + const next = jumpTarget ?? clampWidth(onScreen + step * KEYBOARD_RESIZE_STEP_PX); // An arrow held down at an end of the range repeats, and each repeat would otherwise put the // same width through the store. if (next !== width) onWidthChange(next); From 3933e4a93c5e2d8be7b4d58f69d0183a6c9cd209 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 19 Aug 2026 11:42:28 -0600 Subject: [PATCH 13/29] Drop a stray blank line in the platform React mock --- __mocks__/platform-bible-react.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index 714870f7..11f01c4a 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -121,7 +121,6 @@ export const MOCK_OPEN_ANALYSIS_CATALOG_MENU_ITEM: MenuItemContainingCommand = { localizeNotes: '', }; - /** * Stub toolbar that renders project-menu and view-info buttons using sentinel menu items so tests * can trigger menu commands without a real toolbar implementation. From cc8b93b156ce99588e47aa3913dcd778d7ea5cb7 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Thu, 20 Aug 2026 09:12:20 -0600 Subject: [PATCH 14/29] Keep a remembered panel width through a container clamp Also skip resizing on modified arrow and jump keys, re-clamp the drag origin when the container shrinks mid-gesture, and resolve the rows' shared book label once for the list. --- .../components/AnalysisCatalogPanel.test.tsx | 101 +++++++++++++++++- src/components/AnalysisCatalogPanel.tsx | 19 +++- src/components/CatalogRowView.tsx | 14 +-- src/hooks/usePanelResize.ts | 25 +++-- 4 files changed, 135 insertions(+), 24 deletions(-) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 19d73f24..96e8d18a 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -2,7 +2,7 @@ /// import type { SerializedVerseRef } from '@sillsdev/scripture'; -import { act, fireEvent, render, screen, within } from '@testing-library/react'; +import { act, createEvent, fireEvent, render, screen, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { AssignmentStatus, TextAnalysis, TokenAnalysisLink } from 'interlinearizer'; import { useEffect } from 'react'; @@ -330,6 +330,42 @@ describe('AnalysisCatalogPanel', () => { expect(onWidthChange).not.toHaveBeenCalled(); }); + it('leaves the width alone on an arrow held with a modifier', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { + key: 'ArrowLeft', + altKey: true, + }); + + expect(onWidthChange).not.toHaveBeenCalled(); + }); + + it('leaves a modified arrow for the host to act on', () => { + renderPanel({ width: 320 }); + + const event = createEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { + key: 'ArrowLeft', + ctrlKey: true, + }); + fireEvent(screen.getByTestId('analysis-catalog-resize'), event); + + expect(event.defaultPrevented).toBe(false); + }); + + it('leaves the width alone on a jump key held with a modifier', () => { + const onWidthChange = jest.fn(); + renderPanel({ width: 320, onWidthChange }); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { + key: 'Home', + metaKey: true, + }); + + expect(onWidthChange).not.toHaveBeenCalled(); + }); + it('commits nothing when the handle is pressed without being moved', () => { const onWidthChange = jest.fn(); renderPanel({ width: 320, onWidthChange }); @@ -483,15 +519,72 @@ describe('AnalysisCatalogPanel', () => { }); it('stays at its narrowest in a container with no room for the view either', () => { + stubContainerWidth(300); + renderPanel({ width: 320 }); + + // Something has to give in a container this narrow, and a panel below its own minimum + // would be unreadable, so what is left of the view gives way instead. + expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: '220px' }); + }); + + it('keeps the remembered width when a key lands on the width already on screen', () => { const onWidthChange = jest.fn(); stubContainerWidth(300); renderPanel({ width: 320, onWidthChange }); fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'End' }); - // Something has to give in a container this narrow, and a panel below its own minimum - // would be unreadable, so what is left of the view gives way instead. - expect(onWidthChange).toHaveBeenCalledWith(220); + // The clamp is what the container imposes, not what the reader asked for; reporting it + // would overwrite the width the panel returns to once there is room again. + expect(onWidthChange).not.toHaveBeenCalled(); + }); + + it('keeps a remembered width wider than the container when widened at the clamp', () => { + const onWidthChange = jest.fn(); + stubContainerWidth(600); + renderPanel({ width: 800, onWidthChange }); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowLeft' }); + + expect(onWidthChange).not.toHaveBeenCalled(); + }); + + it('still narrows from a remembered width wider than the container', () => { + const onWidthChange = jest.fn(); + stubContainerWidth(600); + renderPanel({ width: 800, onWidthChange }); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowRight' }); + + // One step in from the clamped maximum, not from the committed width. + expect(onWidthChange).toHaveBeenCalledWith(344); + }); + + it('follows the pointer from the new maximum when the container shrinks mid-drag', () => { + const originalResizeObserver = global.ResizeObserver; + resizeObserverInstances = []; + global.ResizeObserver = TrackingResizeObserver; + const clientWidth = stubContainerWidth(2000); + + try { + const onWidthChange = jest.fn(); + renderPanel({ width: 700, onWidthChange }); + const handle = screen.getByTestId('analysis-catalog-resize'); + fireEvent.mouseDown(handle, { button: 0, clientX: 500 }); + + clientWidth.mockReturnValue(600); + act(() => { + resizeObserverInstances.forEach((observer) => observer.callback([], observer)); + }); + fireEvent.mouseMove(window, { buttons: 1, clientX: 520 }); + fireEvent.mouseUp(window, { button: 0 }); + + // One step in from the shrunken maximum: an unclamped origin would park the panel there + // until the pointer had traveled the whole difference. + expect(onWidthChange).toHaveBeenCalledWith(340); + } finally { + global.ResizeObserver = originalResizeObserver; + } }); it('narrows the panel as the container shrinks under it', () => { diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index d4bc10b1..a0f555fb 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -1,6 +1,8 @@ import { useLocalizedStrings } from '@papi/frontend/react'; +import { Canon } from '@sillsdev/scripture'; import { X } from 'lucide-react'; import { Button } from 'platform-bible-react'; +import { formatReplacementString } from 'platform-bible-utils'; import { useCallback, useMemo, useRef, useState } from 'react'; import { useAnalysisLanguage, useCatalogRows } from './AnalysisStore'; import CatalogRowView, { ROW_STRING_KEYS } from './CatalogRowView'; @@ -104,6 +106,21 @@ export default function AnalysisCatalogPanel({ const rows = useMemo(() => applyCatalogQuery(catalogRows, query), [catalogRows, query]); + /** + * Label every row carries for its per-book usage count, resolved once for the whole list. The + * book's English name rather than its code, because this label reads as prose where the usage + * links below it read as references. A platform-localized name would need PAPI wiring this view + * does not yet have. + */ + const usageCountInBookLabel = useMemo( + () => + formatReplacementString( + localizedStrings['%interlinearizer_analysisCatalog_usageCountInBook%'], + { book: Canon.bookIdToEnglishName(currentBook) }, + ), + [localizedStrings, currentBook], + ); + const { navigate, requestFocusToken } = useInterlinearNav(); /** @@ -199,11 +216,11 @@ export default function AnalysisCatalogPanel({ ))} diff --git a/src/components/CatalogRowView.tsx b/src/components/CatalogRowView.tsx index acf8eae3..8e8b24c2 100644 --- a/src/components/CatalogRowView.tsx +++ b/src/components/CatalogRowView.tsx @@ -1,4 +1,3 @@ -import { Canon } from '@sillsdev/scripture'; import { ChevronDown, ChevronRight } from 'lucide-react'; import { Button } from 'platform-bible-react'; import { formatReplacementString, type LanguageStrings } from 'platform-bible-utils'; @@ -28,8 +27,8 @@ const INLINE_USAGE_LIMIT = 12; type CatalogRowViewProps = Readonly<{ /** The analysis this row lists. */ row: CatalogRow; - /** Book code `row.usageCountInBook` was taken against, whose name that count's label carries. */ - currentBook: string; + /** Finished label for `row.usageCountInBook`, naming the book that count was taken against. */ + usageCountInBookLabel: string; /** Whether this is the row the view was last jumped from. */ isSelected: boolean; /** Jumps the interlinear view to one of this analysis's usages. */ @@ -55,7 +54,7 @@ function usageLabel(usage: CatalogUsage): string { */ function CatalogRowView({ row, - currentBook, + usageCountInBookLabel, isSelected, onUsageSelect, localizedStrings, @@ -77,13 +76,6 @@ function CatalogRowView({ const hiddenUsageCount = row.usages.length - visibleUsages.length; const usageCountLabel = localizedStrings['%interlinearizer_analysisCatalog_usageCount%']; - const usageCountInBookLabel = formatReplacementString( - localizedStrings['%interlinearizer_analysisCatalog_usageCountInBook%'], - // English name rather than the code, because this label reads as prose where the usage links - // below it read as references. A platform-localized name would need PAPI wiring this view does - // not yet have. - { book: Canon.bookIdToEnglishName(currentBook) }, - ); return (
  • { const committed = dragWidthRef.current; // A gesture that ends where it began — a stray click on the handle, a drag that wanders out - // and returns — would otherwise put a width the store already holds back through it. - if (committed !== undefined && committed !== widthRef.current) onWidthChange(committed); + // and returns — puts nothing through the store. Against the width on screen, so ending at a + // clamped end keeps a wider remembered width. + if (committed !== undefined && committed !== clampWidth(widthRef.current)) + onWidthChangeRef.current(committed); setDragWidth(undefined); dragWidthRef.current = undefined; dragOriginRef.current = undefined; @@ -169,7 +171,10 @@ export default function usePanelResize( // listener's removal, which the re-render carrying it has yet to reach. /* v8 ignore next -- a test never interleaves a move into that window: the re-render lands first */ if (!origin) return; - const next = clampWidth(origin.width + origin.widenTravel * (event.clientX - origin.clientX)); + // Re-clamped under the current bounds: a container narrowed mid-gesture would otherwise park + // the panel at the new maximum until the pointer had traveled the difference. + const originWidth = clampWidth(origin.width); + const next = clampWidth(originWidth + origin.widenTravel * (event.clientX - origin.clientX)); dragWidthRef.current = next; setDragWidth(next); }; @@ -180,10 +185,10 @@ export default function usePanelResize( window.removeEventListener('mousemove', handleMouseMove); window.removeEventListener('mouseup', handleMouseUp); }; - // `dragWidth` is read only as the in-flight flag; listing its value as a dep would tear down - // and remount both listeners on every frame of the drag. + // `dragWidth` is read only as the in-flight flag, and `onWidthChange` through its ref; listing + // either value as a dep would tear down and remount both listeners mid-drag. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [dragWidth === undefined, onWidthChange, clampWidth]); + }, [dragWidth === undefined, clampWidth]); // A drag stays local until released, so a panel unmounted while one is in flight would otherwise // discard the width the gesture reached and come back at the width it started from. @@ -197,6 +202,9 @@ export default function usePanelResize( const onKeyDown = useCallback( (event: ReactKeyboardEvent) => { + // The keys below are recognized by name alone, so a modified press — Alt+Arrow, which some + // hosts navigate back on — would both resize the panel and swallow the host's shortcut. + if (event.ctrlKey || event.metaKey || event.altKey) return; const travel = keyTravel(event.key); const jumpTarget = keyJumpTarget(event.key, min, max); if (travel === 0 && jumpTarget === undefined) return; @@ -216,8 +224,9 @@ export default function usePanelResize( const onScreen = clampWidth(width); const next = jumpTarget ?? clampWidth(onScreen + step * KEYBOARD_RESIZE_STEP_PX); // An arrow held down at an end of the range repeats, and each repeat would otherwise put the - // same width through the store. - if (next !== width) onWidthChange(next); + // same width through the store. Against the width on screen, so a press at a clamped end + // keeps a wider remembered width. + if (next !== onScreen) onWidthChange(next); }, [width, onWidthChange, clampWidth, min, max], ); From 07c4a342ca9b0e10dcc41720195d5ac3659ed2a4 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Thu, 20 Aug 2026 12:48:06 -0600 Subject: [PATCH 15/29] Correct the coverage-ignore reason on the stale drag-origin guard The ignore claimed no test could interleave a move into the window between the origin clearing and the listener's removal, which a move dispatched in the same act() as the drag's end disproves. --- src/hooks/usePanelResize.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/usePanelResize.ts b/src/hooks/usePanelResize.ts index 139bf77c..e7e6d2c9 100644 --- a/src/hooks/usePanelResize.ts +++ b/src/hooks/usePanelResize.ts @@ -169,7 +169,7 @@ export default function usePanelResize( // The listener is mounted only with an origin already set, and whatever clears the origin // ends the drag too, so a move reaches this only in the window between that clear and this // listener's removal, which the re-render carrying it has yet to reach. - /* v8 ignore next -- a test never interleaves a move into that window: the re-render lands first */ + /* v8 ignore next -- a guard on that window, which the drag leaves already ended either way */ if (!origin) return; // Re-clamped under the current bounds: a container narrowed mid-gesture would otherwise park // the panel at the new maximum until the pointer had traveled the difference. From ef04752af57e93348ba6509277e683affb314085 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Thu, 20 Aug 2026 16:15:54 -0600 Subject: [PATCH 16/29] Match the unmount width commit to the release comparison A drag only ever reaches a clamped width, so comparing what it reached against the raw committed width never matched when the bounds disallowed that width. A panel that went away mid-gesture then committed the container's clamp, overwriting a wider remembered width that a release in the same gesture would have kept. Compare against the clamped width instead, as the release path does. The comparison reads the bounds through a ref: listing clampWidth as a dependency would tear the effect down on every bounds change and commit part-way through a gesture. Cover the focus request the navigation provider abandons once the reader moves past the book it names. The existing probe claims from inside the provider, where child effects run first, so no test reached that path; routing through a third book leaves the request unclaimed long enough for it to run. --- .../components/AnalysisCatalogPanel.test.tsx | 57 +++++++++++++++++++ src/hooks/usePanelResize.ts | 11 +++- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 96e8d18a..116cf191 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -549,6 +549,22 @@ describe('AnalysisCatalogPanel', () => { expect(onWidthChange).not.toHaveBeenCalled(); }); + it('keeps a remembered width wider than the container when the panel goes away mid-drag', () => { + const onWidthChange = jest.fn(); + stubContainerWidth(600); + const { unmount } = renderPanel({ width: 800, onWidthChange }); + + // Out to the clamped maximum and back: the same gesture a release commits nothing for. + fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); + fireEvent.mouseMove(window, { clientX: 480, buttons: 1 }); + fireEvent.mouseMove(window, { clientX: 500, buttons: 1 }); + unmount(); + + // The clamp is what the container imposes, not what the reader asked for; committing it + // because the panel went away would overwrite the width it returns to once there is room. + expect(onWidthChange).not.toHaveBeenCalled(); + }); + it('still narrows from a remembered width wider than the container', () => { const onWidthChange = jest.fn(); stubContainerWidth(600); @@ -1017,6 +1033,47 @@ describe('AnalysisCatalogPanel', () => { expect(claimedFocusRequest).toBe('EXO 3:14:8'); }); + it('abandons the focus request when the reader navigates past the book it names', async () => { + const { rerender } = renderPanel({ analysis: TWO_BOOKS, mountedBook: 'GEN' }); + + await clickUsage('EXO 3:14:8'); + + // EXO's load never arrives; the reader navigates somewhere else entirely in the meantime. + // The probe stands in for a view of that third book, which claims nothing here. + rerender( + + + + + , + ); + + // EXO finally mounts, long after the reader moved on. + rerender( + + + + + , + ); + + // Honoring it now would yank focus on a visit the reader made for their own reasons, long + // after the click that asked for it. + expect(claimedFocusRequest).toBeUndefined(); + }); + it('leaves the panel open and the clicked row selected', async () => { renderPanel({ analysis: TWO_BOOKS }); diff --git a/src/hooks/usePanelResize.ts b/src/hooks/usePanelResize.ts index e7e6d2c9..797ec133 100644 --- a/src/hooks/usePanelResize.ts +++ b/src/hooks/usePanelResize.ts @@ -109,6 +109,9 @@ export default function usePanelResize( [min, max], ); + /** Latest bounds to clamp against, for a commit that must not re-run when they change. */ + const clampWidthRef = useLatestRef(clampWidth); + const onMouseDown = useCallback( (event: ReactMouseEvent) => { // The drag below asks only whether some button is held, so one begun by any other button @@ -195,9 +198,13 @@ export default function usePanelResize( useEffect( () => () => { const reached = dragWidthRef.current; - if (reached !== undefined && reached !== widthRef.current) onWidthChangeRef.current(reached); + // Against the width on screen, as a release is: a drag only ever reaches a clamped width, so + // measuring against a committed width the bounds disallow never matches, and a gesture that + // ended where it began would commit the clamp and lose the wider remembered width. + if (reached !== undefined && reached !== clampWidthRef.current(widthRef.current)) + onWidthChangeRef.current(reached); }, - [widthRef, onWidthChangeRef], + [widthRef, onWidthChangeRef, clampWidthRef], ); const onKeyDown = useCallback( From d2631024c184e65fc21225998d08e3f78b21c271 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 09:59:26 -0600 Subject: [PATCH 17/29] Ask for the per-book count label where it is rendered --- src/components/AnalysisCatalogPanel.tsx | 1 + src/components/CatalogRowView.tsx | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index a0f555fb..f4113d89 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -23,6 +23,7 @@ const STRING_KEYS = [ '%interlinearizer_analysisCatalog_close%', '%interlinearizer_analysisCatalog_resize%', '%interlinearizer_analysisCatalog_empty%', + '%interlinearizer_analysisCatalog_usageCountInBook%', ...ROW_STRING_KEYS, ] as const satisfies `%${string}%`[]; diff --git a/src/components/CatalogRowView.tsx b/src/components/CatalogRowView.tsx index 8e8b24c2..f41463a1 100644 --- a/src/components/CatalogRowView.tsx +++ b/src/components/CatalogRowView.tsx @@ -12,7 +12,6 @@ import type { CatalogRow, CatalogUsage } from '../utils/analysis-query'; export const ROW_STRING_KEYS = [ '%interlinearizer_analysisCatalog_noGloss%', '%interlinearizer_analysisCatalog_usageCount%', - '%interlinearizer_analysisCatalog_usageCountInBook%', '%interlinearizer_analysisCatalog_noUsages%', '%interlinearizer_analysisCatalog_showAllUsages%', ] as const satisfies `%${string}%`[]; From d8b6b002f9d4a78256149263ab18441da0adbfc5 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 14:11:26 -0600 Subject: [PATCH 18/29] Adopt the platform resizable panels for the catalog The catalog's splitter was a hand-rolled hook pair: window-level drag listeners, a container measurement feeding a clamped maximum, and a focusable separator carrying its own aria-value triple. platform-bible- react already exports ResizablePanelGroup over react-resizable-panels, which does all of it, so the view and the catalog become two panels either side of a ResizableHandle and the interlinear view's floor is a declarative minSize. Three behaviors the library lacks stay ours, in a keydown layer over the handle: arrows mirrored for a right-to-left interface, and Home and End as jumps to either end of the range rather than a step. Everything else yields to the platform handler, which honors an already-defaulted event. The persisted width becomes the group's layout, keyed per panel, so what is restored is a layout the library laid out rather than a pixel count reapplied to it. --- __mocks__/platform-bible-react.tsx | 76 +++ .../components/AnalysisCatalogPanel.test.tsx | 592 +----------------- .../components/InterlinearizerLoader.test.tsx | 25 +- .../hooks/usePanelResizeKeys.test.tsx | 183 ++++++ src/components/AnalysisCatalogPanel.tsx | 146 ++--- src/components/InterlinearizerLoader.tsx | 114 +++- src/hooks/useContainerWidth.ts | 31 - src/hooks/usePanelResize.ts | 244 -------- src/hooks/usePanelResizeKeys.ts | 73 +++ 9 files changed, 477 insertions(+), 1007 deletions(-) create mode 100644 src/__tests__/hooks/usePanelResizeKeys.test.tsx delete mode 100644 src/hooks/useContainerWidth.ts delete mode 100644 src/hooks/usePanelResize.ts create mode 100644 src/hooks/usePanelResizeKeys.ts diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index 11f01c4a..008808ca 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -1008,3 +1008,79 @@ export function TooltipProvider({ }: Readonly<{ children?: ReactNode; delayDuration?: number }>): ReactElement { return <>{children}; } + +/** The layout the enclosing {@link ResizablePanelGroup} was given, empty outside any group. */ +const PanelLayoutContext = createContext>>({}); + +/** + * Stub resizable group, rendering its panels in order under the layout it was given. Real layout + * needs measurement jsdom does not do, so the layout is published rather than applied. + */ +export function ResizablePanelGroup({ + children, + className, + defaultLayout, +}: Readonly<{ + children?: ReactNode; + className?: string; + defaultLayout?: Readonly>; + onLayoutChanged?: (layout: Readonly>) => void; + orientation?: 'horizontal' | 'vertical'; +}>): ReactElement { + return ( + +
    + {children} +
    +     + ); +} + +/** Stub resizable panel, publishing the share of the group it holds as `data-panel-layout`. */ +export function ResizablePanel({ + children, + id, + minSize, + maxSize, +}: Readonly<{ + children?: ReactNode; + id?: string; + minSize?: string | number; + maxSize?: string | number; +}>): ReactElement { + const layout = useContext(PanelLayoutContext); + return ( +
    + {children} +
    + ); +} + +/** + * Stub resize handle, focusable and keyboard-driven as the real one is. Dragging it needs pointer + * behavior jsdom does not have, so only its keyboard half stands. + */ +export function ResizableHandle({ + onKeyDown, + ...props +}: Readonly<{ + onKeyDown?: KeyboardEventHandler; + 'aria-label'?: string; + 'data-testid'?: string; + withHandle?: boolean; +}>): ReactElement { + return ( +
    + ); +} diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 116cf191..5a95c06a 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -2,7 +2,7 @@ /// import type { SerializedVerseRef } from '@sillsdev/scripture'; -import { act, createEvent, fireEvent, render, screen, within } from '@testing-library/react'; +import { render, screen, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { AssignmentStatus, TextAnalysis, TokenAnalysisLink } from 'interlinearizer'; import { useEffect } from 'react'; @@ -39,43 +39,8 @@ function FocusRequestProbe({ bookCode }: Readonly<{ bookCode: string }>) { return undefined; } -/** Every {@link TrackingResizeObserver} created since the last reset, newest last. */ -let resizeObserverInstances: TrackingResizeObserver[] = []; - -/** - * A ResizeObserver test double that records its callback and appends itself to - * {@link resizeObserverInstances}, so a test can fire the container resize jsdom never raises. - * Module-scoped (rather than an inline class per test) so the file stays under - * `max-classes-per-file`. - */ -class TrackingResizeObserver implements ResizeObserver { - constructor(public callback: ResizeObserverCallback) { - resizeObserverInstances.push(this); - } - - // eslint-disable-next-line @typescript-eslint/class-methods-use-this - observe() {} - - // eslint-disable-next-line @typescript-eslint/class-methods-use-this - unobserve() {} - - // eslint-disable-next-line @typescript-eslint/class-methods-use-this - disconnect() {} -} - -/** - * Reports `width` as the client width of every element, standing in for the layout jsdom does not - * do: the panel measures the container it is rendered into, which is otherwise zero-width. The spy - * it returns reports a different width on demand, for a test that fires a resize. - */ -function stubContainerWidth(width: number) { - return jest.spyOn(Element.prototype, 'clientWidth', 'get').mockReturnValue(width); -} - /** Options every `renderPanel` call may override. */ type PanelOptions = Partial<{ - width: number; - onWidthChange: (width: number) => void; onClose: () => void; currentBook: string; analysis: TextAnalysis; @@ -100,9 +65,7 @@ function renderPanel(overrides: PanelOptions = {}) { {})} - onWidthChange={overrides.onWidthChange ?? (() => {})} sourceLanguageTag="el" - width={overrides.width ?? 320} /> , @@ -118,565 +81,12 @@ function rowFor(analysisId: string): HTMLElement { return row; } -/** - * Drags the resize handle from `fromClientX` to `toClientX`. The handle listens on the window for - * the move and release, so those are dispatched there rather than on the handle itself — a real - * drag routinely leaves the handle's own box between frames. - * - * The move carries the button it holds — a real one does, jsdom does not unless told — because the - * panel reads that to tell a live drag from a release it never saw. - */ -function dragHandle(fromClientX: number, toClientX: number): void { - fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: fromClientX }); - fireEvent.mouseMove(window, { clientX: toClientX, buttons: 1 }); - fireEvent.mouseUp(window, { clientX: toClientX }); -} - describe('AnalysisCatalogPanel', () => { beforeEach(() => { claimedFocusRequest = undefined; mockKeyAsValueLocalizedStrings(); }); - describe('resizing', () => { - it('widens the panel when the handle is dragged toward the start edge', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - // The panel is anchored to the end edge, so the handle moving toward the start edge widens it - // by the distance traveled. - dragHandle(500, 460); - - expect(onWidthChange).toHaveBeenCalledWith(360); - }); - - it('narrows the panel when the handle is dragged toward the end edge', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - dragHandle(500, 540); - - expect(onWidthChange).toHaveBeenCalledWith(280); - }); - - it('does not commit a width until the drag is released', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); - fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); - - // The width is persisted, so a write per frame would put the whole gesture through the host. - expect(onWidthChange).not.toHaveBeenCalled(); - expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: '360px' }); - }); - - it('stops narrowing at the minimum width', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - dragHandle(500, 5000); - - expect(onWidthChange).toHaveBeenCalledWith(220); - }); - - it('stops widening at the maximum width', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - dragHandle(500, -5000); - - expect(onWidthChange).toHaveBeenCalledWith(800); - }); - - it('widens the panel by one step on ArrowLeft', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowLeft' }); - - expect(onWidthChange).toHaveBeenCalledWith(336); - }); - - it('narrows the panel by one step on ArrowRight', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowRight' }); - - expect(onWidthChange).toHaveBeenCalledWith(304); - }); - - it('jumps to the narrowest width on Home', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - // The ends of the range are the same widths whichever side the panel is anchored to, so - // unlike the arrow keys these need no right-to-left counterpart. - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'Home' }); - - expect(onWidthChange).toHaveBeenCalledWith(220); - }); - - it('jumps to the widest width on End', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'End' }); - - expect(onWidthChange).toHaveBeenCalledWith(800); - }); - - it('leaves the width alone on Home while a drag is in flight', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - const handle = screen.getByTestId('analysis-catalog-resize'); - fireEvent.mouseDown(handle, { clientX: 500 }); - fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); - fireEvent.keyDown(handle, { key: 'Home' }); - - expect(onWidthChange).not.toHaveBeenCalled(); - }); - - it('draws a committed width the bounds do not allow at the nearest width they do', () => { - // Only the gestures clamp, so a width arriving from the store is the one way the panel can be - // asked to draw itself outside the range it reports. - renderPanel({ width: 5000 }); - - const handle = screen.getByTestId('analysis-catalog-resize'); - expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: '800px' }); - expect(handle).toHaveAttribute('aria-valuenow', '800'); - }); - - it('holds a committed width the bounds do not allow at that same width under a press', () => { - renderPanel({ width: 5000 }); - - fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); - - // A drag that began at the committed width would widen the panel to it on the press alone, - // past the maximum the handle announces. - expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: '800px' }); - }); - - it('drags from the width on screen when the committed width is one the bounds do not allow', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 5000, onWidthChange }); - - dragHandle(500, 540); - - // A drag measured from the committed width would sit at the maximum until the pointer had - // traveled the whole difference, rather than following it from the first pixel. - expect(onWidthChange).toHaveBeenCalledWith(760); - }); - - it('steps from the width on screen when the committed width is one the bounds do not allow', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 5000, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowRight' }); - - // A step measured from the committed width would land on the maximum the panel is already - // drawn at, leaving the first press of the arrow to move nothing. - expect(onWidthChange).toHaveBeenCalledWith(784); - }); - - it('leaves the width alone on End when the panel is already at its widest', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 800, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'End' }); - - // The width is persisted through the host, so an identical write is not free. - expect(onWidthChange).not.toHaveBeenCalled(); - }); - - it('leaves the width alone on an arrow that would widen the panel past its widest', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 800, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowLeft' }); - - expect(onWidthChange).not.toHaveBeenCalled(); - }); - - it('commits the width a drag reached when the panel goes away under it', () => { - const onWidthChange = jest.fn(); - const { unmount } = renderPanel({ width: 320, onWidthChange }); - - fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); - fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); - // The panel is remounted by anything that replaces the draft, so a gesture can outlive it. - unmount(); - - expect(onWidthChange).toHaveBeenCalledWith(360); - }); - - it('commits nothing when the panel goes away with no drag in flight', () => { - const onWidthChange = jest.fn(); - const { unmount } = renderPanel({ width: 320, onWidthChange }); - - unmount(); - - expect(onWidthChange).not.toHaveBeenCalled(); - }); - - it('leaves the width alone on a key that does not resize', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'Enter' }); - - expect(onWidthChange).not.toHaveBeenCalled(); - }); - - it('leaves the width alone on an arrow held with a modifier', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { - key: 'ArrowLeft', - altKey: true, - }); - - expect(onWidthChange).not.toHaveBeenCalled(); - }); - - it('leaves a modified arrow for the host to act on', () => { - renderPanel({ width: 320 }); - - const event = createEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { - key: 'ArrowLeft', - ctrlKey: true, - }); - fireEvent(screen.getByTestId('analysis-catalog-resize'), event); - - expect(event.defaultPrevented).toBe(false); - }); - - it('leaves the width alone on a jump key held with a modifier', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { - key: 'Home', - metaKey: true, - }); - - expect(onWidthChange).not.toHaveBeenCalled(); - }); - - it('commits nothing when the handle is pressed without being moved', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); - fireEvent.mouseUp(window, { clientX: 500 }); - - // The width is persisted, so a stray click on the handle would otherwise put an unchanged - // width through the host. - expect(onWidthChange).not.toHaveBeenCalled(); - }); - - it('commits nothing when a drag returns to the width it started from', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); - fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); - fireEvent.mouseMove(window, { clientX: 500, buttons: 1 }); - fireEvent.mouseUp(window, { clientX: 500 }); - - expect(onWidthChange).not.toHaveBeenCalled(); - }); - - it('commits nothing when the panel goes away with the drag back where it started', () => { - const onWidthChange = jest.fn(); - const { unmount } = renderPanel({ width: 320, onWidthChange }); - - fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); - fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); - fireEvent.mouseMove(window, { clientX: 500, buttons: 1 }); - unmount(); - - expect(onWidthChange).not.toHaveBeenCalled(); - }); - - it('leaves the width alone when a button other than the primary one presses the handle', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { - button: 1, - buttons: 4, - clientX: 500, - }); - // The middle button reports itself held on every move that follows, so a drag begun from one - // would follow the pointer and persist wherever the release found it. - fireEvent.mouseMove(window, { buttons: 4, clientX: 460 }); - fireEvent.mouseUp(window, { clientX: 460 }); - - expect(onWidthChange).not.toHaveBeenCalled(); - expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: '320px' }); - }); - - it('keeps a drag running when a button other than the primary one is released', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); - fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); - // `button` is the one released, `buttons` the one still down: a middle-button click made - // without letting go of the primary. - fireEvent.mouseUp(window, { button: 1, buttons: 1, clientX: 460 }); - - expect(onWidthChange).not.toHaveBeenCalled(); - - fireEvent.mouseMove(window, { clientX: 440, buttons: 1 }); - fireEvent.mouseUp(window, { clientX: 440 }); - - // Otherwise the gesture ends at the stray click, committing the width it had reached there - // rather than the one it finished on. - expect(onWidthChange).toHaveBeenCalledTimes(1); - expect(onWidthChange).toHaveBeenCalledWith(380); - }); - - it('ends the drag at the width it reached when a move reports no button held', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); - fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); - // Stands in for a release the window never saw — one over a native menu, say. - fireEvent.mouseMove(window, { clientX: 400, buttons: 0 }); - - expect(onWidthChange).toHaveBeenCalledWith(360); - }); - - it('stops following the pointer once a release it never saw has ended the drag', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); - fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); - fireEvent.mouseMove(window, { clientX: 400, buttons: 0 }); - fireEvent.mouseMove(window, { clientX: 200, buttons: 0 }); - fireEvent.mouseUp(window, { clientX: 200 }); - - // Otherwise a pointer merely crossing the window keeps widening the panel, and the click that - // ends up committing reports wherever it got to. - expect(onWidthChange).toHaveBeenCalledTimes(1); - expect(onWidthChange).toHaveBeenLastCalledWith(360); - }); - - it('leaves an arrow key alone while a drag is in flight', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - const handle = screen.getByTestId('analysis-catalog-resize'); - fireEvent.mouseDown(handle, { clientX: 500 }); - fireEvent.mouseMove(window, { clientX: 460, buttons: 1 }); - fireEvent.keyDown(handle, { key: 'ArrowLeft' }); - - // Stepping off the width the drag began at would report a width the panel is not showing, - // which the release then overwrites. - expect(onWidthChange).not.toHaveBeenCalled(); - - fireEvent.mouseUp(window, { clientX: 460 }); - - expect(onWidthChange).toHaveBeenCalledWith(360); - }); - - describe('within its container', () => { - it('holds the widest width to the room the interlinear view is left', () => { - const onWidthChange = jest.fn(); - stubContainerWidth(600); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'End' }); - - expect(onWidthChange).toHaveBeenCalledWith(360); - }); - - it('announces the width the container holds it to', () => { - stubContainerWidth(600); - renderPanel({ width: 320 }); - - expect(screen.getByTestId('analysis-catalog-resize')).toHaveAttribute( - 'aria-valuemax', - '360', - ); - }); - - it('holds to its own widest width in a container with room to spare', () => { - const onWidthChange = jest.fn(); - stubContainerWidth(2000); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'End' }); - - expect(onWidthChange).toHaveBeenCalledWith(800); - }); - - it('stays at its narrowest in a container with no room for the view either', () => { - stubContainerWidth(300); - renderPanel({ width: 320 }); - - // Something has to give in a container this narrow, and a panel below its own minimum - // would be unreadable, so what is left of the view gives way instead. - expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: '220px' }); - }); - - it('keeps the remembered width when a key lands on the width already on screen', () => { - const onWidthChange = jest.fn(); - stubContainerWidth(300); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'End' }); - - // The clamp is what the container imposes, not what the reader asked for; reporting it - // would overwrite the width the panel returns to once there is room again. - expect(onWidthChange).not.toHaveBeenCalled(); - }); - - it('keeps a remembered width wider than the container when widened at the clamp', () => { - const onWidthChange = jest.fn(); - stubContainerWidth(600); - renderPanel({ width: 800, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowLeft' }); - - expect(onWidthChange).not.toHaveBeenCalled(); - }); - - it('keeps a remembered width wider than the container when the panel goes away mid-drag', () => { - const onWidthChange = jest.fn(); - stubContainerWidth(600); - const { unmount } = renderPanel({ width: 800, onWidthChange }); - - // Out to the clamped maximum and back: the same gesture a release commits nothing for. - fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); - fireEvent.mouseMove(window, { clientX: 480, buttons: 1 }); - fireEvent.mouseMove(window, { clientX: 500, buttons: 1 }); - unmount(); - - // The clamp is what the container imposes, not what the reader asked for; committing it - // because the panel went away would overwrite the width it returns to once there is room. - expect(onWidthChange).not.toHaveBeenCalled(); - }); - - it('still narrows from a remembered width wider than the container', () => { - const onWidthChange = jest.fn(); - stubContainerWidth(600); - renderPanel({ width: 800, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowRight' }); - - // One step in from the clamped maximum, not from the committed width. - expect(onWidthChange).toHaveBeenCalledWith(344); - }); - - it('follows the pointer from the new maximum when the container shrinks mid-drag', () => { - const originalResizeObserver = global.ResizeObserver; - resizeObserverInstances = []; - global.ResizeObserver = TrackingResizeObserver; - const clientWidth = stubContainerWidth(2000); - - try { - const onWidthChange = jest.fn(); - renderPanel({ width: 700, onWidthChange }); - const handle = screen.getByTestId('analysis-catalog-resize'); - fireEvent.mouseDown(handle, { button: 0, clientX: 500 }); - - clientWidth.mockReturnValue(600); - act(() => { - resizeObserverInstances.forEach((observer) => observer.callback([], observer)); - }); - fireEvent.mouseMove(window, { buttons: 1, clientX: 520 }); - fireEvent.mouseUp(window, { button: 0 }); - - // One step in from the shrunken maximum: an unclamped origin would park the panel there - // until the pointer had traveled the whole difference. - expect(onWidthChange).toHaveBeenCalledWith(340); - } finally { - global.ResizeObserver = originalResizeObserver; - } - }); - - it('narrows the panel as the container shrinks under it', () => { - const originalResizeObserver = global.ResizeObserver; - resizeObserverInstances = []; - global.ResizeObserver = TrackingResizeObserver; - const clientWidth = stubContainerWidth(2000); - - try { - renderPanel({ width: 320 }); - clientWidth.mockReturnValue(600); - act(() => { - resizeObserverInstances.forEach((observer) => observer.callback([], observer)); - }); - - // A tab redocked narrower carries the width it was given in the wide one. - expect(screen.getByTestId('analysis-catalog-resize')).toHaveAttribute( - 'aria-valuemax', - '360', - ); - } finally { - global.ResizeObserver = originalResizeObserver; - } - }); - }); - - describe('in a right-to-left interface', () => { - beforeEach(() => { - document.documentElement.dir = 'rtl'; - }); - - // Plain assignment to the document, which `restoreMocks` cannot undo. - afterEach(() => { - document.documentElement.dir = ''; - }); - - it('widens the panel when the handle is dragged toward the start edge', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - // The end edge the panel is anchored to is the screen's left here, putting the handle on - // its right, so the travel that widens it is the mirror of the left-to-right one. - dragHandle(500, 540); - - expect(onWidthChange).toHaveBeenCalledWith(360); - }); - - it('narrows the panel when the handle is dragged toward the end edge', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - dragHandle(500, 460); - - expect(onWidthChange).toHaveBeenCalledWith(280); - }); - - it('widens the panel by one step on ArrowRight', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowRight' }); - - expect(onWidthChange).toHaveBeenCalledWith(336); - }); - - it('narrows the panel by one step on ArrowLeft', () => { - const onWidthChange = jest.fn(); - renderPanel({ width: 320, onWidthChange }); - - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowLeft' }); - - expect(onWidthChange).toHaveBeenCalledWith(304); - }); - }); - }); - describe('rows', () => { it('says the catalog is empty rather than leaving the panel blank', () => { // A draft records nothing until the first gloss is entered, so the empty catalog is what most diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index f37ef5c0..f0b7ca36 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -400,6 +400,16 @@ jest.mock('../../components/modals/ProjectModals', () => ({ }, })); +/** + * The resizable panel the catalog is laid out in, which is what carries the share of the group it + * holds. Fails the test when the catalog is closed. + */ +function catalogPanelElement(): HTMLElement { + const panel = screen.getByTestId('analysis-catalog-panel').parentElement; + if (!panel) throw new Error('the catalog panel is not inside a resizable panel'); + return panel; +} + /** * Renders {@link InterlinearizerLoader} with the given props, supplying a fresh * `updateWebViewDefinition` spy (which tests can read back) and sensible defaults for the scroll @@ -1835,28 +1845,23 @@ describe('InterlinearizerLoader', () => { expect(screen.getByTestId('analysis-catalog-panel')).toBeInTheDocument(); }); - it('restores a resized catalog panel to its dragged width on remount', async () => { + it('restores a resized catalog panel to its remembered layout on remount', async () => { const useWebViewState = makeWebViewState(); await act(async () => { renderLoader({ useWebViewState }); }); await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); - fireEvent.mouseDown(screen.getByTestId('analysis-catalog-resize'), { clientX: 500 }); - // The move has to report a button held — jsdom sends none unless told — because one reporting - // none reads as a release the window missed, ending the drag before it resizes anything. - fireEvent.mouseMove(window, { buttons: 1, clientX: 420 }); - fireEvent.mouseUp(window, { clientX: 420 }); - // The default is what both a dead drag and a dropped write leave behind, so each assertion - // names a width only a live drag reaches. - expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: '420px' }); + // By key rather than by drag, dragging needing measurement jsdom does not do. Home lands + // somewhere the default is not, so a layout read back on remount can only be a stored one. + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'Home' }); cleanup(); await act(async () => { renderLoader({ useWebViewState }); }); - expect(screen.getByTestId('analysis-catalog-panel')).toHaveStyle({ width: '420px' }); + expect(catalogPanelElement()).toHaveAttribute('data-panel-layout', '0.15'); }); it('leaves the catalog panel closed on remount when it was never opened', async () => { diff --git a/src/__tests__/hooks/usePanelResizeKeys.test.tsx b/src/__tests__/hooks/usePanelResizeKeys.test.tsx new file mode 100644 index 00000000..dfd958ba --- /dev/null +++ b/src/__tests__/hooks/usePanelResizeKeys.test.tsx @@ -0,0 +1,183 @@ +/// + +import { fireEvent, render, screen } from '@testing-library/react'; +import usePanelResizeKeys from '../../hooks/usePanelResizeKeys'; + +/** Narrowest and widest shares of the group a press may reach. */ +const BOUNDS = { min: 0.15, max: 0.5 }; + +/** Renders a separator driven by the hook, standing in for the platform resize handle. */ +function renderHandle(fraction: number, onFractionChange: (fraction: number) => void) { + function Handle() { + const onKeyDown = usePanelResizeKeys(fraction, onFractionChange, BOUNDS); + // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/no-noninteractive-tabindex + return
    ; + } + render(); + return screen.getByTestId('handle'); +} + +/** + * Presses `key` on the handle, with `init` supplying any modifiers held. + * + * @returns Whether the press was claimed, leaving the platform handle to ignore it. + */ +function press(handle: HTMLElement, key: string, init: object = {}): boolean { + return !fireEvent.keyDown(handle, { key, ...init }); +} + +describe('usePanelResizeKeys', () => { + afterEach(() => { + document.documentElement.removeAttribute('dir'); + }); + + describe('in a left-to-right interface', () => { + it('leaves the arrows to the platform handle, which already reads them correctly', () => { + const onFractionChange = jest.fn(); + const handle = renderHandle(0.25, onFractionChange); + + const defaulted = press(handle, 'ArrowLeft'); + + expect(onFractionChange).not.toHaveBeenCalled(); + expect(defaulted).toBe(false); + }); + }); + + describe('in a right-to-left interface', () => { + beforeEach(() => { + document.documentElement.dir = 'rtl'; + }); + + it('narrows the panel on ArrowLeft, which points away from the edge it is anchored to', () => { + const onFractionChange = jest.fn(); + const handle = renderHandle(0.25, onFractionChange); + + press(handle, 'ArrowLeft'); + + expect(onFractionChange).toHaveBeenCalledWith(0.2); + }); + + it('widens the panel on ArrowRight', () => { + const onFractionChange = jest.fn(); + const handle = renderHandle(0.25, onFractionChange); + + press(handle, 'ArrowRight'); + + expect(onFractionChange).toHaveBeenCalledWith(0.3); + }); + + it('claims the mirrored arrow, so the platform handle leaves it alone', () => { + const handle = renderHandle(0.25, () => {}); + + expect(press(handle, 'ArrowRight')).toBe(true); + }); + + it('holds a widening arrow to the widest the panel may be', () => { + const onFractionChange = jest.fn(); + const handle = renderHandle(0.48, onFractionChange); + + press(handle, 'ArrowRight'); + + expect(onFractionChange).toHaveBeenCalledWith(0.5); + }); + + it('reports nothing for an arrow held down at the end of the range', () => { + const onFractionChange = jest.fn(); + const handle = renderHandle(0.5, onFractionChange); + + press(handle, 'ArrowRight'); + + expect(onFractionChange).not.toHaveBeenCalled(); + }); + }); + + describe('jumping to an end of the range', () => { + it('sends the panel to its narrowest on Home', () => { + const onFractionChange = jest.fn(); + const handle = renderHandle(0.25, onFractionChange); + + press(handle, 'Home'); + + expect(onFractionChange).toHaveBeenCalledWith(BOUNDS.min); + }); + + it('sends the panel to its widest on End', () => { + const onFractionChange = jest.fn(); + const handle = renderHandle(0.25, onFractionChange); + + press(handle, 'End'); + + expect(onFractionChange).toHaveBeenCalledWith(BOUNDS.max); + }); + + it('jumps the same way whichever side the interface anchors the panel to', () => { + document.documentElement.dir = 'rtl'; + const onFractionChange = jest.fn(); + const handle = renderHandle(0.25, onFractionChange); + + press(handle, 'Home'); + + expect(onFractionChange).toHaveBeenCalledWith(BOUNDS.min); + }); + + it('reports nothing on End when the panel is already at its widest', () => { + const onFractionChange = jest.fn(); + const handle = renderHandle(BOUNDS.max, onFractionChange); + + press(handle, 'End'); + + expect(onFractionChange).not.toHaveBeenCalled(); + }); + }); + + describe('keys it does not act on', () => { + it('leaves the panel alone on a key that resizes nothing', () => { + const onFractionChange = jest.fn(); + const handle = renderHandle(0.25, onFractionChange); + + press(handle, 'a'); + + expect(onFractionChange).not.toHaveBeenCalled(); + }); + + it('leaves a modified jump key for the host to act on', () => { + const onFractionChange = jest.fn(); + const handle = renderHandle(0.25, onFractionChange); + + // Ctrl+Home is a document-level shortcut in some hosts, which swallowing it would break. + const defaulted = press(handle, 'Home', { ctrlKey: true }); + + expect(onFractionChange).not.toHaveBeenCalled(); + expect(defaulted).toBe(false); + }); + + it.each(['metaKey', 'altKey'])('leaves a %s-modified arrow alone', (modifier) => { + document.documentElement.dir = 'rtl'; + const onFractionChange = jest.fn(); + const handle = renderHandle(0.25, onFractionChange); + + press(handle, 'ArrowRight', { [modifier]: true }); + + expect(onFractionChange).not.toHaveBeenCalled(); + }); + }); + + it('resizes from the share it is given rather than one it remembers', () => { + // The caller holds the layout, so a share changed elsewhere — by a drag, or by a restored + // layout — is what the next press has to step from. + document.documentElement.dir = 'rtl'; + const onFractionChange = jest.fn(); + + function Handle({ fraction }: Readonly<{ fraction: number }>) { + const onKeyDown = usePanelResizeKeys(fraction, onFractionChange, BOUNDS); + // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/no-noninteractive-tabindex + return
    ; + } + const { rerender } = render(); + rerender(); + + press(screen.getByTestId('handle'), 'ArrowRight'); + + expect(onFractionChange).toHaveBeenCalledWith(0.45); + }); +}); diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index f4113d89..d3f226c6 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -3,12 +3,10 @@ import { Canon } from '@sillsdev/scripture'; import { X } from 'lucide-react'; import { Button } from 'platform-bible-react'; import { formatReplacementString } from 'platform-bible-utils'; -import { useCallback, useMemo, useRef, useState } from 'react'; +import { useCallback, useMemo, useState } from 'react'; import { useAnalysisLanguage, useCatalogRows } from './AnalysisStore'; import CatalogRowView, { ROW_STRING_KEYS } from './CatalogRowView'; import { useInterlinearNav } from './InterlinearNavContext'; -import useContainerWidth from '../hooks/useContainerWidth'; -import usePanelResize, { type PanelWidthBounds } from '../hooks/usePanelResize'; import { applyCatalogQuery, type CatalogQuery, type CatalogUsage } from '../utils/analysis-query'; import { collatorForTag } from '../utils/language-tags'; @@ -27,45 +25,10 @@ const STRING_KEYS = [ ...ROW_STRING_KEYS, ] as const satisfies `%${string}%`[]; -/** - * How far the panel may be resized on its own account: from narrow enough that the usage counts - * still fit to wide enough to read a gloss whole. A container with no room for the wide end gives - * the panel less than this — see {@link MIN_VIEW_WIDTH_PX}. - */ -const WIDTH_BOUNDS: PanelWidthBounds = { min: 220, max: 800 }; - -/** - * How much of the container the interlinear view keeps whatever width the panel is asked for, in - * pixels. The panel sits beside the text rather than over it, so a container too narrow for both at - * their full width narrows the panel rather than pushing the text off the screen — unless it has no - * room for this and the narrowest panel together. - */ -const MIN_VIEW_WIDTH_PX = 240; - -/** - * How far a panel in a container this wide may be resized: {@link WIDTH_BOUNDS}, its wide end held - * to the room {@link MIN_VIEW_WIDTH_PX} leaves. A container of unknown width — one nothing has laid - * out yet — constrains nothing. - */ -function boundsWithin(containerWidth: number | undefined): PanelWidthBounds { - if (containerWidth === undefined) return WIDTH_BOUNDS; - return { - min: WIDTH_BOUNDS.min, - max: Math.max(WIDTH_BOUNDS.min, Math.min(WIDTH_BOUNDS.max, containerWidth - MIN_VIEW_WIDTH_PX)), - }; -} - /** Props for {@link AnalysisCatalogPanel}. */ type AnalysisCatalogPanelProps = Readonly<{ /** Dismisses the panel. */ onClose: () => void; - /** The panel's committed width in pixels. */ - width: number; - /** - * Records a new committed width. Called once a drag is released rather than per frame: the width - * is persisted, and a write per mouse move would put the whole drag through the host. - */ - onWidthChange: (width: number) => void; /** Book code each row's per-book usage count is taken against. */ currentBook: string; /** BCP 47 tag of the source text, so surface forms collate by their own language. */ @@ -81,8 +44,6 @@ type AnalysisCatalogPanelProps = Readonly<{ */ export default function AnalysisCatalogPanel({ onClose, - width, - onWidthChange, currentBook, sourceLanguageTag, }: AnalysisCatalogPanelProps) { @@ -151,82 +112,45 @@ export default function AnalysisCatalogPanel({ [navigate, requestFocusToken], ); - /** The panel itself, so the room its container leaves for the view can be measured. */ - // eslint-disable-next-line no-null/no-null - const panelRef = useRef(null); - const containerWidth = useContainerWidth(panelRef); - const bounds = boundsWithin(containerWidth); - - const { - displayWidth, - onMouseDown: handleResizeMouseDown, - onKeyDown: handleResizeKeyDown, - } = usePanelResize(width, onWidthChange, bounds); - return (
    - {/* - The ARIA window-splitter pattern: a focusable `separator` carrying the width it controls as - its value, driven by drag or by arrow key. jsx-a11y reads `separator` as non-interactive and - objects to both the listeners and the tab stop, but a splitter is exactly the case where the - role is focusable and operable — an unfocusable one would be resizable by mouse only. - */} - {/* eslint-disable jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/no-noninteractive-tabindex */} -
    - {/* eslint-enable jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/no-noninteractive-tabindex */} -
    -
    -

    - {localizedStrings['%interlinearizer_analysisCatalog_title%']} -

    - -
    - - {rows.length === 0 ? ( -

    - {localizedStrings['%interlinearizer_analysisCatalog_empty%']} -

    - ) : ( -
      - {rows.map((row) => ( - - ))} -
    - )} +
    +

    + {localizedStrings['%interlinearizer_analysisCatalog_title%']} +

    +
    + + {rows.length === 0 ? ( +

    + {localizedStrings['%interlinearizer_analysisCatalog_empty%']} +

    + ) : ( +
      + {rows.map((row) => ( + + ))} +
    + )}
    ); } diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 8ad21acb..2fd93cb8 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -5,7 +5,12 @@ import type { } from '@papi/core'; import papi, { logger } from '@papi/frontend'; import { useData, useLocalizedStrings, useSetting } from '@papi/frontend/react'; -import { TabToolbar } from 'platform-bible-react'; +import { + ResizableHandle, + ResizablePanel, + ResizablePanelGroup, + TabToolbar, +} from 'platform-bible-react'; import type { SelectMenuItemHandler } from 'platform-bible-react'; import { isPlatformError } from 'platform-bible-utils'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; @@ -35,6 +40,7 @@ import { InterlinearNavProvider, useInterlinearNav, type FadePhase } from './Int import { RECENTER_FADE_TRANSITION_STYLE } from './recenter-fade'; import { firstVerseNumber, segmentContainsVerse } from '../utils/verse-ref'; import { resolvedOrEmpty } from '../utils/localized-strings'; +import usePanelResizeKeys from '../hooks/usePanelResizeKeys'; /** Host-injected callback to update this WebView's definition (used to toggle the tab title). */ type UpdateWebViewDefinition = WebViewProps['updateWebViewDefinition']; @@ -92,8 +98,39 @@ function BookFadeWrapper({ fadePhase, children }: BookFadeWrapperProps) { /** Glyph appended to the tab title while the draft has unsaved changes. */ const UNSAVED_TAB_MARKER = ' ●'; -/** Width the analysis catalog panel opens at before the user has ever resized it, in pixels. */ -const DEFAULT_CATALOG_WIDTH_PX = 340; +/** Identifies the interlinear view within the catalog group, in a {@link PanelLayout} and the DOM. */ +const VIEW_PANEL_ID = 'interlinearView'; + +/** Identifies the catalog within its group, in a {@link PanelLayout} and the DOM. */ +const CATALOG_PANEL_ID = 'analysisCatalog'; + +/** + * How much of the container the interlinear view keeps whatever the catalog is resized to. The + * panel sits beside the text rather than over it, so a container too narrow for both narrows the + * catalog rather than pushing the text off the screen. + */ +const MIN_VIEW_WIDTH = '240px'; + +/** Narrowest the catalog may be resized to, below which its usage counts stop fitting. */ +const MIN_CATALOG_WIDTH = '220px'; + +/** Widest the catalog may be resized to, past which no gloss needs the room. */ +const MAX_CATALOG_WIDTH = '800px'; + +/** A resizable group's layout: the share of the group each of its panels holds, by panel id. */ +type PanelLayout = Readonly>; + +/** + * How the catalog group is laid out before the user has ever resized it: enough of the container + * for a gloss to be read beside the text without crowding it. + */ +const DEFAULT_CATALOG_LAYOUT: PanelLayout = { [VIEW_PANEL_ID]: 0.75, [CATALOG_PANEL_ID]: 0.25 }; + +/** + * How much of the group Home and End aim the catalog at. What it settles on is whatever + * {@link MIN_CATALOG_WIDTH} and {@link MAX_CATALOG_WIDTH} allow, those being the real limits. + */ +const CATALOG_FRACTION_BOUNDS = { min: 0.15, max: 0.5 }; /** * Localized string keys the load/error placeholder needs. Hoisted to module scope so the reference @@ -104,6 +141,7 @@ const STRING_KEYS = [ '%interlinearizer_error_load_book_heading%', '%interlinearizer_error_process_book_heading%', '%interlinearizer_loading%', + '%interlinearizer_analysisCatalog_resize%', ] as const satisfies `%${string}%`[]; /** @@ -425,10 +463,13 @@ function InterlinearizerLoaderInner({ */ const [catalogOpen, setCatalogOpen] = useWebViewState('analysisCatalogOpen', false); - /** The catalog panel's width in pixels, tab-scoped for the same reason its open flag is. */ - const [catalogWidth, setCatalogWidth] = useWebViewState( - 'analysisCatalogWidth', - DEFAULT_CATALOG_WIDTH_PX, + /** + * How the interlinear view and the catalog beside it divide the room between them, tab-scoped for + * the same reason the catalog's open flag is. + */ + const [catalogLayout, setCatalogLayout] = useWebViewState( + 'analysisCatalogLayout', + DEFAULT_CATALOG_LAYOUT, ); const [modal, setModal] = useState('none'); @@ -522,6 +563,20 @@ function InterlinearizerLoaderInner({ /** Dismisses the analysis catalog panel. */ const handleCatalogClose = useCallback(() => setCatalogOpen(false), [setCatalogOpen]); + /** Records a share of the group a key press asked the catalog be given, the view taking the rest. */ + const handleCatalogFractionChange = useCallback( + (fraction: number) => + setCatalogLayout({ [VIEW_PANEL_ID]: 1 - fraction, [CATALOG_PANEL_ID]: fraction }), + [setCatalogLayout], + ); + + const handleCatalogResizeKeyDown = usePanelResizeKeys( + /* v8 ignore next -- every layout names both panels, the default's and each write's alike */ + catalogLayout[CATALOG_PANEL_ID] ?? DEFAULT_CATALOG_LAYOUT[CATALOG_PANEL_ID], + handleCatalogFractionChange, + CATALOG_FRACTION_BOUNDS, + ); + /** * Routes top-menu commands to the appropriate action. The project commands open their modals; the * file commands save (or open Save As); the draft command opens the wipe dialog. @@ -695,19 +750,38 @@ function InterlinearizerLoaderInner({ onPendingEditsChange={setPendingEdits} showSuggestions={showSuggestions} > - {bookArea} - - {catalogOpen && ( - + {catalogOpen ? ( + + + {bookArea} + + + + + + + ) : ( + {bookArea} )} )} diff --git a/src/hooks/useContainerWidth.ts b/src/hooks/useContainerWidth.ts deleted file mode 100644 index 66e2b90c..00000000 --- a/src/hooks/useContainerWidth.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { useEffect, useState } from 'react'; -import type { RefObject } from 'react'; - -/** - * How wide, in pixels, the element the ref'd one sits in currently is, kept current as that - * container resizes, and `undefined` before anything has been laid out. - * - * For an element sizing itself against the room its siblings are left: the width that governs is - * the container's rather than its own, and having been placed in that container rather than - * rendering it, it has no ref of its own to measure. - */ -export default function useContainerWidth(ref: RefObject): number | undefined { - const [containerWidth, setContainerWidth] = useState(undefined); - - useEffect(() => { - const container = ref.current?.parentElement; - /* v8 ignore next -- the ref is attached by the very render that mounts this effect */ - if (!container) return undefined; - - const measure = () => { - setContainerWidth(container.clientWidth > 0 ? container.clientWidth : undefined); - }; - measure(); - - const observer = new ResizeObserver(measure); - observer.observe(container); - return () => observer.disconnect(); - }, [ref]); - - return containerWidth; -} diff --git a/src/hooks/usePanelResize.ts b/src/hooks/usePanelResize.ts deleted file mode 100644 index 797ec133..00000000 --- a/src/hooks/usePanelResize.ts +++ /dev/null @@ -1,244 +0,0 @@ -import { useCallback, useEffect, useRef, useState } from 'react'; -import type { KeyboardEvent as ReactKeyboardEvent, MouseEvent as ReactMouseEvent } from 'react'; -import useLatestRef from './useLatestRef'; - -/** How far one arrow-key press resizes the panel, in pixels. */ -const KEYBOARD_RESIZE_STEP_PX = 16; - -/** `MouseEvent.button` for the primary button, the only one that drags the handle. */ -const PRIMARY_MOUSE_BUTTON = 0; - -/** - * Which way along the screen the handle has to travel to widen the panel: `-1` toward smaller - * `clientX`, `1` toward larger. - * - * The panel is anchored to the container's end edge, which the interface language decides the side - * of. A function rather than a constant, so a panel that outlives a language change still resizes - * the way it is pointing. - */ -function widenTravel(): number { - return document.documentElement.dir === 'rtl' ? 1 : -1; -} - -/** Which way along the screen a key moves the handle, `0` for a key that moves it nowhere. */ -function keyTravel(key: string): number { - if (key === 'ArrowLeft') return -1; - if (key === 'ArrowRight') return 1; - return 0; -} - -/** - * The width a key sends the panel straight to, or `undefined` for a key that sends it nowhere. A - * jump lands on an end of the range the splitter announces rather than on a side of the screen, so - * unlike an arrow key it needs no mirroring for a right-to-left interface. - */ -function keyJumpTarget(key: string, min: number, max: number): number | undefined { - if (key === 'Home') return min; - if (key === 'End') return max; - return undefined; -} - -/** How wide a panel may be dragged. */ -export interface PanelWidthBounds { - /** Narrowest the panel may be dragged, in pixels. */ - min: number; - /** Widest the panel may be dragged, in pixels. */ - max: number; -} - -/** The width to draw now, and the handlers that change it. */ -export interface PanelResize { - /** - * Width the panel should be drawn at: the committed width, or the in-flight one while a drag is - * running. - */ - displayWidth: number; - /** Begins a drag on a primary-button press, ignoring any other. Attach to the resize handle. */ - onMouseDown: (event: ReactMouseEvent) => void; - /** - * Resizes by one step per arrow key, or to an end of the range on Home/End. Attach to the resize - * handle. - */ - onKeyDown: (event: ReactKeyboardEvent) => void; -} - -/** - * Drives a panel anchored to the container's end edge, resized in pixels by dragging a handle on - * its start edge or by arrowing that handle once focused. - * - * The committed width is the caller's to hold and persist, and a drag stays local until released, - * so a gesture crossing a hundred pixels reports once rather than once per frame — which matters - * when the caller's store is the host's. A gesture that leaves the width where it found it reports - * nothing at all — a press that never moves, a drag that wanders out and returns, a key at an end - * of the range. An arrow key otherwise reports on each press, save while a drag is holding the - * width. A width a drag reached and never released is committed if the panel goes away under it. - */ -export default function usePanelResize( - width: number, - onWidthChange: (width: number) => void, - bounds: PanelWidthBounds, -): PanelResize { - /** Width the panel is drawn at while a drag is in flight, or `undefined` when none is. */ - const [dragWidth, setDragWidth] = useState(undefined); - - /** Where the in-flight drag started, the width it started from, and which way widens it. */ - const dragOriginRef = useRef<{ clientX: number; width: number; widenTravel: number } | undefined>( - undefined, - ); - - /** - * The width the in-flight drag has reached, or `undefined` until it moves. A release commits from - * here because a state updater has to stay free of side effects — React may run one more than - * once. - */ - const dragWidthRef = useRef(undefined); - - /** - * Latest committed width and `onWidthChange`, read through refs by the commits below. An effect - * listing either as a dependency tears down whenever it changes, committing a width part-way - * through a gesture. - */ - const widthRef = useLatestRef(width); - const onWidthChangeRef = useLatestRef(onWidthChange); - - const { min, max } = bounds; - - /** Holds a width within the range a drag may reach. */ - const clampWidth = useCallback( - (candidate: number) => Math.min(max, Math.max(min, candidate)), - [min, max], - ); - - /** Latest bounds to clamp against, for a commit that must not re-run when they change. */ - const clampWidthRef = useLatestRef(clampWidth); - - const onMouseDown = useCallback( - (event: ReactMouseEvent) => { - // The drag below asks only whether some button is held, so one begun by any other button - // would follow the pointer to its release and commit the width it reached. - if (event.button !== PRIMARY_MOUSE_BUTTON) return; - // The width on screen rather than the committed one behind it: a drag starting from a - // committed width the bounds disallow would jump to it on the press and then hold still until - // the pointer had traveled the whole difference. - const startWidth = clampWidth(width); - dragOriginRef.current = { - clientX: event.clientX, - width: startWidth, - widenTravel: widenTravel(), - }; - setDragWidth(startWidth); - // Suppresses the text selection a drag across the panel would otherwise sweep up. - event.preventDefault(); - }, - [width, clampWidth], - ); - - // Runs the in-flight drag. Mounted only while one is in flight, so an idle panel listens to - // nothing. The listeners sit on the window rather than the handle because the pointer leaves the - // handle's box the moment the drag begins, and a release outside it must still end the drag. - useEffect(() => { - if (dragWidth === undefined) return undefined; - - const endDrag = () => { - const committed = dragWidthRef.current; - // A gesture that ends where it began — a stray click on the handle, a drag that wanders out - // and returns — puts nothing through the store. Against the width on screen, so ending at a - // clamped end keeps a wider remembered width. - if (committed !== undefined && committed !== clampWidth(widthRef.current)) - onWidthChangeRef.current(committed); - setDragWidth(undefined); - dragWidthRef.current = undefined; - dragOriginRef.current = undefined; - }; - - const handleMouseUp = (event: MouseEvent) => { - // A middle- or right-button click made mid-drag raises `mouseup` too, and ending the drag - // there would commit the width it had reached and leave the pointer still holding a handle - // the panel does not follow. - if (event.button !== PRIMARY_MOUSE_BUTTON) return; - endDrag(); - }; - - const handleMouseMove = (event: MouseEvent) => { - // A release the window never saw — one over a native menu, which takes the pointer with it — - // would otherwise leave the drag running, resizing the panel under a pointer holding nothing - // and committing that width at the next click anywhere. A move reporting no button held is - // the only word the window gets of such a release. - if (event.buttons === 0) { - endDrag(); - return; - } - const origin = dragOriginRef.current; - // The listener is mounted only with an origin already set, and whatever clears the origin - // ends the drag too, so a move reaches this only in the window between that clear and this - // listener's removal, which the re-render carrying it has yet to reach. - /* v8 ignore next -- a guard on that window, which the drag leaves already ended either way */ - if (!origin) return; - // Re-clamped under the current bounds: a container narrowed mid-gesture would otherwise park - // the panel at the new maximum until the pointer had traveled the difference. - const originWidth = clampWidth(origin.width); - const next = clampWidth(originWidth + origin.widenTravel * (event.clientX - origin.clientX)); - dragWidthRef.current = next; - setDragWidth(next); - }; - - window.addEventListener('mousemove', handleMouseMove); - window.addEventListener('mouseup', handleMouseUp); - return () => { - window.removeEventListener('mousemove', handleMouseMove); - window.removeEventListener('mouseup', handleMouseUp); - }; - // `dragWidth` is read only as the in-flight flag, and `onWidthChange` through its ref; listing - // either value as a dep would tear down and remount both listeners mid-drag. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [dragWidth === undefined, clampWidth]); - - // A drag stays local until released, so a panel unmounted while one is in flight would otherwise - // discard the width the gesture reached and come back at the width it started from. - useEffect( - () => () => { - const reached = dragWidthRef.current; - // Against the width on screen, as a release is: a drag only ever reaches a clamped width, so - // measuring against a committed width the bounds disallow never matches, and a gesture that - // ended where it began would commit the clamp and lose the wider remembered width. - if (reached !== undefined && reached !== clampWidthRef.current(widthRef.current)) - onWidthChangeRef.current(reached); - }, - [widthRef, onWidthChangeRef, clampWidthRef], - ); - - const onKeyDown = useCallback( - (event: ReactKeyboardEvent) => { - // The keys below are recognized by name alone, so a modified press — Alt+Arrow, which some - // hosts navigate back on — would both resize the panel and swallow the host's shortcut. - if (event.ctrlKey || event.metaKey || event.altKey) return; - const travel = keyTravel(event.key); - const jumpTarget = keyJumpTarget(event.key, min, max); - if (travel === 0 && jumpTarget === undefined) return; - // The pointer owns the width while it is held: a key press mid-drag would step off the width - // the drag began at, which is neither what the panel is showing nor what the release is about - // to commit. Read off a ref, so the handler stays stable across a drag's frames. The case is - // unusual but reachable — the press that begins a drag suppresses the focus change, so the - // handle has to have been tabbed to first. - if (dragOriginRef.current) return; - event.preventDefault(); - // The arrow that moves the handle the way a drag would widen the panel widens it too, - // whichever side of the container the interface language anchors it to. - const step = travel === widenTravel() ? 1 : -1; - // Stepping from the width on screen rather than the committed one behind it, as a press - // starts from: a step measured from a committed width the bounds disallow lands on the width - // the panel is already showing, so the first press of an arrow would move nothing. - const onScreen = clampWidth(width); - const next = jumpTarget ?? clampWidth(onScreen + step * KEYBOARD_RESIZE_STEP_PX); - // An arrow held down at an end of the range repeats, and each repeat would otherwise put the - // same width through the store. Against the width on screen, so a press at a clamped end - // keeps a wider remembered width. - if (next !== onScreen) onWidthChange(next); - }, - [width, onWidthChange, clampWidth, min, max], - ); - - // Clamped rather than passed through, so the bounds govern what is drawn and reported even when - // a committed width arrives from outside them. - return { displayWidth: dragWidth ?? clampWidth(width), onMouseDown, onKeyDown }; -} diff --git a/src/hooks/usePanelResizeKeys.ts b/src/hooks/usePanelResizeKeys.ts new file mode 100644 index 00000000..879629c9 --- /dev/null +++ b/src/hooks/usePanelResizeKeys.ts @@ -0,0 +1,73 @@ +import { useCallback } from 'react'; +import type { KeyboardEvent as ReactKeyboardEvent } from 'react'; + +/** + * How far one arrow-key press resizes the panel, as a share of the group. Matches the step the + * platform handle takes, so an arrow moves the panel equally far whichever of the two answers it. + */ +const KEYBOARD_RESIZE_STEP = 0.05; + +/** + * Which way along the screen the handle travels to widen the panel: `-1` toward the screen's left, + * `1` toward its right. Read afresh on each press, so a panel that outlives a change of interface + * language resizes the way it is currently pointing. + */ +function widenTravel(): number { + return document.documentElement.dir === 'rtl' ? 1 : -1; +} + +/** Which way along the screen a key moves the handle, `0` for a key that moves it nowhere. */ +function keyTravel(key: string): number { + if (key === 'ArrowLeft') return -1; + if (key === 'ArrowRight') return 1; + return 0; +} + +/** + * Resizes a panel by key press: mirrored arrows for a right-to-left interface, and Home and End to + * either end of the range. Handles only what the platform resize handle leaves undone, and yields + * every other key to it, so the two together answer a full set. + * + * Sizes are shares of the group the panel is laid out in, `0.25` being a quarter of it. + * + * @param fraction - Share the panel currently holds, which a press resizes from. + * @param onFractionChange - Records a share a press asked for. Not called for a press that would + * leave the panel where it already is. + * @param bounds - Narrowest and widest shares a press may reach. + * @returns A `keydown` handler for the resize handle. + */ +export default function usePanelResizeKeys( + fraction: number, + onFractionChange: (fraction: number) => void, + bounds: { min: number; max: number }, +): (event: ReactKeyboardEvent) => void { + const { min, max } = bounds; + + return useCallback( + (event: ReactKeyboardEvent) => { + // The keys below are recognized by name alone, so a modified press — Alt+Arrow, which some + // hosts navigate back on — would both resize the panel and swallow the host's shortcut. + if (event.ctrlKey || event.metaKey || event.altKey) return; + + const travel = keyTravel(event.key); + // Left alone in a left-to-right interface, where the platform handle already reads these + // arrows the way the panel is pointing; stepping here as well would move it twice. + const mirrors = travel !== 0 && widenTravel() === 1; + const jumpTarget = + // eslint-disable-next-line no-nested-ternary + event.key === 'Home' ? min : event.key === 'End' ? max : undefined; + if (!mirrors && jumpTarget === undefined) return; + + // Claims the press, which the platform handle honors by leaving a defaulted event alone. + event.preventDefault(); + + const next = + jumpTarget ?? + Math.min(max, Math.max(min, fraction + travel * widenTravel() * KEYBOARD_RESIZE_STEP)); + // An arrow held down at an end of the range repeats, and each repeat would otherwise put an + // unchanged layout through the store. + if (next !== fraction) onFractionChange(next); + }, + [fraction, onFractionChange, min, max], + ); +} From 83b14e849683e5f63d1e648c3fede099c19e3004 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 14:24:22 -0600 Subject: [PATCH 19/29] Format catalog references and collation through the platform usageLabel hand-built "GEN 1:1" and collatorForTag wrapped Intl.Collator directly. platform-bible-utils exports formatScrRef and a Collator class for both, so route through those and keep intl use inside the platform layer. The try/catch around the collator stays: analysis languages are free text from the project modals, so an unparsable tag reaches the sort unchecked and has to degrade to some ordering rather than take the view down. --- __mocks__/platform-bible-utils.ts | 32 ++++++++++++++++++++++ src/__tests__/utils/analysis-query.test.ts | 11 ++++---- src/components/CatalogRowView.tsx | 8 ++++-- src/utils/analysis-query.ts | 7 +++-- src/utils/language-tags.ts | 14 ++++++---- 5 files changed, 56 insertions(+), 16 deletions(-) diff --git a/__mocks__/platform-bible-utils.ts b/__mocks__/platform-bible-utils.ts index c1c28606..d6734e80 100644 --- a/__mocks__/platform-bible-utils.ts +++ b/__mocks__/platform-bible-utils.ts @@ -106,9 +106,41 @@ const formatReplacementString = (str: string, replacers: { [key: string]: unknow .map((part) => String(part)) .join(''); +/** + * Language-sensitive string comparison, wrapping `Intl.Collator` as the real class does. Throws on + * a tag `Intl` cannot parse, as the real one does. + */ +class Collator { + private collator: Intl.Collator; + + constructor(locales?: string | string[], options?: Intl.CollatorOptions) { + this.collator = new Intl.Collator(locales, options); + } + + compare(string1: string, string2: string): number { + return this.collator.compare(string1, string2); + } + + resolvedOptions(): Intl.ResolvedCollatorOptions { + return this.collator.resolvedOptions(); + } +} + +/** The book, chapter, and verse of a scripture reference; the real type carries more. */ +type SerializedVerseRef = { book: string; chapterNum: number; verseNum: number }; + +/** + * Formats a scripture reference as the real function does for its default options, e.g. `GEN 1:1`. + * The book-name and separator options are unused here. + */ +const formatScrRef = (scrRef: SerializedVerseRef): string => + `${scrRef.book} ${scrRef.chapterNum}:${scrRef.verseNum}`; + export { + Collator, UnsubscriberAsyncList, formatReplacementString, formatReplacementStringToArray, + formatScrRef, isPlatformError, }; diff --git a/src/__tests__/utils/analysis-query.test.ts b/src/__tests__/utils/analysis-query.test.ts index 51bb0a52..0689d492 100644 --- a/src/__tests__/utils/analysis-query.test.ts +++ b/src/__tests__/utils/analysis-query.test.ts @@ -1,6 +1,7 @@ /// import type { AssignmentStatus, TextAnalysis, TokenAnalysisLink } from 'interlinearizer'; +import { Collator } from 'platform-bible-utils'; import { emptyAnalysis } from '../../types/empty-factories'; import { FIXTURE_STAMPS } from '../test-helpers'; import { @@ -19,8 +20,8 @@ function makeQuery(overrides: Partial = {}): CatalogQuery { search: '', sort: 'usageCount', filters: {}, - surfaceCollator: new Intl.Collator('el'), - glossCollator: new Intl.Collator('en'), + surfaceCollator: new Collator('el'), + glossCollator: new Collator('en'), ...overrides, }; } @@ -390,11 +391,11 @@ describe('applyCatalogQuery sort', () => { ], }; const rows = buildCatalogRows(glossed, scope); - const sortByGloss = (glossCollator: Intl.Collator) => + const sortByGloss = (glossCollator: Collator) => applyCatalogQuery(rows, makeQuery({ sort: 'gloss', glossCollator })).map((r) => r.analysisId); - expect(sortByGloss(new Intl.Collator('sv'))).toEqual(['ta-2', 'ta-1']); - expect(sortByGloss(new Intl.Collator('en'))).toEqual(['ta-1', 'ta-2']); + expect(sortByGloss(new Collator('sv'))).toEqual(['ta-2', 'ta-1']); + expect(sortByGloss(new Collator('en'))).toEqual(['ta-1', 'ta-2']); }); // Collating alone would open the list with ta-2: a missing gloss is the empty string, which sorts diff --git a/src/components/CatalogRowView.tsx b/src/components/CatalogRowView.tsx index f41463a1..7e5a73d7 100644 --- a/src/components/CatalogRowView.tsx +++ b/src/components/CatalogRowView.tsx @@ -1,6 +1,6 @@ import { ChevronDown, ChevronRight } from 'lucide-react'; import { Button } from 'platform-bible-react'; -import { formatReplacementString, type LanguageStrings } from 'platform-bible-utils'; +import { formatReplacementString, formatScrRef, type LanguageStrings } from 'platform-bible-utils'; import { memo, useCallback, useState } from 'react'; import type { CatalogRow, CatalogUsage } from '../utils/analysis-query'; @@ -40,7 +40,11 @@ type CatalogRowViewProps = Readonly<{ /** Renders a usage's location the way scripture references are written, e.g. `GEN 1:1`. */ function usageLabel(usage: CatalogUsage): string { - return `${usage.book} ${usage.chapter}:${usage.verse}`; + return formatScrRef({ + book: usage.book, + chapterNum: usage.chapter, + verseNum: usage.verse, + }); } /** diff --git a/src/utils/analysis-query.ts b/src/utils/analysis-query.ts index e523611d..d80d3d82 100644 --- a/src/utils/analysis-query.ts +++ b/src/utils/analysis-query.ts @@ -6,6 +6,7 @@ import type { TokenAnalysis, TokenAnalysisLink, } from 'interlinearizer'; +import type { Collator } from 'platform-bible-utils'; import { bookOfRef } from './analysis-book'; import { foldForSearch } from './search-fold'; import { firstVerseNumber } from './verse-ref'; @@ -84,9 +85,9 @@ export interface CatalogQuery { sort: CatalogSort; filters: CatalogFilters; /** Collates surface forms, so ordering follows the source language rather than code points. */ - surfaceCollator: Intl.Collator; + surfaceCollator: Collator; /** Collates glosses, so ordering follows the analysis language rather than code points. */ - glossCollator: Intl.Collator; + glossCollator: Collator; } /** @@ -259,7 +260,7 @@ function compareFirstUsage(a: CatalogRow, b: CatalogRow): number { * Orders two rows by gloss, an analysis with none in the scope's language coming after every one * that has one. */ -function compareGloss(a: CatalogRow, b: CatalogRow, glossCollator: Intl.Collator): number { +function compareGloss(a: CatalogRow, b: CatalogRow, glossCollator: Collator): number { if (!a.gloss) return b.gloss ? 1 : 0; if (!b.gloss) return -1; return glossCollator.compare(a.gloss, b.gloss); diff --git a/src/utils/language-tags.ts b/src/utils/language-tags.ts index b949426e..460e8e2e 100644 --- a/src/utils/language-tags.ts +++ b/src/utils/language-tags.ts @@ -1,3 +1,5 @@ +import { Collator } from 'platform-bible-utils'; + /** * Parses a comma-separated analysis-language field into BCP 47 tags. The single source of this * parse, so no field can interpret the same input differently. @@ -15,16 +17,16 @@ export function parseLanguageTags(input: string): string[] { } /** - * A collator for `tag`, falling back to the host's default collation when `Intl` rejects it. + * A collator for `tag`, falling back to the host's default collation when the tag is unusable. * * Language tags reach this as free text — nothing checks them for BCP 47 structure on the way in — - * and `Intl` throws on one it cannot parse, so an unusable tag has to degrade to some ordering - * rather than throw. + * and constructing a collator for an unparsable tag throws, so it has to degrade to some ordering + * rather than take the view down. */ -export function collatorForTag(tag: string): Intl.Collator { +export function collatorForTag(tag: string): Collator { try { - return new Intl.Collator(tag); + return new Collator(tag); } catch { - return new Intl.Collator(); + return new Collator(); } } From c85ef27735e49be78024d4e69e4dfc28bf338558 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 14:28:29 -0600 Subject: [PATCH 20/29] Adopt the platform empty state and truncation tooltips MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The catalog's empty message was a hand-rolled

    with the same classes EmptyState renders, minus its role="status" — so a list that went empty under a reader announced nothing. The surface form and gloss both truncate to keep rows one line, with no way to read what was cut off. Use EmptyState for the message, and useTruncationTooltip on each span, which opens only when that span's own text is clipped. Both go through TooltipTrigger asChild so no interactive element nests inside the row button. --- __mocks__/platform-bible-react.tsx | 29 ++++++++++ .../components/AnalysisCatalogPanel.test.tsx | 24 ++++++++ src/components/AnalysisCatalogPanel.tsx | 9 +-- src/components/CatalogRowView.tsx | 56 ++++++++++++++----- 4 files changed, 101 insertions(+), 17 deletions(-) diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index 008808ca..31d15acf 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -24,6 +24,7 @@ import type { MouseEventHandler, ReactElement, ReactNode, + RefObject, } from 'react'; export interface MenuItemContainingCommand { @@ -999,6 +1000,34 @@ export function Tooltip({ children }: Readonly<{ children?: ReactNode }>): React return cloneElement(triggerChild, { title: text === '' ? undefined : text }); } +/** + * Drives a tooltip that opens only when its trigger's text is clipped. Clipping needs measurement + * jsdom does not do, so this one never opens; {@link Tooltip} keeps its text assertable regardless. + */ +export function useTruncationTooltip(): { + ref: RefObject; + open: boolean; + onPointerEnter: () => void; + onPointerLeave: () => void; +} { + // eslint-disable-next-line no-null/no-null + const ref = useRef(null); + return { ref, open: false, onPointerEnter: () => {}, onPointerLeave: () => {} }; +} + +/** Stub empty-state message, carrying the `role="status"` the real component announces through. */ +export function EmptyState({ + message, + id, + className, +}: Readonly<{ message: string; id?: string; className?: string }>): ReactElement { + return ( +

    + {message} +

    + ); +} + /** * Stub tooltip provider that shares hover-delay config across nested tooltips. The stub renders its * children unchanged; the delay has no effect in tests. diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 5a95c06a..c1c93026 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -99,6 +99,14 @@ describe('AnalysisCatalogPanel', () => { ); }); + it('announces the empty catalog rather than leaving it to be noticed', () => { + renderPanel({ analysis: emptyAnalysis() }); + + expect(screen.getByRole('status')).toHaveTextContent( + '%interlinearizer_analysisCatalog_empty%', + ); + }); + it('renders an analysis with its gloss and its usage counts inside and outside the book', () => { const analysis: TextAnalysis = { ...emptyAnalysis(), @@ -219,6 +227,22 @@ describe('AnalysisCatalogPanel', () => { '%interlinearizer_analysisCatalog_noGloss%', ); }); + + it('offers the surface form and gloss in full, both being truncated to one line', () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'λόγος', gloss: { en: 'word' } }, + ], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0')], + }; + renderPanel({ analysis }); + + // The tooltip stub projects its content onto the trigger, hover having no jsdom equivalent. + const row = within(rowFor('ta-1')); + expect(row.getByTestId('catalog-row-surface')).toHaveAttribute('title', 'λόγος'); + expect(row.getByTestId('catalog-row-gloss')).toHaveAttribute('title', 'word'); + }); }); describe('row detail', () => { diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index d3f226c6..69170857 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -1,7 +1,7 @@ import { useLocalizedStrings } from '@papi/frontend/react'; import { Canon } from '@sillsdev/scripture'; import { X } from 'lucide-react'; -import { Button } from 'platform-bible-react'; +import { Button, EmptyState } from 'platform-bible-react'; import { formatReplacementString } from 'platform-bible-utils'; import { useCallback, useMemo, useState } from 'react'; import { useAnalysisLanguage, useCatalogRows } from './AnalysisStore'; @@ -133,9 +133,10 @@ export default function AnalysisCatalogPanel({
    {rows.length === 0 ? ( -

    - {localizedStrings['%interlinearizer_analysisCatalog_empty%']} -

    + ) : (
      {rows.map((row) => ( diff --git a/src/components/CatalogRowView.tsx b/src/components/CatalogRowView.tsx index 7e5a73d7..f69cb83d 100644 --- a/src/components/CatalogRowView.tsx +++ b/src/components/CatalogRowView.tsx @@ -1,5 +1,11 @@ import { ChevronDown, ChevronRight } from 'lucide-react'; -import { Button } from 'platform-bible-react'; +import { + Button, + Tooltip, + TooltipContent, + TooltipTrigger, + useTruncationTooltip, +} from 'platform-bible-react'; import { formatReplacementString, formatScrRef, type LanguageStrings } from 'platform-bible-utils'; import { memo, useCallback, useState } from 'react'; import type { CatalogRow, CatalogUsage } from '../utils/analysis-query'; @@ -80,6 +86,14 @@ function CatalogRowView({ const usageCountLabel = localizedStrings['%interlinearizer_analysisCatalog_usageCount%']; + /** The row's gloss, or the placeholder standing in for an analysis that carries none. */ + const glossLabel = row.gloss || localizedStrings['%interlinearizer_analysisCatalog_noGloss%']; + + // One tooltip each rather than one for the row: either column may be the clipped one, and a + // tooltip is worth opening only over the text that is actually cut off. + const surfaceTooltip = useTruncationTooltip(); + const glossTooltip = useTruncationTooltip(); + return (
    • )} - - {row.surfaceText} - - - {row.gloss || localizedStrings['%interlinearizer_analysisCatalog_noGloss%']} - + + + + {row.surfaceText} + + + {row.surfaceText} + + + + + {glossLabel} + + + {glossLabel} + {/* Native `title` rather than the platform Tooltip because these counts sit inside the row's own button, where a tooltip trigger would nest one interactive element in another. A From 878b311b9b8a081abb932cf5976bd4f73f0c292f Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 14:31:15 -0600 Subject: [PATCH 21/29] Drop a stale claim about the catalog list from the row doc Nothing windows the list or handles a keypress on the
        today, so the sentence described intent for #231 rather than the code as it is. --- src/components/CatalogRowView.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/CatalogRowView.tsx b/src/components/CatalogRowView.tsx index f69cb83d..dea01c1f 100644 --- a/src/components/CatalogRowView.tsx +++ b/src/components/CatalogRowView.tsx @@ -58,8 +58,7 @@ function usageLabel(usage: CatalogUsage): string { * for — the whole draft's usage count beside the current book's. Expanding it reveals the morpheme * breakdown and the places the analysis is applied. * - * Each row owns its own layout so that its detail can be nested inside it. One element per analysis - * is what lets the list window and be walked by keyboard a row at a time. + * Each row owns its own layout so that its detail can be nested inside it. */ function CatalogRowView({ row, From a99c505ce21ef7bec594ff7a1b9c534b61302aef Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 14:45:09 -0600 Subject: [PATCH 22/29] Take book names and layout direction from the platform The per-book count label built its book name with Canon.bookIdToEnglishName and a comment claiming a localized name would need PAPI wiring the view lacked, but useLocalizedStrings was already here. Ask for %LocalizedId.{book}% in its own memoized array so a book change re-resolves that key alone, falling back to the English name for the languages core ships no name for. widenTravel read document.documentElement.dir while core's direction-aware components read readDirection(), which is exported from platform-bible-react/experimental. Take direction from there instead, with a mock for the subpath since the existing platform-bible-react mapping is anchored to the package root. --- .../platform-bible-react-experimental.ts | 16 +++++++++ jest.config.ts | 3 ++ .../components/AnalysisCatalogPanel.test.tsx | 17 +++++++++ src/components/AnalysisCatalogPanel.tsx | 36 +++++++++++++------ src/hooks/usePanelResizeKeys.ts | 3 +- 5 files changed, 63 insertions(+), 12 deletions(-) create mode 100644 __mocks__/platform-bible-react-experimental.ts diff --git a/__mocks__/platform-bible-react-experimental.ts b/__mocks__/platform-bible-react-experimental.ts new file mode 100644 index 00000000..b5509820 --- /dev/null +++ b/__mocks__/platform-bible-react-experimental.ts @@ -0,0 +1,16 @@ +/** + * @file Jest mock for platform-bible-react/experimental. The real package ships ESM which Jest + * cannot parse without extra transform configuration. This stub provides the subset used by the + * extension. + */ + +/** Text and layout direction. */ +export type Direction = 'rtl' | 'ltr'; + +/** + * Layout direction the interface runs in, read from the document rather than from localStorage as + * the real function does, so a test sets it the way it would for any other RTL assertion. + */ +export function readDirection(): Direction { + return document.documentElement.dir === 'rtl' ? 'rtl' : 'ltr'; +} diff --git a/jest.config.ts b/jest.config.ts index 13f141af..4d3148f2 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -100,6 +100,9 @@ const config: Config = { '^platform-bible-utils$': '/__mocks__/platform-bible-utils.ts', /** Mock ESM deps that Jest cannot parse. */ '^platform-bible-react$': '/__mocks__/platform-bible-react.tsx', + /** Mock the experimental entry point separately; the mapping above is anchored to the root. */ + '^platform-bible-react/experimental$': + '/__mocks__/platform-bible-react-experimental.ts', /** Mock ESM-only icon library. */ '^lucide-react$': '/__mocks__/lucide-react.tsx', /** Resolve webpack ?inline imports. */ diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index c1c93026..b07ea147 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -212,6 +212,23 @@ describe('AnalysisCatalogPanel', () => { ).toHaveTextContent('Uses in Genesis'); }); + it('names the book in the interface language where the platform has a name for it', () => { + mockKeyAsValueLocalizedStrings({ + '%interlinearizer_analysisCatalog_usageCountInBook%': 'Uses in {book}', + '%LocalizedId.GEN%': 'Genèse', + }); + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [{ ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'λόγος' }], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0')], + }; + renderPanel({ analysis, currentBook: 'GEN' }); + + expect( + within(rowFor('ta-1')).getByTestId('catalog-row-usage-count-in-book'), + ).toHaveTextContent('Uses in Genèse'); + }); + it('marks an analysis with no gloss in the active language rather than leaving the cell blank', () => { const analysis: TextAnalysis = { ...emptyAnalysis(), diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index 69170857..302d9004 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -69,19 +69,33 @@ export default function AnalysisCatalogPanel({ const rows = useMemo(() => applyCatalogQuery(catalogRows, query), [catalogRows, query]); /** - * Label every row carries for its per-book usage count, resolved once for the whole list. The - * book's English name rather than its code, because this label reads as prose where the usage - * links below it read as references. A platform-localized name would need PAPI wiring this view - * does not yet have. + * The current book's name key, asked for separately from {@link STRING_KEYS} so that changing book + * re-resolves this alone rather than every string the panel shows. */ - const usageCountInBookLabel = useMemo( - () => - formatReplacementString( - localizedStrings['%interlinearizer_analysisCatalog_usageCountInBook%'], - { book: Canon.bookIdToEnglishName(currentBook) }, - ), - [localizedStrings, currentBook], + const bookNameKeys = useMemo( + () => [`%LocalizedId.${currentBook}%`] as const satisfies `%${string}%`[], + [currentBook], ); + const [localizedBookName] = useLocalizedStrings(bookNameKeys); + + /** + * Label every row carries for its per-book usage count, resolved once for the whole list. Names + * the book rather than giving its code, because this label reads as prose where the usage links + * below it read as references. + * + * Falls back to the English name, the platform carrying a localized one for only some languages. + * An unresolved key comes back as itself, which is what distinguishes the two. + */ + const usageCountInBookLabel = useMemo(() => { + const [bookKey] = bookNameKeys; + const resolved = localizedBookName?.[bookKey]; + return formatReplacementString( + localizedStrings['%interlinearizer_analysisCatalog_usageCountInBook%'], + { + book: resolved && resolved !== bookKey ? resolved : Canon.bookIdToEnglishName(currentBook), + }, + ); + }, [localizedStrings, localizedBookName, bookNameKeys, currentBook]); const { navigate, requestFocusToken } = useInterlinearNav(); diff --git a/src/hooks/usePanelResizeKeys.ts b/src/hooks/usePanelResizeKeys.ts index 879629c9..3e839f96 100644 --- a/src/hooks/usePanelResizeKeys.ts +++ b/src/hooks/usePanelResizeKeys.ts @@ -1,3 +1,4 @@ +import { readDirection } from 'platform-bible-react/experimental'; import { useCallback } from 'react'; import type { KeyboardEvent as ReactKeyboardEvent } from 'react'; @@ -13,7 +14,7 @@ const KEYBOARD_RESIZE_STEP = 0.05; * language resizes the way it is currently pointing. */ function widenTravel(): number { - return document.documentElement.dir === 'rtl' ? 1 : -1; + return readDirection() === 'rtl' ? 1 : -1; } /** Which way along the screen a key moves the handle, `0` for a key that moves it nowhere. */ From 97a3f92cae3e381e1fb8aab465277f4687916ddd Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 15:07:45 -0600 Subject: [PATCH 23/29] Lay the analysis catalog out in percentages, not fractions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The platform resizable group rescales any layout it is handed to sum to 100, and reports that rescaled layout back through onLayoutChanged. The catalog was laid out in fractions, so the group stored percentages the moment it mounted while the keyboard resize went on reading them as fractions. Initial sizing was unaffected — rescaling 0.75/0.25 is visually identical to 75/25 — but an arrow press stepped 0.05 from a value of 25 and clamped against a 0.5 bound, jumping the catalog from a quarter of the group to half in one press. Make percentages the single representation rather than converting at the boundary, so no layout has two possible units: scale the default layout, the keyboard bounds and the step, and rename the hook's fraction vocabulary to match what it now carries. The stub group echoed defaultLayout verbatim and never invoked onLayoutChanged, so fractions survived in tests in a way they never do in the app. Normalize and report back there too, which is what lets a test reach the failing step. --- __mocks__/platform-bible-react.tsx | 23 ++++- .../components/InterlinearizerLoader.test.tsx | 25 ++++- .../hooks/usePanelResizeKeys.test.tsx | 98 +++++++++---------- src/components/InterlinearizerLoader.tsx | 25 +++-- src/hooks/usePanelResizeKeys.ts | 25 ++--- 5 files changed, 124 insertions(+), 72 deletions(-) diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index 31d15acf..419d8f1f 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -1041,14 +1041,29 @@ export function TooltipProvider({ /** The layout the enclosing {@link ResizablePanelGroup} was given, empty outside any group. */ const PanelLayoutContext = createContext>>({}); +/** Rescales a layout to sum to 100, as the real group does to whatever it is handed. */ +function normalizeLayout( + layout: Readonly>, +): Readonly> { + const total = Object.values(layout).reduce((sum, size) => sum + size, 0); + if (total === 0 || total === 100) return layout; + return Object.fromEntries( + Object.entries(layout).map(([id, size]) => [id, (size / total) * 100]), + ); +} + /** * Stub resizable group, rendering its panels in order under the layout it was given. Real layout * needs measurement jsdom does not do, so the layout is published rather than applied. + * + * The layout is normalized and reported back through `onLayoutChanged`, as the real group does, so + * a caller storing what it is handed stores it in the unit the app would give it. */ export function ResizablePanelGroup({ children, className, defaultLayout, + onLayoutChanged, }: Readonly<{ children?: ReactNode; className?: string; @@ -1056,8 +1071,14 @@ export function ResizablePanelGroup({ onLayoutChanged?: (layout: Readonly>) => void; orientation?: 'horizontal' | 'vertical'; }>): ReactElement { + const layout = normalizeLayout(defaultLayout ?? {}); + useEffect(() => { + onLayoutChanged?.(layout); + // Keyed on the layout's contents, a fresh object each render otherwise reporting every render. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [JSON.stringify(layout)]); return ( - +
        {children}
        diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index f0b7ca36..54be288a 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -1861,7 +1861,30 @@ describe('InterlinearizerLoader', () => { renderLoader({ useWebViewState }); }); - expect(catalogPanelElement()).toHaveAttribute('data-panel-layout', '0.15'); + expect(catalogPanelElement()).toHaveAttribute('data-panel-layout', '15'); + }); + + it('lays the catalog out in the unit the group reports back, so a press steps rather than jumps', async () => { + // Arrows resize only in a right-to-left interface, the platform handle already reading them + // correctly in a left-to-right one. Only a step discriminates: Home and End clamp to a bound + // whatever unit the layout is in. + document.documentElement.dir = 'rtl'; + try { + await act(async () => { + renderLoader(); + }); + await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); + + // The group rescales whatever layout it is handed to sum to 100 and reports that back, so a + // layout written in fractions is stored in percentages the moment it mounts. A step taken + // in one unit while bounded by the other clamps to an end of the range instead of landing + // one step along. + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowRight' }); + + expect(catalogPanelElement()).toHaveAttribute('data-panel-layout', '30'); + } finally { + document.documentElement.removeAttribute('dir'); + } }); it('leaves the catalog panel closed on remount when it was never opened', async () => { diff --git a/src/__tests__/hooks/usePanelResizeKeys.test.tsx b/src/__tests__/hooks/usePanelResizeKeys.test.tsx index dfd958ba..d98fa791 100644 --- a/src/__tests__/hooks/usePanelResizeKeys.test.tsx +++ b/src/__tests__/hooks/usePanelResizeKeys.test.tsx @@ -3,13 +3,13 @@ import { fireEvent, render, screen } from '@testing-library/react'; import usePanelResizeKeys from '../../hooks/usePanelResizeKeys'; -/** Narrowest and widest shares of the group a press may reach. */ -const BOUNDS = { min: 0.15, max: 0.5 }; +/** Narrowest and widest percentages of the group a press may reach. */ +const BOUNDS = { min: 15, max: 50 }; /** Renders a separator driven by the hook, standing in for the platform resize handle. */ -function renderHandle(fraction: number, onFractionChange: (fraction: number) => void) { +function renderHandle(percentage: number, onPercentageChange: (percentage: number) => void) { function Handle() { - const onKeyDown = usePanelResizeKeys(fraction, onFractionChange, BOUNDS); + const onKeyDown = usePanelResizeKeys(percentage, onPercentageChange, BOUNDS); // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/no-noninteractive-tabindex return
        ; } @@ -33,12 +33,12 @@ describe('usePanelResizeKeys', () => { describe('in a left-to-right interface', () => { it('leaves the arrows to the platform handle, which already reads them correctly', () => { - const onFractionChange = jest.fn(); - const handle = renderHandle(0.25, onFractionChange); + const onPercentageChange = jest.fn(); + const handle = renderHandle(25, onPercentageChange); const defaulted = press(handle, 'ArrowLeft'); - expect(onFractionChange).not.toHaveBeenCalled(); + expect(onPercentageChange).not.toHaveBeenCalled(); expect(defaulted).toBe(false); }); }); @@ -49,135 +49,135 @@ describe('usePanelResizeKeys', () => { }); it('narrows the panel on ArrowLeft, which points away from the edge it is anchored to', () => { - const onFractionChange = jest.fn(); - const handle = renderHandle(0.25, onFractionChange); + const onPercentageChange = jest.fn(); + const handle = renderHandle(25, onPercentageChange); press(handle, 'ArrowLeft'); - expect(onFractionChange).toHaveBeenCalledWith(0.2); + expect(onPercentageChange).toHaveBeenCalledWith(20); }); it('widens the panel on ArrowRight', () => { - const onFractionChange = jest.fn(); - const handle = renderHandle(0.25, onFractionChange); + const onPercentageChange = jest.fn(); + const handle = renderHandle(25, onPercentageChange); press(handle, 'ArrowRight'); - expect(onFractionChange).toHaveBeenCalledWith(0.3); + expect(onPercentageChange).toHaveBeenCalledWith(30); }); it('claims the mirrored arrow, so the platform handle leaves it alone', () => { - const handle = renderHandle(0.25, () => {}); + const handle = renderHandle(25, () => {}); expect(press(handle, 'ArrowRight')).toBe(true); }); it('holds a widening arrow to the widest the panel may be', () => { - const onFractionChange = jest.fn(); - const handle = renderHandle(0.48, onFractionChange); + const onPercentageChange = jest.fn(); + const handle = renderHandle(48, onPercentageChange); press(handle, 'ArrowRight'); - expect(onFractionChange).toHaveBeenCalledWith(0.5); + expect(onPercentageChange).toHaveBeenCalledWith(50); }); it('reports nothing for an arrow held down at the end of the range', () => { - const onFractionChange = jest.fn(); - const handle = renderHandle(0.5, onFractionChange); + const onPercentageChange = jest.fn(); + const handle = renderHandle(50, onPercentageChange); press(handle, 'ArrowRight'); - expect(onFractionChange).not.toHaveBeenCalled(); + expect(onPercentageChange).not.toHaveBeenCalled(); }); }); describe('jumping to an end of the range', () => { it('sends the panel to its narrowest on Home', () => { - const onFractionChange = jest.fn(); - const handle = renderHandle(0.25, onFractionChange); + const onPercentageChange = jest.fn(); + const handle = renderHandle(25, onPercentageChange); press(handle, 'Home'); - expect(onFractionChange).toHaveBeenCalledWith(BOUNDS.min); + expect(onPercentageChange).toHaveBeenCalledWith(BOUNDS.min); }); it('sends the panel to its widest on End', () => { - const onFractionChange = jest.fn(); - const handle = renderHandle(0.25, onFractionChange); + const onPercentageChange = jest.fn(); + const handle = renderHandle(25, onPercentageChange); press(handle, 'End'); - expect(onFractionChange).toHaveBeenCalledWith(BOUNDS.max); + expect(onPercentageChange).toHaveBeenCalledWith(BOUNDS.max); }); it('jumps the same way whichever side the interface anchors the panel to', () => { document.documentElement.dir = 'rtl'; - const onFractionChange = jest.fn(); - const handle = renderHandle(0.25, onFractionChange); + const onPercentageChange = jest.fn(); + const handle = renderHandle(25, onPercentageChange); press(handle, 'Home'); - expect(onFractionChange).toHaveBeenCalledWith(BOUNDS.min); + expect(onPercentageChange).toHaveBeenCalledWith(BOUNDS.min); }); it('reports nothing on End when the panel is already at its widest', () => { - const onFractionChange = jest.fn(); - const handle = renderHandle(BOUNDS.max, onFractionChange); + const onPercentageChange = jest.fn(); + const handle = renderHandle(BOUNDS.max, onPercentageChange); press(handle, 'End'); - expect(onFractionChange).not.toHaveBeenCalled(); + expect(onPercentageChange).not.toHaveBeenCalled(); }); }); describe('keys it does not act on', () => { it('leaves the panel alone on a key that resizes nothing', () => { - const onFractionChange = jest.fn(); - const handle = renderHandle(0.25, onFractionChange); + const onPercentageChange = jest.fn(); + const handle = renderHandle(25, onPercentageChange); press(handle, 'a'); - expect(onFractionChange).not.toHaveBeenCalled(); + expect(onPercentageChange).not.toHaveBeenCalled(); }); it('leaves a modified jump key for the host to act on', () => { - const onFractionChange = jest.fn(); - const handle = renderHandle(0.25, onFractionChange); + const onPercentageChange = jest.fn(); + const handle = renderHandle(25, onPercentageChange); // Ctrl+Home is a document-level shortcut in some hosts, which swallowing it would break. const defaulted = press(handle, 'Home', { ctrlKey: true }); - expect(onFractionChange).not.toHaveBeenCalled(); + expect(onPercentageChange).not.toHaveBeenCalled(); expect(defaulted).toBe(false); }); it.each(['metaKey', 'altKey'])('leaves a %s-modified arrow alone', (modifier) => { document.documentElement.dir = 'rtl'; - const onFractionChange = jest.fn(); - const handle = renderHandle(0.25, onFractionChange); + const onPercentageChange = jest.fn(); + const handle = renderHandle(25, onPercentageChange); press(handle, 'ArrowRight', { [modifier]: true }); - expect(onFractionChange).not.toHaveBeenCalled(); + expect(onPercentageChange).not.toHaveBeenCalled(); }); }); - it('resizes from the share it is given rather than one it remembers', () => { - // The caller holds the layout, so a share changed elsewhere — by a drag, or by a restored + it('resizes from the percentage it is given rather than one it remembers', () => { + // The caller holds the layout, so a percentage changed elsewhere — by a drag, or by a restored // layout — is what the next press has to step from. document.documentElement.dir = 'rtl'; - const onFractionChange = jest.fn(); + const onPercentageChange = jest.fn(); - function Handle({ fraction }: Readonly<{ fraction: number }>) { - const onKeyDown = usePanelResizeKeys(fraction, onFractionChange, BOUNDS); + function Handle({ percentage }: Readonly<{ percentage: number }>) { + const onKeyDown = usePanelResizeKeys(percentage, onPercentageChange, BOUNDS); // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/no-noninteractive-tabindex return
        ; } - const { rerender } = render(); - rerender(); + const { rerender } = render(); + rerender(); press(screen.getByTestId('handle'), 'ArrowRight'); - expect(onFractionChange).toHaveBeenCalledWith(0.45); + expect(onPercentageChange).toHaveBeenCalledWith(45); }); }); diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 2fd93cb8..5325c427 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -117,20 +117,24 @@ const MIN_CATALOG_WIDTH = '220px'; /** Widest the catalog may be resized to, past which no gloss needs the room. */ const MAX_CATALOG_WIDTH = '800px'; -/** A resizable group's layout: the share of the group each of its panels holds, by panel id. */ +/** + * A resizable group's layout: the percentage of the group each of its panels holds, by panel id. + * Percentages rather than fractions because a group rescales any layout it is handed to sum to 100, + * so that is the unit one comes back in. + */ type PanelLayout = Readonly>; /** * How the catalog group is laid out before the user has ever resized it: enough of the container * for a gloss to be read beside the text without crowding it. */ -const DEFAULT_CATALOG_LAYOUT: PanelLayout = { [VIEW_PANEL_ID]: 0.75, [CATALOG_PANEL_ID]: 0.25 }; +const DEFAULT_CATALOG_LAYOUT: PanelLayout = { [VIEW_PANEL_ID]: 75, [CATALOG_PANEL_ID]: 25 }; /** * How much of the group Home and End aim the catalog at. What it settles on is whatever * {@link MIN_CATALOG_WIDTH} and {@link MAX_CATALOG_WIDTH} allow, those being the real limits. */ -const CATALOG_FRACTION_BOUNDS = { min: 0.15, max: 0.5 }; +const CATALOG_PERCENTAGE_BOUNDS = { min: 15, max: 50 }; /** * Localized string keys the load/error placeholder needs. Hoisted to module scope so the reference @@ -563,18 +567,21 @@ function InterlinearizerLoaderInner({ /** Dismisses the analysis catalog panel. */ const handleCatalogClose = useCallback(() => setCatalogOpen(false), [setCatalogOpen]); - /** Records a share of the group a key press asked the catalog be given, the view taking the rest. */ - const handleCatalogFractionChange = useCallback( - (fraction: number) => - setCatalogLayout({ [VIEW_PANEL_ID]: 1 - fraction, [CATALOG_PANEL_ID]: fraction }), + /** + * Records a percentage of the group a key press asked the catalog be given, the view taking the + * rest. + */ + const handleCatalogPercentageChange = useCallback( + (percentage: number) => + setCatalogLayout({ [VIEW_PANEL_ID]: 100 - percentage, [CATALOG_PANEL_ID]: percentage }), [setCatalogLayout], ); const handleCatalogResizeKeyDown = usePanelResizeKeys( /* v8 ignore next -- every layout names both panels, the default's and each write's alike */ catalogLayout[CATALOG_PANEL_ID] ?? DEFAULT_CATALOG_LAYOUT[CATALOG_PANEL_ID], - handleCatalogFractionChange, - CATALOG_FRACTION_BOUNDS, + handleCatalogPercentageChange, + CATALOG_PERCENTAGE_BOUNDS, ); /** diff --git a/src/hooks/usePanelResizeKeys.ts b/src/hooks/usePanelResizeKeys.ts index 3e839f96..bb61956b 100644 --- a/src/hooks/usePanelResizeKeys.ts +++ b/src/hooks/usePanelResizeKeys.ts @@ -3,10 +3,10 @@ import { useCallback } from 'react'; import type { KeyboardEvent as ReactKeyboardEvent } from 'react'; /** - * How far one arrow-key press resizes the panel, as a share of the group. Matches the step the + * How far one arrow-key press resizes the panel, as a percentage of the group. Matches the step the * platform handle takes, so an arrow moves the panel equally far whichever of the two answers it. */ -const KEYBOARD_RESIZE_STEP = 0.05; +const KEYBOARD_RESIZE_STEP = 5; /** * Which way along the screen the handle travels to widen the panel: `-1` toward the screen's left, @@ -29,17 +29,18 @@ function keyTravel(key: string): number { * either end of the range. Handles only what the platform resize handle leaves undone, and yields * every other key to it, so the two together answer a full set. * - * Sizes are shares of the group the panel is laid out in, `0.25` being a quarter of it. + * Sizes are percentages of the group the panel is laid out in, `25` being a quarter of it, matching + * the unit a platform group lays out in and hands back. * - * @param fraction - Share the panel currently holds, which a press resizes from. - * @param onFractionChange - Records a share a press asked for. Not called for a press that would - * leave the panel where it already is. - * @param bounds - Narrowest and widest shares a press may reach. + * @param percentage - Percentage the panel currently holds, which a press resizes from. + * @param onPercentageChange - Records a percentage a press asked for. Not called for a press that + * would leave the panel where it already is. + * @param bounds - Narrowest and widest percentages a press may reach. * @returns A `keydown` handler for the resize handle. */ export default function usePanelResizeKeys( - fraction: number, - onFractionChange: (fraction: number) => void, + percentage: number, + onPercentageChange: (percentage: number) => void, bounds: { min: number; max: number }, ): (event: ReactKeyboardEvent) => void { const { min, max } = bounds; @@ -64,11 +65,11 @@ export default function usePanelResizeKeys( const next = jumpTarget ?? - Math.min(max, Math.max(min, fraction + travel * widenTravel() * KEYBOARD_RESIZE_STEP)); + Math.min(max, Math.max(min, percentage + travel * widenTravel() * KEYBOARD_RESIZE_STEP)); // An arrow held down at an end of the range repeats, and each repeat would otherwise put an // unchanged layout through the store. - if (next !== fraction) onFractionChange(next); + if (next !== percentage) onPercentageChange(next); }, - [fraction, onFractionChange, min, max], + [percentage, onPercentageChange, min, max], ); } From 162997a907e7320ee40632a420b1012522dc2e99 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 15:27:31 -0600 Subject: [PATCH 24/29] Give the analysis catalog its own tooltip provider The catalog panel renders as a sibling of the interlinear view rather than within it, so its row truncation tooltips had no enclosing TooltipProvider. Radix builds its provider context with no default value, which makes a Tooltip without a provider throw rather than degrade, so opening the panel crashed the render. Controlling `open` does not avoid this: Tooltip.Root reads the provider context before it reads the prop. Wrap the panel in its own provider so it stays self-sufficient wherever it is mounted, with no delay since these tooltips open on truncation, not hover time. The mock's TooltipProvider was a passthrough fragment and so could not express the requirement that made this a bug. It now publishes its presence through context and Tooltip throws without one, matching the real component. That guard exposed four suites mounting subtrees that sit under the view's provider in the app; they now supply one via the shared wrapper and their local render helpers. --- __mocks__/platform-bible-react.tsx | 18 +++- .../components/PhraseStripParts.test.tsx | 73 +++++++++------- .../components/TokenLinkIcon.test.tsx | 32 ++++--- src/__tests__/components/test-helpers.tsx | 10 ++- src/components/AnalysisCatalogPanel.tsx | 85 ++++++++++--------- 5 files changed, 130 insertions(+), 88 deletions(-) diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index 419d8f1f..97e94b62 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -986,8 +986,15 @@ 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. + * + * Throws outside a {@link TooltipProvider}, as the real component does: Radix builds its provider + * context with no default value, so a tooltip with no provider above it crashes the render rather + * than degrading. */ -export function Tooltip({ children }: Readonly<{ children?: ReactNode }>): ReactNode { +export function Tooltip({ children }: Readonly<{ children?: ReactNode; open?: boolean }>): ReactNode { + if (!useContext(TooltipProviderContext)) + throw new Error('`Tooltip` must be used within `TooltipProvider`'); + let tooltipText: ReactNode; let triggerChild: ReactNode; Children.forEach(children, (child) => { @@ -1028,14 +1035,17 @@ export function EmptyState({ ); } +/** Whether a {@link TooltipProvider} encloses the tree. */ +const TooltipProviderContext = createContext(false); + /** - * Stub tooltip provider that shares hover-delay config across nested tooltips. The stub renders its - * children unchanged; the delay has no effect in tests. + * Stub tooltip provider that shares hover-delay config across nested tooltips. The stub carries only + * its own presence, the delay being unobservable in jsdom. */ export function TooltipProvider({ children, }: Readonly<{ children?: ReactNode; delayDuration?: number }>): ReactElement { - return <>{children}; + return {children}; } /** The layout the enclosing {@link ResizablePanelGroup} was given, empty outside any group. */ diff --git a/src/__tests__/components/PhraseStripParts.test.tsx b/src/__tests__/components/PhraseStripParts.test.tsx index 27213084..d7c69a32 100644 --- a/src/__tests__/components/PhraseStripParts.test.tsx +++ b/src/__tests__/components/PhraseStripParts.test.tsx @@ -3,6 +3,7 @@ import { fireEvent, render, screen } from '@testing-library/react'; import type { PhraseAnalysisLink, Segment, Token } from 'interlinearizer'; +import { TooltipProvider } from 'platform-bible-react'; import type { ReactElement } from 'react'; import { PhraseSlot, @@ -104,12 +105,18 @@ function slotProps(slot: LinkSlot): Parameters[0] { * Wraps `ui` in a {@link PhraseStripProvider} so components that call {@link usePhraseStripContext} * can render without a provider in the tree, defaulting every context value and layering the given * overrides on top — e.g. standing a book's token refs up in `tokenDocOrder`. + * + * Also supplies a `TooltipProvider`, without which a `Tooltip` throws. */ function withProvider( ui: ReactElement, overrides: Partial = {}, ): ReactElement { - return {ui}; + return ( + + {ui} + + ); } /** A `tokenDocOrder` standing the given refs up as the book's word tokens, in the order listed. */ @@ -390,21 +397,23 @@ describe('PhraseSlot boundary controls', () => { straddledBoundaryRefs: options.straddledBoundaryRefs ?? new Set(), }; render( - - - - - - - , + + + + + + + + + , ); return dispatch; } @@ -660,21 +669,23 @@ describe('PhraseSlot boundary controls', () => { verseStarts: [{ charStart: 0, number: '1', chapter: 1 }], }; render( - - - - - - - , + + + + + + + + + , ); fireEvent.click(screen.getByTestId('boundary-split-marker'), { altKey: true }); expect(dispatch.split).toHaveBeenCalledWith('q'); diff --git a/src/__tests__/components/TokenLinkIcon.test.tsx b/src/__tests__/components/TokenLinkIcon.test.tsx index 1e1d9848..a4960c70 100644 --- a/src/__tests__/components/TokenLinkIcon.test.tsx +++ b/src/__tests__/components/TokenLinkIcon.test.tsx @@ -3,6 +3,7 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import { TooltipProvider } from 'platform-bible-react'; import type { ComponentProps, ReactElement } from 'react'; import { TokenLinkIcon } from '../../components/TokenLinkIcon'; import { @@ -53,10 +54,15 @@ function requiredProps(): ComponentProps { }; } -/** Renders a `TokenLinkIcon` inside a strip provider carrying the given context overrides. */ +/** + * Renders a `TokenLinkIcon` inside a strip provider carrying the given context overrides, plus a + * `TooltipProvider`, without which a `Tooltip` throws. + */ function renderIcon(ui: ReactElement, context: Partial = {}) { return render( - {ui}, + + {ui} + , ); } @@ -504,16 +510,18 @@ describe('TokenLinkIcon', () => { */ function renderCrossSegment(focusedSideIsPrev: boolean) { return render( - - - , + + + + + , ); } diff --git a/src/__tests__/components/test-helpers.tsx b/src/__tests__/components/test-helpers.tsx index 5a644efc..a7599695 100644 --- a/src/__tests__/components/test-helpers.tsx +++ b/src/__tests__/components/test-helpers.tsx @@ -1,4 +1,5 @@ import { useLocalizedStrings } from '@papi/frontend/react'; +import { TooltipProvider } from 'platform-bible-react'; import type { ReactNode } from 'react'; import { AnalysisStoreProvider } from '../../components/AnalysisStore'; import { ViewOptions } from '../../types/view-options'; @@ -35,10 +36,17 @@ 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. + * + * Supplies a `TooltipProvider` too: a `Tooltip` throws without one, so a subject that renders + * tooltips under an enclosing provider in the app needs it to mount in isolation at all. */ export const withAnalysisStore = { wrapper({ children }: Readonly<{ children: ReactNode }>) { - return {children}; + return ( + + {children} + + ); }, }; diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index 302d9004..f6aae721 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -1,7 +1,7 @@ import { useLocalizedStrings } from '@papi/frontend/react'; import { Canon } from '@sillsdev/scripture'; import { X } from 'lucide-react'; -import { Button, EmptyState } from 'platform-bible-react'; +import { Button, EmptyState, TooltipProvider } from 'platform-bible-react'; import { formatReplacementString } from 'platform-bible-utils'; import { useCallback, useMemo, useState } from 'react'; import { useAnalysisLanguage, useCatalogRows } from './AnalysisStore'; @@ -127,45 +127,50 @@ export default function AnalysisCatalogPanel({ ); return ( -
        -
        -

        - {localizedStrings['%interlinearizer_analysisCatalog_title%']} -

        - -
        + // The panel sits beside the interlinear view rather than within it, so the row tooltips have no + // enclosing provider to inherit, and a Tooltip without one throws. The delay is irrelevant here: + // these tooltips open on truncation rather than on hover time. + +
        +
        +

        + {localizedStrings['%interlinearizer_analysisCatalog_title%']} +

        + +
        - {rows.length === 0 ? ( - - ) : ( -
          - {rows.map((row) => ( - - ))} -
        - )} -
        + {rows.length === 0 ? ( + + ) : ( +
          + {rows.map((row) => ( + + ))} +
        + )} +
        + ); } From de3fdd18fa6af8c9a85cf5ac2db1c92175bfb510 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 15:51:09 -0600 Subject: [PATCH 25/29] Move the analysis catalog panel when a key resizes it The resize keys wrote the new layout to WebView state and passed it back to the group as defaultLayout, but the group reads that prop only while registering itself on mount, and a layout it has since settled on outranks it even then. So a Home, End, or arrow press updated the stored layout without moving the panel, and a later drag overwrote the stored value with the on-screen one, discarding the press entirely. Take the group's imperative handle through groupRef and call setLayout alongside the state write, so a press moves the panel now and the state write only decides where the next mount opens. The stub group in the platform-bible-react mock re-applied defaultLayout on every render, so it moved the panel from the prop alone and the existing layout tests passed against the broken path. Seed its layout on mount and expose the handle, matching the real group, and cover the distinction the mock had been hiding. --- __mocks__/platform-bible-react.tsx | 44 +++++++++++++++++-- .../components/InterlinearizerLoader.test.tsx | 13 ++++++ src/components/InterlinearizerLoader.tsx | 27 +++++++++--- 3 files changed, 75 insertions(+), 9 deletions(-) diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index 97e94b62..52863cbd 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -1048,9 +1048,17 @@ export function TooltipProvider({ return {children}; } -/** The layout the enclosing {@link ResizablePanelGroup} was given, empty outside any group. */ +/** The layout the enclosing {@link ResizablePanelGroup} currently holds, empty outside any group. */ const PanelLayoutContext = createContext>>({}); +/** A resizable group's handle for reading and moving its panels once it has mounted. */ +interface GroupImperativeHandle { + /** The layout the panels currently hold. */ + getLayout: () => Readonly>; + /** Moves the panels, returning the layout as the group normalized it. */ + setLayout: (layout: Readonly>) => Readonly>; +} + /** Rescales a layout to sum to 100, as the real group does to whatever it is handed. */ function normalizeLayout( layout: Readonly>, @@ -1063,8 +1071,12 @@ function normalizeLayout( } /** - * Stub resizable group, rendering its panels in order under the layout it was given. Real layout - * needs measurement jsdom does not do, so the layout is published rather than applied. + * Stub resizable group, rendering its panels in order under the layout it holds. Real layout needs + * measurement jsdom does not do, so the layout is published rather than applied. + * + * `defaultLayout` seeds the layout on mount and is ignored thereafter, as the real group's is, so a + * caller that resizes by writing that prop alone moves nothing here either. Moving the panels after + * mount goes through the handle on `groupRef`. * * The layout is normalized and reported back through `onLayoutChanged`, as the real group does, so * a caller storing what it is handed stores it in the unit the app would give it. @@ -1073,20 +1085,44 @@ export function ResizablePanelGroup({ children, className, defaultLayout, + groupRef, onLayoutChanged, }: Readonly<{ children?: ReactNode; className?: string; defaultLayout?: Readonly>; + groupRef?: RefObject; onLayoutChanged?: (layout: Readonly>) => void; orientation?: 'horizontal' | 'vertical'; }>): ReactElement { - const layout = normalizeLayout(defaultLayout ?? {}); + const [layout, setLayout] = useState(() => normalizeLayout(defaultLayout ?? {})); + + // Read through a ref so the handle can be installed once rather than replaced on every resize. + const layoutRef = useRef(layout); + layoutRef.current = layout; + + useLayoutEffect(() => { + if (!groupRef) return undefined; + const handle = groupRef; + handle.current = { + getLayout: () => layoutRef.current, + setLayout: (next) => { + const normalized = normalizeLayout(next); + setLayout(normalized); + return normalized; + }, + }; + return () => { + handle.current = null; + }; + }, [groupRef]); + useEffect(() => { onLayoutChanged?.(layout); // Keyed on the layout's contents, a fresh object each render otherwise reporting every render. // eslint-disable-next-line react-hooks/exhaustive-deps }, [JSON.stringify(layout)]); + return (
        diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 54be288a..59c9aa7b 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -1864,6 +1864,19 @@ describe('InterlinearizerLoader', () => { expect(catalogPanelElement()).toHaveAttribute('data-panel-layout', '15'); }); + it('moves the catalog panel on the press that resized it, not only on the next mount', async () => { + await act(async () => { + renderLoader(); + }); + await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'Home' }); + + // The group reads its defaultLayout only as it mounts, so the panel can only have moved by + // the press itself rather than by the layout reaching state. + expect(catalogPanelElement()).toHaveAttribute('data-panel-layout', '15'); + }); + it('lays the catalog out in the unit the group reports back, so a press steps rather than jumps', async () => { // Arrows resize only in a right-to-left interface, the platform handle already reading them // correctly in a left-to-right one. Only a step discriminates: Home and End clamp to a bound diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 5325c427..b970e3a6 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -14,7 +14,7 @@ import { import type { SelectMenuItemHandler } from 'platform-bible-react'; import { isPlatformError } from 'platform-bible-utils'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import type { ReactNode } from 'react'; +import type { ComponentProps, ReactNode, RefObject } from 'react'; import { resegmentBook } from 'parsers/papi/resegmentBook'; import useDraftProject from '../hooks/useDraftProject'; import useInterlinearizerBookData from '../hooks/useInterlinearizerBookData'; @@ -124,6 +124,12 @@ const MAX_CATALOG_WIDTH = '800px'; */ type PanelLayout = Readonly>; +/** Holds the handle a resizable group exposes for moving its panels after it has mounted. */ +type GroupHandleRef = Extract< + ComponentProps['groupRef'], + RefObject +>; + /** * How the catalog group is laid out before the user has ever resized it: enough of the container * for a gloss to be read beside the text without crowding it. @@ -568,12 +574,22 @@ function InterlinearizerLoaderInner({ const handleCatalogClose = useCallback(() => setCatalogOpen(false), [setCatalogOpen]); /** - * Records a percentage of the group a key press asked the catalog be given, the view taking the - * rest. + * Moves the catalog group's panels, the group reading its `defaultLayout` only as it mounts and + * so staying where it is for any later layout written to state alone. + */ + // eslint-disable-next-line no-null/no-null + const catalogGroupRef: GroupHandleRef = useRef(null); + + /** + * Moves the catalog to a percentage of the group a key press asked it be given, the view taking + * the rest, and records where it was moved to so the next mount opens there. */ const handleCatalogPercentageChange = useCallback( - (percentage: number) => - setCatalogLayout({ [VIEW_PANEL_ID]: 100 - percentage, [CATALOG_PANEL_ID]: percentage }), + (percentage: number) => { + const layout = { [VIEW_PANEL_ID]: 100 - percentage, [CATALOG_PANEL_ID]: percentage }; + catalogGroupRef.current?.setLayout(layout); + setCatalogLayout(layout); + }, [setCatalogLayout], ); @@ -761,6 +777,7 @@ function InterlinearizerLoaderInner({ From 4ba1f8e4c0427ea40483e81ff72bf2d5116df4ed Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 16:21:01 -0600 Subject: [PATCH 26/29] Answer arrow keys where the platform handle gets them wrong MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The platform binds its key handler to the separator element directly, so a React onKeyDown prop runs after it — too late for preventDefault to suppress its step. Home and End were claimed that way and moved the panel twice on one press, in opposite directions; in a right-to-left interface both the mirrored arrow and the platform's unmirrored one landed. Hand Home and End back to the platform, which already implements them, and bind the arrow listener to the handle's own element in the capture phase so a claimed press is seen before the platform acts on it. Keep the resizable group mounted whether or not the catalog is open, letting only the catalog's panel come and go. Swapping the group in and out put a different element type where the view sits, remounting it and discarding the segment list's scroll position, a gloss typed but not committed, and any open breakdown editor. The group honors defaultLayout only while every panel it names is mounted, so the remembered width is now restored as the panel opens. Model the handle's native listener in the mock, without which neither the double step nor the remount is reachable from a test. --- __mocks__/platform-bible-react.tsx | 69 +++++++- .../components/InterlinearizerLoader.test.tsx | 84 ++++++--- .../hooks/usePanelResizeKeys.test.tsx | 161 ++++++++++-------- src/components/InterlinearizerLoader.tsx | 86 ++++++---- src/hooks/usePanelResizeKeys.ts | 59 ++++--- 5 files changed, 308 insertions(+), 151 deletions(-) diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index 52863cbd..fbb09b21 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -9,6 +9,7 @@ import { createContext, forwardRef, isValidElement, + useCallback, useContext, useEffect, useId, @@ -1051,6 +1052,12 @@ export function TooltipProvider({ /** The layout the enclosing {@link ResizablePanelGroup} currently holds, empty outside any group. */ const PanelLayoutContext = createContext>>({}); +/** + * Moves the enclosing group's panels by a percentage of it, as the real handle's own key presses + * do. A positive step widens the last panel, the one the handle is anchored beside. + */ +const PanelStepContext = createContext<(step: number) => void>(() => {}); + /** A resizable group's handle for reading and moving its panels once it has mounted. */ interface GroupImperativeHandle { /** The layout the panels currently hold. */ @@ -1123,11 +1130,24 @@ export function ResizablePanelGroup({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [JSON.stringify(layout)]); + const step = useCallback((percentage: number) => { + setLayout((current) => { + const ids = Object.keys(current); + if (ids.length < 2) return current; + const first = ids[0]; + const last = ids[ids.length - 1]; + const moved = Math.min(100, Math.max(0, current[last] + percentage)); + return normalizeLayout({ ...current, [first]: 100 - moved, [last]: moved }); + }); + }, []); + return ( -
        - {children} -
        + +
        + {children} +
        +         ); } @@ -1157,24 +1177,61 @@ export function ResizablePanel({ ); } +/** + * How far each key the real handle acts on moves it, as a percentage of the group. The jump keys + * are given more of it than any panel may hold, so they land against a limit rather than part way. + */ +const HANDLE_KEY_STEPS: Readonly> = { + ArrowLeft: -5, + ArrowRight: 5, + Home: -100, + End: 100, +}; + /** * Stub resize handle, focusable and keyboard-driven as the real one is. Dragging it needs pointer * behavior jsdom does not have, so only its keyboard half stands. + * + * Its keys are answered from a listener on the element rather than a React prop, as the real + * handle's are, so that a caller binding its own listener meets the same ordering it would in the + * app — the ordering that decides whether a press it means to claim reaches this one anyway. The + * steps are signed without reference to the interface direction, mirroring nothing, because the + * real handle mirrors nothing either. */ export function ResizableHandle({ - onKeyDown, + elementRef, ...props }: Readonly<{ - onKeyDown?: KeyboardEventHandler; + elementRef?: (element: HTMLElement | null) => void; 'aria-label'?: string; 'data-testid'?: string; withHandle?: boolean; }>): ReactElement { + const onStep = useContext(PanelStepContext); + const onStepRef = useRef(onStep); + onStepRef.current = onStep; + + const attach = useCallback( + (element: HTMLDivElement | null) => { + elementRef?.(element); + if (!element) return; + element.addEventListener('keydown', (event: KeyboardEvent) => { + // The real handle starts by standing down for a press another listener already claimed. + if (event.defaultPrevented) return; + const step = HANDLE_KEY_STEPS[event.key]; + if (step === undefined) return; + event.preventDefault(); + onStepRef.current(step); + }); + }, + [elementRef], + ); + return (
        diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 59c9aa7b..5649e441 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -1846,41 +1846,53 @@ describe('InterlinearizerLoader', () => { }); it('restores a resized catalog panel to its remembered layout on remount', async () => { - const useWebViewState = makeWebViewState(); - await act(async () => { - renderLoader({ useWebViewState }); - }); - await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); + // Resized by the mirrored arrow, that being the press the extension answers itself: dragging + // needs measurement jsdom does not do, and the platform handle owns Home and End. + document.documentElement.dir = 'rtl'; + try { + const useWebViewState = makeWebViewState(); + await act(async () => { + renderLoader({ useWebViewState }); + }); + await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); - // By key rather than by drag, dragging needing measurement jsdom does not do. Home lands - // somewhere the default is not, so a layout read back on remount can only be a stored one. - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'Home' }); + // A step lands somewhere the default is not, so a layout read back on remount can only be + // a stored one. + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowRight' }); - cleanup(); - await act(async () => { - renderLoader({ useWebViewState }); - }); + cleanup(); + await act(async () => { + renderLoader({ useWebViewState }); + }); - expect(catalogPanelElement()).toHaveAttribute('data-panel-layout', '15'); + expect(catalogPanelElement()).toHaveAttribute('data-panel-layout', '30'); + } finally { + document.documentElement.removeAttribute('dir'); + } }); it('moves the catalog panel on the press that resized it, not only on the next mount', async () => { - await act(async () => { - renderLoader(); - }); - await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); + document.documentElement.dir = 'rtl'; + try { + await act(async () => { + renderLoader(); + }); + await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); - fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'Home' }); + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowRight' }); - // The group reads its defaultLayout only as it mounts, so the panel can only have moved by - // the press itself rather than by the layout reaching state. - expect(catalogPanelElement()).toHaveAttribute('data-panel-layout', '15'); + // The group reads its defaultLayout only while every panel it names is mounted, so the + // panel can only have moved by the press itself rather than by the layout reaching state. + expect(catalogPanelElement()).toHaveAttribute('data-panel-layout', '30'); + } finally { + document.documentElement.removeAttribute('dir'); + } }); it('lays the catalog out in the unit the group reports back, so a press steps rather than jumps', async () => { // Arrows resize only in a right-to-left interface, the platform handle already reading them - // correctly in a left-to-right one. Only a step discriminates: Home and End clamp to a bound - // whatever unit the layout is in. + // correctly in a left-to-right one. Only a step discriminates between the units, a jump + // landing against a bound whichever the layout is in. document.documentElement.dir = 'rtl'; try { await act(async () => { @@ -1900,6 +1912,32 @@ describe('InterlinearizerLoader', () => { } }); + it('keeps the interlinear view mounted as the catalog opens', async () => { + // Identity rather than presence: a view that changed place in the tree as the catalog + // appeared would still be found here, having remounted and lost everything it holds locally + // — where the segment list was scrolled to, a gloss typed but not yet committed. + await act(async () => { + renderLoader(); + }); + const viewBeforeOpening = screen.getByTestId('interlinearizer'); + + await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); + + expect(screen.getByTestId('interlinearizer')).toBe(viewBeforeOpening); + }); + + it('keeps the interlinear view mounted as the catalog closes', async () => { + await act(async () => { + renderLoader(); + }); + await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); + const viewBeforeClosing = screen.getByTestId('interlinearizer'); + + await userEvent.click(screen.getByTestId('analysis-catalog-close')); + + expect(screen.getByTestId('interlinearizer')).toBe(viewBeforeClosing); + }); + it('leaves the catalog panel closed on remount when it was never opened', async () => { const useWebViewState = makeWebViewState(); await act(async () => { diff --git a/src/__tests__/hooks/usePanelResizeKeys.test.tsx b/src/__tests__/hooks/usePanelResizeKeys.test.tsx index d98fa791..bb3dc8c8 100644 --- a/src/__tests__/hooks/usePanelResizeKeys.test.tsx +++ b/src/__tests__/hooks/usePanelResizeKeys.test.tsx @@ -6,24 +6,46 @@ import usePanelResizeKeys from '../../hooks/usePanelResizeKeys'; /** Narrowest and widest percentages of the group a press may reach. */ const BOUNDS = { min: 15, max: 50 }; -/** Renders a separator driven by the hook, standing in for the platform resize handle. */ +/** + * Renders a separator driven by the hook, standing in for the platform resize handle. + * + * The handle's own key listener is bound to the element rather than passed as a React prop, as the + * platform's is, so that a press reaches the hook in the order it would in the app. + * + * @returns The rendered handle, and a spy called with the key of every press the handle was left to + * act on itself. + */ function renderHandle(percentage: number, onPercentageChange: (percentage: number) => void) { + const platformSteps = jest.fn(); + function Handle() { - const onKeyDown = usePanelResizeKeys(percentage, onPercentageChange, BOUNDS); - // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/no-noninteractive-tabindex - return
        ; + const ref = usePanelResizeKeys(percentage, onPercentageChange, BOUNDS); + return ( +
        { + ref(element); + // Bound after the hook's, and in the bubble phase, as the platform's listener is: it + // stands down only for a press the hook has already claimed. + element?.addEventListener('keydown', (event) => { + if (event.defaultPrevented) return; + platformSteps(event.key); + }); + }} + role="separator" + // Focusable as the real separator is, which is what puts key presses within its reach. + // eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex + tabIndex={0} + /> + ); } render(); - return screen.getByTestId('handle'); + return { handle: screen.getByTestId('handle'), platformSteps }; } -/** - * Presses `key` on the handle, with `init` supplying any modifiers held. - * - * @returns Whether the press was claimed, leaving the platform handle to ignore it. - */ -function press(handle: HTMLElement, key: string, init: object = {}): boolean { - return !fireEvent.keyDown(handle, { key, ...init }); +/** Presses `key` on the handle, with `init` supplying any modifiers held. */ +function press(handle: HTMLElement, key: string, init: object = {}): void { + fireEvent.keyDown(handle, { key, ...init }); } describe('usePanelResizeKeys', () => { @@ -34,12 +56,12 @@ describe('usePanelResizeKeys', () => { describe('in a left-to-right interface', () => { it('leaves the arrows to the platform handle, which already reads them correctly', () => { const onPercentageChange = jest.fn(); - const handle = renderHandle(25, onPercentageChange); + const { handle, platformSteps } = renderHandle(25, onPercentageChange); - const defaulted = press(handle, 'ArrowLeft'); + press(handle, 'ArrowLeft'); expect(onPercentageChange).not.toHaveBeenCalled(); - expect(defaulted).toBe(false); + expect(platformSteps).toHaveBeenCalledWith('ArrowLeft'); }); }); @@ -50,7 +72,7 @@ describe('usePanelResizeKeys', () => { it('narrows the panel on ArrowLeft, which points away from the edge it is anchored to', () => { const onPercentageChange = jest.fn(); - const handle = renderHandle(25, onPercentageChange); + const { handle } = renderHandle(25, onPercentageChange); press(handle, 'ArrowLeft'); @@ -59,22 +81,24 @@ describe('usePanelResizeKeys', () => { it('widens the panel on ArrowRight', () => { const onPercentageChange = jest.fn(); - const handle = renderHandle(25, onPercentageChange); + const { handle } = renderHandle(25, onPercentageChange); press(handle, 'ArrowRight'); expect(onPercentageChange).toHaveBeenCalledWith(30); }); - it('claims the mirrored arrow, so the platform handle leaves it alone', () => { - const handle = renderHandle(25, () => {}); + it('claims the mirrored arrow, so the platform handle does not step it a second time', () => { + const { handle, platformSteps } = renderHandle(25, () => {}); + + press(handle, 'ArrowRight'); - expect(press(handle, 'ArrowRight')).toBe(true); + expect(platformSteps).not.toHaveBeenCalled(); }); it('holds a widening arrow to the widest the panel may be', () => { const onPercentageChange = jest.fn(); - const handle = renderHandle(48, onPercentageChange); + const { handle } = renderHandle(48, onPercentageChange); press(handle, 'ArrowRight'); @@ -83,7 +107,7 @@ describe('usePanelResizeKeys', () => { it('reports nothing for an arrow held down at the end of the range', () => { const onPercentageChange = jest.fn(); - const handle = renderHandle(50, onPercentageChange); + const { handle } = renderHandle(50, onPercentageChange); press(handle, 'ArrowRight'); @@ -91,75 +115,52 @@ describe('usePanelResizeKeys', () => { }); }); - describe('jumping to an end of the range', () => { - it('sends the panel to its narrowest on Home', () => { + describe('keys the platform handle owns', () => { + it.each(['Home', 'End'])('leaves %s to the platform handle, which jumps to an end', (key) => { const onPercentageChange = jest.fn(); - const handle = renderHandle(25, onPercentageChange); + const { handle, platformSteps } = renderHandle(25, onPercentageChange); - press(handle, 'Home'); - - expect(onPercentageChange).toHaveBeenCalledWith(BOUNDS.min); - }); - - it('sends the panel to its widest on End', () => { - const onPercentageChange = jest.fn(); - const handle = renderHandle(25, onPercentageChange); + press(handle, key); - press(handle, 'End'); - - expect(onPercentageChange).toHaveBeenCalledWith(BOUNDS.max); + expect(onPercentageChange).not.toHaveBeenCalled(); + expect(platformSteps).toHaveBeenCalledWith(key); }); - it('jumps the same way whichever side the interface anchors the panel to', () => { + it('leaves the jump keys to the platform handle in a right-to-left interface too', () => { document.documentElement.dir = 'rtl'; const onPercentageChange = jest.fn(); - const handle = renderHandle(25, onPercentageChange); + const { handle, platformSteps } = renderHandle(25, onPercentageChange); press(handle, 'Home'); - expect(onPercentageChange).toHaveBeenCalledWith(BOUNDS.min); - }); - - it('reports nothing on End when the panel is already at its widest', () => { - const onPercentageChange = jest.fn(); - const handle = renderHandle(BOUNDS.max, onPercentageChange); - - press(handle, 'End'); - expect(onPercentageChange).not.toHaveBeenCalled(); + expect(platformSteps).toHaveBeenCalledWith('Home'); }); }); describe('keys it does not act on', () => { it('leaves the panel alone on a key that resizes nothing', () => { const onPercentageChange = jest.fn(); - const handle = renderHandle(25, onPercentageChange); + const { handle } = renderHandle(25, onPercentageChange); press(handle, 'a'); expect(onPercentageChange).not.toHaveBeenCalled(); }); - it('leaves a modified jump key for the host to act on', () => { - const onPercentageChange = jest.fn(); - const handle = renderHandle(25, onPercentageChange); - - // Ctrl+Home is a document-level shortcut in some hosts, which swallowing it would break. - const defaulted = press(handle, 'Home', { ctrlKey: true }); - - expect(onPercentageChange).not.toHaveBeenCalled(); - expect(defaulted).toBe(false); - }); - - it.each(['metaKey', 'altKey'])('leaves a %s-modified arrow alone', (modifier) => { - document.documentElement.dir = 'rtl'; - const onPercentageChange = jest.fn(); - const handle = renderHandle(25, onPercentageChange); + it.each(['metaKey', 'altKey', 'ctrlKey'])( + 'leaves a %s-modified arrow for the host to act on', + (modifier) => { + document.documentElement.dir = 'rtl'; + const onPercentageChange = jest.fn(); + const { handle, platformSteps } = renderHandle(25, onPercentageChange); - press(handle, 'ArrowRight', { [modifier]: true }); + press(handle, 'ArrowRight', { [modifier]: true }); - expect(onPercentageChange).not.toHaveBeenCalled(); - }); + expect(onPercentageChange).not.toHaveBeenCalled(); + expect(platformSteps).toHaveBeenCalledWith('ArrowRight'); + }, + ); }); it('resizes from the percentage it is given rather than one it remembers', () => { @@ -169,9 +170,9 @@ describe('usePanelResizeKeys', () => { const onPercentageChange = jest.fn(); function Handle({ percentage }: Readonly<{ percentage: number }>) { - const onKeyDown = usePanelResizeKeys(percentage, onPercentageChange, BOUNDS); - // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/no-noninteractive-tabindex - return
        ; + const ref = usePanelResizeKeys(percentage, onPercentageChange, BOUNDS); + // eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex + return
        ; } const { rerender } = render(); rerender(); @@ -180,4 +181,26 @@ describe('usePanelResizeKeys', () => { expect(onPercentageChange).toHaveBeenCalledWith(45); }); + + it('stops resizing once the handle it was on has gone', () => { + // The catalog's handle is unmounted when the panel closes, and a listener left bound to it + // would keep answering presses for a panel that is no longer there. + document.documentElement.dir = 'rtl'; + const onPercentageChange = jest.fn(); + + function Handle({ present }: Readonly<{ present: boolean }>) { + const ref = usePanelResizeKeys(25, onPercentageChange, BOUNDS); + return present ? ( + // eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex +
        + ) : undefined; + } + const { rerender } = render(); + const handle = screen.getByTestId('handle'); + rerender(); + + press(handle, 'ArrowRight'); + + expect(onPercentageChange).not.toHaveBeenCalled(); + }); }); diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index b970e3a6..2b4f5c03 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -593,7 +593,18 @@ function InterlinearizerLoaderInner({ [setCatalogLayout], ); - const handleCatalogResizeKeyDown = usePanelResizeKeys( + /** + * Restores the width the catalog was last left at as it opens. The group outlives the panel and + * honors `defaultLayout` only while every panel it names is mounted, so the layout held for a + * closed catalog is not one the group will have applied by itself. + */ + const catalogLayoutRef = useRef(catalogLayout); + catalogLayoutRef.current = catalogLayout; + useEffect(() => { + if (catalogOpen) catalogGroupRef.current?.setLayout(catalogLayoutRef.current); + }, [catalogOpen]); + + const catalogResizeRef = usePanelResizeKeys( /* v8 ignore next -- every layout names both panels, the default's and each write's alike */ catalogLayout[CATALOG_PANEL_ID] ?? DEFAULT_CATALOG_LAYOUT[CATALOG_PANEL_ID], handleCatalogPercentageChange, @@ -773,40 +784,47 @@ function InterlinearizerLoaderInner({ onPendingEditsChange={setPendingEdits} showSuggestions={showSuggestions} > - {catalogOpen ? ( - - - {bookArea} - - - - + + {bookArea} + + {catalogOpen && ( + <> + - - - ) : ( - {bookArea} - )} + + + + + )} + )}
        diff --git a/src/hooks/usePanelResizeKeys.ts b/src/hooks/usePanelResizeKeys.ts index bb61956b..251c9508 100644 --- a/src/hooks/usePanelResizeKeys.ts +++ b/src/hooks/usePanelResizeKeys.ts @@ -1,6 +1,5 @@ import { readDirection } from 'platform-bible-react/experimental'; -import { useCallback } from 'react'; -import type { KeyboardEvent as ReactKeyboardEvent } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; /** * How far one arrow-key press resizes the panel, as a percentage of the group. Matches the step the @@ -25,9 +24,14 @@ function keyTravel(key: string): number { } /** - * Resizes a panel by key press: mirrored arrows for a right-to-left interface, and Home and End to - * either end of the range. Handles only what the platform resize handle leaves undone, and yields - * every other key to it, so the two together answer a full set. + * Resizes a panel by arrow key in a right-to-left interface, where the platform handle would + * otherwise move it the wrong way: the handle steps by a signed amount that never consults the + * interface direction, so an arrow pointing at the panel's own edge widens it instead of narrowing + * it. Every other key, and every arrow in a left-to-right interface, is left to the handle. + * + * Returns a ref rather than a handler because the platform binds its own key handler to the + * handle's element directly, and only a listener on that element in the capture phase runs early + * enough to claim a press before it. A press claimed too late is stepped twice, once by each. * * Sizes are percentages of the group the panel is laid out in, `25` being a quarter of it, matching * the unit a platform group lays out in and hands back. @@ -36,40 +40,57 @@ function keyTravel(key: string): number { * @param onPercentageChange - Records a percentage a press asked for. Not called for a press that * would leave the panel where it already is. * @param bounds - Narrowest and widest percentages a press may reach. - * @returns A `keydown` handler for the resize handle. + * @returns A ref for the resize handle's element. */ export default function usePanelResizeKeys( percentage: number, onPercentageChange: (percentage: number) => void, bounds: { min: number; max: number }, -): (event: ReactKeyboardEvent) => void { +): (element: HTMLElement | null) => void { const { min, max } = bounds; - return useCallback( - (event: ReactKeyboardEvent) => { - // The keys below are recognized by name alone, so a modified press — Alt+Arrow, which some + const handleKeyDown = useCallback( + (event: KeyboardEvent) => { + // The arrows below are recognized by name alone, so a modified press — Alt+Arrow, which some // hosts navigate back on — would both resize the panel and swallow the host's shortcut. if (event.ctrlKey || event.metaKey || event.altKey) return; const travel = keyTravel(event.key); // Left alone in a left-to-right interface, where the platform handle already reads these // arrows the way the panel is pointing; stepping here as well would move it twice. - const mirrors = travel !== 0 && widenTravel() === 1; - const jumpTarget = - // eslint-disable-next-line no-nested-ternary - event.key === 'Home' ? min : event.key === 'End' ? max : undefined; - if (!mirrors && jumpTarget === undefined) return; + if (travel === 0 || widenTravel() !== 1) return; - // Claims the press, which the platform handle honors by leaving a defaulted event alone. + // Claims the press before the platform's own handler sees it, that handler starting by + // returning on an event already defaulted. event.preventDefault(); - const next = - jumpTarget ?? - Math.min(max, Math.max(min, percentage + travel * widenTravel() * KEYBOARD_RESIZE_STEP)); + const next = Math.min( + max, + Math.max(min, percentage + travel * widenTravel() * KEYBOARD_RESIZE_STEP), + ); // An arrow held down at an end of the range repeats, and each repeat would otherwise put an // unchanged layout through the store. if (next !== percentage) onPercentageChange(next); }, [percentage, onPercentageChange, min, max], ); + + // Read through a ref so a press runs the current handler without the listener being rebound for + // every resize, which would rebind it under a held-down arrow. + const handlerRef = useRef(handleKeyDown); + handlerRef.current = handleKeyDown; + + // Held in state rather than a ref so that attaching the listener re-runs once the handle mounts; + // a ref filled in during commit would change without rendering, leaving the effect never re-run. + // eslint-disable-next-line no-null/no-null + const [element, setElement] = useState(null); + + useEffect(() => { + if (!element) return undefined; + const listener = (event: KeyboardEvent) => handlerRef.current(event); + element.addEventListener('keydown', listener, true); + return () => element.removeEventListener('keydown', listener, true); + }, [element]); + + return setElement; } From a056da50bb1b0995fec137ff7cf850df1160ff4c Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 16:36:59 -0600 Subject: [PATCH 27/29] Keep the catalog width when it is closed and reopened --- __mocks__/platform-bible-react.tsx | 52 ++++++++++++++++--- .../components/InterlinearizerLoader.test.tsx | 21 ++++++++ src/components/InterlinearizerLoader.tsx | 16 +++++- 3 files changed, 81 insertions(+), 8 deletions(-) diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index fbb09b21..5a9e943f 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -1058,6 +1058,12 @@ const PanelLayoutContext = createContext>>({}); */ const PanelStepContext = createContext<(step: number) => void>(() => {}); +/** + * Registers a panel with the enclosing group for as long as it is mounted, returning its removal. + * A group reports a layout over the panels mounted at the time, so it has to know which those are. + */ +const PanelRegistryContext = createContext<(id: string) => () => void>(() => () => {}); + /** A resizable group's handle for reading and moving its panels once it has mounted. */ interface GroupImperativeHandle { /** The layout the panels currently hold. */ @@ -1086,7 +1092,9 @@ function normalizeLayout( * mount goes through the handle on `groupRef`. * * The layout is normalized and reported back through `onLayoutChanged`, as the real group does, so - * a caller storing what it is handed stores it in the unit the app would give it. + * a caller storing what it is handed stores it in the unit the app would give it. What is reported + * covers only the panels mounted at the time, as the real group's does, so unmounting one has the + * rest reported holding the whole group between them. */ export function ResizablePanelGroup({ children, @@ -1104,6 +1112,29 @@ export function ResizablePanelGroup({ }>): ReactElement { const [layout, setLayout] = useState(() => normalizeLayout(defaultLayout ?? {})); + // Held in state rather than a ref so registering or unregistering a panel re-renders the group, + // that render being what reports the layout afresh. + const [mountedIds, setMountedIds] = useState([]); + + const registerPanel = useCallback((id: string) => { + setMountedIds((current) => [...current, id]); + return () => { + setMountedIds((current) => { + const index = current.indexOf(id); + if (index < 0) return current; + return [...current.slice(0, index), ...current.slice(index + 1)]; + }); + }; + }, []); + + // Before any panel registers there is nothing to report a layout over, so the seeded one stands. + const reportedLayout = useMemo(() => { + if (mountedIds.length === 0) return layout; + return normalizeLayout( + Object.fromEntries(mountedIds.map((id) => [id, layout[id] ?? 0])), + ); + }, [layout, mountedIds]); + // Read through a ref so the handle can be installed once rather than replaced on every resize. const layoutRef = useRef(layout); layoutRef.current = layout; @@ -1125,10 +1156,10 @@ export function ResizablePanelGroup({ }, [groupRef]); useEffect(() => { - onLayoutChanged?.(layout); + onLayoutChanged?.(reportedLayout); // Keyed on the layout's contents, a fresh object each render otherwise reporting every render. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [JSON.stringify(layout)]); + }, [JSON.stringify(reportedLayout)]); const step = useCallback((percentage: number) => { setLayout((current) => { @@ -1144,9 +1175,11 @@ export function ResizablePanelGroup({ return ( -
        - {children} -
        + +
        + {children} +
        +         ); @@ -1165,6 +1198,13 @@ export function ResizablePanel({ maxSize?: string | number; }>): ReactElement { const layout = useContext(PanelLayoutContext); + + const registerPanel = useContext(PanelRegistryContext); + useEffect(() => { + if (id === undefined) return undefined; + return registerPanel(id); + }, [id, registerPanel]); + return (
        { } }); + it('restores a resized catalog panel to its remembered layout on reopening', async () => { + // Closing unmounts the catalog's panel while its group stays mounted, and a group reports a + // layout over the panels it still has — a report that, stored, would lose the resize. + document.documentElement.dir = 'rtl'; + try { + await act(async () => { + renderLoader(); + }); + await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'ArrowRight' }); + + await userEvent.click(screen.getByTestId('analysis-catalog-close')); + await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); + + expect(catalogPanelElement()).toHaveAttribute('data-panel-layout', '30'); + } finally { + document.documentElement.removeAttribute('dir'); + } + }); + it('moves the catalog panel on the press that resized it, not only on the next mount', async () => { document.documentElement.dir = 'rtl'; try { diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 2b4f5c03..dafae3b7 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -573,6 +573,18 @@ function InterlinearizerLoaderInner({ /** Dismisses the analysis catalog panel. */ const handleCatalogClose = useCallback(() => setCatalogOpen(false), [setCatalogOpen]); + /** + * Records a layout the group reports, keeping the stored one naming both panels. A group reports + * a layout over the panels mounted at the time, so a closed catalog is reported absent rather + * than at the width it was left at, and storing that would lose the width for the reopening. + */ + const handleCatalogLayoutChanged = useCallback( + (layout: PanelLayout) => { + if (VIEW_PANEL_ID in layout && CATALOG_PANEL_ID in layout) setCatalogLayout(layout); + }, + [setCatalogLayout], + ); + /** * Moves the catalog group's panels, the group reading its `defaultLayout` only as it mounts and * so staying where it is for any later layout written to state alone. @@ -605,7 +617,7 @@ function InterlinearizerLoaderInner({ }, [catalogOpen]); const catalogResizeRef = usePanelResizeKeys( - /* v8 ignore next -- every layout names both panels, the default's and each write's alike */ + /* v8 ignore next -- every stored layout names the catalog, the default included */ catalogLayout[CATALOG_PANEL_ID] ?? DEFAULT_CATALOG_LAYOUT[CATALOG_PANEL_ID], handleCatalogPercentageChange, CATALOG_PERCENTAGE_BOUNDS, @@ -795,7 +807,7 @@ function InterlinearizerLoaderInner({ className="tw:flex tw:flex-1 tw:min-h-0" defaultLayout={catalogLayout} groupRef={catalogGroupRef} - onLayoutChanged={setCatalogLayout} + onLayoutChanged={handleCatalogLayoutChanged} orientation="horizontal" > From aa8a59946bed8852fdf9be11015193f99df1950c Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 16:58:12 -0600 Subject: [PATCH 28/29] Mirror Home and End in a right-to-left interface The platform handle reads Home and End as narrowest and widest without consulting the interface direction, so they landed against the bound opposite the arrows once those were mirrored. Give each key its own travel and step, letting the jump keys ride the arrow path with a step farther than the widest panel, which the existing clamp lands on a bound. Left-to-right is untouched: the handle still owns every key there. --- .../hooks/usePanelResizeKeys.test.tsx | 48 ++++++++++++++----- src/hooks/usePanelResizeKeys.ts | 42 +++++++++------- 2 files changed, 61 insertions(+), 29 deletions(-) diff --git a/src/__tests__/hooks/usePanelResizeKeys.test.tsx b/src/__tests__/hooks/usePanelResizeKeys.test.tsx index bb3dc8c8..0e6b7efc 100644 --- a/src/__tests__/hooks/usePanelResizeKeys.test.tsx +++ b/src/__tests__/hooks/usePanelResizeKeys.test.tsx @@ -96,6 +96,41 @@ describe('usePanelResizeKeys', () => { expect(platformSteps).not.toHaveBeenCalled(); }); + it('narrows the panel fully on Home, landing where ArrowLeft points', () => { + const onPercentageChange = jest.fn(); + const { handle } = renderHandle(25, onPercentageChange); + + press(handle, 'Home'); + + expect(onPercentageChange).toHaveBeenCalledWith(BOUNDS.min); + }); + + it('widens the panel fully on End, landing where ArrowRight points', () => { + const onPercentageChange = jest.fn(); + const { handle } = renderHandle(25, onPercentageChange); + + press(handle, 'End'); + + expect(onPercentageChange).toHaveBeenCalledWith(BOUNDS.max); + }); + + it('claims the mirrored jump key, so the platform handle does not step it a second time', () => { + const { handle, platformSteps } = renderHandle(25, () => {}); + + press(handle, 'Home'); + + expect(platformSteps).not.toHaveBeenCalled(); + }); + + it('reports nothing for a jump key pressed at the bound it lands on', () => { + const onPercentageChange = jest.fn(); + const { handle } = renderHandle(BOUNDS.max, onPercentageChange); + + press(handle, 'End'); + + expect(onPercentageChange).not.toHaveBeenCalled(); + }); + it('holds a widening arrow to the widest the panel may be', () => { const onPercentageChange = jest.fn(); const { handle } = renderHandle(48, onPercentageChange); @@ -115,7 +150,7 @@ describe('usePanelResizeKeys', () => { }); }); - describe('keys the platform handle owns', () => { + describe('jump keys in a left-to-right interface', () => { it.each(['Home', 'End'])('leaves %s to the platform handle, which jumps to an end', (key) => { const onPercentageChange = jest.fn(); const { handle, platformSteps } = renderHandle(25, onPercentageChange); @@ -125,17 +160,6 @@ describe('usePanelResizeKeys', () => { expect(onPercentageChange).not.toHaveBeenCalled(); expect(platformSteps).toHaveBeenCalledWith(key); }); - - it('leaves the jump keys to the platform handle in a right-to-left interface too', () => { - document.documentElement.dir = 'rtl'; - const onPercentageChange = jest.fn(); - const { handle, platformSteps } = renderHandle(25, onPercentageChange); - - press(handle, 'Home'); - - expect(onPercentageChange).not.toHaveBeenCalled(); - expect(platformSteps).toHaveBeenCalledWith('Home'); - }); }); describe('keys it does not act on', () => { diff --git a/src/hooks/usePanelResizeKeys.ts b/src/hooks/usePanelResizeKeys.ts index 251c9508..c1194f55 100644 --- a/src/hooks/usePanelResizeKeys.ts +++ b/src/hooks/usePanelResizeKeys.ts @@ -7,6 +7,12 @@ import { useCallback, useEffect, useRef, useState } from 'react'; */ const KEYBOARD_RESIZE_STEP = 5; +/** + * How far one jump-key press resizes the panel. Farther than the widest panel, so a press lands + * against whichever bound it points at rather than part way to it. + */ +const KEYBOARD_JUMP_STEP = 100; + /** * Which way along the screen the handle travels to widen the panel: `-1` toward the screen's left, * `1` toward its right. Read afresh on each press, so a panel that outlives a change of interface @@ -17,17 +23,22 @@ function widenTravel(): number { } /** Which way along the screen a key moves the handle, `0` for a key that moves it nowhere. */ -function keyTravel(key: string): number { - if (key === 'ArrowLeft') return -1; - if (key === 'ArrowRight') return 1; - return 0; +function keyTravel(key: string): { travel: number; step: number } { + if (key === 'ArrowLeft') return { travel: -1, step: KEYBOARD_RESIZE_STEP }; + if (key === 'ArrowRight') return { travel: 1, step: KEYBOARD_RESIZE_STEP }; + // Jump keys travel the way the arrow beside them points: `Home` toward the screen's left, `End` + // toward its right. + if (key === 'Home') return { travel: -1, step: KEYBOARD_JUMP_STEP }; + if (key === 'End') return { travel: 1, step: KEYBOARD_JUMP_STEP }; + return { travel: 0, step: 0 }; } /** - * Resizes a panel by arrow key in a right-to-left interface, where the platform handle would - * otherwise move it the wrong way: the handle steps by a signed amount that never consults the - * interface direction, so an arrow pointing at the panel's own edge widens it instead of narrowing - * it. Every other key, and every arrow in a left-to-right interface, is left to the handle. + * Resizes a panel by arrow or jump key in a right-to-left interface, where the platform handle + * would otherwise move it the wrong way: the handle steps by a signed amount that never consults + * the interface direction, so an arrow pointing at the panel's own edge widens it instead of + * narrowing it, and `Home`/`End` land against the bound opposite the arrow beside them. Every other + * key, and every key in a left-to-right interface, is left to the handle. * * Returns a ref rather than a handler because the platform binds its own key handler to the * handle's element directly, and only a listener on that element in the capture phase runs early @@ -51,25 +62,22 @@ export default function usePanelResizeKeys( const handleKeyDown = useCallback( (event: KeyboardEvent) => { - // The arrows below are recognized by name alone, so a modified press — Alt+Arrow, which some + // The keys below are recognized by name alone, so a modified press — Alt+Arrow, which some // hosts navigate back on — would both resize the panel and swallow the host's shortcut. if (event.ctrlKey || event.metaKey || event.altKey) return; - const travel = keyTravel(event.key); + const { travel, step } = keyTravel(event.key); // Left alone in a left-to-right interface, where the platform handle already reads these - // arrows the way the panel is pointing; stepping here as well would move it twice. + // keys the way the panel is pointing; stepping here as well would move it twice. if (travel === 0 || widenTravel() !== 1) return; // Claims the press before the platform's own handler sees it, that handler starting by // returning on an event already defaulted. event.preventDefault(); - const next = Math.min( - max, - Math.max(min, percentage + travel * widenTravel() * KEYBOARD_RESIZE_STEP), - ); - // An arrow held down at an end of the range repeats, and each repeat would otherwise put an - // unchanged layout through the store. + const next = Math.min(max, Math.max(min, percentage + travel * widenTravel() * step)); + // A key pressed at the end of the range it moves toward — an arrow held down there repeating, + // or a jump key aimed at it — would otherwise put an unchanged layout through the store. if (next !== percentage) onPercentageChange(next); }, [percentage, onPercentageChange, min, max], From 5fbc1c1ba4d608d239853e093f4c0abba31be279 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 17:17:34 -0600 Subject: [PATCH 29/29] Store the catalog width the panel group settled on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A keyboard resize asked the group for a percentage and then wrote that same percentage to state itself. The group holds the catalog within MIN_CATALOG_WIDTH and MAX_CATALOG_WIDTH, both in pixels, so a press aimed past either limit was clamped on the way in and the percentage stored was one the catalog never took. The stored layout then seeded the next mount and the next press, which stepped from a width the panel had never had and appeared to do nothing until it caught up. Drop the second write and leave the recording to the group's layout report, which carries the width it settled on. The mock group reported that layout from an effect, after the render, so the settled width always landed after the extension's own write and silently corrected it — the reason 1950 passing tests missed this. Report from within setLayout, as the real group does, and model the pixel limits the report reflects: panels register their limits, which resolve against a fixed width since jsdom measures every element at zero. --- __mocks__/platform-bible-react.tsx | 171 +++++++++++++++--- .../components/InterlinearizerLoader.test.tsx | 29 +++ src/components/InterlinearizerLoader.tsx | 18 +- 3 files changed, 182 insertions(+), 36 deletions(-) diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index 5a9e943f..a9da2264 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -1058,11 +1058,43 @@ const PanelLayoutContext = createContext>>({}); */ const PanelStepContext = createContext<(step: number) => void>(() => {}); +/** The limits a panel is held within, in the CSS units the real panel takes them in. */ +interface PanelConstraints { + /** Narrowest the panel may be, unlimited when absent. */ + minSize?: string | number; + /** Widest the panel may be, unlimited when absent. */ + maxSize?: string | number; +} + +/** + * Registers a panel and its limits with the enclosing group for as long as it is mounted, returning + * its removal. A group reports a layout over the panels mounted at the time and holds them within + * their limits, so it has to know both. + */ +const PanelRegistryContext = createContext<(id: string, constraints: PanelConstraints) => () => void>( + () => () => {}, +); + /** - * Registers a panel with the enclosing group for as long as it is mounted, returning its removal. - * A group reports a layout over the panels mounted at the time, so it has to know which those are. + * How wide a group is taken to be when resolving a panel's pixel limits against it. jsdom lays + * nothing out, so every element measures zero and a real measurement would leave every pixel limit + * unenforceable; a fixed width gives the limits something to bite on. */ -const PanelRegistryContext = createContext<(id: string) => () => void>(() => () => {}); +const GROUP_WIDTH = 1000; + +/** + * Converts one of a panel's limits to a percentage of the group, the unit a layout is held in. + * + * @returns The limit as a percentage, or nothing for a limit that is absent or in a unit jsdom + * cannot resolve — either way one to leave unenforced. + */ +function limitAsPercentage(limit: string | number | undefined): number | undefined { + if (limit === undefined) return undefined; + if (typeof limit === 'number') return limit; + const pixels = /^(\d+(?:\.\d+)?)px$/.exec(limit); + if (!pixels) return undefined; + return (Number(pixels[1]) / GROUP_WIDTH) * 100; +} /** A resizable group's handle for reading and moving its panels once it has mounted. */ interface GroupImperativeHandle { @@ -1083,6 +1115,69 @@ function normalizeLayout( ); } +/** + * Narrows a layout to the panels currently mounted, the shape the real group reports one in. + * + * @returns The mounted panels' shares, normalized so they hold the whole group between them. The + * layout as it came when no panel has registered, there being nothing to report it over. + */ +function layoutOverMounted( + layout: Readonly>, + mountedIds: readonly string[], +): Readonly> { + if (mountedIds.length === 0) return layout; + return normalizeLayout(Object.fromEntries(mountedIds.map((id) => [id, layout[id] ?? 0]))); +} + +/** Whether two layouts name the same panels at the same sizes. */ +function shallowEqualLayout( + a: Readonly>, + b: Readonly>, +): boolean { + const ids = Object.keys(a); + if (ids.length !== Object.keys(b).length) return false; + return ids.every((id) => a[id] === b[id]); +} + +/** + * Holds a layout within its panels' limits, as the real group does to whatever it is handed. + * + * @returns A layout no panel exceeds its limits in, still normalized: width taken off a panel at a + * limit is passed to one with room for it. Panels whose limits jsdom cannot resolve are left as + * they came. + */ +function clampLayout( + layout: Readonly>, + constraints: ReadonlyMap, +): Readonly> { + const clamped = { ...layout }; + let spare = 0; + Object.keys(clamped).forEach((id) => { + const limits = constraints.get(id); + if (!limits) return; + const min = limitAsPercentage(limits.minSize) ?? 0; + const max = limitAsPercentage(limits.maxSize) ?? 100; + const held = Math.min(max, Math.max(min, clamped[id])); + spare += clamped[id] - held; + clamped[id] = held; + }); + + // Whatever clamping freed goes to the first panel that can take it, the others having been held + // at a limit precisely so they would not grow. + if (spare !== 0) { + const taker = Object.keys(clamped).find((id) => { + const limits = constraints.get(id); + if (!limits) return true; + const min = limitAsPercentage(limits.minSize) ?? 0; + const max = limitAsPercentage(limits.maxSize) ?? 100; + return clamped[id] + spare >= min && clamped[id] + spare <= max; + }); + if (taker !== undefined) clamped[taker] += spare; + } + + return clamped; +} + /** * Stub resizable group, rendering its panels in order under the layout it holds. Real layout needs * measurement jsdom does not do, so the layout is published rather than applied. @@ -1091,10 +1186,13 @@ function normalizeLayout( * caller that resizes by writing that prop alone moves nothing here either. Moving the panels after * mount goes through the handle on `groupRef`. * - * The layout is normalized and reported back through `onLayoutChanged`, as the real group does, so - * a caller storing what it is handed stores it in the unit the app would give it. What is reported - * covers only the panels mounted at the time, as the real group's does, so unmounting one has the - * rest reported holding the whole group between them. + * The layout is normalized, held within its panels' `minSize`/`maxSize`, and reported back through + * `onLayoutChanged`, as the real group does, so a caller storing what it is handed stores the + * layout the group settled on rather than the one it asked for. What is reported covers only the + * panels mounted at the time, as the real group's does, so unmounting one has the rest reported + * holding the whole group between them. + * + * Pixel limits are resolved against {@link GROUP_WIDTH} rather than a measurement. */ export function ResizablePanelGroup({ children, @@ -1116,9 +1214,15 @@ export function ResizablePanelGroup({ // that render being what reports the layout afresh. const [mountedIds, setMountedIds] = useState([]); - const registerPanel = useCallback((id: string) => { + // A ref rather than state because the limits are read while resizing rather than rendered, and a + // panel registering during commit would otherwise need a further render to be clamped against. + const constraintsRef = useRef(new Map()); + + const registerPanel = useCallback((id: string, constraints: PanelConstraints) => { + constraintsRef.current.set(id, constraints); setMountedIds((current) => [...current, id]); return () => { + constraintsRef.current.delete(id); setMountedIds((current) => { const index = current.indexOf(id); if (index < 0) return current; @@ -1127,17 +1231,25 @@ export function ResizablePanelGroup({ }; }, []); - // Before any panel registers there is nothing to report a layout over, so the seeded one stands. - const reportedLayout = useMemo(() => { - if (mountedIds.length === 0) return layout; - return normalizeLayout( - Object.fromEntries(mountedIds.map((id) => [id, layout[id] ?? 0])), - ); - }, [layout, mountedIds]); + const reportedLayout = useMemo(() => layoutOverMounted(layout, mountedIds), [layout, mountedIds]); - // Read through a ref so the handle can be installed once rather than replaced on every resize. + // Read through refs so the handle can be installed once rather than replaced on every resize. const layoutRef = useRef(layout); layoutRef.current = layout; + const mountedIdsRef = useRef(mountedIds); + mountedIdsRef.current = mountedIds; + + // The layout last handed to `onLayoutChanged`, so that the same one is not reported twice. Held + // through a resize and the render it causes, hence a ref rather than state. + const reportedRef = useRef> | undefined>(undefined); + const onLayoutChangedRef = useRef(onLayoutChanged); + onLayoutChangedRef.current = onLayoutChanged; + + const report = useCallback((next: Readonly>) => { + if (reportedRef.current && shallowEqualLayout(reportedRef.current, next)) return; + reportedRef.current = next; + onLayoutChangedRef.current?.(next); + }, []); useLayoutEffect(() => { if (!groupRef) return undefined; @@ -1145,21 +1257,23 @@ export function ResizablePanelGroup({ handle.current = { getLayout: () => layoutRef.current, setLayout: (next) => { - const normalized = normalizeLayout(next); - setLayout(normalized); - return normalized; + const settled = clampLayout(normalizeLayout(next), constraintsRef.current); + setLayout(settled); + // Reported within the call rather than from an effect, as the real group reports it, so a + // caller that writes its own layout after calling this overwrites the settled one rather + // than being corrected by it afterwards. + report(layoutOverMounted(settled, mountedIdsRef.current)); + return settled; }, }; return () => { handle.current = null; }; - }, [groupRef]); + }, [groupRef, report]); useEffect(() => { - onLayoutChanged?.(reportedLayout); - // Keyed on the layout's contents, a fresh object each render otherwise reporting every render. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [JSON.stringify(reportedLayout)]); + report(reportedLayout); + }, [report, reportedLayout]); const step = useCallback((percentage: number) => { setLayout((current) => { @@ -1168,7 +1282,10 @@ export function ResizablePanelGroup({ const first = ids[0]; const last = ids[ids.length - 1]; const moved = Math.min(100, Math.max(0, current[last] + percentage)); - return normalizeLayout({ ...current, [first]: 100 - moved, [last]: moved }); + return clampLayout( + normalizeLayout({ ...current, [first]: 100 - moved, [last]: moved }), + constraintsRef.current, + ); }); }, []); @@ -1202,8 +1319,8 @@ export function ResizablePanel({ const registerPanel = useContext(PanelRegistryContext); useEffect(() => { if (id === undefined) return undefined; - return registerPanel(id); - }, [id, registerPanel]); + return registerPanel(id, { minSize, maxSize }); + }, [id, maxSize, minSize, registerPanel]); return (
        { } }); + it('stores the width a bounded press settled on rather than the one it asked for', async () => { + // Home aims the catalog narrower than its pixel floor allows, so the group holds it at that + // floor instead. Arrows and jumps resize only in a right-to-left interface, the platform + // handle already reading them correctly in a left-to-right one. + document.documentElement.dir = 'rtl'; + try { + const useWebViewState = makeWebViewState(); + await act(async () => { + renderLoader({ useWebViewState }); + }); + await userEvent.click(screen.getByTestId('tab-toolbar-analysis-catalog')); + + fireEvent.keyDown(screen.getByTestId('analysis-catalog-resize'), { key: 'Home' }); + + // Read back on a remount, which lays the group out from what was stored, so the assertion + // covers the stored layout rather than only the one on screen. + cleanup(); + await act(async () => { + renderLoader({ useWebViewState }); + }); + + // The catalog's narrowest width as a percentage of the width the mock group resolves pixel + // limits against, rather than the narrower one the press aimed at. + expect(catalogPanelElement()).toHaveAttribute('data-panel-layout', '22'); + } finally { + document.documentElement.removeAttribute('dir'); + } + }); + it('keeps the interlinear view mounted as the catalog opens', async () => { // Identity rather than presence: a view that changed place in the tree as the catalog // appeared would still be found here, having remounted and lost everything it holds locally diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index dafae3b7..b9579d3c 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -594,16 +594,16 @@ function InterlinearizerLoaderInner({ /** * Moves the catalog to a percentage of the group a key press asked it be given, the view taking - * the rest, and records where it was moved to so the next mount opens there. + * the rest. Storing the new width is left to the group's own report of what it settled on: a + * percentage the pixel limits do not allow is clamped on the way in, and storing the percentage + * asked for instead would record a width the catalog never took. */ - const handleCatalogPercentageChange = useCallback( - (percentage: number) => { - const layout = { [VIEW_PANEL_ID]: 100 - percentage, [CATALOG_PANEL_ID]: percentage }; - catalogGroupRef.current?.setLayout(layout); - setCatalogLayout(layout); - }, - [setCatalogLayout], - ); + const handleCatalogPercentageChange = useCallback((percentage: number) => { + catalogGroupRef.current?.setLayout({ + [VIEW_PANEL_ID]: 100 - percentage, + [CATALOG_PANEL_ID]: percentage, + }); + }, []); /** * Restores the width the catalog was last left at as it opens. The group outlives the panel and