From eaf3f1dade5da08645fc42de25308f4f70d498ad Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Thu, 11 Jun 2026 14:37:50 -0600 Subject: [PATCH 01/16] Add UI for morphosyntactic analysis of tokens --- contributions/localizedStrings.json | 3 + contributions/projectSettings.json | 5 + cspell.json | 2 + .../components/AnalysisStore.test.tsx | 250 ++++++++++++++++++ .../components/ContinuousView.test.tsx | 4 + .../components/Interlinearizer.test.tsx | 18 ++ .../components/MorphemeEditor.test.tsx | 175 ++++++++++++ src/__tests__/components/SegmentView.test.tsx | 2 + src/__tests__/components/TokenChip.test.tsx | 190 +++++++++++++ .../controls/ViewOptionsDropdown.test.tsx | 2 + src/__tests__/store/analysisSlice.test.ts | 190 +++++++++++++ src/__tests__/test-helpers.ts | 1 + src/components/AnalysisStore.tsx | 101 ++++++- src/components/ContinuousView.tsx | 6 + src/components/Interlinearizer.tsx | 9 + src/components/InterlinearizerLoader.tsx | 12 +- src/components/MorphemeEditor.tsx | 149 +++++++++++ src/components/PhraseBox.tsx | 16 +- src/components/PhraseStripContext.tsx | 5 + src/components/SegmentListView.tsx | 6 + src/components/SegmentView.tsx | 6 + src/components/TokenChip.tsx | 92 ++++++- src/components/__mocks__/AnalysisStore.tsx | 50 +++- .../controls/ViewOptionsDropdown.tsx | 20 +- src/hooks/usePhraseStripSetup.ts | 5 + src/store/analysisSlice.ts | 127 ++++++++- src/types/interlinearizer.d.ts | 12 + 27 files changed, 1446 insertions(+), 12 deletions(-) create mode 100644 src/__tests__/components/MorphemeEditor.test.tsx create mode 100644 src/components/MorphemeEditor.tsx diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index ed8f1dca..fdf5e937 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -24,6 +24,9 @@ "%interlinearizer_projectSettings_simplifyPhrasesDescription%": "Hide interactive controls (split, unlink, remove-token) on phrases that are not currently focused, leaving only their style change on hover", "%interlinearizer_projectSettings_chapterLabelInVerse%": "Show Chapter in Verse Label", "%interlinearizer_projectSettings_chapterLabelInVerseDescription%": "Mark chapter boundaries by labeling the first verse of each chapter as chapter:verse instead of showing an inline chapter header above it", + "%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_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/contributions/projectSettings.json b/contributions/projectSettings.json index 769233ef..16f6a64d 100644 --- a/contributions/projectSettings.json +++ b/contributions/projectSettings.json @@ -21,6 +21,11 @@ "label": "%interlinearizer_projectSettings_chapterLabelInVerse%", "description": "%interlinearizer_projectSettings_chapterLabelInVerseDescription%", "default": false + }, + "interlinearizer.showMorphology": { + "label": "%interlinearizer_projectSettings_showMorphology%", + "description": "%interlinearizer_projectSettings_showMorphologyDescription%", + "default": false } } } diff --git a/cspell.json b/cspell.json index 04ae63db..d754598f 100644 --- a/cspell.json +++ b/cspell.json @@ -18,6 +18,7 @@ "bara", "baselining", "BBCCCVVV", + "believ", "clickability", "cullable", "deconflict", @@ -63,6 +64,7 @@ "struc", "Stylesheet", "typedefs", + "unanalyzed", "unhover", "unobserves", "unphrased", diff --git a/src/__tests__/components/AnalysisStore.test.tsx b/src/__tests__/components/AnalysisStore.test.tsx index 9c823cb4..94b6b4b5 100644 --- a/src/__tests__/components/AnalysisStore.test.tsx +++ b/src/__tests__/components/AnalysisStore.test.tsx @@ -8,8 +8,12 @@ import type { TextAnalysis, TokenAnalysis, TokenAnalysisLink } from 'interlinear import { AnalysisStoreProvider, useAnalysis, + useAnalysisLanguage, useGloss, useGlossDispatch, + useMorphemeBreakdownDispatch, + useMorphemeGlossDispatch, + useMorphemes, usePhraseLinkByIdMap, usePhraseLinkForToken, usePhraseLinkMap, @@ -812,3 +816,249 @@ describe('usePhraseGlossDispatch', () => { ); }); }); + +// --------------------------------------------------------------------------- +// Morpheme hooks +// --------------------------------------------------------------------------- + +/** + * Renders the morpheme forms for a token, used to assert on `useMorphemes`. + * + * @param props.tokenRef - Token ref to subscribe to. + * @returns JSX element with joined morpheme forms. + */ +function MorphemeReader({ tokenRef }: Readonly<{ tokenRef: string }>) { + const morphemes = useMorphemes(tokenRef); + return {morphemes.map((m) => m.form).join(',')}; +} + +/** + * Renders the analysis language, used to assert on `useAnalysisLanguage`. + * + * @returns JSX element with the analysis language string. + */ +function LanguageReader() { + const lang = useAnalysisLanguage(); + return {lang}; +} + +/** + * Renders a button that dispatches a morpheme breakdown, used to test + * `useMorphemeBreakdownDispatch`. + * + * @param props.tokenRef - Token ref to write. + * @param props.surfaceText - Surface text of the token. + * @param props.forms - Morpheme forms to write. + * @returns JSX element suitable for passing to `render`. + */ +function MorphemeWriter({ + tokenRef, + surfaceText, + forms, +}: Readonly<{ tokenRef: string; surfaceText: string; forms: string[] }>) { + const dispatch = useMorphemeBreakdownDispatch(); + return ( + + ); +} + +/** + * Renders a button that dispatches a morpheme gloss, used to test `useMorphemeGlossDispatch`. + * + * @param props.tokenRef - Token ref to write. + * @param props.morphemeId - Morpheme id to gloss. + * @param props.value - Gloss value. + * @returns JSX element suitable for passing to `render`. + */ +function MorphemeGlossWriter({ + tokenRef, + morphemeId, + value, +}: Readonly<{ tokenRef: string; morphemeId: string; value: string }>) { + const dispatch = useMorphemeGlossDispatch(); + return ( + + ); +} + +/** + * Renders a component that calls `useMorphemes` without a provider, used to test the error. + * + * @returns Nothing — only mounted to trigger the throw. + */ +function MorphemesUser() { + useMorphemes('tok-1'); + return undefined; +} + +/** + * Renders a component that calls `useAnalysisLanguage` without a provider, used to test the error. + * + * @returns Nothing — only mounted to trigger the throw. + */ +function LanguageUser() { + useAnalysisLanguage(); + return undefined; +} + +/** + * Renders a component that calls `useMorphemeBreakdownDispatch` without a provider. + * + * @returns Nothing — only mounted to trigger the throw. + */ +function MorphemeBreakdownDispatchUser() { + useMorphemeBreakdownDispatch(); + return undefined; +} + +/** + * Renders a component that calls `useMorphemeGlossDispatch` without a provider. + * + * @returns Nothing — only mounted to trigger the throw. + */ +function MorphemeGlossDispatchUser() { + useMorphemeGlossDispatch(); + return undefined; +} + +describe('useMorphemes', () => { + it('returns empty array when no morphemes exist', () => { + render( + + + , + ); + expect(screen.getByTestId('morphemes')).toHaveTextContent(''); + }); + + it('returns morphemes from an approved analysis with morphemes', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'unbelievable', + morphemes: [ + { id: 'm-1', form: 'un-', writingSystem: 'und' }, + { id: 'm-2', form: 'believe', writingSystem: 'und' }, + ], + }; + const link: TokenAnalysisLink = { + analysisId: 'ta-1', + status: 'approved', + token: { tokenRef: 'tok-1', surfaceText: 'unbelievable' }, + }; + const analysis: TextAnalysis = { + segmentAnalyses: [], + segmentAnalysisLinks: [], + tokenAnalyses: [ta], + tokenAnalysisLinks: [link], + phraseAnalyses: [], + phraseAnalysisLinks: [], + }; + render( + + + , + ); + expect(screen.getByTestId('morphemes')).toHaveTextContent('un-,believe'); + }); + + it('throws when called outside an AnalysisStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => render()).toThrow( + 'useMorphemes must be used inside an AnalysisStoreProvider', + ); + }); +}); + +describe('useAnalysisLanguage', () => { + it('returns the analysis language from the provider', () => { + render( + + + , + ); + expect(screen.getByTestId('lang')).toHaveTextContent('fr'); + }); + + it('throws when called outside an AnalysisStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => render()).toThrow( + 'useAnalysisLanguage must be used inside an AnalysisStoreProvider', + ); + }); +}); + +describe('useMorphemeBreakdownDispatch', () => { + it('writes morphemes and calls onSave', async () => { + const onSave = jest.fn(); + render( + + + + , + ); + + await userEvent.click(screen.getByRole('button', { name: 'break' })); + + expect(screen.getByTestId('morphemes')).toHaveTextContent('ca,-t'); + expect(onSave).toHaveBeenCalledTimes(1); + const saved: TextAnalysis = onSave.mock.calls[0][0]; + expect(saved.tokenAnalyses).toHaveLength(1); + expect(saved.tokenAnalyses[0].morphemes).toHaveLength(2); + }); + + it('throws when called outside an AnalysisStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => render()).toThrow( + 'useMorphemeBreakdownDispatch must be used inside an AnalysisStoreProvider', + ); + }); +}); + +describe('useMorphemeGlossDispatch', () => { + it('writes a morpheme gloss and calls onSave', async () => { + const onSave = jest.fn(); + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'cat', + morphemes: [ + { id: 'm-1', form: 'ca', writingSystem: 'und' }, + { id: 'm-2', form: '-t', writingSystem: 'und' }, + ], + }; + const link: TokenAnalysisLink = { + analysisId: 'ta-1', + status: 'approved', + token: { tokenRef: 'tok-1', surfaceText: 'cat' }, + }; + const analysis: TextAnalysis = { + segmentAnalyses: [], + segmentAnalysisLinks: [], + tokenAnalyses: [ta], + tokenAnalysisLinks: [link], + phraseAnalyses: [], + phraseAnalysisLinks: [], + }; + render( + + + , + ); + + await userEvent.click(screen.getByRole('button', { name: 'gloss' })); + + expect(onSave).toHaveBeenCalledTimes(1); + const saved: TextAnalysis = onSave.mock.calls[0][0]; + expect(saved.tokenAnalyses[0].morphemes?.[0].gloss).toStrictEqual({ und: 'prefix' }); + }); + + it('throws when called outside an AnalysisStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => render()).toThrow( + 'useMorphemeGlossDispatch must be used inside an AnalysisStoreProvider', + ); + }); +}); diff --git a/src/__tests__/components/ContinuousView.test.tsx b/src/__tests__/components/ContinuousView.test.tsx index d6458220..87af7ac7 100644 --- a/src/__tests__/components/ContinuousView.test.tsx +++ b/src/__tests__/components/ContinuousView.test.tsx @@ -419,6 +419,7 @@ function requiredProps( wordTokenByRef: ReadonlyMap; hideInactiveLinkButtons: boolean; simplifyPhrases: boolean; + showMorphology: boolean; } { const { tokenSegmentMap, tokenDocOrder, wordTokenByRef } = buildLookups(book); return { @@ -433,6 +434,7 @@ function requiredProps( wordTokenByRef, hideInactiveLinkButtons: false, simplifyPhrases: false, + showMorphology: false, }; } @@ -899,6 +901,7 @@ describe('ContinuousView scroll behavior', () => { wordTokenByRef={wordTokenByRef} hideInactiveLinkButtons={false} simplifyPhrases={false} + showMorphology={false} /> ); } @@ -945,6 +948,7 @@ describe('ContinuousView scroll behavior', () => { wordTokenByRef={wordTokenByRef} hideInactiveLinkButtons simplifyPhrases={false} + showMorphology={false} /> ); } diff --git a/src/__tests__/components/Interlinearizer.test.tsx b/src/__tests__/components/Interlinearizer.test.tsx index 3e8f5d44..40997d10 100644 --- a/src/__tests__/components/Interlinearizer.test.tsx +++ b/src/__tests__/components/Interlinearizer.test.tsx @@ -353,6 +353,7 @@ function renderInterlinearizer({ hideInactiveLinkButtons = false, simplifyPhrases = false, chapterLabelInVerse = false, + showMorphology = false, }: { book?: Book; continuousScroll?: boolean; @@ -361,6 +362,7 @@ function renderInterlinearizer({ hideInactiveLinkButtons?: boolean; simplifyPhrases?: boolean; chapterLabelInVerse?: boolean; + showMorphology?: boolean; } = {}) { return render( withNav( @@ -374,6 +376,7 @@ function renderInterlinearizer({ hideInactiveLinkButtons={hideInactiveLinkButtons} simplifyPhrases={simplifyPhrases} chapterLabelInVerse={chapterLabelInVerse} + showMorphology={showMorphology} />, navigate, ), @@ -514,6 +517,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons={false} simplifyPhrases={false} chapterLabelInVerse={false} + showMorphology={false} />, ), ); @@ -626,6 +630,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons={false} simplifyPhrases={false} chapterLabelInVerse={false} + showMorphology={false} />, ), ); @@ -662,6 +667,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons={false} simplifyPhrases={false} chapterLabelInVerse={false} + showMorphology={false} />, ), ); @@ -703,6 +709,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons={false} simplifyPhrases={false} chapterLabelInVerse={false} + showMorphology={false} />, ), ); @@ -721,6 +728,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons={false} simplifyPhrases={false} chapterLabelInVerse={false} + showMorphology={false} />, ), ); @@ -764,6 +772,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons={false} simplifyPhrases={false} chapterLabelInVerse={false} + showMorphology={false} />, ), ); @@ -801,6 +810,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons={false} simplifyPhrases={false} chapterLabelInVerse={false} + showMorphology={false} />, ), ); @@ -897,6 +907,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons={false} simplifyPhrases={false} chapterLabelInVerse={false} + showMorphology={false} />, ), ); @@ -922,6 +933,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons={false} simplifyPhrases={false} chapterLabelInVerse={false} + showMorphology={false} />, ), ); @@ -941,6 +953,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons={false} simplifyPhrases={false} chapterLabelInVerse={false} + showMorphology={false} />, ), ); @@ -962,6 +975,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons={false} simplifyPhrases={false} chapterLabelInVerse={false} + showMorphology={false} />, ), ); @@ -983,6 +997,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons={false} simplifyPhrases={false} chapterLabelInVerse={false} + showMorphology={false} />, ), ); @@ -1003,6 +1018,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons: false, simplifyPhrases: false, chapterLabelInVerse: false, + showMorphology: false, }; const { container, rerender } = render( withNav( @@ -1071,6 +1087,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons={false} simplifyPhrases={false} chapterLabelInVerse={false} + showMorphology={false} /> ); } @@ -1102,6 +1119,7 @@ describe('Interlinearizer', () => { hideInactiveLinkButtons: false, simplifyPhrases: false, chapterLabelInVerse: false, + showMorphology: false, }; const { container, rerender } = render( withNav(), diff --git a/src/__tests__/components/MorphemeEditor.test.tsx b/src/__tests__/components/MorphemeEditor.test.tsx new file mode 100644 index 00000000..9af49639 --- /dev/null +++ b/src/__tests__/components/MorphemeEditor.test.tsx @@ -0,0 +1,175 @@ +/** @file Unit tests for components/MorphemeEditor.tsx. */ +/// +/// + +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import * as AnalysisStore from '../../components/AnalysisStore'; +import { MorphemeBreakdownPopover, MorphemeGlossInput } from '../../components/MorphemeEditor'; + +jest.mock('../../components/AnalysisStore'); + +describe('MorphemeBreakdownPopover', () => { + it('renders with the initial value pre-filled', () => { + render( + , + ); + const input = screen.getByRole('textbox'); + expect(input).toHaveValue('un- believe -able'); + }); + + it('auto-focuses and selects the input on mount', () => { + render(); + 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( + , + ); + await userEvent.click(screen.getByRole('button', { name: 'Done' })); + expect(onSave).toHaveBeenCalledWith('un- believe'); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('calls onSave with the edited value', async () => { + const onSave = jest.fn(); + render(); + await userEvent.clear(screen.getByRole('textbox')); + await userEvent.type(screen.getByRole('textbox'), 'wor -d'); + await userEvent.click(screen.getByRole('button', { name: 'Done' })); + expect(onSave).toHaveBeenCalledWith('wor -d'); + }); + + it('commits on Enter key', async () => { + const onSave = jest.fn(); + const onClose = jest.fn(); + render(); + await userEvent.keyboard('{Enter}'); + expect(onSave).toHaveBeenCalledWith('test'); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('dismisses without saving on Escape key', async () => { + const onSave = jest.fn(); + const onClose = jest.fn(); + render(); + await userEvent.keyboard('{Escape}'); + expect(onSave).not.toHaveBeenCalled(); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('dismisses when the backdrop is clicked', async () => { + const onClose = jest.fn(); + render(); + // The backdrop is the fixed full-screen div; getByRole won't find it, so query the DOM. + const backdrop = document.querySelector('.tw\\:fixed.tw\\:inset-0'); + if (!backdrop) throw new Error('Backdrop not found'); + await userEvent.click(backdrop); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('does not dismiss when clicking inside the popover panel', async () => { + const onClose = jest.fn(); + render(); + const label = screen.getByText('Split into morphemes'); + await userEvent.click(label); + expect(onClose).not.toHaveBeenCalled(); + }); + + it('does not call onSave when the input is empty', async () => { + const onSave = jest.fn(); + render(); + await userEvent.click(screen.getByRole('button', { name: 'Done' })); + expect(onSave).not.toHaveBeenCalled(); + }); + + it('does not call onSave when the input is only whitespace', async () => { + const onSave = jest.fn(); + render(); + await userEvent.click(screen.getByRole('button', { name: 'Done' })); + expect(onSave).not.toHaveBeenCalled(); + }); +}); + +describe('MorphemeGlossInput', () => { + it('renders an empty input when no gloss exists', () => { + render( + , + ); + expect(screen.getByRole('textbox', { name: 'Gloss for morpheme un-' })).toHaveValue(''); + }); + + it('renders the existing gloss value', () => { + render( + , + ); + expect(screen.getByRole('textbox', { name: 'Gloss for morpheme un-' })).toHaveValue('not'); + }); + + it('does not dispatch when blurring without changes', async () => { + const dispatchMock = jest.fn(); + jest.spyOn(AnalysisStore, 'useMorphemeGlossDispatch').mockReturnValue(dispatchMock); + + render( + , + ); + await userEvent.click(screen.getByRole('textbox', { name: 'Gloss for morpheme un-' })); + await userEvent.tab(); + expect(dispatchMock).not.toHaveBeenCalled(); + }); + + it('dispatches the gloss on blur when the draft differs', async () => { + const dispatchMock = jest.fn(); + jest.spyOn(AnalysisStore, 'useMorphemeGlossDispatch').mockReturnValue(dispatchMock); + + render( + , + ); + await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for morpheme un-' }), 'not'); + await userEvent.tab(); + expect(dispatchMock).toHaveBeenCalledWith('tok-1', 'm-1', 'not'); + }); + + it('does not dispatch when disabled', async () => { + const dispatchMock = jest.fn(); + jest.spyOn(AnalysisStore, 'useMorphemeGlossDispatch').mockReturnValue(dispatchMock); + + render( + , + ); + const input = screen.getByRole('textbox', { name: 'Gloss for morpheme un-' }); + expect(input).toBeDisabled(); + }); +}); diff --git a/src/__tests__/components/SegmentView.test.tsx b/src/__tests__/components/SegmentView.test.tsx index cf560a30..1e2f4cf8 100644 --- a/src/__tests__/components/SegmentView.test.tsx +++ b/src/__tests__/components/SegmentView.test.tsx @@ -193,6 +193,7 @@ function requiredProps(): { hideInactiveLinkButtons: boolean; simplifyPhrases: boolean; chapterLabelInVerse: boolean; + showMorphology: boolean; } { return { displayMode: 'token-chip', @@ -211,6 +212,7 @@ function requiredProps(): { hideInactiveLinkButtons: false, simplifyPhrases: false, chapterLabelInVerse: false, + showMorphology: false, }; } diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index 98a868b7..921c1d3e 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -5,10 +5,45 @@ import { fireEvent, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { AssignmentStatus, Token, TokenSnapshot } from 'interlinearizer'; +import * as AnalysisStore from '../../components/AnalysisStore'; import { AnalysisStoreProvider } from '../../components/AnalysisStore'; import { InertTokenChip, TokenChip } from '../../components/TokenChip'; jest.mock('../../components/AnalysisStore'); +jest.mock('../../components/MorphemeEditor', () => ({ + /** + * Stub popover that renders a save button so tests can trigger onSave. + * + * @param props - Receives the same props as the real popover. + * @returns A test stub element with a save button. + */ + MorphemeBreakdownPopover({ + onSave, + onClose, + }: Readonly<{ onSave: (v: string) => void; onClose: () => void }>) { + return ( +
+ + + +
+ ); + }, + /** + * Stub gloss input that renders a placeholder for test assertions. + * + * @returns A test stub element. + */ + MorphemeGlossInput() { + return ; + }, +})); const WORD_TOKEN = { ref: 'GEN 1:1:0', @@ -308,4 +343,159 @@ describe('TokenChip', () => { const label = screen.getByText('hello').closest('label'); expect(label?.className).not.toContain('tw:border-destructive'); }); + + describe('morphology UI', () => { + it('does not render morpheme row when showMorphology is false', () => { + render( + + + , + ); + expect( + screen.queryByRole('button', { name: 'Define morpheme breakdown for hello' }), + ).not.toBeInTheDocument(); + }); + + it('renders a "define" button when showMorphology is true and no morphemes exist', () => { + render( + + + , + ); + expect( + screen.getByRole('button', { name: 'Define morpheme breakdown for hello' }), + ).toBeInTheDocument(); + }); + + it('shows surface text on the define button for unanalyzed tokens', () => { + render( + + + , + ); + const btn = screen.getByRole('button', { name: 'Define morpheme breakdown for hello' }); + expect(btn).toHaveTextContent('hello'); + }); + + it('opens the popover when the define button is clicked', async () => { + render( + + + , + ); + expect(screen.queryByTestId('morpheme-popover')).not.toBeInTheDocument(); + await userEvent.click( + screen.getByRole('button', { name: 'Define morpheme breakdown for hello' }), + ); + expect(screen.getByTestId('morpheme-popover')).toBeInTheDocument(); + }); + + it('does not open the popover when disabled', async () => { + render( + + + , + ); + await userEvent.click( + screen.getByRole('button', { name: 'Define morpheme breakdown for hello' }), + ); + expect(screen.queryByTestId('morpheme-popover')).not.toBeInTheDocument(); + }); + + it('renders morpheme forms when morphemes exist', () => { + // AnalysisStore imported at top level + jest.spyOn(AnalysisStore, 'useMorphemes').mockReturnValue([ + { id: 'm-1', form: 'hel', writingSystem: 'und' }, + { id: 'm-2', form: '-lo', writingSystem: 'und' }, + ]); + render( + + + , + ); + expect( + screen.getByRole('button', { name: 'Edit morpheme breakdown for hello' }), + ).toBeInTheDocument(); + expect(screen.getByText('hel')).toBeInTheDocument(); + expect(screen.getByText('-lo')).toBeInTheDocument(); + }); + + it('renders morpheme gloss inputs when morphemes exist', () => { + // AnalysisStore imported at top level + jest.spyOn(AnalysisStore, 'useMorphemes').mockReturnValue([ + { id: 'm-1', form: 'hel', writingSystem: 'und' }, + { id: 'm-2', form: '-lo', writingSystem: 'und' }, + ]); + render( + + + , + ); + expect(screen.getAllByTestId('morpheme-gloss')).toHaveLength(2); + }); + + it('opens the popover when the edit button is clicked on analyzed token', async () => { + // AnalysisStore imported at top level + jest + .spyOn(AnalysisStore, 'useMorphemes') + .mockReturnValue([{ id: 'm-1', form: 'hel', writingSystem: 'und' }]); + render( + + + , + ); + await userEvent.click( + screen.getByRole('button', { name: 'Edit morpheme breakdown for hello' }), + ); + expect(screen.getByTestId('morpheme-popover')).toBeInTheDocument(); + }); + + it('dispatches morpheme breakdown when saving from the popover', async () => { + const mockDispatch = jest.fn(); + // AnalysisStore imported at top level + jest.spyOn(AnalysisStore, 'useMorphemeBreakdownDispatch').mockReturnValue(mockDispatch); + + render( + + + , + ); + await userEvent.click( + screen.getByRole('button', { name: 'Define morpheme breakdown for hello' }), + ); + await userEvent.click(screen.getByRole('button', { name: 'mock-save' })); + expect(mockDispatch).toHaveBeenCalledWith('GEN 1:1:0', 'hello', ['hel', '-lo']); + }); + + it('does not dispatch when the popover saves only whitespace', async () => { + const mockDispatch = jest.fn(); + // AnalysisStore imported at top level + jest.spyOn(AnalysisStore, 'useMorphemeBreakdownDispatch').mockReturnValue(mockDispatch); + + render( + + + , + ); + await userEvent.click( + screen.getByRole('button', { name: 'Define morpheme breakdown for hello' }), + ); + await userEvent.click(screen.getByRole('button', { name: 'mock-save-empty' })); + expect(mockDispatch).not.toHaveBeenCalled(); + }); + + it('closes the popover via onClose', async () => { + render( + + + , + ); + await userEvent.click( + screen.getByRole('button', { name: 'Define morpheme breakdown for hello' }), + ); + expect(screen.getByTestId('morpheme-popover')).toBeInTheDocument(); + await userEvent.click(screen.getByRole('button', { name: 'mock-close' })); + expect(screen.queryByTestId('morpheme-popover')).not.toBeInTheDocument(); + }); + }); }); diff --git a/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx b/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx index acdec59e..6326e03e 100644 --- a/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx +++ b/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx @@ -27,6 +27,8 @@ const DEFAULT_PROPS = { onSimplifyPhrasesChange: jest.fn(), chapterLabelInVerse: false, onChapterLabelInVerseChange: jest.fn(), + showMorphology: false, + onShowMorphologyChange: jest.fn(), }; describe('ViewOptionsDropdown', () => { diff --git a/src/__tests__/store/analysisSlice.test.ts b/src/__tests__/store/analysisSlice.test.ts index 74e8360a..5c055500 100644 --- a/src/__tests__/store/analysisSlice.test.ts +++ b/src/__tests__/store/analysisSlice.test.ts @@ -17,6 +17,7 @@ import { deletePhrase, mergePhrases, selectApprovedGloss, + selectApprovedMorphemes, selectPhraseLinkByTokenRef, selectPhraseAnalysisById, selectPhraseGloss, @@ -24,6 +25,8 @@ import { setAnalysis, updatePhrase, writeGloss, + writeMorphemeGloss, + writeMorphemes, writePhraseGloss, } from '../../store/analysisSlice'; @@ -544,3 +547,190 @@ describe('selectPhraseGloss', () => { expect(selectPhraseGloss(store.getState().analysis, 'nonexistent')).toBe(''); }); }); + +describe('writeMorphemes', () => { + it('creates a new approved analysis with morphemes when none exists', () => { + const store = createAnalysisStore(); + store.dispatch(writeMorphemes('tok-1', 'unbelievable', ['un-', 'believe', '-able'])); + + const { tokenAnalyses, tokenAnalysisLinks } = store.getState().analysis.analysis; + expect(tokenAnalysisLinks).toHaveLength(1); + expect(tokenAnalysisLinks[0].status).toBe('approved'); + expect(tokenAnalysisLinks[0].token.tokenRef).toBe('tok-1'); + + const ta = tokenAnalyses.find((a) => a.id === tokenAnalysisLinks[0].analysisId); + expect(ta).toBeDefined(); + expect(ta?.morphemes).toHaveLength(3); + expect(ta?.morphemes?.[0].form).toBe('un-'); + expect(ta?.morphemes?.[1].form).toBe('believe'); + expect(ta?.morphemes?.[2].form).toBe('-able'); + }); + + it('updates morphemes on an existing approved analysis, preserving glosses by form', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'unbelievable', + morphemes: [ + { id: 'm-1', form: 'un-', writingSystem: 'und', gloss: { und: 'not' } }, + { id: 'm-2', form: 'believe', writingSystem: 'und', gloss: { und: 'trust' } }, + { id: 'm-3', form: '-able', writingSystem: 'und' }, + ], + }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + store.dispatch(writeMorphemes('tok-1', 'unbelievable', ['un-', 'believ', '-able'])); + + const updated = store + .getState() + .analysis.analysis.tokenAnalyses.find( + (a) => + a.id === + store + .getState() + .analysis.analysis.tokenAnalysisLinks.find( + (l) => l.status === 'approved' && l.token.tokenRef === 'tok-1', + )?.analysisId, + ); + expect(updated?.morphemes).toHaveLength(3); + expect(updated?.morphemes?.[0].gloss).toStrictEqual({ und: 'not' }); + expect(updated?.morphemes?.[1].gloss).toBeUndefined(); + expect(updated?.morphemes?.[2].form).toBe('-able'); + }); + + it('removes an orphaned approved link and creates a fresh analysis', () => { + const orphanLink: TokenAnalysisLink = { + analysisId: 'old-uuid', + status: 'approved', + token: { tokenRef: 'tok-1', surfaceText: 'word' }, + }; + const store = createAnalysisStore({ + analysis: { + analysis: { ...emptyAnalysis(), tokenAnalysisLinks: [orphanLink] }, + analysisLanguage: 'und', + }, + }); + + store.dispatch(writeMorphemes('tok-1', 'word', ['wor', '-d'])); + + const { tokenAnalyses, tokenAnalysisLinks } = store.getState().analysis.analysis; + expect(tokenAnalysisLinks).toHaveLength(1); + expect(tokenAnalysisLinks[0].analysisId).not.toBe('old-uuid'); + + const ta = tokenAnalyses.find((a) => a.id === tokenAnalysisLinks[0].analysisId); + expect(ta?.morphemes).toHaveLength(2); + }); + + it('adds morphemes to an existing approved analysis that has no morphemes', () => { + const ta: TokenAnalysis = { id: 'ta-1', surfaceText: 'hello' }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + store.dispatch(writeMorphemes('tok-1', 'hello', ['hel', '-lo'])); + + const updated = store.getState().analysis.analysis.tokenAnalyses.find((a) => a.id === 'ta-1'); + expect(updated?.morphemes).toHaveLength(2); + expect(updated?.morphemes?.[0].form).toBe('hel'); + }); + + it('assigns unique ids to each morpheme via prepare', () => { + const store = createAnalysisStore(); + store.dispatch(writeMorphemes('tok-1', 'abc', ['a', 'b', 'c'])); + + const ta = store.getState().analysis.analysis.tokenAnalyses[0]; + const ids = ta.morphemes?.map((m) => m.id); + expect(new Set(ids).size).toBe(3); + }); +}); + +describe('writeMorphemeGloss', () => { + it('writes a gloss onto the specified morpheme', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'unbelievable', + morphemes: [ + { id: 'm-1', form: 'un-', writingSystem: 'und' }, + { id: 'm-2', form: 'believe', writingSystem: 'und' }, + ], + }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + store.dispatch(writeMorphemeGloss({ tokenRef: 'tok-1', morphemeId: 'm-1', value: 'not' })); + + const updated = store.getState().analysis.analysis.tokenAnalyses.find((a) => a.id === 'ta-1'); + expect(updated?.morphemes?.[0].gloss).toStrictEqual({ und: 'not' }); + expect(updated?.morphemes?.[1].gloss).toBeUndefined(); + }); + + it('no-ops when the token has no approved link', () => { + const store = createAnalysisStore(); + store.dispatch(writeMorphemeGloss({ tokenRef: 'tok-1', morphemeId: 'm-1', value: 'not' })); + expect(store.getState().analysis.analysis.tokenAnalyses).toHaveLength(0); + }); + + it('no-ops when the morpheme id is not found', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'word', + morphemes: [{ id: 'm-1', form: 'word', writingSystem: 'und' }], + }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + store.dispatch( + writeMorphemeGloss({ tokenRef: 'tok-1', morphemeId: 'nonexistent', value: 'x' }), + ); + + expect( + store.getState().analysis.analysis.tokenAnalyses[0].morphemes?.[0].gloss, + ).toBeUndefined(); + }); +}); + +describe('selectApprovedMorphemes', () => { + it('returns morphemes from the approved analysis', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'word', + morphemes: [ + { id: 'm-1', form: 'wor', writingSystem: 'und' }, + { id: 'm-2', form: '-d', writingSystem: 'und' }, + ], + }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + const morphemes = selectApprovedMorphemes(store.getState().analysis, 'tok-1'); + expect(morphemes).toHaveLength(2); + expect(morphemes[0].form).toBe('wor'); + }); + + it('returns an empty array when no approved link exists', () => { + const store = createAnalysisStore(); + const morphemes = selectApprovedMorphemes(store.getState().analysis, 'tok-1'); + expect(morphemes).toHaveLength(0); + }); + + it('returns an empty array when approved analysis has no morphemes', () => { + const ta: TokenAnalysis = { id: 'ta-1', surfaceText: 'word' }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + const morphemes = selectApprovedMorphemes(store.getState().analysis, 'tok-1'); + expect(morphemes).toHaveLength(0); + }); + + it('returns the same reference for repeated calls (stable empty array)', () => { + const store = createAnalysisStore(); + const a = selectApprovedMorphemes(store.getState().analysis, 'tok-1'); + const b = selectApprovedMorphemes(store.getState().analysis, 'tok-1'); + expect(a).toBe(b); + }); +}); diff --git a/src/__tests__/test-helpers.ts b/src/__tests__/test-helpers.ts index 7edb2222..befaecac 100644 --- a/src/__tests__/test-helpers.ts +++ b/src/__tests__/test-helpers.ts @@ -83,6 +83,7 @@ export function makePhraseStripContext( onHoverSplitFreeTokens: () => {}, hideInactiveLinkButtons: false, simplifyPhrases: false, + showMorphology: false, activeSegmentId: undefined, crossSegmentLinkTooltip: '', skipLinkTransition: false, diff --git a/src/components/AnalysisStore.tsx b/src/components/AnalysisStore.tsx index 3a9de42a..0e1f7df3 100644 --- a/src/components/AnalysisStore.tsx +++ b/src/components/AnalysisStore.tsx @@ -1,5 +1,10 @@ /** @file Analysis store backed by Redux Toolkit with per-token subscriptions via `useSelector`. */ -import type { PhraseAnalysisLink, TextAnalysis, TokenSnapshot } from 'interlinearizer'; +import type { + MorphemeAnalysis, + PhraseAnalysisLink, + TextAnalysis, + TokenSnapshot, +} from 'interlinearizer'; import { createContext, useCallback, useContext, useMemo, useRef } from 'react'; import type { ReactNode } from 'react'; import { Provider as ReduxProvider, useDispatch, useSelector, useStore } from 'react-redux'; @@ -8,12 +13,16 @@ import { deletePhrase, mergePhrases, selectAnalysis, + selectAnalysisLanguage, selectApprovedGloss, + selectApprovedMorphemes, selectPhraseLinkByAnalysisId, selectPhraseLinkByTokenRef, selectPhraseGloss, updatePhrase, writeGloss, + writeMorphemeGloss, + writeMorphemes, writePhraseGloss, } from '../store/analysisSlice'; import { createAnalysisStore, type AnalysisDispatch, type AnalysisRootState } from '../store'; @@ -125,6 +134,37 @@ export function useGloss(tokenRef: string): string { return useSelector((state: AnalysisRootState) => selectApprovedGloss(state.analysis, tokenRef)); } +/** + * Returns the morpheme breakdown from the approved `TokenAnalysis` for `tokenRef`, re-rendering + * only when the morpheme array changes. Returns a stable empty array when no approved analysis + * exists or it has no morphemes. + * + * @param tokenRef - The `Token.ref` to look up. + * @returns The morpheme array from the approved analysis, or a stable empty array. + * @throws When called outside an {@link AnalysisStoreProvider}. + */ +export function useMorphemes(tokenRef: string): readonly MorphemeAnalysis[] { + const ctx = useContext(AnalysisCallbackCtx); + if (!ctx) throw new Error('useMorphemes must be used inside an AnalysisStoreProvider'); + + return useSelector((state: AnalysisRootState) => + selectApprovedMorphemes(state.analysis, tokenRef), + ); +} + +/** + * Returns the active BCP 47 analysis-language tag from the nearest {@link AnalysisStoreProvider}. + * + * @returns The analysis language string. + * @throws When called outside an {@link AnalysisStoreProvider}. + */ +export function useAnalysisLanguage(): string { + const ctx = useContext(AnalysisCallbackCtx); + if (!ctx) throw new Error('useAnalysisLanguage must be used inside an AnalysisStoreProvider'); + + return useSelector((state: AnalysisRootState) => selectAnalysisLanguage(state.analysis)); +} + /** * Returns the current `TextAnalysis` snapshot, re-rendering on every analysis change. Intended for * components that need the full analysis (e.g. an analysis-selection popup). @@ -165,6 +205,65 @@ export function useGlossDispatch(): (tokenRef: string, surfaceText: string, valu ); } +/** + * Returns a stable callback that replaces the morpheme breakdown on the approved `TokenAnalysis` + * for a given token. Dispatches the `writeMorphemes` action and triggers `onSave`. + * + * @returns A function `(tokenRef, surfaceText, forms) => void`. + * @throws When called outside an {@link AnalysisStoreProvider}. + */ +export function useMorphemeBreakdownDispatch(): ( + tokenRef: string, + surfaceText: string, + forms: string[], +) => void { + const callbacks = useContext(AnalysisCallbackCtx); + if (!callbacks) + throw new Error('useMorphemeBreakdownDispatch must be used inside an AnalysisStoreProvider'); + + const dispatch = useDispatch(); + const store = useStore(); + + return useCallback( + (tokenRef: string, surfaceText: string, forms: string[]) => { + dispatch(writeMorphemes(tokenRef, surfaceText, forms)); + const { analysis } = store.getState().analysis; + callbacks.onSaveRef.current?.(analysis); + }, + [dispatch, store, callbacks], + ); +} + +/** + * Returns a stable callback that writes a gloss on a single morpheme within the approved + * `TokenAnalysis` for a given token. Dispatches the `writeMorphemeGloss` action and triggers + * `onSave`. + * + * @returns A function `(tokenRef, morphemeId, value) => void`. + * @throws When called outside an {@link AnalysisStoreProvider}. + */ +export function useMorphemeGlossDispatch(): ( + tokenRef: string, + morphemeId: string, + value: string, +) => void { + const callbacks = useContext(AnalysisCallbackCtx); + if (!callbacks) + throw new Error('useMorphemeGlossDispatch must be used inside an AnalysisStoreProvider'); + + const dispatch = useDispatch(); + const store = useStore(); + + return useCallback( + (tokenRef: string, morphemeId: string, value: string) => { + dispatch(writeMorphemeGloss({ tokenRef, morphemeId, value })); + const { analysis } = store.getState().analysis; + callbacks.onSaveRef.current?.(analysis); + }, + [dispatch, store, callbacks], + ); +} + // #endregion // #region Phrase hooks diff --git a/src/components/ContinuousView.tsx b/src/components/ContinuousView.tsx index 3102e704..847876ad 100644 --- a/src/components/ContinuousView.tsx +++ b/src/components/ContinuousView.tsx @@ -117,6 +117,8 @@ type ContinuousViewProps = Readonly<{ * every phrase except the focused one. */ simplifyPhrases: boolean; + /** When `true`, morpheme rows and per-morpheme glosses are shown beneath each word token. */ + showMorphology: boolean; }>; /** @@ -146,6 +148,8 @@ type ContinuousViewProps = Readonly<{ * the focused token's segment. * @param props.simplifyPhrases - When true, phrase-level controls are hidden on every phrase except * the focused one. + * @param props.showMorphology - When true, morpheme rows and per-morpheme glosses are shown beneath + * each word token. * @returns A horizontal phrase strip with previous/next navigation arrows and edge-fade overlays */ export default function ContinuousView({ @@ -160,6 +164,7 @@ export default function ContinuousView({ wordTokenByRef, hideInactiveLinkButtons, simplifyPhrases, + showMorphology, }: ContinuousViewProps) { const isRtl = document.documentElement.dir === 'rtl'; @@ -663,6 +668,7 @@ export default function ContinuousView({ crossSegmentLinkTooltip: localizedStrings['%interlinearizer_linkButton_crossSegmentDisabledTooltip%'], skipLinkTransition: !isVisible || skipSlotTransitionForJump, + showMorphology, }); /** diff --git a/src/components/Interlinearizer.tsx b/src/components/Interlinearizer.tsx index db026049..6f370901 100644 --- a/src/components/Interlinearizer.tsx +++ b/src/components/Interlinearizer.tsx @@ -63,6 +63,8 @@ type InterlinearizerProps = Readonly<{ * bare verse numbers. */ chapterLabelInVerse: boolean; + /** When true, morpheme rows and per-morpheme glosses are shown beneath each word token. */ + showMorphology: boolean; }>; /** @@ -83,6 +85,8 @@ type InterlinearizerProps = Readonly<{ * the focused one. * @param props.chapterLabelInVerse - When true, every verse is labeled `chapter:verse` instead of * showing an inline chapter header. + * @param props.showMorphology - When true, morpheme rows and per-morpheme glosses are shown beneath + * each word token. * @returns The interlinearizer layout without the provider wrapper. */ function InterlinearizerInner({ @@ -94,6 +98,7 @@ function InterlinearizerInner({ hideInactiveLinkButtons, simplifyPhrases, chapterLabelInVerse, + showMorphology, }: Omit) { // Navigation surface from the context: `navigate` writes the reference (classifying internal vs // external at the call site), `consumeInternalNav` lets the segment window suppress the fade for @@ -284,6 +289,7 @@ function InterlinearizerInner({ wordTokenByRef={wordTokenByRef} hideInactiveLinkButtons={hideInactiveLinkButtons} simplifyPhrases={simplifyPhrases} + showMorphology={showMorphology} /> )} @@ -302,6 +308,7 @@ function InterlinearizerInner({ hideInactiveLinkButtons={hideInactiveLinkButtons} simplifyPhrases={simplifyPhrases} chapterLabelInVerse={chapterLabelInVerse} + showMorphology={showMorphology} hoveredPhraseId={hoveredPhraseId} setHoveredPhraseId={setHoveredPhraseId} editPhraseSegmentId={editPhraseSegmentId} @@ -334,6 +341,8 @@ function InterlinearizerInner({ * segments other than the active verse. * @param props.simplifyPhrases - When true, phrase-level controls are hidden on every phrase except * the focused one. + * @param props.showMorphology - When true, morpheme rows and per-morpheme glosses are shown beneath + * each word token. * @returns The full interlinearizer layout with optional continuous strip and segment list */ export default function Interlinearizer({ diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 470064a9..6199a8b1 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -220,6 +220,12 @@ function InterlinearizerLoaderInner({ value: chapterLabelInVerse, } = useOptimisticBooleanSetting(projectId, 'interlinearizer.chapterLabelInVerse', false); + const { + isLoading: isShowMorphologyLoading, + onChange: handleShowMorphologyChange, + value: showMorphology, + } = useOptimisticBooleanSetting(projectId, 'interlinearizer.showMorphology', false); + const { book, isLoading, bookError, tokenizeError } = useInterlinearizerBookData({ projectId, scrRef, @@ -230,7 +236,8 @@ function InterlinearizerLoaderInner({ isContinuousScrollLoading || isHideInactiveLinkButtonsLoading || isSimplifyPhrasesLoading || - isChapterLabelInVerseLoading; + isChapterLabelInVerseLoading || + isShowMorphologyLoading; // True during a cross-book swap: the live `scrRef` already names the new book but the loaded `book` // is still the previous one (its USJ hasn't arrived yet). The old `Interlinearizer` is still // mounted here; showing it (even frozen on its last in-book reference) lets the previous book's @@ -337,6 +344,8 @@ function InterlinearizerLoaderInner({ onSimplifyPhrasesChange={handleSimplifyPhrasesChange} chapterLabelInVerse={chapterLabelInVerse} onChapterLabelInVerseChange={handleChapterLabelInVerseChange} + showMorphology={showMorphology} + onShowMorphologyChange={handleShowMorphologyChange} /> ) : undefined } @@ -397,6 +406,7 @@ function InterlinearizerLoaderInner({ hideInactiveLinkButtons={hideInactiveLinkButtons} simplifyPhrases={simplifyPhrases} chapterLabelInVerse={chapterLabelInVerse} + showMorphology={showMorphology} /> )} diff --git a/src/components/MorphemeEditor.tsx b/src/components/MorphemeEditor.tsx new file mode 100644 index 00000000..3cfb1f7f --- /dev/null +++ b/src/components/MorphemeEditor.tsx @@ -0,0 +1,149 @@ +/** + * @file Inline morpheme editing components rendered inside {@link TokenChip} when the morphology + * toggle is active. {@link MorphemeBreakdownPopover} lets the user define or re-split a token's + * morpheme forms; {@link MorphemeGlossInput} provides a per-morpheme gloss field. + */ +import type { MorphemeAnalysis } from 'interlinearizer'; +import { useEffect, useRef, useState } from 'react'; +import type { KeyboardEvent, MouseEvent } from 'react'; +import { useMorphemeGlossDispatch } from './AnalysisStore'; + +/** + * 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 or Done. + * + * @param props - Component props. + * @param props.initialValue - Pre-filled text for the input (current morpheme forms joined by + * spaces, or the full surface text when no breakdown exists yet). + * @param props.onSave - Called with the raw input string when the user commits. + * @param props.onClose - Called to dismiss the popover without saving. + * @returns A positioned popover panel with a text input and Done button. + */ +export function MorphemeBreakdownPopover({ + initialValue, + onSave, + onClose, +}: Readonly<{ + initialValue: string; + onSave: (value: string) => void; + onClose: () => void; +}>) { + const [draft, setDraft] = useState(initialValue); + // eslint-disable-next-line no-null/no-null + const inputRef = useRef(null); + + useEffect(() => { + inputRef.current?.focus(); + inputRef.current?.select(); + }, []); + + /** Commits the current draft and closes the popover. */ + const handleSave = () => { + const trimmed = draft.trim(); + if (trimmed) onSave(trimmed); + onClose(); + }; + + /** + * Handles Enter to commit and Escape to dismiss. + * + * @param e - The keyboard event. + */ + const handleKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Enter') { + e.preventDefault(); + handleSave(); + } else if (e.key === 'Escape') { + e.preventDefault(); + onClose(); + } + }; + + /** + * Prevents the click from bubbling to the backdrop dismiss handler. + * + * @param e - The mouse event on the popover panel. + */ + const stopPropagation = (e: MouseEvent) => { + e.stopPropagation(); + }; + + return ( + <> + {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} +
+ {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} +
+ + setDraft(e.target.value)} + onKeyDown={handleKeyDown} + type="text" + /> + +
+ + ); +} + +/** + * Renders a single morpheme's gloss as an inline editable input. Writes to the store on blur when + * the draft differs from the committed value. + * + * @param props - Component props. + * @param props.morpheme - The morpheme whose gloss is being edited. + * @param props.tokenRef - The token ref for dispatching gloss writes. + * @param props.analysisLanguage - BCP 47 tag for reading/writing the gloss. + * @param props.disabled - When true, the input is read-only. + * @returns A sized text input for the morpheme gloss. + */ +export function MorphemeGlossInput({ + morpheme, + tokenRef, + analysisLanguage, + disabled, +}: Readonly<{ + morpheme: MorphemeAnalysis; + tokenRef: string; + analysisLanguage: string; + disabled: boolean; +}>) { + const committed = morpheme.gloss?.[analysisLanguage] ?? ''; + const dispatchMorphemeGloss = useMorphemeGlossDispatch(); + const [draft, setDraft] = useState(committed); + + useEffect(() => { + setDraft(committed); + }, [committed]); + + return ( + setDraft(e.target.value)} + onBlur={() => { + if (!disabled && draft !== committed) dispatchMorphemeGloss(tokenRef, morpheme.id, draft); + }} + type="text" + /> + ); +} diff --git a/src/components/PhraseBox.tsx b/src/components/PhraseBox.tsx index 74e866bd..7afbd54d 100644 --- a/src/components/PhraseBox.tsx +++ b/src/components/PhraseBox.tsx @@ -162,6 +162,7 @@ export function PhraseBox({ tokenSegmentMap, tokenDocOrder, simplifyPhrases, + showMorphology, } = usePhraseStripContext(); // When simplifyPhrases is on, a phrase exposes its interactive controls only while focused. // Intra-phrase unlink icons are hidden via opacity/pointer-events (not unmounted) so the layout @@ -394,6 +395,7 @@ export function PhraseBox({ ? () => handleViewPopOut(token.ref) : undefined } + showMorphology={showMorphology} token={token} /> @@ -432,7 +434,12 @@ export function PhraseBox({ ))} )} - + ))} @@ -488,7 +495,12 @@ export function PhraseBox({ onClick={() => handleEditRemove(token.ref)} onKeyDown={handlePerTokenKeyDown(token.ref)} > - + ))} diff --git a/src/components/PhraseStripContext.tsx b/src/components/PhraseStripContext.tsx index d9bc5ad3..e0a7fb05 100644 --- a/src/components/PhraseStripContext.tsx +++ b/src/components/PhraseStripContext.tsx @@ -73,6 +73,11 @@ export type PhraseStripContextValue = Readonly<{ * before the strip fades in, rather than animating while the strip is becoming visible. */ skipLinkTransition: boolean; + /** + * When `true`, each word token displays its morpheme breakdown and per-morpheme glosses beneath + * the token-level gloss input. + */ + showMorphology: boolean; }>; /** The phrase-strip context. `undefined` outside a provider so consumers can fail loudly. */ diff --git a/src/components/SegmentListView.tsx b/src/components/SegmentListView.tsx index c715cd32..46f6f567 100644 --- a/src/components/SegmentListView.tsx +++ b/src/components/SegmentListView.tsx @@ -53,6 +53,8 @@ type SegmentListViewProps = Readonly<{ * verse of each chapter. */ chapterLabelInVerse: boolean; + /** When true, morpheme rows and per-morpheme glosses are shown beneath each word token. */ + showMorphology: boolean; /** PhraseId currently hovered anywhere in the interlinearizer; shared across all SegmentViews. */ hoveredPhraseId: string | undefined; /** Sets the hovered phraseId when the pointer enters or leaves a phrase box. */ @@ -96,6 +98,8 @@ type SegmentListViewProps = Readonly<{ * phrase. * @param props.chapterLabelInVerse - When true, every verse is labeled `chapter:verse` instead of * showing an inline chapter header. + * @param props.showMorphology - When true, morpheme rows and per-morpheme glosses are shown beneath + * each word token. * @param props.hoveredPhraseId - PhraseId currently hovered anywhere in the interlinearizer. * @param props.setHoveredPhraseId - Sets the hovered phraseId. * @param props.editPhraseSegmentId - Segment id containing the phrase being edited, or `undefined`. @@ -119,6 +123,7 @@ export default function SegmentListView({ hideInactiveLinkButtons, simplifyPhrases, chapterLabelInVerse, + showMorphology, hoveredPhraseId, setHoveredPhraseId, editPhraseSegmentId, @@ -250,6 +255,7 @@ export default function SegmentListView({ hideInactiveLinkButtons={hideInactiveLinkButtons} simplifyPhrases={simplifyPhrases} chapterLabelInVerse={chapterLabelInVerse} + showMorphology={showMorphology} /> ))} diff --git a/src/components/SegmentView.tsx b/src/components/SegmentView.tsx index 493f1dcb..649b5f75 100644 --- a/src/components/SegmentView.tsx +++ b/src/components/SegmentView.tsx @@ -89,6 +89,8 @@ type SegmentViewProps = Readonly<{ * chapter's first segment). */ chapterLabelInVerse: boolean; + /** When `true`, morpheme rows and per-morpheme glosses are shown beneath each word token. */ + showMorphology: boolean; }>; /** @@ -122,6 +124,8 @@ type SegmentViewProps = Readonly<{ * the focused one. * @param props.chapterLabelInVerse - When true, every segment is labeled `chapter:verse`; when * false, segments show a bare verse number. + * @param props.showMorphology - When true, morpheme rows and per-morpheme glosses are shown beneath + * each word token. * @returns A button (baseline-text mode) or div (token-chip mode) containing a verse label and * segment content */ @@ -142,6 +146,7 @@ export function SegmentView({ hideInactiveLinkButtons, simplifyPhrases, chapterLabelInVerse, + showMorphology, }: SegmentViewProps) { const { book, chapter, verse } = segment.startRef; const ref: ScriptureRef = useMemo(() => ({ book, chapter, verse }), [book, chapter, verse]); @@ -321,6 +326,7 @@ export function SegmentView({ crossSegmentLinkTooltip: localizedStrings['%interlinearizer_linkButton_crossSegmentDisabledTooltip%'], skipLinkTransition: !hasMounted, + showMorphology, }); /** True when any committed phrase exists in this segment. */ diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index 4d5ba172..d569643b 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -1,7 +1,14 @@ import type { Token } from 'interlinearizer'; import { X } from 'lucide-react'; import { memo, type MouseEventHandler, useEffect, useRef, useState } from 'react'; -import { useGloss, useGlossDispatch } from './AnalysisStore'; +import { + useAnalysisLanguage, + useGloss, + useGlossDispatch, + useMorphemeBreakdownDispatch, + useMorphemes, +} from './AnalysisStore'; +import { MorphemeBreakdownPopover, MorphemeGlossInput } from './MorphemeEditor'; /** * Renders a single word token as an inline chip with an editable gloss input below the surface @@ -10,15 +17,22 @@ import { useGloss, useGlossDispatch } from './AnalysisStore'; * only when the draft differs from the committed value, to avoid creating empty analysis entries on * focus/blur cycles with no edits. * + * When `showMorphology` is true, a morpheme row is shown below the surface text. For unanalyzed + * tokens this is a clickable button showing the surface text; for analyzed tokens it shows the + * morpheme forms. Clicking either opens an inline popover where the user can define or edit the + * morpheme breakdown. Per-morpheme gloss inputs appear below the morpheme forms. + * * @param props - Component props * @param props.token - The word token to render. * @param props.onFocus - Called when the gloss input receives focus. * @param props.disabled - When true, the gloss input is read-only and non-interactive. - * @param props.onRemove - When provided, renders a small ✕ button in the top-right corner of the + * @param props.onRemove - When provided, renders a small X button in the top-right corner of the * chip; clicking it calls this callback to remove the token from its phrase. * @param props.isSplitFree - When true, this token would become free (solo) if the currently * hovered split/unlink button were clicked; previewed with a destructive border on the chip. - * @returns A styled label containing the surface text and a gloss input. + * @param props.showMorphology - When true, morpheme breakdown and per-morpheme glosses are shown + * below the surface text. + * @returns A styled label containing the surface text, optionally morpheme rows, and a gloss input. */ export function TokenChip({ token, @@ -26,16 +40,22 @@ export function TokenChip({ disabled = false, onRemove, isSplitFree = false, + showMorphology = false, }: Readonly<{ token: Token & { type: 'word' }; onFocus: () => void; disabled?: boolean; onRemove?: () => void; isSplitFree?: boolean; + showMorphology?: boolean; }>) { const committedGloss = useGloss(token.ref); const onGlossChange = useGlossDispatch(); + const morphemes = useMorphemes(token.ref); + const analysisLanguage = useAnalysisLanguage(); + const dispatchMorphemeBreakdown = useMorphemeBreakdownDispatch(); const [draft, setDraft] = useState(committedGloss); + const [popoverOpen, setPopoverOpen] = useState(false); // Tracks whether the X button itself is hovered, so only that button hover reddens the border. const [isRemoveHovered, setIsRemoveHovered] = useState(false); // Reset remove-hover state when onRemove is cleared so the red border doesn't linger. @@ -75,6 +95,20 @@ export function TokenChip({ e.currentTarget.querySelector('input')?.focus({ preventScroll: true }); }; + /** + * Commits the morpheme breakdown from the popover input, splitting on whitespace. + * + * @param value - The raw text from the popover input. + */ + const handleMorphemeSave = (value: string) => { + const forms = value.split(/\s+/).filter(Boolean); + if (forms.length > 0) { + dispatchMorphemeBreakdown(token.ref, token.surfaceText, forms); + } + }; + + const hasMorphemes = morphemes.length > 0; + // The X button is positioned outside the
, document.body, diff --git a/src/hooks/usePhraseStripSetup.ts b/src/hooks/usePhraseStripSetup.ts index f584caba..e09e0303 100644 --- a/src/hooks/usePhraseStripSetup.ts +++ b/src/hooks/usePhraseStripSetup.ts @@ -121,6 +121,8 @@ export type PhraseStripContextParams = Readonly<{ crossSegmentLinkTooltip: string; /** When true, the link-slot sliding-door transition is suppressed (duration 0ms). */ skipLinkTransition: boolean; + /** When true, morpheme rows and per-morpheme glosses are shown beneath each word token. */ + showMorphology: boolean; }>; /** @@ -151,6 +153,7 @@ export function usePhraseStripContextValue( activeSegmentId, crossSegmentLinkTooltip, skipLinkTransition, + showMorphology, } = params; return useMemo( @@ -169,6 +172,7 @@ export function usePhraseStripContextValue( activeSegmentId, crossSegmentLinkTooltip, skipLinkTransition, + showMorphology, }), [ phraseMode, @@ -185,6 +189,7 @@ export function usePhraseStripContextValue( activeSegmentId, crossSegmentLinkTooltip, skipLinkTransition, + showMorphology, ], ); } diff --git a/src/store/analysisSlice.ts b/src/store/analysisSlice.ts index 0218964f..06a11d0e 100644 --- a/src/store/analysisSlice.ts +++ b/src/store/analysisSlice.ts @@ -1,5 +1,6 @@ import { createSelector, createSlice, type PayloadAction } from '@reduxjs/toolkit'; import type { + MorphemeAnalysis, PhraseAnalysis, PhraseAnalysisLink, TextAnalysis, @@ -185,6 +186,108 @@ const analysisSlice = createSlice({ state.analysis.tokenAnalysisLinks.push(newLink); }, }, + writeMorphemes: { + /** + * Generates UUIDs for new morpheme records and a potential new `TokenAnalysis` before the + * action reaches the reducer. + * + * @param tokenRef - `Token.ref` of the token whose morphemes are being set. + * @param surfaceText - Surface text of the token. + * @param forms - Ordered morpheme form strings as entered by the user. + * @returns The prepared action payload. + */ + prepare(tokenRef: string, surfaceText: string, forms: string[]) { + return { + payload: { + tokenRef, + surfaceText, + analysisId: crypto.randomUUID(), + morphemes: forms.map((form) => ({ id: crypto.randomUUID(), form })), + }, + }; + }, + /** + * Sets the morpheme breakdown on the approved `TokenAnalysis` for the given token. Preserves + * existing morpheme glosses when a morpheme form is unchanged. When no approved analysis + * exists, creates one. + * + * @param state - Current slice state (Immer draft). + * @param action - Action carrying the morpheme payload. + */ + reducer( + state, + action: PayloadAction<{ + tokenRef: string; + surfaceText: string; + analysisId: string; + morphemes: Array<{ id: string; form: string }>; + }>, + ) { + const { tokenRef, surfaceText, analysisId, morphemes } = action.payload; + const writingSystem = state.analysisLanguage; + + const existingLink = state.analysis.tokenAnalysisLinks.find( + (l) => l.status === 'approved' && l.token.tokenRef === tokenRef, + ); + + if (existingLink) { + const existingAnalysis = state.analysis.tokenAnalyses.find( + (ta) => ta.id === existingLink.analysisId, + ); + if (existingAnalysis) { + const oldByForm = new Map((existingAnalysis.morphemes ?? []).map((m) => [m.form, m])); + existingAnalysis.morphemes = morphemes.map(({ id, form }) => { + const old = oldByForm.get(form); + if (old) return { ...old, id }; + return { id, form, writingSystem }; + }); + return; + } + state.analysis.tokenAnalysisLinks = state.analysis.tokenAnalysisLinks.filter( + (l) => l !== existingLink, + ); + } + + const newAnalysis: TokenAnalysis = { + id: analysisId, + surfaceText, + morphemes: morphemes.map(({ id, form }) => ({ id, form, writingSystem })), + }; + const newLink: TokenAnalysisLink = { + analysisId, + status: 'approved', + token: { tokenRef, surfaceText }, + }; + state.analysis.tokenAnalyses.push(newAnalysis); + state.analysis.tokenAnalysisLinks.push(newLink); + }, + }, + /** + * Writes a gloss string onto a single morpheme within the approved `TokenAnalysis` for the + * given token. No-ops when the token has no approved analysis or the morpheme id is not found. + * + * @param state - Current slice state (Immer draft). + * @param action - Action carrying the morpheme gloss payload. + */ + writeMorphemeGloss( + state, + action: PayloadAction<{ tokenRef: string; morphemeId: string; value: string }>, + ) { + const { tokenRef, morphemeId, value } = action.payload; + const lang = state.analysisLanguage; + + const link = state.analysis.tokenAnalysisLinks.find( + (l) => l.status === 'approved' && l.token.tokenRef === tokenRef, + ); + if (!link) return; + + const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === link.analysisId); + const morpheme = analysis?.morphemes?.find((m) => m.id === morphemeId); + if (!morpheme) return; + + if (!morpheme.gloss) morpheme.gloss = {}; + morpheme.gloss[lang] = value; + }, createPhrase: { /** * Generates a UUID for the new `PhraseAnalysis` before the action reaches the reducer, @@ -287,6 +390,8 @@ const analysisSlice = createSlice({ export const { setAnalysis, writeGloss, + writeMorphemes, + writeMorphemeGloss, createPhrase, updatePhrase, deletePhrase, @@ -321,7 +426,7 @@ const selectTokenAnalysisLinks = (state: AnalysisState) => state.analysis.tokenA * @param state - The analysis slice state. * @returns The active BCP 47 analysis language tag. */ -const selectAnalysisLanguage = (state: AnalysisState) => state.analysisLanguage; +export const selectAnalysisLanguage = (state: AnalysisState) => state.analysisLanguage; /** * Memoized selector that builds a `Map` from `TokenAnalysis.id` to `TokenAnalysis` for O(1) lookup. @@ -373,6 +478,26 @@ export function selectApprovedGloss(state: AnalysisState, tokenRef: string): str return ta?.gloss?.[lang] ?? ''; } +const EMPTY_MORPHEMES: readonly MorphemeAnalysis[] = []; + +/** + * Returns the morpheme array from the approved `TokenAnalysis` for `tokenRef`, or an empty array + * when no approved analysis exists or it has no morphemes. + * + * @param state - The analysis slice state. + * @param tokenRef - The `Token.ref` to look up. + * @returns The morpheme array, or a stable empty array when absent. + */ +export function selectApprovedMorphemes( + state: AnalysisState, + tokenRef: string, +): readonly MorphemeAnalysis[] { + const approvedId = selectApprovedIdByTokenRef(state).get(tokenRef); + if (!approvedId) return EMPTY_MORPHEMES; + const ta = selectAnalysisById(state).get(approvedId); + return ta?.morphemes ?? EMPTY_MORPHEMES; +} + /** * Projects `phraseAnalysisLinks` out of `AnalysisState` for use as a `createSelector` input. * diff --git a/src/types/interlinearizer.d.ts b/src/types/interlinearizer.d.ts index 93c2b967..9a490996 100644 --- a/src/types/interlinearizer.d.ts +++ b/src/types/interlinearizer.d.ts @@ -29,6 +29,11 @@ declare module 'papi-shared-types' { * each chapter is labeled `chapter:verse` instead of a bare verse number. */ 'interlinearizer.chapterLabelInVerse': boolean; + /** + * When true, each word token displays its morpheme breakdown and per-morpheme glosses beneath + * the token-level gloss input. + */ + 'interlinearizer.showMorphology': boolean; } /** @@ -845,6 +850,13 @@ declare module 'interlinearizer' { * exposure — see `GrammarRef`). Absent when MSA-level detail is not available. */ grammarRef?: GrammarRef; + + /** + * Free-form gloss string keyed by BCP 47 analysis-language tag. Analogous to + * `TokenAnalysis.gloss` but scoped to a single morpheme. Takes precedence over any gloss + * resolved through `senseRef` when both are present. + */ + gloss?: MultiString; } // --------------------------------------------------------------------------- From a88b473d29a1f2afb9a908b838eea6aa9412cf0a Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 12 Jun 2026 11:02:35 -0600 Subject: [PATCH 02/16] Improve editor, allow deletion of MSA --- .../components/AnalysisStore.test.tsx | 76 ++++++ .../components/MorphemeEditor.test.tsx | 248 +++++++++++++++++- src/__tests__/components/TokenChip.test.tsx | 41 ++- .../controls/ViewOptionsDropdown.test.tsx | 39 ++- src/__tests__/store/analysisSlice.test.ts | 105 ++++++++ src/components/AnalysisStore.tsx | 27 ++ src/components/MorphemeEditor.tsx | 167 +++++++++--- src/components/TokenChip.tsx | 21 +- src/components/__mocks__/AnalysisStore.tsx | 9 + .../controls/ViewOptionsDropdown.tsx | 14 +- src/store/analysisSlice.ts | 29 ++ 11 files changed, 713 insertions(+), 63 deletions(-) diff --git a/src/__tests__/components/AnalysisStore.test.tsx b/src/__tests__/components/AnalysisStore.test.tsx index 94b6b4b5..adca6cda 100644 --- a/src/__tests__/components/AnalysisStore.test.tsx +++ b/src/__tests__/components/AnalysisStore.test.tsx @@ -12,6 +12,7 @@ import { useGloss, useGlossDispatch, useMorphemeBreakdownDispatch, + useMorphemeDeleteDispatch, useMorphemeGlossDispatch, useMorphemes, usePhraseLinkByIdMap, @@ -864,6 +865,22 @@ function MorphemeWriter({ ); } +/** + * Renders a button that dispatches a morpheme breakdown deletion, used to test + * `useMorphemeDeleteDispatch`. + * + * @param props.tokenRef - Token ref whose breakdown to delete. + * @returns JSX element suitable for passing to `render`. + */ +function MorphemeDeleter({ tokenRef }: Readonly<{ tokenRef: string }>) { + const dispatch = useMorphemeDeleteDispatch(); + return ( + + ); +} + /** * Renders a button that dispatches a morpheme gloss, used to test `useMorphemeGlossDispatch`. * @@ -925,6 +942,16 @@ function MorphemeGlossDispatchUser() { return undefined; } +/** + * Renders a component that calls `useMorphemeDeleteDispatch` without a provider. + * + * @returns Nothing — only mounted to trigger the throw. + */ +function MorphemeDeleteDispatchUser() { + useMorphemeDeleteDispatch(); + return undefined; +} + describe('useMorphemes', () => { it('returns empty array when no morphemes exist', () => { render( @@ -1018,6 +1045,55 @@ describe('useMorphemeBreakdownDispatch', () => { }); }); +describe('useMorphemeDeleteDispatch', () => { + it('removes the morpheme breakdown and calls onSave', async () => { + const onSave = jest.fn(); + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'cat', + morphemes: [ + { id: 'm-1', form: 'ca', writingSystem: 'und' }, + { id: 'm-2', form: '-t', writingSystem: 'und' }, + ], + }; + const link: TokenAnalysisLink = { + analysisId: 'ta-1', + status: 'approved', + token: { tokenRef: 'tok-1', surfaceText: 'cat' }, + }; + const analysis: TextAnalysis = { + segmentAnalyses: [], + segmentAnalysisLinks: [], + tokenAnalyses: [ta], + tokenAnalysisLinks: [link], + phraseAnalyses: [], + phraseAnalysisLinks: [], + }; + render( + + + + , + ); + + await userEvent.click(screen.getByRole('button', { name: 'delete-morphemes' })); + + expect(screen.getByTestId('morphemes')).toHaveTextContent(''); + expect(onSave).toHaveBeenCalledTimes(1); + const saved: TextAnalysis = onSave.mock.calls[0][0]; + // The analysis carried no gloss, so the now-empty record and its link are removed entirely. + expect(saved.tokenAnalyses).toHaveLength(0); + expect(saved.tokenAnalysisLinks).toHaveLength(0); + }); + + it('throws when called outside an AnalysisStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => render()).toThrow( + 'useMorphemeDeleteDispatch must be used inside an AnalysisStoreProvider', + ); + }); +}); + describe('useMorphemeGlossDispatch', () => { it('writes a morpheme gloss and calls onSave', async () => { const onSave = jest.fn(); diff --git a/src/__tests__/components/MorphemeEditor.test.tsx b/src/__tests__/components/MorphemeEditor.test.tsx index 9af49639..6fef8e33 100644 --- a/src/__tests__/components/MorphemeEditor.test.tsx +++ b/src/__tests__/components/MorphemeEditor.test.tsx @@ -13,6 +13,7 @@ describe('MorphemeBreakdownPopover', () => { it('renders with the initial value pre-filled', () => { render( { }); it('auto-focuses and selects the input on mount', () => { - render(); + render( + , + ); expect(screen.getByRole('textbox')).toHaveFocus(); }); @@ -31,7 +39,12 @@ describe('MorphemeBreakdownPopover', () => { const onSave = jest.fn(); const onClose = jest.fn(); render( - , + , ); await userEvent.click(screen.getByRole('button', { name: 'Done' })); expect(onSave).toHaveBeenCalledWith('un- believe'); @@ -40,7 +53,14 @@ describe('MorphemeBreakdownPopover', () => { it('calls onSave with the edited value', async () => { const onSave = jest.fn(); - render(); + render( + , + ); await userEvent.clear(screen.getByRole('textbox')); await userEvent.type(screen.getByRole('textbox'), 'wor -d'); await userEvent.click(screen.getByRole('button', { name: 'Done' })); @@ -50,7 +70,14 @@ describe('MorphemeBreakdownPopover', () => { it('commits on Enter key', async () => { const onSave = jest.fn(); const onClose = jest.fn(); - render(); + render( + , + ); await userEvent.keyboard('{Enter}'); expect(onSave).toHaveBeenCalledWith('test'); expect(onClose).toHaveBeenCalledTimes(1); @@ -59,25 +86,129 @@ describe('MorphemeBreakdownPopover', () => { it('dismisses without saving on Escape key', async () => { const onSave = jest.fn(); const onClose = jest.fn(); - render(); + render( + , + ); await userEvent.keyboard('{Escape}'); expect(onSave).not.toHaveBeenCalled(); expect(onClose).toHaveBeenCalledTimes(1); }); - it('dismisses when the backdrop is clicked', async () => { + it('dismisses without saving when Cancel button is clicked', async () => { + const onSave = jest.fn(); const onClose = jest.fn(); - render(); + render( + , + ); + await userEvent.click(screen.getByRole('button', { name: 'Cancel' })); + expect(onSave).not.toHaveBeenCalled(); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('saves when the backdrop is clicked and a breakdown already exists', async () => { + const onSave = jest.fn(); + render( + , + ); // The backdrop is the fixed full-screen div; getByRole won't find it, so query the DOM. const backdrop = document.querySelector('.tw\\:fixed.tw\\:inset-0'); if (!backdrop) throw new Error('Backdrop not found'); await userEvent.click(backdrop); + expect(onSave).toHaveBeenCalledWith('test'); + }); + + it('closes when the backdrop is clicked', async () => { + const onClose = jest.fn(); + render( + , + ); + const backdrop = document.querySelector('.tw\\:fixed.tw\\:inset-0'); + if (!backdrop) throw new Error('Backdrop not found'); + await userEvent.click(backdrop); expect(onClose).toHaveBeenCalledTimes(1); }); + it('saves on backdrop click when no breakdown exists but the text was edited', async () => { + const onSave = jest.fn(); + render( + , + ); + await userEvent.type(screen.getByRole('textbox'), ' -er'); + const backdrop = document.querySelector('.tw\\:fixed.tw\\:inset-0'); + if (!backdrop) throw new Error('Backdrop not found'); + await userEvent.click(backdrop); + expect(onSave).toHaveBeenCalledWith('test -er'); + }); + + it('does not save on backdrop click when no breakdown exists and the text is unchanged', async () => { + const onSave = jest.fn(); + const onClose = jest.fn(); + render( + , + ); + const backdrop = document.querySelector('.tw\\:fixed.tw\\:inset-0'); + if (!backdrop) throw new Error('Backdrop not found'); + await userEvent.click(backdrop); + expect(onSave).not.toHaveBeenCalled(); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('does not save on backdrop click when the input is only whitespace', async () => { + const onSave = jest.fn(); + render( + , + ); + const backdrop = document.querySelector('.tw\\:fixed.tw\\:inset-0'); + if (!backdrop) throw new Error('Backdrop not found'); + await userEvent.click(backdrop); + expect(onSave).not.toHaveBeenCalled(); + }); + it('does not dismiss when clicking inside the popover panel', async () => { const onClose = jest.fn(); - render(); + render( + , + ); const label = screen.getByText('Split into morphemes'); await userEvent.click(label); expect(onClose).not.toHaveBeenCalled(); @@ -85,17 +216,114 @@ describe('MorphemeBreakdownPopover', () => { it('does not call onSave when the input is empty', async () => { const onSave = jest.fn(); - render(); + render( + , + ); await userEvent.click(screen.getByRole('button', { name: 'Done' })); expect(onSave).not.toHaveBeenCalled(); }); it('does not call onSave when the input is only whitespace', async () => { const onSave = jest.fn(); - render(); + render( + , + ); await userEvent.click(screen.getByRole('button', { name: 'Done' })); expect(onSave).not.toHaveBeenCalled(); }); + + it('does not render a Delete button when onDelete is not provided', () => { + render( + , + ); + expect(screen.queryByRole('button', { name: 'Delete' })).not.toBeInTheDocument(); + }); + + it('calls onDelete and onClose without saving when Delete is clicked', async () => { + const onDelete = jest.fn(); + const onSave = jest.fn(); + const onClose = jest.fn(); + render( + , + ); + await userEvent.click(screen.getByRole('button', { name: 'Delete' })); + expect(onDelete).toHaveBeenCalledTimes(1); + expect(onClose).toHaveBeenCalledTimes(1); + expect(onSave).not.toHaveBeenCalled(); + }); + + it('portals the panel to document.body so segment rows cannot stack above it', () => { + render( + , + ); + const panel = screen.getByText('Split into morphemes').closest('div'); + expect(panel?.parentElement).toBe(document.body); + }); + + it('positions the panel below the anchor when there is room under the viewport bottom', () => { + // The layout effect measures the anchor (panel's DOM parent) first, then the panel itself. + jest + .spyOn(Element.prototype, 'getBoundingClientRect') + .mockReturnValueOnce(new DOMRect(50, 100, 40, 20)) + .mockReturnValueOnce(new DOMRect(0, 0, 200, 100)); + render( + , + ); + const panel = screen.getByText('Split into morphemes').closest('div'); + // Anchor bottom (120) plus the 4px margin. + expect(panel).toHaveStyle({ top: '124px', left: '50px' }); + }); + + it('flips the panel above the anchor when the viewport bottom is too close', () => { + // Anchor bottom at 720 leaves only 48px below in jsdom's 768px-tall window — not enough for + // the 100px-tall panel, so it flips above the anchor. + jest + .spyOn(Element.prototype, 'getBoundingClientRect') + .mockReturnValueOnce(new DOMRect(50, 700, 40, 20)) + .mockReturnValueOnce(new DOMRect(0, 0, 200, 100)); + render( + , + ); + const panel = screen.getByText('Split into morphemes').closest('div'); + // Anchor top (700) minus panel height (100) minus the 4px margin. + expect(panel).toHaveStyle({ top: '596px', left: '50px' }); + }); }); describe('MorphemeGlossInput', () => { diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index 921c1d3e..633de690 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -20,7 +20,8 @@ jest.mock('../../components/MorphemeEditor', () => ({ MorphemeBreakdownPopover({ onSave, onClose, - }: Readonly<{ onSave: (v: string) => void; onClose: () => void }>) { + onDelete, + }: Readonly<{ onSave: (v: string) => void; onClose: () => void; onDelete?: () => void }>) { return (
+ {onDelete && ( + + )}
); }, @@ -484,6 +490,39 @@ describe('TokenChip', () => { expect(mockDispatch).not.toHaveBeenCalled(); }); + it('dispatches morpheme deletion when the popover delete is clicked', async () => { + const mockDispatch = jest.fn(); + // AnalysisStore imported at top level + jest + .spyOn(AnalysisStore, 'useMorphemes') + .mockReturnValue([{ id: 'm-1', form: 'hel', writingSystem: 'und' }]); + jest.spyOn(AnalysisStore, 'useMorphemeDeleteDispatch').mockReturnValue(mockDispatch); + + render( + + + , + ); + await userEvent.click( + screen.getByRole('button', { name: 'Edit morpheme breakdown for hello' }), + ); + await userEvent.click(screen.getByRole('button', { name: 'mock-delete' })); + expect(mockDispatch).toHaveBeenCalledWith('GEN 1:1:0'); + }); + + it('passes no onDelete to the popover when the token has no breakdown', async () => { + render( + + + , + ); + await userEvent.click( + screen.getByRole('button', { name: 'Define morpheme breakdown for hello' }), + ); + expect(screen.getByTestId('morpheme-popover')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'mock-delete' })).not.toBeInTheDocument(); + }); + it('closes the popover via onClose', async () => { render( diff --git a/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx b/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx index 6326e03e..bffef219 100644 --- a/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx +++ b/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx @@ -146,13 +146,40 @@ describe('ViewOptionsDropdown', () => { }); }); + describe('show morphology toggle', () => { + it('reflects the checked value', async () => { + render(); + await userEvent.click(screen.getByTestId('view-options-button')); + + const checkboxes = screen.getAllByRole('checkbox'); + expect(checkboxes[1]).toBeChecked(); + }); + + it('calls onShowMorphologyChange when toggled', async () => { + const onShowMorphologyChange = jest.fn(); + render( + , + ); + await userEvent.click(screen.getByTestId('view-options-button')); + + const checkboxes = screen.getAllByRole('checkbox'); + await userEvent.click(checkboxes[1]); + + expect(onShowMorphologyChange).toHaveBeenCalledWith(true); + }); + }); + describe('hide inactive link buttons toggle', () => { it('reflects the checked value', async () => { render(); await userEvent.click(screen.getByTestId('view-options-button')); const checkboxes = screen.getAllByRole('checkbox'); - expect(checkboxes[1]).toBeChecked(); + expect(checkboxes[2]).toBeChecked(); }); it('calls onHideInactiveLinkButtonsChange when toggled', async () => { @@ -167,7 +194,7 @@ describe('ViewOptionsDropdown', () => { await userEvent.click(screen.getByTestId('view-options-button')); const checkboxes = screen.getAllByRole('checkbox'); - await userEvent.click(checkboxes[1]); + await userEvent.click(checkboxes[2]); expect(onHideInactiveLinkButtonsChange).toHaveBeenCalledWith(true); }); @@ -179,7 +206,7 @@ describe('ViewOptionsDropdown', () => { await userEvent.click(screen.getByTestId('view-options-button')); const checkboxes = screen.getAllByRole('checkbox'); - expect(checkboxes[2]).toBeChecked(); + expect(checkboxes[3]).toBeChecked(); }); it('calls onSimplifyPhrasesChange when toggled', async () => { @@ -194,7 +221,7 @@ describe('ViewOptionsDropdown', () => { await userEvent.click(screen.getByTestId('view-options-button')); const checkboxes = screen.getAllByRole('checkbox'); - await userEvent.click(checkboxes[2]); + await userEvent.click(checkboxes[3]); expect(onSimplifyPhrasesChange).toHaveBeenCalledWith(true); }); @@ -206,7 +233,7 @@ describe('ViewOptionsDropdown', () => { await userEvent.click(screen.getByTestId('view-options-button')); const checkboxes = screen.getAllByRole('checkbox'); - expect(checkboxes[3]).toBeChecked(); + expect(checkboxes[4]).toBeChecked(); }); it('calls onChapterLabelInVerseChange when toggled', async () => { @@ -221,7 +248,7 @@ describe('ViewOptionsDropdown', () => { await userEvent.click(screen.getByTestId('view-options-button')); const checkboxes = screen.getAllByRole('checkbox'); - await userEvent.click(checkboxes[3]); + await userEvent.click(checkboxes[4]); expect(onChapterLabelInVerseChange).toHaveBeenCalledWith(true); }); diff --git a/src/__tests__/store/analysisSlice.test.ts b/src/__tests__/store/analysisSlice.test.ts index 5c055500..51d29969 100644 --- a/src/__tests__/store/analysisSlice.test.ts +++ b/src/__tests__/store/analysisSlice.test.ts @@ -14,6 +14,7 @@ import { emptyAnalysis } from '../../types/empty-factories'; import { createPhrase, defaultState, + deleteMorphemes, deletePhrase, mergePhrases, selectApprovedGloss, @@ -645,6 +646,110 @@ describe('writeMorphemes', () => { }); }); +describe('deleteMorphemes', () => { + it('removes the morphemes but keeps the analysis when it has a gloss', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'unbelievable', + gloss: { und: 'incredible' }, + morphemes: [{ id: 'm-1', form: 'un-', writingSystem: 'und' }], + }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + store.dispatch(deleteMorphemes({ tokenRef: 'tok-1' })); + + const { tokenAnalyses, tokenAnalysisLinks } = store.getState().analysis.analysis; + expect(tokenAnalyses).toHaveLength(1); + expect(tokenAnalyses[0].morphemes).toBeUndefined(); + expect(tokenAnalyses[0].gloss).toStrictEqual({ und: 'incredible' }); + expect(tokenAnalysisLinks).toHaveLength(1); + }); + + it('removes the analysis and its link when it has no gloss', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'word', + morphemes: [{ id: 'm-1', form: 'word', writingSystem: 'und' }], + }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + store.dispatch(deleteMorphemes({ tokenRef: 'tok-1' })); + + const { tokenAnalyses, tokenAnalysisLinks } = store.getState().analysis.analysis; + expect(tokenAnalyses).toHaveLength(0); + expect(tokenAnalysisLinks).toHaveLength(0); + }); + + it('removes the analysis and its link when its gloss object is empty', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'word', + gloss: {}, + morphemes: [{ id: 'm-1', form: 'word', writingSystem: 'und' }], + }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + store.dispatch(deleteMorphemes({ tokenRef: 'tok-1' })); + + const { tokenAnalyses, tokenAnalysisLinks } = store.getState().analysis.analysis; + expect(tokenAnalyses).toHaveLength(0); + expect(tokenAnalysisLinks).toHaveLength(0); + }); + + it('no-ops when the token has no approved link', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'word', + morphemes: [{ id: 'm-1', form: 'word', writingSystem: 'und' }], + }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + store.dispatch(deleteMorphemes({ tokenRef: 'tok-other' })); + + expect(store.getState().analysis.analysis.tokenAnalyses[0].morphemes).toHaveLength(1); + }); + + it('no-ops when the approved analysis has no morphemes', () => { + const ta: TokenAnalysis = { id: 'ta-1', surfaceText: 'word', gloss: { und: 'hi' } }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + store.dispatch(deleteMorphemes({ tokenRef: 'tok-1' })); + + const { tokenAnalyses, tokenAnalysisLinks } = store.getState().analysis.analysis; + expect(tokenAnalyses).toHaveLength(1); + expect(tokenAnalyses[0].gloss).toStrictEqual({ und: 'hi' }); + expect(tokenAnalysisLinks).toHaveLength(1); + }); + + it('no-ops when the approved link references a missing analysis', () => { + const orphanLink: TokenAnalysisLink = { + analysisId: 'missing-uuid', + status: 'approved', + token: { tokenRef: 'tok-1', surfaceText: 'word' }, + }; + const store = createAnalysisStore({ + analysis: { + analysis: { ...emptyAnalysis(), tokenAnalysisLinks: [orphanLink] }, + analysisLanguage: 'und', + }, + }); + + store.dispatch(deleteMorphemes({ tokenRef: 'tok-1' })); + + expect(store.getState().analysis.analysis.tokenAnalysisLinks).toHaveLength(1); + }); +}); + describe('writeMorphemeGloss', () => { it('writes a gloss onto the specified morpheme', () => { const ta: TokenAnalysis = { diff --git a/src/components/AnalysisStore.tsx b/src/components/AnalysisStore.tsx index 0e1f7df3..947a12c6 100644 --- a/src/components/AnalysisStore.tsx +++ b/src/components/AnalysisStore.tsx @@ -10,6 +10,7 @@ import type { ReactNode } from 'react'; import { Provider as ReduxProvider, useDispatch, useSelector, useStore } from 'react-redux'; import { createPhrase, + deleteMorphemes, deletePhrase, mergePhrases, selectAnalysis, @@ -234,6 +235,32 @@ export function useMorphemeBreakdownDispatch(): ( ); } +/** + * Returns a stable callback that removes the morpheme breakdown from the approved `TokenAnalysis` + * for a given token (deleting the analysis record entirely when it carries no gloss). Dispatches + * the `deleteMorphemes` action and triggers `onSave`. + * + * @returns A function `(tokenRef) => void`. + * @throws When called outside an {@link AnalysisStoreProvider}. + */ +export function useMorphemeDeleteDispatch(): (tokenRef: string) => void { + const callbacks = useContext(AnalysisCallbackCtx); + if (!callbacks) + throw new Error('useMorphemeDeleteDispatch must be used inside an AnalysisStoreProvider'); + + const dispatch = useDispatch(); + const store = useStore(); + + return useCallback( + (tokenRef: string) => { + dispatch(deleteMorphemes({ tokenRef })); + const { analysis } = store.getState().analysis; + callbacks.onSaveRef.current?.(analysis); + }, + [dispatch, store, callbacks], + ); +} + /** * Returns a stable callback that writes a gloss on a single morpheme within the approved * `TokenAnalysis` for a given token. Dispatches the `writeMorphemeGloss` action and triggers diff --git a/src/components/MorphemeEditor.tsx b/src/components/MorphemeEditor.tsx index 3cfb1f7f..3866a2ba 100644 --- a/src/components/MorphemeEditor.tsx +++ b/src/components/MorphemeEditor.tsx @@ -4,39 +4,89 @@ * morpheme forms; {@link MorphemeGlossInput} provides a per-morpheme gloss field. */ import type { MorphemeAnalysis } from 'interlinearizer'; -import { useEffect, useRef, useState } from 'react'; +import { useEffect, useLayoutEffect, useRef, useState } from 'react'; import type { KeyboardEvent, MouseEvent } from 'react'; +import { createPortal } from 'react-dom'; import { useMorphemeGlossDispatch } from './AnalysisStore'; +/** Minimum gap in pixels between the popover panel and its anchor or the viewport edges. */ +const POPOVER_MARGIN_PX = 4; + /** * 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 or Done. + * 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, as does an outside click when the token had no breakdown yet and + * the text was not edited — committing then would create a single-morpheme breakdown equal to the + * pre-filled surface text. * * @param props - Component props. * @param props.initialValue - Pre-filled text for the input (current morpheme forms joined by * spaces, or the full surface text when no breakdown exists yet). + * @param props.hasExistingBreakdown - Whether the token already had a morpheme breakdown when the + * editor opened. When false, an outside click with an unedited draft dismisses instead of + * committing. * @param props.onSave - Called with the raw input string when the user commits. - * @param props.onClose - Called to dismiss the popover without saving. - * @returns A positioned popover panel with a text input and Done button. + * @param props.onClose - Called to dismiss the popover. + * @param props.onDelete - When provided, a Delete button is shown that calls this to remove the + * token's existing morpheme breakdown, then dismisses the popover. Callers should omit it when + * the token has no breakdown to delete. + * @returns A positioned popover panel with a text input and Cancel/Done buttons. The panel and + * backdrop are portaled to document.body with `position: fixed`, so they escape both the clipping + * of ancestor scroll viewports (e.g. the continuous view's token strip) and the `token-row` + * stacking contexts that would otherwise paint later segment rows over the panel. The panel opens + * below its anchor and flips above when there is not enough room under the viewport bottom. */ export function MorphemeBreakdownPopover({ initialValue, + hasExistingBreakdown, onSave, onClose, + onDelete, }: Readonly<{ initialValue: string; + hasExistingBreakdown: boolean; onSave: (value: string) => void; onClose: () => void; + onDelete?: () => void; }>) { const [draft, setDraft] = useState(initialValue); // eslint-disable-next-line no-null/no-null const inputRef = useRef(null); + // eslint-disable-next-line no-null/no-null + const panelRef = useRef(null); + // eslint-disable-next-line no-null/no-null + const anchorRef = useRef(null); + const [position, setPosition] = useState<{ top: number; left: number } | undefined>(undefined); useEffect(() => { inputRef.current?.focus(); inputRef.current?.select(); }, []); + // Position the fixed panel from its anchor (the parent of the in-place marker span — the + // morpheme row in the token chip). The panel itself is portaled to document.body with fixed + // positioning because the token chip lives inside scroll viewports that clip vertical overflow + // and inside `token-row` stacking contexts (z-7) that later segment rows would paint over. The + // panel opens below the anchor and flips above when the viewport bottom is too close. + useLayoutEffect(() => { + const panel = panelRef.current; + const anchor = anchorRef.current?.parentElement; + /* v8 ignore next -- the panel and the anchor's parent always exist when the effect runs */ + if (!panel || !anchor) return; + const anchorRect = anchor.getBoundingClientRect(); + const panelRect = panel.getBoundingClientRect(); + let top = anchorRect.bottom + POPOVER_MARGIN_PX; + if (top + panelRect.height > window.innerHeight - POPOVER_MARGIN_PX) { + top = Math.max(POPOVER_MARGIN_PX, anchorRect.top - panelRect.height - POPOVER_MARGIN_PX); + } + const left = Math.max( + POPOVER_MARGIN_PX, + Math.min(anchorRect.left, window.innerWidth - panelRect.width - POPOVER_MARGIN_PX), + ); + setPosition({ top, left }); + }, []); + /** Commits the current draft and closes the popover. */ const handleSave = () => { const trimmed = draft.trim(); @@ -60,43 +110,94 @@ export function MorphemeBreakdownPopover({ }; /** - * Prevents the click from bubbling to the backdrop dismiss handler. + * Stops the click from reaching ancestor click handlers and cancels the click's default action. + * The panel is portaled to document.body, so the browser's native label activation cannot fire, + * but React synthetic events still bubble through the React tree (portal boundary included) to + * the token chip and its phrase-selection handlers. * * @param e - The mouse event on the popover panel. */ - const stopPropagation = (e: MouseEvent) => { + const handlePanelClick = (e: MouseEvent) => { e.stopPropagation(); + e.preventDefault(); + }; + + /** + * Commits the draft when the user clicks outside the popover, except when the token had no + * breakdown yet and the text was not edited — then the click acts like Cancel so the pre-filled + * surface text is not committed as a single-morpheme breakdown. `preventDefault` cancels any + * default action of whatever sits under the backdrop (see {@link handlePanelClick}). + * + * @param e - The mouse event on the full-screen backdrop. + */ + const handleBackdropClick = (e: MouseEvent) => { + e.preventDefault(); + if (!hasExistingBreakdown && draft.trim() === initialValue.trim()) { + onClose(); + return; + } + handleSave(); }; return ( <> - {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} -
- {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} -
- - setDraft(e.target.value)} - onKeyDown={handleKeyDown} - type="text" - /> - -
+ {/* Invisible in-place marker; its parent is the anchor the fixed panel is positioned from. */} + + {createPortal( + <> + {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} +
+ {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} +
+ + setDraft(e.target.value)} + onKeyDown={handleKeyDown} + type="text" + /> +
+ {onDelete && ( + + )} + + +
+
+ , + document.body, + )} ); } diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index d569643b..4291591e 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -1,11 +1,12 @@ import type { Token } from 'interlinearizer'; import { X } from 'lucide-react'; -import { memo, type MouseEventHandler, useEffect, useRef, useState } from 'react'; +import { memo, type MouseEventHandler, useEffect, useId, useRef, useState } from 'react'; import { useAnalysisLanguage, useGloss, useGlossDispatch, useMorphemeBreakdownDispatch, + useMorphemeDeleteDispatch, useMorphemes, } from './AnalysisStore'; import { MorphemeBreakdownPopover, MorphemeGlossInput } from './MorphemeEditor'; @@ -19,8 +20,8 @@ import { MorphemeBreakdownPopover, MorphemeGlossInput } from './MorphemeEditor'; * * When `showMorphology` is true, a morpheme row is shown below the surface text. For unanalyzed * tokens this is a clickable button showing the surface text; for analyzed tokens it shows the - * morpheme forms. Clicking either opens an inline popover where the user can define or edit the - * morpheme breakdown. Per-morpheme gloss inputs appear below the morpheme forms. + * morpheme forms. Clicking either opens an inline popover where the user can define, edit, or + * delete the morpheme breakdown. Per-morpheme gloss inputs appear below the morpheme forms. * * @param props - Component props * @param props.token - The word token to render. @@ -54,8 +55,10 @@ export function TokenChip({ const morphemes = useMorphemes(token.ref); const analysisLanguage = useAnalysisLanguage(); const dispatchMorphemeBreakdown = useMorphemeBreakdownDispatch(); + const dispatchMorphemeDelete = useMorphemeDeleteDispatch(); const [draft, setDraft] = useState(committedGloss); const [popoverOpen, setPopoverOpen] = useState(false); + const glossInputId = useId(); // Tracks whether the X button itself is hovered, so only that button hover reddens the border. const [isRemoveHovered, setIsRemoveHovered] = useState(false); // Reset remove-hover state when onRemove is cleared so the red border doesn't linger. @@ -109,9 +112,11 @@ export function TokenChip({ const hasMorphemes = morphemes.length > 0; - // The X button is positioned outside the
, document.body, diff --git a/src/store/analysisSlice.ts b/src/store/analysisSlice.ts index 06a11d0e..9b7e47a0 100644 --- a/src/store/analysisSlice.ts +++ b/src/store/analysisSlice.ts @@ -262,6 +262,34 @@ const analysisSlice = createSlice({ state.analysis.tokenAnalysisLinks.push(newLink); }, }, + /** + * Removes the morpheme breakdown from the approved `TokenAnalysis` for the given token. When + * the analysis carries no gloss either, the now-empty analysis record and its link are removed + * entirely so empty records do not accumulate in storage. No-ops when the token has no approved + * analysis or the analysis has no morphemes. + * + * @param state - Current slice state (Immer draft). + * @param action - Action carrying the `tokenRef` whose breakdown is removed. + */ + deleteMorphemes(state, action: PayloadAction<{ tokenRef: string }>) { + const { tokenRef } = action.payload; + + const link = state.analysis.tokenAnalysisLinks.find( + (l) => l.status === 'approved' && l.token.tokenRef === tokenRef, + ); + if (!link) return; + + const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === link.analysisId); + if (!analysis?.morphemes) return; + + delete analysis.morphemes; + if (!analysis.gloss || Object.keys(analysis.gloss).length === 0) { + state.analysis.tokenAnalyses = state.analysis.tokenAnalyses.filter((ta) => ta !== analysis); + state.analysis.tokenAnalysisLinks = state.analysis.tokenAnalysisLinks.filter( + (l) => l !== link, + ); + } + }, /** * Writes a gloss string onto a single morpheme within the approved `TokenAnalysis` for the * given token. No-ops when the token has no approved analysis or the morpheme id is not found. @@ -391,6 +419,7 @@ export const { setAnalysis, writeGloss, writeMorphemes, + deleteMorphemes, writeMorphemeGloss, createPhrase, updatePhrase, From 3532e8728a68a83b8dfc563c8a66729e0824a60e Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 12 Jun 2026 13:05:25 -0600 Subject: [PATCH 03/16] Minor adjustments --- .../components/MorphemeEditor.test.tsx | 175 ++---------------- src/__tests__/components/PhraseBox.test.tsx | 25 ++- src/__tests__/store/analysisSlice.test.ts | 20 ++ src/components/MorphemeEditor.tsx | 29 ++- src/components/PhraseBox.tsx | 7 +- src/components/TokenChip.tsx | 1 - src/store/analysisSlice.ts | 11 +- 7 files changed, 93 insertions(+), 175 deletions(-) diff --git a/src/__tests__/components/MorphemeEditor.test.tsx b/src/__tests__/components/MorphemeEditor.test.tsx index 6fef8e33..0201fec4 100644 --- a/src/__tests__/components/MorphemeEditor.test.tsx +++ b/src/__tests__/components/MorphemeEditor.test.tsx @@ -13,7 +13,6 @@ describe('MorphemeBreakdownPopover', () => { it('renders with the initial value pre-filled', () => { render( { }); it('auto-focuses and selects the input on mount', () => { - render( - , - ); + render(); expect(screen.getByRole('textbox')).toHaveFocus(); }); @@ -39,12 +31,7 @@ describe('MorphemeBreakdownPopover', () => { const onSave = jest.fn(); const onClose = jest.fn(); render( - , + , ); await userEvent.click(screen.getByRole('button', { name: 'Done' })); expect(onSave).toHaveBeenCalledWith('un- believe'); @@ -53,14 +40,7 @@ describe('MorphemeBreakdownPopover', () => { it('calls onSave with the edited value', async () => { const onSave = jest.fn(); - render( - , - ); + render(); await userEvent.clear(screen.getByRole('textbox')); await userEvent.type(screen.getByRole('textbox'), 'wor -d'); await userEvent.click(screen.getByRole('button', { name: 'Done' })); @@ -70,14 +50,7 @@ describe('MorphemeBreakdownPopover', () => { it('commits on Enter key', async () => { const onSave = jest.fn(); const onClose = jest.fn(); - render( - , - ); + render(); await userEvent.keyboard('{Enter}'); expect(onSave).toHaveBeenCalledWith('test'); expect(onClose).toHaveBeenCalledTimes(1); @@ -86,14 +59,7 @@ describe('MorphemeBreakdownPopover', () => { it('dismisses without saving on Escape key', async () => { const onSave = jest.fn(); const onClose = jest.fn(); - render( - , - ); + render(); await userEvent.keyboard('{Escape}'); expect(onSave).not.toHaveBeenCalled(); expect(onClose).toHaveBeenCalledTimes(1); @@ -102,62 +68,25 @@ describe('MorphemeBreakdownPopover', () => { it('dismisses without saving when Cancel button is clicked', async () => { const onSave = jest.fn(); const onClose = jest.fn(); - render( - , - ); + render(); await userEvent.click(screen.getByRole('button', { name: 'Cancel' })); expect(onSave).not.toHaveBeenCalled(); expect(onClose).toHaveBeenCalledTimes(1); }); - it('saves when the backdrop is clicked and a breakdown already exists', async () => { - const onSave = jest.fn(); - render( - , - ); - // The backdrop is the fixed full-screen div; getByRole won't find it, so query the DOM. - const backdrop = document.querySelector('.tw\\:fixed.tw\\:inset-0'); - if (!backdrop) throw new Error('Backdrop not found'); - await userEvent.click(backdrop); - expect(onSave).toHaveBeenCalledWith('test'); - }); - it('closes when the backdrop is clicked', async () => { const onClose = jest.fn(); - render( - , - ); + render(); + // The backdrop is the fixed full-screen div; getByRole won't find it, so query the DOM. const backdrop = document.querySelector('.tw\\:fixed.tw\\:inset-0'); if (!backdrop) throw new Error('Backdrop not found'); await userEvent.click(backdrop); expect(onClose).toHaveBeenCalledTimes(1); }); - it('saves on backdrop click when no breakdown exists but the text was edited', async () => { + it('saves on backdrop click when the text was edited', async () => { const onSave = jest.fn(); - render( - , - ); + render(); await userEvent.type(screen.getByRole('textbox'), ' -er'); const backdrop = document.querySelector('.tw\\:fixed.tw\\:inset-0'); if (!backdrop) throw new Error('Backdrop not found'); @@ -165,17 +94,10 @@ describe('MorphemeBreakdownPopover', () => { expect(onSave).toHaveBeenCalledWith('test -er'); }); - it('does not save on backdrop click when no breakdown exists and the text is unchanged', async () => { + it('does not save on backdrop click when the text is unchanged', async () => { const onSave = jest.fn(); const onClose = jest.fn(); - render( - , - ); + render(); const backdrop = document.querySelector('.tw\\:fixed.tw\\:inset-0'); if (!backdrop) throw new Error('Backdrop not found'); await userEvent.click(backdrop); @@ -185,14 +107,7 @@ describe('MorphemeBreakdownPopover', () => { it('does not save on backdrop click when the input is only whitespace', async () => { const onSave = jest.fn(); - render( - , - ); + render(); const backdrop = document.querySelector('.tw\\:fixed.tw\\:inset-0'); if (!backdrop) throw new Error('Backdrop not found'); await userEvent.click(backdrop); @@ -201,14 +116,7 @@ describe('MorphemeBreakdownPopover', () => { it('does not dismiss when clicking inside the popover panel', async () => { const onClose = jest.fn(); - render( - , - ); + render(); const label = screen.getByText('Split into morphemes'); await userEvent.click(label); expect(onClose).not.toHaveBeenCalled(); @@ -216,41 +124,20 @@ describe('MorphemeBreakdownPopover', () => { it('does not call onSave when the input is empty', async () => { const onSave = jest.fn(); - render( - , - ); + render(); await userEvent.click(screen.getByRole('button', { name: 'Done' })); expect(onSave).not.toHaveBeenCalled(); }); it('does not call onSave when the input is only whitespace', async () => { const onSave = jest.fn(); - render( - , - ); + render(); await userEvent.click(screen.getByRole('button', { name: 'Done' })); expect(onSave).not.toHaveBeenCalled(); }); it('does not render a Delete button when onDelete is not provided', () => { - render( - , - ); + render(); expect(screen.queryByRole('button', { name: 'Delete' })).not.toBeInTheDocument(); }); @@ -260,7 +147,6 @@ describe('MorphemeBreakdownPopover', () => { const onClose = jest.fn(); render( { }); it('portals the panel to document.body so segment rows cannot stack above it', () => { - render( - , - ); + render(); const panel = screen.getByText('Split into morphemes').closest('div'); expect(panel?.parentElement).toBe(document.body); }); @@ -292,14 +171,7 @@ describe('MorphemeBreakdownPopover', () => { .spyOn(Element.prototype, 'getBoundingClientRect') .mockReturnValueOnce(new DOMRect(50, 100, 40, 20)) .mockReturnValueOnce(new DOMRect(0, 0, 200, 100)); - render( - , - ); + render(); const panel = screen.getByText('Split into morphemes').closest('div'); // Anchor bottom (120) plus the 4px margin. expect(panel).toHaveStyle({ top: '124px', left: '50px' }); @@ -312,14 +184,7 @@ describe('MorphemeBreakdownPopover', () => { .spyOn(Element.prototype, 'getBoundingClientRect') .mockReturnValueOnce(new DOMRect(50, 700, 40, 20)) .mockReturnValueOnce(new DOMRect(0, 0, 200, 100)); - render( - , - ); + render(); const panel = screen.getByText('Split into morphemes').closest('div'); // Anchor top (700) minus panel height (100) minus the 4px margin. expect(panel).toHaveStyle({ top: '596px', left: '50px' }); diff --git a/src/__tests__/components/PhraseBox.test.tsx b/src/__tests__/components/PhraseBox.test.tsx index 4d6c6193..2e7cd815 100644 --- a/src/__tests__/components/PhraseBox.test.tsx +++ b/src/__tests__/components/PhraseBox.test.tsx @@ -58,6 +58,8 @@ jest.mock('../../components/TokenChip', () => { * @param props.token - The word token to render. * @param props.isSplitFree - When true, marks the chip as a would-be-free token. * @param props.onRemove - Called when the remove button is clicked; omitted for edge tokens. + * @param props.showMorphology - Exposed as a data attribute so tests can verify every PhraseBox + * render path forwards the strip-wide morphology toggle. * @returns A span containing the surface text, a gloss input, and an optional remove button. */ function MockTokenChip({ @@ -65,16 +67,22 @@ jest.mock('../../components/TokenChip', () => { token, isSplitFree, onRemove, + showMorphology, }: Readonly<{ onFocus?: () => void; token: Token; isSplitFree?: boolean; onRemove?: () => void; + showMorphology?: boolean; }>) { const gloss = mockUseGloss(token.ref); const dispatch = mockUseGlossDispatch(); return ( - + {token.surfaceText} { expect(screen.getByTestId('inert-punct-1')).toBeInTheDocument(); }); + it('forwards showMorphology to chips in a non-edit-target box during edit mode', () => { + renderBox( + , + // Edit mode is active for a different phrase, so this free box renders via the fallback + // path; its chips must keep their morpheme rows rather than collapsing while editing. + { + phraseMode: { kind: 'edit', phraseId: 'other-phrase', originalTokens: [] }, + showMorphology: true, + }, + ); + + expect(screen.getByTestId('token-token-1')).toHaveAttribute('data-show-morphology', 'true'); + expect(screen.getByTestId('token-token-2')).toHaveAttribute('data-show-morphology', 'true'); + }); + it('renders punctuation between tokens in confirm-unlink mode', () => { mockUsePhraseLinkForToken.mockReturnValue(TEST_PHRASE_LINK); renderBox( diff --git a/src/__tests__/store/analysisSlice.test.ts b/src/__tests__/store/analysisSlice.test.ts index 51d29969..21c01ff7 100644 --- a/src/__tests__/store/analysisSlice.test.ts +++ b/src/__tests__/store/analysisSlice.test.ts @@ -600,6 +600,26 @@ describe('writeMorphemes', () => { expect(updated?.morphemes?.[2].form).toBe('-able'); }); + it('preserves distinct glosses on duplicate morpheme forms in order (reduplication)', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'baba', + morphemes: [ + { id: 'm-1', form: 'ba', writingSystem: 'und', gloss: { und: 'first' } }, + { id: 'm-2', form: 'ba', writingSystem: 'und', gloss: { und: 'second' } }, + ], + }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + store.dispatch(writeMorphemes('tok-1', 'baba', ['ba', 'ba'])); + + const updated = store.getState().analysis.analysis.tokenAnalyses.find((a) => a.id === 'ta-1'); + expect(updated?.morphemes?.[0].gloss).toStrictEqual({ und: 'first' }); + expect(updated?.morphemes?.[1].gloss).toStrictEqual({ und: 'second' }); + }); + it('removes an orphaned approved link and creates a fresh analysis', () => { const orphanLink: TokenAnalysisLink = { analysisId: 'old-uuid', diff --git a/src/components/MorphemeEditor.tsx b/src/components/MorphemeEditor.tsx index 3866a2ba..fd225f6a 100644 --- a/src/components/MorphemeEditor.tsx +++ b/src/components/MorphemeEditor.tsx @@ -4,7 +4,7 @@ * morpheme forms; {@link MorphemeGlossInput} provides a per-morpheme gloss field. */ import type { MorphemeAnalysis } from 'interlinearizer'; -import { useEffect, useLayoutEffect, useRef, useState } from 'react'; +import { useEffect, useId, useLayoutEffect, useRef, useState } from 'react'; import type { KeyboardEvent, MouseEvent } from 'react'; import { createPortal } from 'react-dom'; import { useMorphemeGlossDispatch } from './AnalysisStore'; @@ -16,16 +16,14 @@ const POPOVER_MARGIN_PX = 4; * 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, as does an outside click when the token had no breakdown yet and - * the text was not edited — committing then would create a single-morpheme breakdown equal to the - * pre-filled surface text. + * Escape dismiss without saving, as does an outside click when the text was not edited — committing + * an unedited draft would either create a single-morpheme breakdown equal to the pre-filled surface + * text (no existing breakdown) or pointlessly re-save identical forms under fresh morpheme ids + * (existing breakdown). * * @param props - Component props. * @param props.initialValue - Pre-filled text for the input (current morpheme forms joined by * spaces, or the full surface text when no breakdown exists yet). - * @param props.hasExistingBreakdown - Whether the token already had a morpheme breakdown when the - * editor opened. When false, an outside click with an unedited draft dismisses instead of - * committing. * @param props.onSave - Called with the raw input string when the user commits. * @param props.onClose - Called to dismiss the popover. * @param props.onDelete - When provided, a Delete button is shown that calls this to remove the @@ -39,17 +37,16 @@ const POPOVER_MARGIN_PX = 4; */ export function MorphemeBreakdownPopover({ initialValue, - hasExistingBreakdown, onSave, onClose, onDelete, }: Readonly<{ initialValue: string; - hasExistingBreakdown: boolean; onSave: (value: string) => void; onClose: () => void; onDelete?: () => void; }>) { + const inputId = useId(); const [draft, setDraft] = useState(initialValue); // eslint-disable-next-line no-null/no-null const inputRef = useRef(null); @@ -123,16 +120,18 @@ export function MorphemeBreakdownPopover({ }; /** - * Commits the draft when the user clicks outside the popover, except when the token had no - * breakdown yet and the text was not edited — then the click acts like Cancel so the pre-filled - * surface text is not committed as a single-morpheme breakdown. `preventDefault` cancels any + * Commits the draft when the user clicks outside the popover, except when the text was not edited + * — then the click acts like Cancel. For a token with no breakdown yet, committing the unedited + * draft would create a single-morpheme breakdown equal to the pre-filled surface text; for a + * token with an existing breakdown, it would re-save identical forms while regenerating every + * morpheme id (which `MorphemeLink.morphemeId` cross-references). `preventDefault` cancels any * default action of whatever sits under the backdrop (see {@link handlePanelClick}). * * @param e - The mouse event on the full-screen backdrop. */ const handleBackdropClick = (e: MouseEvent) => { e.preventDefault(); - if (!hasExistingBreakdown && draft.trim() === initialValue.trim()) { + if (draft.trim() === initialValue.trim()) { onClose(); return; } @@ -154,13 +153,13 @@ export function MorphemeBreakdownPopover({ style={position ?? { visibility: 'hidden' }} onClick={handlePanelClick} > - diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index 4291591e..556fcc25 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -186,7 +186,6 @@ export function TokenChip({ )} {popoverOpen && ( m.form).join(' ') : token.surfaceText } diff --git a/src/store/analysisSlice.ts b/src/store/analysisSlice.ts index 9b7e47a0..5f2e4c55 100644 --- a/src/store/analysisSlice.ts +++ b/src/store/analysisSlice.ts @@ -235,9 +235,16 @@ const analysisSlice = createSlice({ (ta) => ta.id === existingLink.analysisId, ); if (existingAnalysis) { - const oldByForm = new Map((existingAnalysis.morphemes ?? []).map((m) => [m.form, m])); + // Multimap with consumed entries so duplicate forms (e.g. reduplication "ba ba") each + // match a distinct old morpheme in order, instead of all inheriting the last one. + const oldByForm = new Map(); + (existingAnalysis.morphemes ?? []).forEach((m) => { + const bucket = oldByForm.get(m.form); + if (bucket) bucket.push(m); + else oldByForm.set(m.form, [m]); + }); existingAnalysis.morphemes = morphemes.map(({ id, form }) => { - const old = oldByForm.get(form); + const old = oldByForm.get(form)?.shift(); if (old) return { ...old, id }; return { id, form, writingSystem }; }); From 2ecc365beaa5467fc49f4a98fcd74cf5b1619042 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 12 Jun 2026 13:41:32 -0600 Subject: [PATCH 04/16] Add writing system to morpheme pipeline, fix input focus issue, prevent morpheme ID regeneration on unedited done/enter --- .../components/AnalysisStore.test.tsx | 14 +++++-- .../components/MorphemeEditor.test.tsx | 31 +++++++++++++++ src/__tests__/components/TokenChip.test.tsx | 38 +++++++++++++++++- src/__tests__/store/analysisSlice.test.ts | 39 ++++++++++++++++--- src/components/AnalysisStore.tsx | 9 +++-- src/components/MorphemeEditor.tsx | 33 ++++++++++------ src/components/TokenChip.tsx | 13 ++++--- src/components/__mocks__/AnalysisStore.tsx | 1 + src/store/analysisSlice.ts | 15 ++++--- 9 files changed, 158 insertions(+), 35 deletions(-) diff --git a/src/__tests__/components/AnalysisStore.test.tsx b/src/__tests__/components/AnalysisStore.test.tsx index adca6cda..ad57a392 100644 --- a/src/__tests__/components/AnalysisStore.test.tsx +++ b/src/__tests__/components/AnalysisStore.test.tsx @@ -850,16 +850,18 @@ function LanguageReader() { * @param props.tokenRef - Token ref to write. * @param props.surfaceText - Surface text of the token. * @param props.forms - Morpheme forms to write. + * @param props.writingSystem - Writing system of the token's surface text. * @returns JSX element suitable for passing to `render`. */ function MorphemeWriter({ tokenRef, surfaceText, forms, -}: Readonly<{ tokenRef: string; surfaceText: string; forms: string[] }>) { + writingSystem, +}: Readonly<{ tokenRef: string; surfaceText: string; forms: string[]; writingSystem: string }>) { const dispatch = useMorphemeBreakdownDispatch(); return ( - ); @@ -1023,7 +1025,12 @@ describe('useMorphemeBreakdownDispatch', () => { const onSave = jest.fn(); render( - + , ); @@ -1035,6 +1042,7 @@ describe('useMorphemeBreakdownDispatch', () => { const saved: TextAnalysis = onSave.mock.calls[0][0]; expect(saved.tokenAnalyses).toHaveLength(1); expect(saved.tokenAnalyses[0].morphemes).toHaveLength(2); + expect(saved.tokenAnalyses[0].morphemes?.[0].writingSystem).toBe('en'); }); it('throws when called outside an AnalysisStoreProvider', () => { diff --git a/src/__tests__/components/MorphemeEditor.test.tsx b/src/__tests__/components/MorphemeEditor.test.tsx index 0201fec4..8742d6a9 100644 --- a/src/__tests__/components/MorphemeEditor.test.tsx +++ b/src/__tests__/components/MorphemeEditor.test.tsx @@ -47,6 +47,37 @@ describe('MorphemeBreakdownPopover', () => { expect(onSave).toHaveBeenCalledWith('wor -d'); }); + 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( + , + ); + await userEvent.click(screen.getByRole('button', { name: 'Done' })); + expect(onSave).not.toHaveBeenCalled(); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('saves when Done is clicked with edited text and an existing breakdown', async () => { + const onSave = jest.fn(); + render( + , + ); + 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 () => { const onSave = jest.fn(); const onClose = jest.fn(); diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index 633de690..cb5a875c 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -470,7 +470,7 @@ describe('TokenChip', () => { screen.getByRole('button', { name: 'Define morpheme breakdown for hello' }), ); await userEvent.click(screen.getByRole('button', { name: 'mock-save' })); - expect(mockDispatch).toHaveBeenCalledWith('GEN 1:1:0', 'hello', ['hel', '-lo']); + expect(mockDispatch).toHaveBeenCalledWith('GEN 1:1:0', 'hello', ['hel', '-lo'], 'en'); }); it('does not dispatch when the popover saves only whitespace', async () => { @@ -523,6 +523,42 @@ describe('TokenChip', () => { expect(screen.queryByRole('button', { name: 'mock-delete' })).not.toBeInTheDocument(); }); + it('focuses the main gloss input on a surface-text mouse-down when morpheme inputs precede it', () => { + // AnalysisStore imported at top level + jest + .spyOn(AnalysisStore, 'useMorphemes') + .mockReturnValue([{ id: 'm-1', form: 'hel', writingSystem: 'und' }]); + render( + + + , + ); + + // The morpheme gloss inputs sit before the main gloss input inside the label; the label + // handler must still route focus to the main gloss input, not the first input it finds. + fireEvent.mouseDown(screen.getByText('hello')); + + expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toHaveFocus(); + }); + + it('leaves a mouse-down on the morpheme button to the button itself', () => { + const focusSpy = jest.spyOn(HTMLElement.prototype, 'focus'); + render( + + + , + ); + + // The button opens the popover via its own click handler; the label handler must not focus + // an input as a side effect of the same mouse-down. + const defaultAllowed = fireEvent.mouseDown( + screen.getByRole('button', { name: 'Define morpheme breakdown for hello' }), + ); + + expect(defaultAllowed).toBe(true); + expect(focusSpy).not.toHaveBeenCalled(); + }); + it('closes the popover via onClose', async () => { render( diff --git a/src/__tests__/store/analysisSlice.test.ts b/src/__tests__/store/analysisSlice.test.ts index 21c01ff7..2122dc46 100644 --- a/src/__tests__/store/analysisSlice.test.ts +++ b/src/__tests__/store/analysisSlice.test.ts @@ -552,7 +552,7 @@ describe('selectPhraseGloss', () => { describe('writeMorphemes', () => { it('creates a new approved analysis with morphemes when none exists', () => { const store = createAnalysisStore(); - store.dispatch(writeMorphemes('tok-1', 'unbelievable', ['un-', 'believe', '-able'])); + store.dispatch(writeMorphemes('tok-1', 'unbelievable', ['un-', 'believe', '-able'], 'und')); const { tokenAnalyses, tokenAnalysisLinks } = store.getState().analysis.analysis; expect(tokenAnalysisLinks).toHaveLength(1); @@ -581,7 +581,7 @@ describe('writeMorphemes', () => { analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, }); - store.dispatch(writeMorphemes('tok-1', 'unbelievable', ['un-', 'believ', '-able'])); + store.dispatch(writeMorphemes('tok-1', 'unbelievable', ['un-', 'believ', '-able'], 'und')); const updated = store .getState() @@ -613,7 +613,7 @@ describe('writeMorphemes', () => { analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, }); - store.dispatch(writeMorphemes('tok-1', 'baba', ['ba', 'ba'])); + store.dispatch(writeMorphemes('tok-1', 'baba', ['ba', 'ba'], 'und')); const updated = store.getState().analysis.analysis.tokenAnalyses.find((a) => a.id === 'ta-1'); expect(updated?.morphemes?.[0].gloss).toStrictEqual({ und: 'first' }); @@ -633,7 +633,7 @@ describe('writeMorphemes', () => { }, }); - store.dispatch(writeMorphemes('tok-1', 'word', ['wor', '-d'])); + store.dispatch(writeMorphemes('tok-1', 'word', ['wor', '-d'], 'und')); const { tokenAnalyses, tokenAnalysisLinks } = store.getState().analysis.analysis; expect(tokenAnalysisLinks).toHaveLength(1); @@ -649,7 +649,7 @@ describe('writeMorphemes', () => { analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, }); - store.dispatch(writeMorphemes('tok-1', 'hello', ['hel', '-lo'])); + store.dispatch(writeMorphemes('tok-1', 'hello', ['hel', '-lo'], 'und')); const updated = store.getState().analysis.analysis.tokenAnalyses.find((a) => a.id === 'ta-1'); expect(updated?.morphemes).toHaveLength(2); @@ -658,12 +658,39 @@ describe('writeMorphemes', () => { it('assigns unique ids to each morpheme via prepare', () => { const store = createAnalysisStore(); - store.dispatch(writeMorphemes('tok-1', 'abc', ['a', 'b', 'c'])); + store.dispatch(writeMorphemes('tok-1', 'abc', ['a', 'b', 'c'], 'und')); const ta = store.getState().analysis.analysis.tokenAnalyses[0]; const ids = ta.morphemes?.map((m) => m.id); expect(new Set(ids).size).toBe(3); }); + + it('stores the passed writing system on new morphemes, not the analysis language', () => { + const store = createAnalysisStore({ + analysis: { analysis: emptyAnalysis(), analysisLanguage: 'en' }, + }); + store.dispatch(writeMorphemes('tok-1', 'λόγος', ['λόγ', '-ος'], 'grc')); + + const ta = store.getState().analysis.analysis.tokenAnalyses[0]; + expect(ta.morphemes?.map((m) => m.writingSystem)).toStrictEqual(['grc', 'grc']); + }); + + it('refreshes the writing system on preserved morphemes whose form is unchanged', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'λόγος', + morphemes: [{ id: 'm-1', form: 'λόγ', writingSystem: 'en', gloss: { en: 'word' } }], + }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'en' }, + }); + + store.dispatch(writeMorphemes('tok-1', 'λόγος', ['λόγ', '-ος'], 'grc')); + + const updated = store.getState().analysis.analysis.tokenAnalyses.find((a) => a.id === 'ta-1'); + expect(updated?.morphemes?.[0].gloss).toStrictEqual({ en: 'word' }); + expect(updated?.morphemes?.[0].writingSystem).toBe('grc'); + }); }); describe('deleteMorphemes', () => { diff --git a/src/components/AnalysisStore.tsx b/src/components/AnalysisStore.tsx index 947a12c6..10ea3b43 100644 --- a/src/components/AnalysisStore.tsx +++ b/src/components/AnalysisStore.tsx @@ -210,13 +210,16 @@ export function useGlossDispatch(): (tokenRef: string, surfaceText: string, valu * Returns a stable callback that replaces the morpheme breakdown on the approved `TokenAnalysis` * for a given token. Dispatches the `writeMorphemes` action and triggers `onSave`. * - * @returns A function `(tokenRef, surfaceText, forms) => void`. + * @returns A function `(tokenRef, surfaceText, forms, writingSystem) => void`, where + * `writingSystem` is the BCP 47 tag of the token's surface text (`Token.writingSystem`), stored + * on each morpheme as the writing system of its form. * @throws When called outside an {@link AnalysisStoreProvider}. */ export function useMorphemeBreakdownDispatch(): ( tokenRef: string, surfaceText: string, forms: string[], + writingSystem: string, ) => void { const callbacks = useContext(AnalysisCallbackCtx); if (!callbacks) @@ -226,8 +229,8 @@ export function useMorphemeBreakdownDispatch(): ( const store = useStore(); return useCallback( - (tokenRef: string, surfaceText: string, forms: string[]) => { - dispatch(writeMorphemes(tokenRef, surfaceText, forms)); + (tokenRef: string, surfaceText: string, forms: string[], writingSystem: string) => { + dispatch(writeMorphemes(tokenRef, surfaceText, forms, writingSystem)); const { analysis } = store.getState().analysis; callbacks.onSaveRef.current?.(analysis); }, diff --git a/src/components/MorphemeEditor.tsx b/src/components/MorphemeEditor.tsx index fd225f6a..e2170f8c 100644 --- a/src/components/MorphemeEditor.tsx +++ b/src/components/MorphemeEditor.tsx @@ -16,10 +16,12 @@ const POPOVER_MARGIN_PX = 4; * 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, as does an outside click when the text was not edited — committing - * an unedited draft would either create a single-morpheme breakdown equal to the pre-filled surface - * text (no existing breakdown) or pointlessly re-save identical forms under fresh morpheme ids - * (existing breakdown). + * 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 + * regenerate every morpheme id (which `MorphemeLink.morphemeId` cross-references). 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. * * @param props - Component props. * @param props.initialValue - Pre-filled text for the input (current morpheme forms joined by @@ -28,7 +30,8 @@ const POPOVER_MARGIN_PX = 4; * @param props.onClose - Called to dismiss the popover. * @param props.onDelete - When provided, a Delete button is shown that calls this to remove the * token's existing morpheme breakdown, then dismisses the popover. Callers should omit it when - * the token has no breakdown to delete. + * 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. * @returns A positioned popover panel with a text input and Cancel/Done buttons. The panel and * backdrop are portaled to document.body with `position: fixed`, so they escape both the clipping * of ancestor scroll viewports (e.g. the continuous view's token strip) and the `token-row` @@ -84,10 +87,16 @@ export function MorphemeBreakdownPopover({ setPosition({ top, left }); }, []); - /** Commits the current draft and closes the popover. */ + /** + * Commits the current draft and closes the popover. Skips the save when the token already has a + * breakdown (`onDelete` provided) and the text was not edited — re-saving identical forms would + * only regenerate every morpheme id, which `MorphemeLink.morphemeId` cross-references. An + * unedited commit on a token with _no_ breakdown is kept: it deliberately records the token as a + * single whole-word morpheme. + */ const handleSave = () => { const trimmed = draft.trim(); - if (trimmed) onSave(trimmed); + if (trimmed && !(onDelete && trimmed === initialValue.trim())) onSave(trimmed); onClose(); }; @@ -121,11 +130,11 @@ export function MorphemeBreakdownPopover({ /** * Commits the draft when the user clicks outside the popover, except when the text was not edited - * — then the click acts like Cancel. For a token with no breakdown yet, committing the unedited - * draft would create a single-morpheme breakdown equal to the pre-filled surface text; for a - * token with an existing breakdown, it would re-save identical forms while regenerating every - * morpheme id (which `MorphemeLink.morphemeId` cross-references). `preventDefault` cancels any - * default action of whatever sits under the backdrop (see {@link handlePanelClick}). + * — then the click 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. `preventDefault` cancels any default action of whatever sits under the + * backdrop (see {@link handlePanelClick}). * * @param e - The mouse event on the full-screen backdrop. */ diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index 556fcc25..d15b1ade 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -87,15 +87,18 @@ export function TokenChip({ * activation — which forwards focus to the gloss input with the browser's default * scroll-into-view — can never scroll the list under the click. Focuses the input directly with * `preventScroll` instead; the native forwarding then finds it already focused and does nothing. - * A mouse-down on the input itself is left to {@link handleMouseDown}, which bubbles here after - * already handling it. + * The input is looked up by id rather than `querySelector('input')` because the morpheme gloss + * inputs precede it inside the label when morphology is shown. A mouse-down on any input is left + * to that input's own handling ({@link handleMouseDown} for the gloss input, which bubbles here + * after already handling it); a mouse-down on the morpheme trigger button is left to the button's + * own click handler, which opens the popover. * * @param e - The label's mouse-down event. */ const handleLabelMouseDown: MouseEventHandler = (e) => { - if (e.target instanceof Element && e.target.closest('input')) return; + if (e.target instanceof Element && e.target.closest('input, button')) return; e.preventDefault(); - e.currentTarget.querySelector('input')?.focus({ preventScroll: true }); + document.getElementById(glossInputId)?.focus({ preventScroll: true }); }; /** @@ -106,7 +109,7 @@ export function TokenChip({ const handleMorphemeSave = (value: string) => { const forms = value.split(/\s+/).filter(Boolean); if (forms.length > 0) { - dispatchMorphemeBreakdown(token.ref, token.surfaceText, forms); + dispatchMorphemeBreakdown(token.ref, token.surfaceText, forms, token.writingSystem); } }; diff --git a/src/components/__mocks__/AnalysisStore.tsx b/src/components/__mocks__/AnalysisStore.tsx index 80dd6d55..76007a9a 100644 --- a/src/components/__mocks__/AnalysisStore.tsx +++ b/src/components/__mocks__/AnalysisStore.tsx @@ -105,6 +105,7 @@ export function useMorphemeBreakdownDispatch(): ( tokenRef: string, surfaceText: string, forms: string[], + writingSystem: string, ) => void { return () => {}; } diff --git a/src/store/analysisSlice.ts b/src/store/analysisSlice.ts index 5f2e4c55..00ed0d2b 100644 --- a/src/store/analysisSlice.ts +++ b/src/store/analysisSlice.ts @@ -194,13 +194,16 @@ const analysisSlice = createSlice({ * @param tokenRef - `Token.ref` of the token whose morphemes are being set. * @param surfaceText - Surface text of the token. * @param forms - Ordered morpheme form strings as entered by the user. + * @param writingSystem - BCP 47 tag of the token's surface text (`Token.writingSystem`), + * stored on each morpheme as the writing system of its form. * @returns The prepared action payload. */ - prepare(tokenRef: string, surfaceText: string, forms: string[]) { + prepare(tokenRef: string, surfaceText: string, forms: string[], writingSystem: string) { return { payload: { tokenRef, surfaceText, + writingSystem, analysisId: crypto.randomUUID(), morphemes: forms.map((form) => ({ id: crypto.randomUUID(), form })), }, @@ -209,7 +212,9 @@ const analysisSlice = createSlice({ /** * Sets the morpheme breakdown on the approved `TokenAnalysis` for the given token. Preserves * existing morpheme glosses when a morpheme form is unchanged. When no approved analysis - * exists, creates one. + * exists, creates one. Every morpheme — preserved or new — is stamped with the supplied + * writing system, so records written before the writing system was threaded through (which + * wrongly stored the analysis language) self-correct on the next save. * * @param state - Current slice state (Immer draft). * @param action - Action carrying the morpheme payload. @@ -219,12 +224,12 @@ const analysisSlice = createSlice({ action: PayloadAction<{ tokenRef: string; surfaceText: string; + writingSystem: string; analysisId: string; morphemes: Array<{ id: string; form: string }>; }>, ) { - const { tokenRef, surfaceText, analysisId, morphemes } = action.payload; - const writingSystem = state.analysisLanguage; + const { tokenRef, surfaceText, writingSystem, analysisId, morphemes } = action.payload; const existingLink = state.analysis.tokenAnalysisLinks.find( (l) => l.status === 'approved' && l.token.tokenRef === tokenRef, @@ -245,7 +250,7 @@ const analysisSlice = createSlice({ }); existingAnalysis.morphemes = morphemes.map(({ id, form }) => { const old = oldByForm.get(form)?.shift(); - if (old) return { ...old, id }; + if (old) return { ...old, id, writingSystem }; return { id, form, writingSystem }; }); return; From 36151c1581269aa1f9202cea233a9692f2152736 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 12 Jun 2026 13:55:52 -0600 Subject: [PATCH 05/16] Minor adjustments --- src/__tests__/store/analysisSlice.test.ts | 17 +++++++++++++++++ src/components/MorphemeEditor.tsx | 12 ++++++------ src/components/__mocks__/AnalysisStore.tsx | 15 ++++++++++----- src/store/analysisSlice.ts | 10 +++++++--- 4 files changed, 40 insertions(+), 14 deletions(-) diff --git a/src/__tests__/store/analysisSlice.test.ts b/src/__tests__/store/analysisSlice.test.ts index 2122dc46..6fd18f93 100644 --- a/src/__tests__/store/analysisSlice.test.ts +++ b/src/__tests__/store/analysisSlice.test.ts @@ -643,6 +643,23 @@ describe('writeMorphemes', () => { expect(ta?.morphemes).toHaveLength(2); }); + it('refreshes the surface text on the analysis and the link snapshot when the token text changed', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'word', + morphemes: [{ id: 'm-1', form: 'word', writingSystem: 'und' }], + }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + store.dispatch(writeMorphemes('tok-1', 'words', ['word', '-s'], 'und')); + + const { tokenAnalyses, tokenAnalysisLinks } = store.getState().analysis.analysis; + expect(tokenAnalyses.find((a) => a.id === 'ta-1')?.surfaceText).toBe('words'); + expect(tokenAnalysisLinks[0].token.surfaceText).toBe('words'); + }); + it('adds morphemes to an existing approved analysis that has no morphemes', () => { const ta: TokenAnalysis = { id: 'ta-1', surfaceText: 'hello' }; const store = createAnalysisStore({ diff --git a/src/components/MorphemeEditor.tsx b/src/components/MorphemeEditor.tsx index e2170f8c..5d9249f3 100644 --- a/src/components/MorphemeEditor.tsx +++ b/src/components/MorphemeEditor.tsx @@ -116,16 +116,16 @@ export function MorphemeBreakdownPopover({ }; /** - * Stops the click from reaching ancestor click handlers and cancels the click's default action. - * The panel is portaled to document.body, so the browser's native label activation cannot fire, - * but React synthetic events still bubble through the React tree (portal boundary included) to - * the token chip and its phrase-selection handlers. + * Stops the click from reaching ancestor click handlers. The panel is portaled to document.body, + * so the browser's native label activation cannot fire, but React synthetic events still bubble + * through the React tree (portal boundary included) to the token chip and its phrase-selection + * handlers. The click's default action is left alone so interactions inside the panel (e.g. the + * panel's own label focusing its input) keep their native behavior. * * @param e - The mouse event on the popover panel. */ const handlePanelClick = (e: MouseEvent) => { e.stopPropagation(); - e.preventDefault(); }; /** @@ -134,7 +134,7 @@ export function MorphemeBreakdownPopover({ * 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. `preventDefault` cancels any default action of whatever sits under the - * backdrop (see {@link handlePanelClick}). + * backdrop. * * @param e - The mouse event on the full-screen backdrop. */ diff --git a/src/components/__mocks__/AnalysisStore.tsx b/src/components/__mocks__/AnalysisStore.tsx index 76007a9a..d32e2bb0 100644 --- a/src/components/__mocks__/AnalysisStore.tsx +++ b/src/components/__mocks__/AnalysisStore.tsx @@ -8,8 +8,9 @@ type GlossMap = Record; type MockCtxValue = { glosses: GlossMap; dispatch: (tokenRef: string, surfaceText: string, value: string) => void; + language: string; }; -const MockCtx = createContext({ glosses: {}, dispatch: () => {} }); +const MockCtx = createContext({ glosses: {}, dispatch: () => {}, language: 'und' }); /** * Test-only provider that seeds glosses from `initialAnalysis` and keeps them in local state, @@ -51,7 +52,10 @@ export function AnalysisStoreProvider({ }, [onGlossChange], ); - const ctx = useMemo(() => ({ glosses, dispatch }), [glosses, dispatch]); + const ctx = useMemo( + () => ({ glosses, dispatch, language: analysisLanguage }), + [glosses, dispatch, analysisLanguage], + ); return {children}; } @@ -88,12 +92,13 @@ export function useMorphemes(_tokenRef: string): readonly MorphemeAnalysis[] { } /** - * Returns the analysis language string from mock context. + * Returns the analysis language string from mock context, mirroring the `analysisLanguage` prop + * passed to the mock provider. * - * @returns The BCP 47 tag `'und'`. + * @returns The BCP 47 tag from mock context (defaults to `'und'` outside a provider). */ export function useAnalysisLanguage(): string { - return 'und'; + return useContext(MockCtx).language; } /** diff --git a/src/store/analysisSlice.ts b/src/store/analysisSlice.ts index 00ed0d2b..348df136 100644 --- a/src/store/analysisSlice.ts +++ b/src/store/analysisSlice.ts @@ -212,9 +212,11 @@ const analysisSlice = createSlice({ /** * Sets the morpheme breakdown on the approved `TokenAnalysis` for the given token. Preserves * existing morpheme glosses when a morpheme form is unchanged. When no approved analysis - * exists, creates one. Every morpheme — preserved or new — is stamped with the supplied - * writing system, so records written before the writing system was threaded through (which - * wrongly stored the analysis language) self-correct on the next save. + * exists, creates one. Also refreshes the stored surface text on both the analysis and the + * link's token snapshot, so neither goes stale when the baseline text changed since the + * analysis was first written. Every morpheme — preserved or new — is stamped with the + * supplied writing system, so records written before the writing system was threaded through + * (which wrongly stored the analysis language) self-correct on the next save. * * @param state - Current slice state (Immer draft). * @param action - Action carrying the morpheme payload. @@ -240,6 +242,8 @@ const analysisSlice = createSlice({ (ta) => ta.id === existingLink.analysisId, ); if (existingAnalysis) { + existingAnalysis.surfaceText = surfaceText; + existingLink.token.surfaceText = surfaceText; // Multimap with consumed entries so duplicate forms (e.g. reduplication "ba ba") each // match a distinct old morpheme in order, instead of all inheriting the last one. const oldByForm = new Map(); From 20aeccd7787e02e5e4048f5bd26b74e704f1e303 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 12 Jun 2026 15:46:10 -0600 Subject: [PATCH 06/16] Migrate MorphemeEditor to Platform.Bible popover, clean up analysisSlice, other minor improvements --- __mocks__/platform-bible-react.tsx | 102 +++++++- .../components/MorphemeEditor.test.tsx | 97 ++++--- src/__tests__/components/PhraseBox.test.tsx | 41 ++- src/__tests__/store/analysisSlice.test.ts | 143 ++++++++++- src/components/MorphemeEditor.tsx | 224 +++++++---------- src/components/PhraseBox.tsx | 10 +- src/components/TokenChip.tsx | 91 ++++--- src/store/analysisSlice.ts | 238 ++++++++++-------- 8 files changed, 615 insertions(+), 331 deletions(-) diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index 496ac0b3..c9373693 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -3,8 +3,8 @@ * without extra transform configuration. This stub provides the subset used by the extension. */ -import { forwardRef } from 'react'; -import type { ReactElement, ReactNode } from 'react'; +import { forwardRef, useEffect, useRef } from 'react'; +import type { MouseEventHandler, ReactElement, ReactNode } from 'react'; export interface MenuItemContainingCommand { label: `%${string}%`; @@ -323,6 +323,104 @@ export function Switch({ ); } +/** + * Stub popover root that renders its children unconditionally. The extension conditionally mounts + * the content component while open (so its draft state re-initializes per open), so visibility + * needs no simulation here. + * + * @param props - Component props. + * @param props.children - Anchor and (while open) content elements. + * @returns The children unchanged. + */ +export function Popover({ + children, +}: Readonly<{ children?: ReactNode; open?: boolean; modal?: boolean }>): ReactElement { + return <>{children}; +} + +/** + * Stub popover anchor that renders its children as-is, matching the real component's `asChild` + * pass-through behavior. + * + * @param props - Component props. + * @param props.children - The element the popover is anchored to. + * @returns The children unchanged. + */ +export function PopoverAnchor({ + children, +}: Readonly<{ children?: ReactNode; asChild?: boolean }>): ReactElement { + return <>{children}; +} + +/** + * Stub popover content rendered as a plain `
`. The real + * component implements positioning, portaling, and dismissal internally; this stub exposes the + * dismissal callbacks so tests can simulate them: + * + * - `onOpenAutoFocus` is invoked once on mount (mirroring Radix's open auto-focus event). + * - 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. + * + * @param props - Component props. + * @param props.children - Panel content. + * @param props.className - CSS class names forwarded to the div. + * @param props.onEscapeKeyDown - Called when Escape is pressed inside the content. + * @param props.onInteractOutside - Called when the sentinel outside button is clicked. + * @param props.onOpenAutoFocus - Called once on mount with a plain `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. + */ +export function PopoverContent({ + children, + className, + onEscapeKeyDown, + onInteractOutside, + onOpenAutoFocus, + onClick, + onMouseDown, +}: Readonly<{ + children?: ReactNode; + className?: string; + align?: 'start' | 'center' | 'end'; + sideOffset?: number; + onEscapeKeyDown?: () => void; + onInteractOutside?: () => void; + onOpenAutoFocus?: (event: Event) => void; + onClick?: MouseEventHandler; + onMouseDown?: MouseEventHandler; +}>): ReactElement { + // Capture the mount-time callback so the simulation fires exactly once, like the real event. + const openAutoFocusRef = useRef(onOpenAutoFocus); + useEffect(() => { + openAutoFocusRef.current?.(new Event('openAutoFocus')); + }, []); + return ( + // eslint-disable-next-line jsx-a11y/no-static-element-interactions +
{ + if (e.key === 'Escape') onEscapeKeyDown?.(); + }} + onMouseDown={onMouseDown} + > + {children} + {onInteractOutside && ( + + )} +
+ ); +} + /** * Stub label rendered as a native `
+ )} l.status === 'approved' && l.token.tokenRef === tokenRef, + ); + if (!link) return undefined; + const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === link.analysisId); + if (!analysis) { + state.analysis.tokenAnalysisLinks = state.analysis.tokenAnalysisLinks.filter((l) => l !== link); + return undefined; + } + return { link, analysis }; +} + +/** + * Appends a new `TokenAnalysis` and an approved `TokenAnalysisLink` referencing it in a single + * step, ensuring both collections stay in sync. The link's token snapshot takes its surface text + * from the analysis. + * + * @param state - Current slice state (Immer draft). + * @param analysis - The new `TokenAnalysis` record to append. + * @param tokenRef - `Token.ref` of the token the link points at. + */ +function appendApprovedAnalysis( + state: AnalysisState, + analysis: TokenAnalysis, + tokenRef: string, +): void { + state.analysis.tokenAnalyses.push(analysis); + state.analysis.tokenAnalysisLinks.push({ + analysisId: analysis.id, + status: 'approved', + token: { tokenRef, surfaceText: analysis.surfaceText }, + }); +} + +/** + * Determines whether a `TokenAnalysis` carries no analysis content, so a reducer that just emptied + * one field can decide to drop the whole record instead of letting empty records accumulate in + * storage. Checks every content field of the type — `gloss`, `morphemes`, `pos`, `features`, and + * `glossSenseRef` — not only the field the caller emptied, so records carrying imported + * morphosyntactic or lexicon data are never discarded by an unrelated edit. + * + * @param analysis - The `TokenAnalysis` to inspect. + * @returns `true` when the record holds no analysis content worth keeping. + */ +function isEmptyTokenAnalysis(analysis: TokenAnalysis): boolean { + return ( + (!analysis.gloss || Object.keys(analysis.gloss).length === 0) && + /* v8 ignore next -- the only current caller deletes morphemes first; kept for future callers */ + (!analysis.morphemes || analysis.morphemes.length === 0) && + analysis.pos === undefined && + analysis.features === undefined && + analysis.glossSenseRef === undefined + ); +} + const analysisSlice = createSlice({ name: 'analysis', initialState: defaultState, @@ -143,9 +214,12 @@ const analysisSlice = createSlice({ }, /** * Creates or updates an approved `TokenAnalysis` for the given token. If an approved link - * already exists for `tokenRef`, its analysis is updated in place; otherwise a new - * `TokenAnalysis` and `TokenAnalysisLink` are appended. Non-approved analyses for the token - * are left untouched. + * already exists for `tokenRef`, its analysis is updated in place and the stored surface text + * is refreshed on both the analysis and the link's token snapshot, so neither goes stale when + * the baseline text changed since the analysis was first written. Otherwise a new + * `TokenAnalysis` and `TokenAnalysisLink` are appended (an orphaned approved link is repaired + * first; see {@link resolveApprovedAnalysis}). Non-approved analyses for the token are left + * untouched. * * @param state - Current slice state (Immer draft). * @param action - Action carrying the `WriteGlossPayload`. @@ -154,36 +228,17 @@ const analysisSlice = createSlice({ const { tokenRef, surfaceText, value, id } = action.payload; const lang = state.analysisLanguage; - const existingLink = state.analysis.tokenAnalysisLinks.find( - (l) => l.status === 'approved' && l.token.tokenRef === tokenRef, - ); - - if (existingLink) { - const existingAnalysis = state.analysis.tokenAnalyses.find( - (ta) => ta.id === existingLink.analysisId, - ); - if (existingAnalysis) { - existingAnalysis.surfaceText = surfaceText; - if (!existingAnalysis.gloss) existingAnalysis.gloss = {}; - existingAnalysis.gloss[lang] = value; - return; - } - // The approved link references a missing analysis (orphaned link from corruption or a - // migration). Remove it so we don't accumulate duplicate approved links below. - state.analysis.tokenAnalysisLinks = state.analysis.tokenAnalysisLinks.filter( - (l) => l !== existingLink, - ); + const resolved = resolveApprovedAnalysis(state, tokenRef); + if (resolved) { + const { link, analysis } = resolved; + analysis.surfaceText = surfaceText; + link.token.surfaceText = surfaceText; + if (!analysis.gloss) analysis.gloss = {}; + analysis.gloss[lang] = value; + return; } - // No approved link exists, or the orphaned link was removed above — create a new one. - const newAnalysis: TokenAnalysis = { id, surfaceText, gloss: { [lang]: value } }; - const newLink: TokenAnalysisLink = { - analysisId: id, - status: 'approved', - token: { tokenRef, surfaceText }, - }; - state.analysis.tokenAnalyses.push(newAnalysis); - state.analysis.tokenAnalysisLinks.push(newLink); + appendApprovedAnalysis(state, { id, surfaceText, gloss: { [lang]: value } }, tokenRef); }, }, writeMorphemes: { @@ -210,13 +265,17 @@ const analysisSlice = createSlice({ }; }, /** - * Sets the morpheme breakdown on the approved `TokenAnalysis` for the given token. Preserves - * existing morpheme glosses when a morpheme form is unchanged. When no approved analysis - * exists, creates one. Also refreshes the stored surface text on both the analysis and the - * link's token snapshot, so neither goes stale when the baseline text changed since the - * analysis was first written. Every morpheme — preserved or new — is stamped with the - * supplied writing system, so records written before the writing system was threaded through - * (which wrongly stored the analysis language) self-correct on the next save. + * Sets the morpheme breakdown on the approved `TokenAnalysis` for the given token. When a + * morpheme form is unchanged the existing morpheme record is preserved whole — including its + * id, which `MorphemeLink.morphemeId` cross-references, so alignment links to unchanged + * morphemes survive edits to the rest of the breakdown. When no approved analysis exists, + * creates one (an orphaned approved link is repaired first; see + * {@link resolveApprovedAnalysis}). Also refreshes the stored surface text on both the + * analysis and the link's token snapshot, so neither goes stale when the baseline text + * changed since the analysis was first written. Every morpheme — preserved or new — is + * stamped with the supplied writing system, so records written before the writing system was + * threaded through (which wrongly stored the analysis language) self-correct on the next + * save. * * @param state - Current slice state (Immer draft). * @param action - Action carrying the morpheme payload. @@ -233,56 +292,47 @@ const analysisSlice = createSlice({ ) { const { tokenRef, surfaceText, writingSystem, analysisId, morphemes } = action.payload; - const existingLink = state.analysis.tokenAnalysisLinks.find( - (l) => l.status === 'approved' && l.token.tokenRef === tokenRef, - ); - - if (existingLink) { - const existingAnalysis = state.analysis.tokenAnalyses.find( - (ta) => ta.id === existingLink.analysisId, - ); - if (existingAnalysis) { - existingAnalysis.surfaceText = surfaceText; - existingLink.token.surfaceText = surfaceText; - // Multimap with consumed entries so duplicate forms (e.g. reduplication "ba ba") each - // match a distinct old morpheme in order, instead of all inheriting the last one. - const oldByForm = new Map(); - (existingAnalysis.morphemes ?? []).forEach((m) => { - const bucket = oldByForm.get(m.form); - if (bucket) bucket.push(m); - else oldByForm.set(m.form, [m]); - }); - existingAnalysis.morphemes = morphemes.map(({ id, form }) => { - const old = oldByForm.get(form)?.shift(); - if (old) return { ...old, id, writingSystem }; - return { id, form, writingSystem }; - }); - return; - } - state.analysis.tokenAnalysisLinks = state.analysis.tokenAnalysisLinks.filter( - (l) => l !== existingLink, - ); + const resolved = resolveApprovedAnalysis(state, tokenRef); + if (resolved) { + const { link, analysis } = resolved; + analysis.surfaceText = surfaceText; + link.token.surfaceText = surfaceText; + // Multimap with consumed entries so duplicate forms (e.g. reduplication "ba ba") each + // match a distinct old morpheme in order, instead of all inheriting the last one. + const oldByForm = new Map(); + (analysis.morphemes ?? []).forEach((m) => { + const bucket = oldByForm.get(m.form); + if (bucket) bucket.push(m); + else oldByForm.set(m.form, [m]); + }); + analysis.morphemes = morphemes.map(({ id, form }) => { + const old = oldByForm.get(form)?.shift(); + // Keep the preserved morpheme's id (the prepared id is discarded) so external + // references to it stay valid; only the writing system is refreshed. + if (old) return { ...old, writingSystem }; + return { id, form, writingSystem }; + }); + return; } - const newAnalysis: TokenAnalysis = { - id: analysisId, - surfaceText, - morphemes: morphemes.map(({ id, form }) => ({ id, form, writingSystem })), - }; - const newLink: TokenAnalysisLink = { - analysisId, - status: 'approved', - token: { tokenRef, surfaceText }, - }; - state.analysis.tokenAnalyses.push(newAnalysis); - state.analysis.tokenAnalysisLinks.push(newLink); + appendApprovedAnalysis( + state, + { + id: analysisId, + surfaceText, + morphemes: morphemes.map(({ id, form }) => ({ id, form, writingSystem })), + }, + tokenRef, + ); }, }, /** * Removes the morpheme breakdown from the approved `TokenAnalysis` for the given token. When - * the analysis carries no gloss either, the now-empty analysis record and its link are removed - * entirely so empty records do not accumulate in storage. No-ops when the token has no approved - * analysis or the analysis has no morphemes. + * the analysis carries no other content (gloss, POS, features, or lexicon sense reference — see + * {@link isEmptyTokenAnalysis}), the now-empty analysis record and its link are removed entirely + * so empty records do not accumulate in storage. No-ops when the token has no approved analysis + * or the analysis has no morphemes (an orphaned approved link is still repaired; see + * {@link resolveApprovedAnalysis}). * * @param state - Current slice state (Immer draft). * @param action - Action carrying the `tokenRef` whose breakdown is removed. @@ -290,16 +340,12 @@ const analysisSlice = createSlice({ deleteMorphemes(state, action: PayloadAction<{ tokenRef: string }>) { const { tokenRef } = action.payload; - const link = state.analysis.tokenAnalysisLinks.find( - (l) => l.status === 'approved' && l.token.tokenRef === tokenRef, - ); - if (!link) return; - - const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === link.analysisId); - if (!analysis?.morphemes) return; + const resolved = resolveApprovedAnalysis(state, tokenRef); + if (!resolved?.analysis.morphemes) return; + const { link, analysis } = resolved; delete analysis.morphemes; - if (!analysis.gloss || Object.keys(analysis.gloss).length === 0) { + if (isEmptyTokenAnalysis(analysis)) { state.analysis.tokenAnalyses = state.analysis.tokenAnalyses.filter((ta) => ta !== analysis); state.analysis.tokenAnalysisLinks = state.analysis.tokenAnalysisLinks.filter( (l) => l !== link, @@ -308,7 +354,8 @@ const analysisSlice = createSlice({ }, /** * Writes a gloss string onto a single morpheme within the approved `TokenAnalysis` for the - * given token. No-ops when the token has no approved analysis or the morpheme id is not found. + * given token. No-ops when the token has no approved analysis or the morpheme id is not found + * (an orphaned approved link is still repaired; see {@link resolveApprovedAnalysis}). * * @param state - Current slice state (Immer draft). * @param action - Action carrying the morpheme gloss payload. @@ -320,13 +367,8 @@ const analysisSlice = createSlice({ const { tokenRef, morphemeId, value } = action.payload; const lang = state.analysisLanguage; - const link = state.analysis.tokenAnalysisLinks.find( - (l) => l.status === 'approved' && l.token.tokenRef === tokenRef, - ); - if (!link) return; - - const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === link.analysisId); - const morpheme = analysis?.morphemes?.find((m) => m.id === morphemeId); + const resolved = resolveApprovedAnalysis(state, tokenRef); + const morpheme = resolved?.analysis.morphemes?.find((m) => m.id === morphemeId); if (!morpheme) return; if (!morpheme.gloss) morpheme.gloss = {}; From 352637acbbf628fc7309bea85fe8d210d8898128 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 12 Jun 2026 16:31:33 -0600 Subject: [PATCH 07/16] Recenter on showMorphology toggle --- src/__tests__/components/ContinuousView.test.tsx | 15 +++++++++++++++ src/components/ContinuousView.tsx | 13 +++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/__tests__/components/ContinuousView.test.tsx b/src/__tests__/components/ContinuousView.test.tsx index 87af7ac7..1f61c1d1 100644 --- a/src/__tests__/components/ContinuousView.test.tsx +++ b/src/__tests__/components/ContinuousView.test.tsx @@ -1125,6 +1125,21 @@ describe('ContinuousView scroll behavior', () => { ); expect(scrollIntoViewMock).toHaveBeenCalledTimes(1); }); + + it('re-centers once when showMorphology toggles', () => { + // Morpheme rows beneath tokens can widen phrase boxes, shifting the strip layout, so the + // focused group must be snapped back to center when the toggle flips. + const book = makeBook(); + const props = requiredProps(book, { focusedTokenRef: 'tok-0' }); + const { rerender } = render(, withAnalysisStore); + scrollIntoViewMock.mockClear(); + + rerender(); + expect(scrollIntoViewMock).toHaveBeenCalledWith( + expect.objectContaining({ behavior: 'auto', inline: 'center' }), + ); + expect(scrollIntoViewMock).toHaveBeenCalledTimes(1); + }); }); // --------------------------------------------------------------------------- diff --git a/src/components/ContinuousView.tsx b/src/components/ContinuousView.tsx index 847876ad..833d2c5e 100644 --- a/src/components/ContinuousView.tsx +++ b/src/components/ContinuousView.tsx @@ -583,17 +583,18 @@ export default function ContinuousView({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [committedActiveSegmentId]); - // Re-center the focused group when a view option toggles. Toggling `simplifyPhrases` changes - // the strip's layout, so the previously-centered group may drift off-center; snap it back into - // view. `hideInactiveLinkButtons` is excluded: inactive link slots now reserve their space even - // when hidden (`opacity: 0`; clickability is guarded at the button level), so toggling it does - // not shift the layout. + // Re-center the focused group when a view option toggles. Toggling `simplifyPhrases` or + // `showMorphology` changes the strip's layout (morpheme rows can widen phrase boxes), so the + // previously-centered group may drift off-center; snap it back into view. + // `hideInactiveLinkButtons` is excluded: inactive link slots now reserve their space even when + // hidden (`opacity: 0`; clickability is guarded at the button level), so toggling it does not + // shift the layout. useEffect(() => { centerGroup(focusPhraseIndex, 'auto'); // focusPhraseIndex is intentionally excluded: it has its own scroll effect above. This effect // only re-centers in response to layout-affecting option toggles. centerGroup is stable. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [simplifyPhrases]); + }, [simplifyPhrases, showMorphology]); // When entering edit or confirm-unlink mode, smooth-scroll to the first group of the active // phrase by notifying the parent of the new focused token. Scroll then follows automatically From e9cd471c396284734e0f4dd57dd10bbd55b1f3ed Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 12 Jun 2026 16:59:53 -0600 Subject: [PATCH 08/16] Minor adjustments --- REVIEW.md | 4 ++++ __mocks__/platform-bible-react.tsx | 21 ++++++++++++----- src/__tests__/components/PhraseBox.test.tsx | 1 + src/__tests__/components/TokenChip.test.tsx | 25 +++++++++++++++++++++ src/components/TokenChip.tsx | 7 ++++++ 5 files changed, 52 insertions(+), 6 deletions(-) diff --git a/REVIEW.md b/REVIEW.md index d8a19cfa..ef440b9f 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -18,6 +18,10 @@ Before reporting any documentation as missing, open the file and confirm the JSD Keyboard accessibility is planned but not yet implemented. Do not flag missing `tabIndex` attributes, absent `aria-*` roles, or gaps in focus management as issues — these will be addressed in a dedicated pass once the core interaction model is stable. +## Buttons inside the TokenChip label + +The `
)} @@ -305,10 +285,7 @@ function InterlinearizerInner({ reportSettled={reportSettled} phraseMode={phraseMode} setPhraseMode={setPhraseMode} - hideInactiveLinkButtons={hideInactiveLinkButtons} - simplifyPhrases={simplifyPhrases} - chapterLabelInVerse={chapterLabelInVerse} - showMorphology={showMorphology} + viewOptions={viewOptions} hoveredPhraseId={hoveredPhraseId} setHoveredPhraseId={setHoveredPhraseId} editPhraseSegmentId={editPhraseSegmentId} @@ -337,12 +314,8 @@ function InterlinearizerInner({ * @param props.onSaveAnalysis - Called after each gloss write with the updated `TextAnalysis` * @param props.phraseMode - Current phrase-interaction mode owned by the parent * @param props.setPhraseMode - Setter for `phraseMode` - * @param props.hideInactiveLinkButtons - When true, link buttons between phrases are hidden in - * segments other than the active verse. - * @param props.simplifyPhrases - When true, phrase-level controls are hidden on every phrase except - * the focused one. - * @param props.showMorphology - When true, morpheme rows and per-morpheme glosses are shown beneath - * each word token. + * @param props.viewOptions - Bundled display toggles forwarded to the segment list and continuous + * views. * @returns The full interlinearizer layout with optional continuous strip and segment list */ export default function Interlinearizer({ diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 6199a8b1..d99e5238 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -403,10 +403,12 @@ function InterlinearizerLoaderInner({ onSaveAnalysis={handleSaveAnalysis} phraseMode={phraseMode} setPhraseMode={setPhraseMode} - hideInactiveLinkButtons={hideInactiveLinkButtons} - simplifyPhrases={simplifyPhrases} - chapterLabelInVerse={chapterLabelInVerse} - showMorphology={showMorphology} + viewOptions={{ + hideInactiveLinkButtons, + simplifyPhrases, + chapterLabelInVerse, + showMorphology, + }} /> )}
diff --git a/src/components/SegmentListView.tsx b/src/components/SegmentListView.tsx index 46f6f567..d6e73aed 100644 --- a/src/components/SegmentListView.tsx +++ b/src/components/SegmentListView.tsx @@ -4,6 +4,7 @@ import { LocateFixed } from 'lucide-react'; import { Fragment, useCallback, useEffect, useMemo, useRef } from 'react'; import type { Dispatch, SetStateAction } from 'react'; import type { PhraseMode } from '../types/phrase-mode'; +import type { ViewOptions } from '../types/view-options'; import MemoizedSegmentView from './SegmentView'; import useSegmentWindow from '../hooks/useSegmentWindow'; import { isSameVerse } from '../utils/verse-ref'; @@ -43,18 +44,8 @@ type SegmentListViewProps = Readonly<{ phraseMode: PhraseMode; /** Setter for `phraseMode`; passed down so child components can transition modes. */ setPhraseMode: Dispatch>; - /** When true, link buttons between phrases are hidden in segments other than the active verse. */ - hideInactiveLinkButtons: boolean; - /** When true, phrase-level controls are hidden on every phrase except the focused one. */ - simplifyPhrases: boolean; - /** - * When true, every verse is labeled `chapter:verse` and no inline chapter header is shown; when - * false, verse labels stay bare verse numbers and an inline chapter header precedes the first - * verse of each chapter. - */ - chapterLabelInVerse: boolean; - /** When true, morpheme rows and per-morpheme glosses are shown beneath each word token. */ - showMorphology: boolean; + /** Bundled display toggles forwarded unchanged to each {@link SegmentView}. */ + viewOptions: ViewOptions; /** PhraseId currently hovered anywhere in the interlinearizer; shared across all SegmentViews. */ hoveredPhraseId: string | undefined; /** Sets the hovered phraseId when the pointer enters or leaves a phrase box. */ @@ -92,14 +83,7 @@ type SegmentListViewProps = Readonly<{ * @param props.reportSettled - Reports the window has settled; lifts the cross-book curtain. * @param props.phraseMode - Current phrase-interaction mode passed down for rendering. * @param props.setPhraseMode - Setter for `phraseMode`. - * @param props.hideInactiveLinkButtons - When true, link buttons are hidden outside the active - * verse. - * @param props.simplifyPhrases - When true, phrase controls are hidden except on the focused - * phrase. - * @param props.chapterLabelInVerse - When true, every verse is labeled `chapter:verse` instead of - * showing an inline chapter header. - * @param props.showMorphology - When true, morpheme rows and per-morpheme glosses are shown beneath - * each word token. + * @param props.viewOptions - Bundled display toggles forwarded unchanged to each segment. * @param props.hoveredPhraseId - PhraseId currently hovered anywhere in the interlinearizer. * @param props.setHoveredPhraseId - Sets the hovered phraseId. * @param props.editPhraseSegmentId - Segment id containing the phrase being edited, or `undefined`. @@ -120,10 +104,7 @@ export default function SegmentListView({ reportSettled, phraseMode, setPhraseMode, - hideInactiveLinkButtons, - simplifyPhrases, - chapterLabelInVerse, - showMorphology, + viewOptions, hoveredPhraseId, setHoveredPhraseId, editPhraseSegmentId, @@ -132,6 +113,10 @@ export default function SegmentListView({ tokenDocOrder, wordTokenByRef, }: SegmentListViewProps) { + // Read directly here for the inline chapter headers; the rest of `viewOptions` is forwarded + // unchanged to each SegmentView. + const { chapterLabelInVerse } = viewOptions; + /** * Ids of the segments that begin a new chapter — the first segment of the book and every segment * whose chapter differs from the immediately preceding segment in book order. Computed over the @@ -252,10 +237,7 @@ export default function SegmentListView({ tokenSegmentMap={tokenSegmentMap} tokenDocOrder={tokenDocOrder} wordTokenByRef={wordTokenByRef} - hideInactiveLinkButtons={hideInactiveLinkButtons} - simplifyPhrases={simplifyPhrases} - chapterLabelInVerse={chapterLabelInVerse} - showMorphology={showMorphology} + viewOptions={viewOptions} /> ))} diff --git a/src/components/SegmentView.tsx b/src/components/SegmentView.tsx index 649b5f75..e4e0c14e 100644 --- a/src/components/SegmentView.tsx +++ b/src/components/SegmentView.tsx @@ -11,6 +11,7 @@ import { usePhraseStripContextValue, } from '../hooks/usePhraseStripSetup'; import type { PhraseMode } from '../types/phrase-mode'; +import type { ViewOptions } from '../types/view-options'; import type { RenderUnit } from '../types/token-layout'; import { buildRenderUnits, groupTokens, resolveFocusContext } from '../utils/token-layout'; import { usePhraseLinkByIdMap, usePhraseLinkMap } from './AnalysisStore'; @@ -74,23 +75,11 @@ type SegmentViewProps = Readonly<{ /** Word token ref → token lookup for the whole book; used to resolve focus context. */ wordTokenByRef: ReadonlyMap; /** - * When `true`, the link/unlink buttons between phrase boxes are hidden unless this segment is the - * active verse. Passed through to {@link PhraseStripContextValue}. + * Bundled display toggles. `hideInactiveLinkButtons`, `simplifyPhrases`, and `showMorphology` are + * passed through to {@link PhraseStripContextValue}; `chapterLabelInVerse` controls this segment's + * verse label. */ - hideInactiveLinkButtons: boolean; - /** - * When `true`, phrase-level controls (split, intra-phrase unlink, remove-token) are hidden on - * every phrase except the focused one. Passed through to {@link PhraseStripContextValue}. - */ - simplifyPhrases: boolean; - /** - * When `true`, every segment is labeled `chapter:verse`. When `false`, segments show a bare verse - * number (the chapter is shown by an inline header that {@link SegmentListView} renders above each - * chapter's first segment). - */ - chapterLabelInVerse: boolean; - /** When `true`, morpheme rows and per-morpheme glosses are shown beneath each word token. */ - showMorphology: boolean; + viewOptions: ViewOptions; }>; /** @@ -118,14 +107,8 @@ type SegmentViewProps = Readonly<{ * @param props.tokenDocOrder - Book-level map from word token ref to flat document index; used to * sort phrase tokens across segment boundaries. * @param props.wordTokenByRef - Word token ref → token lookup; used to resolve focus context. - * @param props.hideInactiveLinkButtons - When true, link buttons between phrases are hidden unless - * this segment is the active verse. - * @param props.simplifyPhrases - When true, phrase-level controls are hidden on every phrase except - * the focused one. - * @param props.chapterLabelInVerse - When true, every segment is labeled `chapter:verse`; when - * false, segments show a bare verse number. - * @param props.showMorphology - When true, morpheme rows and per-morpheme glosses are shown beneath - * each word token. + * @param props.viewOptions - Bundled display toggles; `chapterLabelInVerse` sets the verse label, + * the rest pass through to the phrase strip context. * @returns A button (baseline-text mode) or div (token-chip mode) containing a verse label and * segment content */ @@ -143,11 +126,10 @@ export function SegmentView({ tokenSegmentMap, tokenDocOrder, wordTokenByRef, - hideInactiveLinkButtons, - simplifyPhrases, - chapterLabelInVerse, - showMorphology, + viewOptions, }: SegmentViewProps) { + const { hideInactiveLinkButtons, simplifyPhrases, chapterLabelInVerse, showMorphology } = + viewOptions; const { book, chapter, verse } = segment.startRef; const ref: ScriptureRef = useMemo(() => ({ book, chapter, verse }), [book, chapter, verse]); diff --git a/src/store/analysisSlice.ts b/src/store/analysisSlice.ts index 05f79c80..70370252 100644 --- a/src/store/analysisSlice.ts +++ b/src/store/analysisSlice.ts @@ -164,20 +164,40 @@ function appendApprovedAnalysis( }); } +/** + * Removes a `TokenAnalysis` record and its `TokenAnalysisLink` from the draft in a single step, + * keeping the two collections in sync. Called when an edit empties an analysis of all content so + * empty records do not accumulate in storage. + * + * @param state - Current slice state (Immer draft). + * @param analysis - The `TokenAnalysis` record to remove. + * @param link - The `TokenAnalysisLink` referencing it. + */ +function removeTokenAnalysis( + state: AnalysisState, + analysis: TokenAnalysis, + link: TokenAnalysisLink, +): void { + state.analysis.tokenAnalyses = state.analysis.tokenAnalyses.filter((ta) => ta !== analysis); + state.analysis.tokenAnalysisLinks = state.analysis.tokenAnalysisLinks.filter((l) => l !== link); +} + /** * Determines whether a `TokenAnalysis` carries no analysis content, so a reducer that just emptied * one field can decide to drop the whole record instead of letting empty records accumulate in * storage. Checks every content field of the type — `gloss`, `morphemes`, `pos`, `features`, and * `glossSenseRef` — not only the field the caller emptied, so records carrying imported - * morphosyntactic or lexicon data are never discarded by an unrelated edit. + * morphosyntactic or lexicon data are never discarded by an unrelated edit. A gloss counts as empty + * when it has no entries or every entry is blank, so a record left holding only whitespace glosses + * (junk from clearing a gloss field) is treated the same as one with no gloss at all. * * @param analysis - The `TokenAnalysis` to inspect. * @returns `true` when the record holds no analysis content worth keeping. */ function isEmptyTokenAnalysis(analysis: TokenAnalysis): boolean { return ( - (!analysis.gloss || Object.keys(analysis.gloss).length === 0) && - /* v8 ignore next -- the only current caller deletes morphemes first; kept for future callers */ + (!analysis.gloss || Object.values(analysis.gloss).every((g) => g.trim() === '')) && + /* v8 ignore next -- the length===0 sub-branch needs an empty-but-defined morphemes array, which no caller produces */ (!analysis.morphemes || analysis.morphemes.length === 0) && analysis.pos === undefined && analysis.features === undefined && @@ -221,23 +241,39 @@ const analysisSlice = createSlice({ * first; see {@link resolveApprovedAnalysis}). Non-approved analyses for the token are left * untouched. * + * A blank `value` (empty or whitespace) is treated as clearing the gloss rather than writing + * junk: the active language's entry is removed, and when that leaves the analysis with no + * content ({@link isEmptyTokenAnalysis}) the record and its link are removed entirely. A blank + * write to a token with no approved analysis is a no-op, so a focus/blur cycle on an empty + * gloss never creates a record. + * * @param state - Current slice state (Immer draft). * @param action - Action carrying the `WriteGlossPayload`. */ reducer(state, action: PayloadAction) { const { tokenRef, surfaceText, value, id } = action.payload; const lang = state.analysisLanguage; + const isBlank = value.trim() === ''; const resolved = resolveApprovedAnalysis(state, tokenRef); if (resolved) { const { link, analysis } = resolved; analysis.surfaceText = surfaceText; link.token.surfaceText = surfaceText; + if (isBlank) { + if (analysis.gloss) { + delete analysis.gloss[lang]; + if (Object.keys(analysis.gloss).length === 0) delete analysis.gloss; + } + if (isEmptyTokenAnalysis(analysis)) removeTokenAnalysis(state, analysis, link); + return; + } if (!analysis.gloss) analysis.gloss = {}; analysis.gloss[lang] = value; return; } + if (isBlank) return; appendApprovedAnalysis(state, { id, surfaceText, gloss: { [lang]: value } }, tokenRef); }, }, @@ -345,12 +381,7 @@ const analysisSlice = createSlice({ const { link, analysis } = resolved; delete analysis.morphemes; - if (isEmptyTokenAnalysis(analysis)) { - state.analysis.tokenAnalyses = state.analysis.tokenAnalyses.filter((ta) => ta !== analysis); - state.analysis.tokenAnalysisLinks = state.analysis.tokenAnalysisLinks.filter( - (l) => l !== link, - ); - } + if (isEmptyTokenAnalysis(analysis)) removeTokenAnalysis(state, analysis, link); }, /** * Writes a gloss string onto a single morpheme within the approved `TokenAnalysis` for the @@ -606,15 +637,11 @@ export const selectPhraseLinks = createSelector(selectPhraseAnalysisLinksRaw, (l * containing it. When a token appears in multiple approved links (data-model violation), the last * wins. Recomputes only when approved phrase links change. */ -export const selectPhraseLinkByTokenRef = createSelector(selectPhraseLinks, (links) => - links.reduce((map, link) => { - link.tokens.reduce((m, snap) => { - m.set(snap.tokenRef, link); - return m; - }, map); - return map; - }, new Map()), -); +export const selectPhraseLinkByTokenRef = createSelector(selectPhraseLinks, (links) => { + const map = new Map(); + links.forEach((link) => link.tokens.forEach((snap) => map.set(snap.tokenRef, link))); + return map; +}); /** * Memoized selector that builds a `Map` from `analysisId` to approved `PhraseAnalysisLink` for O(1) diff --git a/src/types/view-options.ts b/src/types/view-options.ts new file mode 100644 index 00000000..483a4c0a --- /dev/null +++ b/src/types/view-options.ts @@ -0,0 +1,20 @@ +/** + * Bundled display toggles threaded from {@link InterlinearizerLoader} down through the segment and + * continuous views to the phrase strips. Grouping them in one object lets intermediate components + * forward `viewOptions` unchanged, so adding a new toggle only touches the loader that builds it + * and the leaf that reads it — not every component in between. + */ +export type ViewOptions = Readonly<{ + /** When true, link buttons between phrases are hidden in segments other than the active verse. */ + hideInactiveLinkButtons: boolean; + /** When true, phrase-level controls are hidden on every phrase except the focused one. */ + simplifyPhrases: boolean; + /** + * When true, every verse is labeled `chapter:verse` and no inline chapter header is shown; when + * false, an inline chapter header precedes the first verse of each chapter and verse labels stay + * bare verse numbers. + */ + chapterLabelInVerse: boolean; + /** When true, morpheme rows and per-morpheme glosses are shown beneath each word token. */ + showMorphology: boolean; +}>; From d150774963f17ee8c37a116cb13d585a1c9ef186 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 16 Jun 2026 14:38:11 -0600 Subject: [PATCH 10/16] Minor adjustments --- .../components/InterlinearizerLoader.test.tsx | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index f1aff1ee..841b4601 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -32,6 +32,8 @@ jest.mock('../../components/controls/ViewOptionsDropdown', () => ({ onSimplifyPhrasesChange, chapterLabelInVerse, onChapterLabelInVerseChange, + showMorphology, + onShowMorphologyChange, }: { continuousScroll: boolean; onContinuousScrollChange: (v: boolean) => void; @@ -41,6 +43,8 @@ jest.mock('../../components/controls/ViewOptionsDropdown', () => ({ onSimplifyPhrasesChange: (v: boolean) => void; chapterLabelInVerse: boolean; onChapterLabelInVerseChange: (v: boolean) => void; + showMorphology: boolean; + onShowMorphologyChange: (v: boolean) => void; }) => (
), })); @@ -608,6 +619,32 @@ describe('InterlinearizerLoader', () => { expect(onChangeByKey.get('interlinearizer.chapterLabelInVerse')).toHaveBeenCalledWith(true); }); + it('passes showMorphology=false to Interlinearizer by default', () => { + render( + , + ); + + expect(capturedInterlinearizerProps?.viewOptions.showMorphology).toBe(false); + }); + + it('wires ViewOptionsDropdown show-morphology to onChange from useOptimisticBooleanSetting', async () => { + const onChangeByKey = mockOptimisticSetting(); + render( + , + ); + + await userEvent.click(screen.getByTestId('show-morphology-toggle')); + expect(onChangeByKey.get('interlinearizer.showMorphology')).toHaveBeenCalledWith(true); + }); + it('passes continuousScroll=true to Interlinearizer when the setting is true', () => { mockOptimisticSetting(true); render( From dc43d0a459370dd6e36d351148b0b4da9fbc43cc Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 16 Jun 2026 16:52:19 -0600 Subject: [PATCH 11/16] Memoize viewOptions, clear blank morpheme gloss --- src/__tests__/store/analysisSlice.test.ts | 50 +++++++++++++++++++++++ src/components/InterlinearizerLoader.tsx | 16 +++++--- src/store/analysisSlice.ts | 14 +++++++ 3 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/__tests__/store/analysisSlice.test.ts b/src/__tests__/store/analysisSlice.test.ts index 2a20a29d..02cf794b 100644 --- a/src/__tests__/store/analysisSlice.test.ts +++ b/src/__tests__/store/analysisSlice.test.ts @@ -1023,6 +1023,56 @@ describe('writeMorphemeGloss', () => { expect(updated?.morphemes?.[1].gloss).toBeUndefined(); }); + it('drops the gloss object when a blank value clears its only entry', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'unbelievable', + morphemes: [{ id: 'm-1', form: 'un-', writingSystem: 'und', gloss: { und: 'not' } }], + }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + store.dispatch(writeMorphemeGloss({ tokenRef: 'tok-1', morphemeId: 'm-1', value: ' ' })); + + const updated = store.getState().analysis.analysis.tokenAnalyses.find((a) => a.id === 'ta-1'); + expect(updated?.morphemes?.[0].gloss).toBeUndefined(); + }); + + it('removes only the active language entry from a multi-language gloss when cleared', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'unbelievable', + morphemes: [ + { id: 'm-1', form: 'un-', writingSystem: 'und', gloss: { und: 'not', fr: 'non' } }, + ], + }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + store.dispatch(writeMorphemeGloss({ tokenRef: 'tok-1', morphemeId: 'm-1', value: '' })); + + const updated = store.getState().analysis.analysis.tokenAnalyses.find((a) => a.id === 'ta-1'); + expect(updated?.morphemes?.[0].gloss).toStrictEqual({ fr: 'non' }); + }); + + it('no-ops when a blank value clears a morpheme that has no gloss', () => { + const ta: TokenAnalysis = { + id: 'ta-1', + surfaceText: 'unbelievable', + morphemes: [{ id: 'm-1', form: 'un-', writingSystem: 'und' }], + }; + const store = createAnalysisStore({ + analysis: { analysis: makeAnalysis(ta), analysisLanguage: 'und' }, + }); + + store.dispatch(writeMorphemeGloss({ tokenRef: 'tok-1', morphemeId: 'm-1', value: '' })); + + const updated = store.getState().analysis.analysis.tokenAnalyses.find((a) => a.id === 'ta-1'); + expect(updated?.morphemes?.[0].gloss).toBeUndefined(); + }); + it('no-ops when the token has no approved link', () => { const store = createAnalysisStore(); store.dispatch(writeMorphemeGloss({ tokenRef: 'tok-1', morphemeId: 'm-1', value: 'not' })); diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index d99e5238..adcc6d05 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -226,6 +226,15 @@ function InterlinearizerLoaderInner({ value: showMorphology, } = useOptimisticBooleanSetting(projectId, 'interlinearizer.showMorphology', false); + // Bundle the display toggles into one stable object. Memoizing on the four 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. + const viewOptions = useMemo( + () => ({ hideInactiveLinkButtons, simplifyPhrases, chapterLabelInVerse, showMorphology }), + [hideInactiveLinkButtons, simplifyPhrases, chapterLabelInVerse, showMorphology], + ); + const { book, isLoading, bookError, tokenizeError } = useInterlinearizerBookData({ projectId, scrRef, @@ -403,12 +412,7 @@ function InterlinearizerLoaderInner({ onSaveAnalysis={handleSaveAnalysis} phraseMode={phraseMode} setPhraseMode={setPhraseMode} - viewOptions={{ - hideInactiveLinkButtons, - simplifyPhrases, - chapterLabelInVerse, - showMorphology, - }} + viewOptions={viewOptions} /> )} diff --git a/src/store/analysisSlice.ts b/src/store/analysisSlice.ts index 70370252..0f91b369 100644 --- a/src/store/analysisSlice.ts +++ b/src/store/analysisSlice.ts @@ -388,6 +388,12 @@ const analysisSlice = createSlice({ * given token. No-ops when the token has no approved analysis or the morpheme id is not found * (an orphaned approved link is still repaired; see {@link resolveApprovedAnalysis}). * + * A blank `value` (empty or whitespace) clears the gloss rather than storing junk: the active + * language's entry is removed, and when that leaves the morpheme with no glosses the `gloss` + * object is dropped entirely — mirroring the token-level {@link writeGloss}. The morpheme record + * itself is kept (a breakdown is content in its own right), so unlike `writeGloss` this never + * removes the enclosing analysis. + * * @param state - Current slice state (Immer draft). * @param action - Action carrying the morpheme gloss payload. */ @@ -402,6 +408,14 @@ const analysisSlice = createSlice({ const morpheme = resolved?.analysis.morphemes?.find((m) => m.id === morphemeId); if (!morpheme) return; + if (value.trim() === '') { + if (morpheme.gloss) { + delete morpheme.gloss[lang]; + if (Object.keys(morpheme.gloss).length === 0) delete morpheme.gloss; + } + return; + } + if (!morpheme.gloss) morpheme.gloss = {}; morpheme.gloss[lang] = value; }, From 86311e12f1bef6a71cdb641c2b68363d8678796b Mon Sep 17 00:00:00 2001 From: "D. Ror." Date: Wed, 17 Jun 2026 09:17:40 -0600 Subject: [PATCH 12/16] Tiny tweaks (#109) * Address token-analysis review comments - MorphemeEditor: normalize internal whitespace in isUnedited to avoid no-op saves on spacing-only changes - TokenChip: document the deliberate onOpenChange omission on the controlled Popover Co-Authored-By: Claude Opus 4.8 (1M context) * Add JSDoc to normalize helper in MorphemeEditor Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- src/components/MorphemeEditor.tsx | 14 ++++++++++++-- src/components/TokenChip.tsx | 5 +++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/components/MorphemeEditor.tsx b/src/components/MorphemeEditor.tsx index b4218638..12b79059 100644 --- a/src/components/MorphemeEditor.tsx +++ b/src/components/MorphemeEditor.tsx @@ -58,9 +58,19 @@ export function MorphemeBreakdownPopover({ inputRef.current?.select(); }, []); + /** + * Collapses leading/trailing and repeated internal whitespace to a single space. + * + * @param s - The string to normalize. + * @returns The string with surrounding whitespace trimmed and internal runs collapsed. + */ + const normalize = (s: string) => s.trim().replace(/\s+/g, ' '); + // Whether the draft matches the pre-filled value. Shared by the Done/Enter and outside-click - // commit paths so the two can never disagree about what counts as an edit. - const isUnedited = draft.trim() === initialValue.trim(); + // commit paths so the two can never disagree about what counts as an edit. Whitespace is + // normalized because the save path splits on /\s+/, so differing spacing yields identical forms — + // comparing normalized text avoids a no-op persistence round-trip. + const isUnedited = normalize(draft) === normalize(initialValue); /** * Commits the current draft and closes the popover. Skips the save when the token already has a diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index ade4aa22..0795d055 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -161,6 +161,11 @@ export function TokenChip({ // otherwise paint later segment rows over it. The popover is modal so interactions // outside the panel are blocked while it is open. The popover component is mounted only // while open so its draft state re-initializes from the current forms on every open. + // + // `onOpenChange` is intentionally omitted: this consumer owns every dismissal path + // (onEscapeKeyDown, onInteractOutside, explicit button clicks), so Radix's internal close + // requests aren't needed. Don't wire onOpenChange without also removing those, or closes + // would double-fire.
From 6053234593b8fcbf7c41cb94b889424ebd7a76ab Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 17 Jun 2026 09:27:14 -0600 Subject: [PATCH 13/16] Add validator for show morphology setting --- src/__tests__/main.test.ts | 2 +- src/main.ts | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/__tests__/main.test.ts b/src/__tests__/main.test.ts index 78a9259d..8eab3912 100644 --- a/src/__tests__/main.test.ts +++ b/src/__tests__/main.test.ts @@ -264,7 +264,7 @@ describe('main', () => { await activate(context); - expect(context.registrations.unsubscribers.size).toBe(17); + expect(context.registrations.unsubscribers.size).toBe(18); }); it('logs activation start and finish', async () => { diff --git a/src/main.ts b/src/main.ts index 3ab484df..79f65b0f 100644 --- a/src/main.ts +++ b/src/main.ts @@ -382,6 +382,11 @@ export async function activate(context: ExecutionActivationContext): Promise typeof newValue === 'boolean', ); + const showMorphologyValidatorRegistration = await papi.projectSettings.registerValidator( + 'interlinearizer.showMorphology', + async (newValue) => typeof newValue === 'boolean', + ); + const createProjectCommandRegistration = await papi.commands.registerCommand( 'interlinearizer.createProject', createInterlinearProject, @@ -630,6 +635,7 @@ export async function activate(context: ExecutionActivationContext): Promise Date: Wed, 17 Jun 2026 11:07:31 -0600 Subject: [PATCH 14/16] Close morphology popover when token chip disabled, --- src/__tests__/components/TokenChip.test.tsx | 20 +++++++++++++++++++ .../controls/ViewOptionsDropdown.test.tsx | 6 ++---- src/components/TokenChip.tsx | 8 +++++--- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index 321a6da7..6753dbd2 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -584,6 +584,26 @@ describe('TokenChip', () => { expect(screen.queryByTestId('morpheme-popover')).not.toBeInTheDocument(); }); + it('closes the popover when the chip becomes disabled', async () => { + const { rerender } = render( + + + , + ); + await userEvent.click( + screen.getByRole('button', { name: 'Define morpheme breakdown for hello' }), + ); + expect(screen.getByTestId('morpheme-popover')).toBeInTheDocument(); + // The popover content renders on `popoverOpen` alone, not gated on `disabled`; a chip whose + // popover is open while it transitions to disabled would otherwise stay editable. + rerender( + + + , + ); + expect(screen.queryByTestId('morpheme-popover')).not.toBeInTheDocument(); + }); + it('closes the popover via onClose', async () => { render( diff --git a/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx b/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx index bffef219..55ca467d 100644 --- a/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx +++ b/src/__tests__/components/controls/ViewOptionsDropdown.test.tsx @@ -151,8 +151,7 @@ describe('ViewOptionsDropdown', () => { render(); await userEvent.click(screen.getByTestId('view-options-button')); - const checkboxes = screen.getAllByRole('checkbox'); - expect(checkboxes[1]).toBeChecked(); + expect(screen.getByRole('checkbox', { name: /morphology/i })).toBeChecked(); }); it('calls onShowMorphologyChange when toggled', async () => { @@ -166,8 +165,7 @@ describe('ViewOptionsDropdown', () => { ); await userEvent.click(screen.getByTestId('view-options-button')); - const checkboxes = screen.getAllByRole('checkbox'); - await userEvent.click(checkboxes[1]); + await userEvent.click(screen.getByRole('checkbox', { name: /morphology/i })); expect(onShowMorphologyChange).toHaveBeenCalledWith(true); }); diff --git a/src/components/TokenChip.tsx b/src/components/TokenChip.tsx index 0795d055..a22ae0fb 100644 --- a/src/components/TokenChip.tsx +++ b/src/components/TokenChip.tsx @@ -78,10 +78,12 @@ export function TokenChip({ // The popover tree unmounts with the morpheme row when showMorphology turns off, but this state // lives on the chip and would survive — silently reopening the popover when morphology is shown - // again. Clear it so hiding morphology also closes the popover. + // again. Clearing it on hide also closes the popover. We also close it when the chip becomes + // disabled: the popover content renders on `popoverOpen` alone (it isn't gated on `disabled`), so + // a chip whose popover is open while it transitions to disabled would otherwise stay editable. useEffect(() => { - if (!showMorphology) setPopoverOpen(false); - }, [showMorphology]); + if (!showMorphology || disabled) setPopoverOpen(false); + }, [showMorphology, disabled]); const handleMouseDown: MouseEventHandler = (e) => { // Prevent the browser's built-in focus-and-scroll so only the React-controlled From 53bfe5495374911ea045b01afb9f7be63b8e2375 Mon Sep 17 00:00:00 2001 From: "D. Ror." Date: Wed, 17 Jun 2026 14:14:51 -0600 Subject: [PATCH 15/16] Morpheme analysis UI: suggestions for #104 (#111) * Address review feedback on morpheme analysis UI - Don't save empty or whole-word morpheme breakdowns: disable the Done button and guard the Enter / outside-click commit paths - Localize morpheme editor strings and morpheme/trigger aria-labels - Validate morpheme element shape in isTextAnalysis at the persistence boundary (isTokenAnalysisRecord / isMorphemeAnalysis) - Restore focus to the token gloss input when the morpheme popover closes - Gate the disabled morpheme trigger's hover/cursor affordance - De-duplicate the boolean project-setting validators into one isBoolean - Document the deliberate provenance exclusion in isEmptyTokenAnalysis and de-stale the viewOptions memo comment Co-Authored-By: Claude Opus 4.8 (1M context) * Focus on 1st morph gloss * Tweak * Extract handle --------- Co-authored-by: Claude Opus 4.8 (1M context) --- __mocks__/platform-bible-react.tsx | 15 ++ contributions/localizedStrings.json | 7 + .../components/MorphemeEditor.test.tsx | 192 +++++++++++++----- src/__tests__/components/TokenChip.test.tsx | 10 + src/components/InterlinearizerLoader.tsx | 2 +- src/components/MorphemeEditor.tsx | 94 +++++++-- src/components/TokenChip.tsx | 20 +- src/main.ts | 21 +- src/store/analysisSlice.ts | 7 + src/types/type-guards.ts | 46 ++++- 10 files changed, 325 insertions(+), 89 deletions(-) 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({