diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index f4297e44..1007fc6f 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -361,6 +361,8 @@ export function PopoverAnchor({ * - An Escape keydown anywhere inside the content invokes `onEscapeKeyDown`. * - A sentinel `data-testid="popover-outside"` button invokes `onInteractOutside` on click, * simulating a pointer interaction outside the popover. + * - A sentinel `data-testid="popover-close"` button invokes `onCloseAutoFocus` on click, + * simulating Radix's focus-restoration event fired as the popover closes. * * @param props - Component props. * @param props.children - Panel content. @@ -371,6 +373,8 @@ export function PopoverAnchor({ * in `detail.originalEvent` when the sentinel outside button is clicked, matching the shape of * Radix's `PointerDownOutsideEvent`. * @param props.onOpenAutoFocus - Called once on mount with a plain `Event`. + * @param props.onCloseAutoFocus - Called with a plain `Event` when the sentinel close button is + * clicked, mirroring Radix's close-time focus-restoration event. * @param props.onClick - Click handler forwarded to the div. * @param props.onMouseDown - Mouse-down handler forwarded to the div. * @returns A `
` with the panel content and sentinel controls. @@ -381,6 +385,7 @@ export function PopoverContent({ onEscapeKeyDown, onInteractOutside, onOpenAutoFocus, + onCloseAutoFocus, onClick, onMouseDown, }: Readonly<{ @@ -391,6 +396,7 @@ export function PopoverContent({ onEscapeKeyDown?: (event: KeyboardEvent) => void; onInteractOutside?: (event: CustomEvent) => void; onOpenAutoFocus?: (event: Event) => void; + onCloseAutoFocus?: (event: Event) => void; onClick?: MouseEventHandler; onMouseDown?: MouseEventHandler; }>): ReactElement { @@ -426,6 +432,15 @@ export function PopoverContent({ outside )} + {onCloseAutoFocus && ( + + )}
); } diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index fdf5e937..5d1cbaba 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -27,6 +27,13 @@ "%interlinearizer_viewOption_showMorphology%": "Show morphology", "%interlinearizer_projectSettings_showMorphology%": "Show Morphology", "%interlinearizer_projectSettings_showMorphologyDescription%": "Display morpheme breakdown and per-morpheme glosses beneath each word token", + "%interlinearizer_morphemeEditor_splitLabel%": "Split into morphemes", + "%interlinearizer_morphemeEditor_delete%": "Delete", + "%interlinearizer_morphemeEditor_cancel%": "Cancel", + "%interlinearizer_morphemeEditor_done%": "Done", + "%interlinearizer_morphemeGloss_label%": "Gloss for morpheme {form}", + "%interlinearizer_tokenChip_editMorphemes%": "Edit morpheme breakdown for {token}", + "%interlinearizer_tokenChip_defineMorphemes%": "Define morpheme breakdown for {token}", "%interlinearizer_linkButton_crossSegmentDisabledTooltip%": "Cross-segment phrases are not supported. This link button is outside the current segment.", "%interlinearizer_modal_create_title%": "Create Interlinear Project", diff --git a/src/__tests__/components/MorphemeEditor.test.tsx b/src/__tests__/components/MorphemeEditor.test.tsx index fd38f53d..4c3eff3e 100644 --- a/src/__tests__/components/MorphemeEditor.test.tsx +++ b/src/__tests__/components/MorphemeEditor.test.tsx @@ -2,37 +2,64 @@ /// /// +import { useLocalizedStrings } from '@papi/frontend/react'; import { fireEvent, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import type { ComponentProps } from 'react'; import * as AnalysisStore from '../../components/AnalysisStore'; import { MorphemeBreakdownPopover, MorphemeGlossInput } from '../../components/MorphemeEditor'; jest.mock('../../components/AnalysisStore'); +const LOCALIZED = { + '%interlinearizer_morphemeEditor_splitLabel%': 'Split into morphemes', + '%interlinearizer_morphemeEditor_delete%': 'Delete', + '%interlinearizer_morphemeEditor_cancel%': 'Cancel', + '%interlinearizer_morphemeEditor_done%': 'Done', + '%interlinearizer_morphemeGloss_label%': 'Gloss for morpheme {form}', +}; + +beforeEach(() => { + jest.mocked(useLocalizedStrings).mockReturnValue([LOCALIZED, false]); +}); + +/** + * Renders {@link MorphemeBreakdownPopover} with the two structural props (`surfaceText`, + * `glossInputId`) defaulted so each test only supplies what it asserts on. + * + * @param props - Overrides merged over the defaults; callers pass their own `onSave`/`onClose` + * spies. + * @returns The render result. + */ +function renderPopover(props: Partial> = {}) { + return render( + , + ); +} + describe('MorphemeBreakdownPopover', () => { it('renders with the initial value pre-filled', () => { - render( - , - ); + renderPopover({ initialValue: 'un- believe -able' }); const input = screen.getByRole('textbox'); expect(input).toHaveValue('un- believe -able'); }); it('auto-focuses and selects the input on mount', () => { - render(); + renderPopover({ initialValue: 'word' }); expect(screen.getByRole('textbox')).toHaveFocus(); }); it('calls onSave and onClose when Done button is clicked', async () => { const onSave = jest.fn(); const onClose = jest.fn(); - render( - , - ); + renderPopover({ initialValue: 'un- believe', onSave, onClose, surfaceText: 'unbelieve' }); await userEvent.click(screen.getByRole('button', { name: 'Done' })); expect(onSave).toHaveBeenCalledWith('un- believe'); expect(onClose).toHaveBeenCalledTimes(1); @@ -40,7 +67,7 @@ describe('MorphemeBreakdownPopover', () => { it('calls onSave with the edited value', async () => { const onSave = jest.fn(); - render(); + renderPopover({ initialValue: 'word', onSave, surfaceText: 'word' }); await userEvent.clear(screen.getByRole('textbox')); await userEvent.type(screen.getByRole('textbox'), 'wor -d'); await userEvent.click(screen.getByRole('button', { name: 'Done' })); @@ -50,14 +77,13 @@ describe('MorphemeBreakdownPopover', () => { it('does not save when Done is clicked with unchanged text and an existing breakdown', async () => { const onSave = jest.fn(); const onClose = jest.fn(); - render( - , - ); + renderPopover({ + initialValue: 'un- believe', + onSave, + onClose, + onDelete: jest.fn(), + surfaceText: 'unbelieve', + }); await userEvent.click(screen.getByRole('button', { name: 'Done' })); expect(onSave).not.toHaveBeenCalled(); expect(onClose).toHaveBeenCalledTimes(1); @@ -65,32 +91,30 @@ describe('MorphemeBreakdownPopover', () => { it('saves when Done is clicked with edited text and an existing breakdown', async () => { const onSave = jest.fn(); - render( - , - ); + renderPopover({ + initialValue: 'un- believe', + onSave, + onDelete: jest.fn(), + surfaceText: 'unbelieve', + }); await userEvent.type(screen.getByRole('textbox'), ' -r'); await userEvent.click(screen.getByRole('button', { name: 'Done' })); expect(onSave).toHaveBeenCalledWith('un- believe -r'); }); - it('commits on Enter key', async () => { + it('commits a multi-morpheme breakdown on Enter key', async () => { const onSave = jest.fn(); const onClose = jest.fn(); - render(); + renderPopover({ initialValue: 'te -st', onSave, onClose, surfaceText: 'test' }); await userEvent.keyboard('{Enter}'); - expect(onSave).toHaveBeenCalledWith('test'); + expect(onSave).toHaveBeenCalledWith('te -st'); expect(onClose).toHaveBeenCalledTimes(1); }); it('dismisses without saving on Escape key', async () => { const onSave = jest.fn(); const onClose = jest.fn(); - render(); + renderPopover({ initialValue: 'te -st', onSave, onClose, surfaceText: 'test' }); await userEvent.keyboard('{Escape}'); expect(onSave).not.toHaveBeenCalled(); expect(onClose).toHaveBeenCalledTimes(1); @@ -99,7 +123,7 @@ describe('MorphemeBreakdownPopover', () => { it('dismisses without saving when Cancel button is clicked', async () => { const onSave = jest.fn(); const onClose = jest.fn(); - render(); + renderPopover({ initialValue: 'te -st', onSave, onClose, surfaceText: 'test' }); await userEvent.click(screen.getByRole('button', { name: 'Cancel' })); expect(onSave).not.toHaveBeenCalled(); expect(onClose).toHaveBeenCalledTimes(1); @@ -108,7 +132,7 @@ describe('MorphemeBreakdownPopover', () => { it('closes without saving when interacting outside with unchanged text', async () => { const onSave = jest.fn(); const onClose = jest.fn(); - render(); + renderPopover({ initialValue: 'te -st', onSave, onClose, surfaceText: 'test' }); // The platform-bible-react mock exposes a sentinel button that fires onInteractOutside, // simulating a pointer interaction outside the popover. await userEvent.click(screen.getByTestId('popover-outside')); @@ -118,7 +142,7 @@ describe('MorphemeBreakdownPopover', () => { it('saves on outside interaction when the text was edited', async () => { const onSave = jest.fn(); - render(); + renderPopover({ initialValue: 'test', onSave, surfaceText: 'whole' }); await userEvent.type(screen.getByRole('textbox'), ' -er'); await userEvent.click(screen.getByTestId('popover-outside')); expect(onSave).toHaveBeenCalledWith('test -er'); @@ -126,14 +150,14 @@ describe('MorphemeBreakdownPopover', () => { it('does not save on outside interaction when the input is only whitespace', async () => { const onSave = jest.fn(); - render(); + renderPopover({ initialValue: ' ', onSave }); await userEvent.click(screen.getByTestId('popover-outside')); expect(onSave).not.toHaveBeenCalled(); }); it('does not dismiss when clicking inside the popover panel', async () => { const onClose = jest.fn(); - render(); + renderPopover({ onClose }); const label = screen.getByText('Split into morphemes'); await userEvent.click(label); expect(onClose).not.toHaveBeenCalled(); @@ -145,7 +169,13 @@ describe('MorphemeBreakdownPopover', () => { const ancestorClick = jest.fn(); render(
- +
, ); await userEvent.click(screen.getByText('Split into morphemes')); @@ -158,29 +188,45 @@ describe('MorphemeBreakdownPopover', () => { const ancestorMouseDown = jest.fn(); render(
- +
, ); fireEvent.mouseDown(screen.getByText('Split into morphemes')); expect(ancestorMouseDown).not.toHaveBeenCalled(); }); - it('does not call onSave when the input is empty', async () => { + it('disables Done when the input is only whitespace', () => { + renderPopover({ initialValue: ' ' }); + expect(screen.getByRole('button', { name: 'Done' })).toBeDisabled(); + }); + + it('does not save whitespace on Enter', async () => { const onSave = jest.fn(); - render(); - await userEvent.click(screen.getByRole('button', { name: 'Done' })); + renderPopover({ initialValue: ' ', onSave, surfaceText: 'word' }); + await userEvent.keyboard('{Enter}'); expect(onSave).not.toHaveBeenCalled(); }); - it('does not call onSave when the input is only whitespace', async () => { + it('disables Done for a breakdown that is just the whole word as one morpheme', () => { + renderPopover({ initialValue: 'word', surfaceText: 'word' }); + expect(screen.getByRole('button', { name: 'Done' })).toBeDisabled(); + }); + + it('does not save a breakdown that is just the whole word as one morpheme on Enter', async () => { const onSave = jest.fn(); - render(); - await userEvent.click(screen.getByRole('button', { name: 'Done' })); + renderPopover({ initialValue: 'word', onSave, surfaceText: 'word' }); + await userEvent.keyboard('{Enter}'); expect(onSave).not.toHaveBeenCalled(); }); it('does not render a Delete button when onDelete is not provided', () => { - render(); + renderPopover(); expect(screen.queryByRole('button', { name: 'Delete' })).not.toBeInTheDocument(); }); @@ -188,14 +234,13 @@ describe('MorphemeBreakdownPopover', () => { const onDelete = jest.fn(); const onSave = jest.fn(); const onClose = jest.fn(); - render( - , - ); + renderPopover({ + initialValue: 'un- believe', + onSave, + onClose, + onDelete, + surfaceText: 'unbelieve', + }); await userEvent.click(screen.getByRole('button', { name: 'Delete' })); expect(onDelete).toHaveBeenCalledTimes(1); expect(onClose).toHaveBeenCalledTimes(1); @@ -205,10 +250,47 @@ describe('MorphemeBreakdownPopover', () => { it('renders inside the popover content panel', () => { // Positioning, portaling, and flipping are owned by the platform-bible-react Popover; this // only verifies the editor renders as the popover's content. - render(); + renderPopover(); const content = screen.getByTestId('popover-content'); expect(content).toContainElement(screen.getByText('Split into morphemes')); }); + + it('focuses the first morpheme gloss field of the chip when the popover closes', async () => { + // The chip label holds the morpheme gloss inputs before the token gloss input; on close, focus + // should land on the first morpheme gloss, scoped to this token's label via glossInputId. + render( + , + ); + await userEvent.click(screen.getByTestId('popover-close')); + expect(screen.getByRole('textbox', { name: 'morpheme gloss' })).toHaveFocus(); + }); + + it('falls back to the token gloss input on close when the chip has no morpheme gloss field', async () => { + render( + , + ); + await userEvent.click(screen.getByTestId('popover-close')); + expect(screen.getByRole('textbox', { name: 'token gloss' })).toHaveFocus(); + }); }); describe('MorphemeGlossInput', () => { diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index 6753dbd2..89c431d6 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -2,6 +2,7 @@ /// /// +import { useLocalizedStrings } from '@papi/frontend/react'; import { fireEvent, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { AssignmentStatus, Token, TokenSnapshot } from 'interlinearizer'; @@ -10,6 +11,15 @@ import { AnalysisStoreProvider } from '../../components/AnalysisStore'; import { InertTokenChip, TokenChip } from '../../components/TokenChip'; jest.mock('../../components/AnalysisStore'); + +const LOCALIZED = { + '%interlinearizer_tokenChip_editMorphemes%': 'Edit morpheme breakdown for {token}', + '%interlinearizer_tokenChip_defineMorphemes%': 'Define morpheme breakdown for {token}', +}; + +beforeEach(() => { + jest.mocked(useLocalizedStrings).mockReturnValue([LOCALIZED, false]); +}); jest.mock('../../components/MorphemeEditor', () => ({ /** * Stub popover that renders a save button so tests can trigger onSave. diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index adcc6d05..395ac8fa 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -226,7 +226,7 @@ function InterlinearizerLoaderInner({ value: showMorphology, } = useOptimisticBooleanSetting(projectId, 'interlinearizer.showMorphology', false); - // Bundle the display toggles into one stable object. Memoizing on the four primitive values keeps + // Bundle the display toggles into one stable object. Memoizing on the primitive values keeps // the reference identical across the loader's frequent re-renders (driven by `useData`, // `useSetting`, etc.), so the `memo()` wrapping `SegmentView` can shallow-compare it away instead // of re-rendering every windowed segment when no toggle actually changed. diff --git a/src/components/MorphemeEditor.tsx b/src/components/MorphemeEditor.tsx index 12b79059..82c6bf4f 100644 --- a/src/components/MorphemeEditor.tsx +++ b/src/components/MorphemeEditor.tsx @@ -4,20 +4,32 @@ * morpheme forms; {@link MorphemeGlossInput} provides a per-morpheme gloss field. */ import type { MorphemeAnalysis } from 'interlinearizer'; +import { useLocalizedStrings } from '@papi/frontend/react'; import { PopoverContent } from 'platform-bible-react'; import { useEffect, useId, useRef, useState } from 'react'; import type { KeyboardEvent, MouseEvent } from 'react'; import { useMorphemeGlossDispatch } from './AnalysisStore'; +const POPOVER_STRING_KEYS = [ + '%interlinearizer_morphemeEditor_splitLabel%', + '%interlinearizer_morphemeEditor_delete%', + '%interlinearizer_morphemeEditor_cancel%', + '%interlinearizer_morphemeEditor_done%', +] as const satisfies `%${string}%`[]; + +const MORPHEME_GLOSS_STRING_KEYS = [ + '%interlinearizer_morphemeGloss_label%', +] as const satisfies `%${string}%`[]; + /** * Inline popover for defining or editing a token's morpheme breakdown. The user types * space-separated morpheme forms (e.g. "un- believe -able") and commits with Enter, Done, or by * clicking outside the popover (matching the commit-on-blur behavior of gloss inputs). Cancel and * Escape dismiss without saving. An unedited draft is never re-saved over an existing breakdown — * Enter, Done, and outside clicks all dismiss instead, because re-saving identical forms would only - * rewrite identical data. When no breakdown exists yet, Enter and Done with the unedited pre-filled - * surface text deliberately record the token as a single whole-word morpheme, but an outside click - * dismisses without saving so an accidental click cannot create one. + * rewrite identical data. A breakdown that is empty or just the whole word as a single morpheme + * carries no real segmentation, so it is never saved either: the Done button is disabled for it and + * the Enter / outside-click paths dismiss without writing. * * Renders the content of a `platform-bible-react` `Popover`; the caller owns the `Popover` root and * the `PopoverAnchor` the panel is positioned from, and must render this component only while the @@ -33,6 +45,11 @@ import { useMorphemeGlossDispatch } from './AnalysisStore'; * token's existing morpheme breakdown, then dismisses the popover. Callers should omit it when * the token has no breakdown to delete; its presence is also how the popover knows a breakdown * already exists when deciding whether an unedited commit should save. + * @param props.surfaceText - The token's surface text, used to reject a "breakdown" that is just + * the whole word as a single morpheme (no real segmentation). + * @param props.glossInputId - Id of the token's gloss input; used to locate the chip on close so + * focus lands on its first morpheme gloss field (falling back to the gloss input itself), rather + * than on the non-tabbable morpheme trigger. * @returns A popover panel with a text input and Cancel/Done buttons. */ export function MorphemeBreakdownPopover({ @@ -40,12 +57,17 @@ export function MorphemeBreakdownPopover({ onSave, onClose, onDelete, + surfaceText, + glossInputId, }: Readonly<{ initialValue: string; onSave: (value: string) => void; onClose: () => void; onDelete?: () => void; + surfaceText: string; + glossInputId: string; }>) { + const [localizedStrings] = useLocalizedStrings(POPOVER_STRING_KEYS); const inputId = useId(); const [draft, setDraft] = useState(initialValue); // eslint-disable-next-line no-null/no-null @@ -72,15 +94,25 @@ export function MorphemeBreakdownPopover({ // comparing normalized text avoids a no-op persistence round-trip. const isUnedited = normalize(draft) === normalize(initialValue); + // A breakdown carries no real segmentation when it is empty or is just the whole word as a single + // morpheme equal to the surface text; in both cases there is nothing worth persisting. + const normalized = normalize(draft); + const forms = normalized === '' ? [] : normalized.split(' '); + const isMeaningless = + forms.length === 0 || (forms.length === 1 && forms[0] === normalize(surfaceText)); + /** - * Commits the current draft and closes the popover. Skips the save when the token already has a + * Commits the current draft and closes the popover. Skips the save when the breakdown is + * meaningless (empty, or the whole word as one morpheme), or when the token already has a * breakdown (`onDelete` provided) and the text was not edited — re-saving identical forms would - * only rewrite identical data. An unedited commit on a token with _no_ breakdown is kept: it - * deliberately records the token as a single whole-word morpheme. + * only rewrite identical data. */ const handleSave = () => { - const trimmed = draft.trim(); - if (trimmed && !(onDelete && isUnedited)) onSave(trimmed); + if (isMeaningless || (onDelete && isUnedited)) { + onClose(); + return; + } + onSave(draft.trim()); onClose(); }; @@ -98,10 +130,9 @@ export function MorphemeBreakdownPopover({ /** * Commits the draft when the user interacts outside the popover, except when the text was not - * edited — then the interaction acts like Cancel. Unlike Enter and Done, the unedited check here - * applies even when the token has no breakdown yet ({@link handleSave} would otherwise create a - * single-morpheme breakdown equal to the pre-filled surface text), because an accidental outside - * click is not a deliberate commit. + * edited — then the interaction acts like Cancel, because an accidental outside click is not a + * deliberate commit. An edited-but-meaningless draft is also dismissed without saving by + * {@link handleSave}. */ const handleInteractOutside = () => { if (isUnedited) { @@ -125,18 +156,38 @@ export function MorphemeBreakdownPopover({ e.stopPropagation(); }; + /** + * Overrides Radix's default close-focus behavior to land focus on the chip's first morpheme gloss + * field. The morpheme gloss inputs sit before the token gloss input inside the same label, so the + * lookup is scoped to that label — the panel is portaled to `document.body`, so a document-wide + * query could match another token's field. Falls back to the token gloss input when no morpheme + * field exists (dismissed with no breakdown, or deleted). + * + * @param e - The Radix close auto-focus event. + */ + const handleCloseAutoFocus = (e: Event) => { + e.preventDefault(); + const glossInput = document.getElementById(glossInputId); + const firstMorphemeGloss = glossInput + ?.closest('label') + ?.querySelector('input[data-morpheme-gloss]'); + // `preventScroll` keeps the React-controlled scroll the sole scroller. + (firstMorphemeGloss ?? glossInput)?.focus({ preventScroll: true }); + }; + return ( e.preventDefault()} > - Delete + {localizedStrings['%interlinearizer_morphemeEditor_delete%']} )} @@ -206,6 +258,7 @@ export function MorphemeGlossInput({ const committed = morpheme.gloss?.[analysisLanguage] ?? ''; const dispatchMorphemeGloss = useMorphemeGlossDispatch(); const [draft, setDraft] = useState(committed); + const [localizedStrings] = useLocalizedStrings(MORPHEME_GLOSS_STRING_KEYS); useEffect(() => { setDraft(committed); @@ -213,7 +266,10 @@ export function MorphemeGlossInput({ return ( morpheme.form, + )} className="tw:gloss-input tw:text-xs" data-morpheme-gloss="true" disabled={disabled} diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index a22ae0fb..7e3f4cfe 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -1,4 +1,5 @@ import type { Token } from 'interlinearizer'; +import { useLocalizedStrings } from '@papi/frontend/react'; import { X } from 'lucide-react'; import { Popover, PopoverAnchor } from 'platform-bible-react'; import { memo, type MouseEventHandler, useEffect, useId, useRef, useState } from 'react'; @@ -12,6 +13,11 @@ import { } from './AnalysisStore'; import { MorphemeBreakdownPopover, MorphemeGlossInput } from './MorphemeEditor'; +const STRING_KEYS = [ + '%interlinearizer_tokenChip_editMorphemes%', + '%interlinearizer_tokenChip_defineMorphemes%', +] as const satisfies `%${string}%`[]; + /** * Renders a single word token as an inline chip with an editable gloss input below the surface * text. Gloss value and dispatch are read from {@link AnalysisStoreProvider} context via @@ -51,6 +57,7 @@ export function TokenChip({ isSplitFree?: boolean; showMorphology?: boolean; }>) { + const [localizedStrings] = useLocalizedStrings(STRING_KEYS); const committedGloss = useGloss(token.ref); const onGlossChange = useGlossDispatch(); const morphemes = useMorphemes(token.ref); @@ -172,12 +179,11 @@ export function TokenChip({