From 6586d0f0dbb74aa471d391c4ceb64cd00f0bd96b Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 31 Aug 2026 12:15:05 -0600 Subject: [PATCH 01/14] Add analysis-keyed mutation reducers for the analysis catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The catalog's rows are shared TokenAnalysis payloads with no token in context, so the existing tokenRef-keyed reducers cannot serve them: those fork a shared payload to keep an edit local to one token, which is the opposite of what editing a catalog row means. Add a second reducer family keyed by analysisId — writeAnalysisGloss, writeAnalysisMorphemes, writeAnalysisMorphemeGloss, deleteAnalysis, and mergeAnalysisInto — that never forks. The key alone determines the blast radius, so neither family needs a scope flag. Each write re-converges onto a content-identical sibling as the per-token path does. Add selectAnalysisDeletionOutcome, which reports whether deleting a row leaves its tokens blank or falling back to a surviving homograph, so the confirmation can name the concrete consequence. It rebuilds the pool with the payload dropped outright rather than discounting a single approval, which would leave a multi-token payload competing to replace itself. Add selectAnalysisMergePeers to offer merge targets from the row's own pool bucket, so merging is confined to genuine homographs. --- src/__tests__/store/analysisSlice.test.ts | 385 ++++++++++++++++++++++ src/store/analysisSlice.ts | 259 ++++++++++++++- 2 files changed, 643 insertions(+), 1 deletion(-) diff --git a/src/__tests__/store/analysisSlice.test.ts b/src/__tests__/store/analysisSlice.test.ts index 2687fb87..c5d9b6e4 100644 --- a/src/__tests__/store/analysisSlice.test.ts +++ b/src/__tests__/store/analysisSlice.test.ts @@ -13,9 +13,13 @@ import { createAnalysisStore } from '../../store'; import { approveAnalysisForToken, createPhrase, + deleteAnalysis, deleteMorphemes, deletePhrase, + mergeAnalysisInto, mergePhrases, + selectAnalysisDeletionOutcome, + selectAnalysisMergePeers, selectApprovedGloss, selectApprovedMorphemes, selectCatalogRows, @@ -28,6 +32,9 @@ import { selectSuggestionAfterClearing, selectSegmentFreeTranslation, updatePhrase, + writeAnalysisGloss, + writeAnalysisMorphemeGloss, + writeAnalysisMorphemes, writeGloss, writeMorphemeGloss, writeMorphemes, @@ -2525,3 +2532,381 @@ describe('analysis timestamps', () => { }); }); }); + +describe('analysis-keyed reducers', () => { + /** + * Builds a store where one payload is shared by two approved tokens, the shape the catalog's + * whole reason for existing rests on: one row, many usages. + */ + function makeSharedStore(overrides?: Partial) { + const shared: TokenAnalysis = { + ...FIXTURE_STAMPS, + id: 'ta-shared', + surfaceText: 'word', + gloss: { und: 'first' }, + ...overrides, + }; + const links: TokenAnalysisLink[] = ['tok-1', 'tok-2'].map((tokenRef) => ({ + ...FIXTURE_STAMPS, + analysisId: shared.id, + status: 'approved', + token: { tokenRef, surfaceText: 'word' }, + })); + return createAnalysisStore({ + analysis: { + analysis: { ...emptyAnalysis(), tokenAnalyses: [shared], tokenAnalysisLinks: links }, + analysisLanguage: 'und', + }, + }); + } + + describe('writeAnalysisGloss', () => { + it('rewrites the gloss for every token linked to the payload', () => { + const store = makeSharedStore(); + + store.dispatch(writeAnalysisGloss({ analysisId: 'ta-shared', value: 'second' })); + + const state = store.getState().analysis; + expect(selectApprovedGloss(state, 'tok-1')).toBe('second'); + expect(selectApprovedGloss(state, 'tok-2')).toBe('second'); + }); + + it('does not fork the shared payload', () => { + const store = makeSharedStore(); + + store.dispatch(writeAnalysisGloss({ analysisId: 'ta-shared', value: 'second' })); + + expect(store.getState().analysis.analysis.tokenAnalyses).toHaveLength(1); + }); + + it('removes the record entirely when the edit empties it', () => { + const store = makeSharedStore(); + + store.dispatch(writeAnalysisGloss({ analysisId: 'ta-shared', value: ' ' })); + + const { tokenAnalyses, tokenAnalysisLinks } = store.getState().analysis.analysis; + expect(tokenAnalyses).toHaveLength(0); + expect(tokenAnalysisLinks).toHaveLength(0); + }); + + it('keeps the record when clearing the gloss leaves other content behind', () => { + const store = makeSharedStore({ + morphemes: [{ id: 'm-1', form: 'word', writingSystem: 'en' }], + }); + + store.dispatch(writeAnalysisGloss({ analysisId: 'ta-shared', value: '' })); + + const { tokenAnalyses } = store.getState().analysis.analysis; + expect(tokenAnalyses).toHaveLength(1); + expect(tokenAnalyses[0].gloss).toBeUndefined(); + }); + + it('collapses onto a content-identical sibling, leaving the sibling as the survivor', () => { + const store = makeSharedStore(); + // A second payload for the same word, glossed differently — a homograph the edit will match. + store.dispatch(writeGloss('tok-3', 'word', 'second')); + const sibling = store + .getState() + .analysis.analysis.tokenAnalyses.find((ta) => ta.id !== 'ta-shared'); + + store.dispatch(writeAnalysisGloss({ analysisId: 'ta-shared', value: 'second' })); + + const { tokenAnalyses } = store.getState().analysis.analysis; + expect(tokenAnalyses).toHaveLength(1); + expect(tokenAnalyses[0].id).toBe(sibling?.id); + }); + + it('moves the collapsed payload’s links onto the surviving sibling', () => { + const store = makeSharedStore(); + store.dispatch(writeGloss('tok-3', 'word', 'second')); + + store.dispatch(writeAnalysisGloss({ analysisId: 'ta-shared', value: 'second' })); + + const state = store.getState().analysis; + expect(selectApprovedGloss(state, 'tok-1')).toBe('second'); + expect(selectApprovedGloss(state, 'tok-2')).toBe('second'); + expect(selectApprovedGloss(state, 'tok-3')).toBe('second'); + }); + + it('ignores an analysisId that resolves to no payload', () => { + const store = makeSharedStore(); + + store.dispatch(writeAnalysisGloss({ analysisId: 'nope', value: 'second' })); + + expect(selectApprovedGloss(store.getState().analysis, 'tok-1')).toBe('first'); + }); + }); + + describe('writeAnalysisMorphemes', () => { + it('rewrites the breakdown for every token linked to the payload', () => { + const store = makeSharedStore(); + + store.dispatch( + writeAnalysisMorphemes({ + analysisId: 'ta-shared', + forms: ['wor', 'd'], + writingSystem: 'en', + }), + ); + + const state = store.getState().analysis; + expect(selectApprovedMorphemes(state, 'tok-1').map((m) => m.form)).toEqual(['wor', 'd']); + expect(selectApprovedMorphemes(state, 'tok-2').map((m) => m.form)).toEqual(['wor', 'd']); + }); + + it('does not fork the shared payload', () => { + const store = makeSharedStore(); + + store.dispatch( + writeAnalysisMorphemes({ + analysisId: 'ta-shared', + forms: ['wor', 'd'], + writingSystem: 'en', + }), + ); + + expect(store.getState().analysis.analysis.tokenAnalyses).toHaveLength(1); + }); + + it('removes the breakdown when given no forms', () => { + const store = makeSharedStore({ + morphemes: [{ id: 'm-1', form: 'word', writingSystem: 'en' }], + }); + + store.dispatch( + writeAnalysisMorphemes({ analysisId: 'ta-shared', forms: [], writingSystem: 'en' }), + ); + + expect(store.getState().analysis.analysis.tokenAnalyses[0].morphemes).toBeUndefined(); + }); + + it('removes a record whose only content was the breakdown it just lost', () => { + const store = makeSharedStore({ + gloss: undefined, + morphemes: [{ id: 'm-1', form: 'word', writingSystem: 'en' }], + }); + + store.dispatch( + writeAnalysisMorphemes({ analysisId: 'ta-shared', forms: [], writingSystem: 'en' }), + ); + + expect(store.getState().analysis.analysis.tokenAnalyses).toHaveLength(0); + }); + + it('ignores an analysisId that resolves to no payload', () => { + const store = makeSharedStore(); + + store.dispatch( + writeAnalysisMorphemes({ analysisId: 'nope', forms: ['x'], writingSystem: 'en' }), + ); + + expect(store.getState().analysis.analysis.tokenAnalyses[0].morphemes).toBeUndefined(); + }); + }); + + describe('writeAnalysisMorphemeGloss', () => { + /** A shared payload carrying a two-morpheme breakdown, the unit this reducer edits. */ + function makeMorphemeStore() { + return makeSharedStore({ + morphemes: [ + { id: 'm-1', form: 'wor', writingSystem: 'en' }, + { id: 'm-2', form: 'd', writingSystem: 'en' }, + ], + }); + } + + it('writes a morpheme gloss for every token linked to the payload', () => { + const store = makeMorphemeStore(); + + store.dispatch( + writeAnalysisMorphemeGloss({ analysisId: 'ta-shared', morphemeId: 'm-1', value: 'WORD' }), + ); + + const state = store.getState().analysis; + expect(selectApprovedMorphemes(state, 'tok-1')[0].gloss?.und).toBe('WORD'); + expect(selectApprovedMorphemes(state, 'tok-2')[0].gloss?.und).toBe('WORD'); + }); + + it('keeps the morpheme when its gloss is cleared', () => { + const store = makeMorphemeStore(); + store.dispatch( + writeAnalysisMorphemeGloss({ analysisId: 'ta-shared', morphemeId: 'm-1', value: 'WORD' }), + ); + + store.dispatch( + writeAnalysisMorphemeGloss({ analysisId: 'ta-shared', morphemeId: 'm-1', value: '' }), + ); + + const morphemes = selectApprovedMorphemes(store.getState().analysis, 'tok-1'); + expect(morphemes).toHaveLength(2); + expect(morphemes[0].gloss).toBeUndefined(); + }); + + it('ignores a morphemeId the payload does not carry', () => { + const store = makeMorphemeStore(); + + store.dispatch( + writeAnalysisMorphemeGloss({ analysisId: 'ta-shared', morphemeId: 'nope', value: 'WORD' }), + ); + + expect(selectApprovedMorphemes(store.getState().analysis, 'tok-1')[0].gloss).toBeUndefined(); + }); + }); + + describe('deleteAnalysis', () => { + it('removes the payload and every link to it', () => { + const store = makeSharedStore(); + + store.dispatch(deleteAnalysis({ analysisId: 'ta-shared' })); + + const { tokenAnalyses, tokenAnalysisLinks } = store.getState().analysis.analysis; + expect(tokenAnalyses).toHaveLength(0); + expect(tokenAnalysisLinks).toHaveLength(0); + }); + + it('leaves the affected tokens reading as blank', () => { + const store = makeSharedStore(); + + store.dispatch(deleteAnalysis({ analysisId: 'ta-shared' })); + + const state = store.getState().analysis; + expect(selectApprovedGloss(state, 'tok-1')).toBe(''); + expect(selectApprovedGloss(state, 'tok-2')).toBe(''); + }); + + it('leaves a surviving homograph untouched', () => { + const store = makeSharedStore(); + store.dispatch(writeGloss('tok-3', 'word', 'second')); + + store.dispatch(deleteAnalysis({ analysisId: 'ta-shared' })); + + expect(selectApprovedGloss(store.getState().analysis, 'tok-3')).toBe('second'); + }); + }); + + describe('mergeAnalysisInto', () => { + it('moves every link to the target and drops the source', () => { + const store = makeSharedStore(); + store.dispatch(writeGloss('tok-3', 'word', 'second')); + const target = store + .getState() + .analysis.analysis.tokenAnalyses.find((ta) => ta.id !== 'ta-shared'); + + store.dispatch( + mergeAnalysisInto({ + sourceAnalysisId: 'ta-shared', + targetAnalysisId: target?.id ?? '', + }), + ); + + const { tokenAnalyses } = store.getState().analysis.analysis; + expect(tokenAnalyses).toHaveLength(1); + expect(tokenAnalyses[0].id).toBe(target?.id); + }); + + it('sums the usage count onto the target', () => { + const store = makeSharedStore(); + store.dispatch(writeGloss('tok-3', 'word', 'second')); + const target = store + .getState() + .analysis.analysis.tokenAnalyses.find((ta) => ta.id !== 'ta-shared'); + + store.dispatch( + mergeAnalysisInto({ sourceAnalysisId: 'ta-shared', targetAnalysisId: target?.id ?? '' }), + ); + + const state = store.getState().analysis; + expect(selectApprovedGloss(state, 'tok-1')).toBe('second'); + expect(selectApprovedGloss(state, 'tok-2')).toBe('second'); + expect(selectApprovedGloss(state, 'tok-3')).toBe('second'); + }); + + it('ignores a merge of a record into itself', () => { + const store = makeSharedStore(); + + store.dispatch( + mergeAnalysisInto({ sourceAnalysisId: 'ta-shared', targetAnalysisId: 'ta-shared' }), + ); + + expect(store.getState().analysis.analysis.tokenAnalyses).toHaveLength(1); + }); + + it('ignores a target that resolves to no payload', () => { + const store = makeSharedStore(); + + store.dispatch( + mergeAnalysisInto({ sourceAnalysisId: 'ta-shared', targetAnalysisId: 'nope' }), + ); + + expect(selectApprovedGloss(store.getState().analysis, 'tok-1')).toBe('first'); + }); + + it('ignores a source that resolves to no payload', () => { + const store = makeSharedStore(); + + store.dispatch( + mergeAnalysisInto({ sourceAnalysisId: 'nope', targetAnalysisId: 'ta-shared' }), + ); + + expect(store.getState().analysis.analysis.tokenAnalyses).toHaveLength(1); + }); + }); + + describe('selectAnalysisDeletionOutcome', () => { + it('reports a blank outcome when no homograph survives the deletion', () => { + const store = makeSharedStore(); + + const outcome = selectAnalysisDeletionOutcome(store.getState().analysis, 'ta-shared'); + + expect(outcome).toEqual({ kind: 'blank', usageCount: 2 }); + }); + + it('reports a fallback outcome naming the gloss the tokens will read', () => { + const store = makeSharedStore(); + store.dispatch(writeGloss('tok-3', 'word', 'second')); + + const outcome = selectAnalysisDeletionOutcome(store.getState().analysis, 'ta-shared'); + + expect(outcome).toEqual({ kind: 'fallback', usageCount: 2, fallbackGloss: 'second' }); + }); + + it('omits the fallback gloss when the surviving peer has none in the analysis language', () => { + const store = makeSharedStore(); + store.dispatch(writeMorphemes('tok-3', 'word', ['wor', 'd'], 'en')); + + const outcome = selectAnalysisDeletionOutcome(store.getState().analysis, 'ta-shared'); + + expect(outcome).toMatchObject({ kind: 'fallback', usageCount: 2 }); + expect(outcome?.fallbackGloss).toBeUndefined(); + }); + + it('returns undefined for an analysisId that resolves to no payload', () => { + const store = makeSharedStore(); + + expect(selectAnalysisDeletionOutcome(store.getState().analysis, 'nope')).toBeUndefined(); + }); + }); + + describe('selectAnalysisMergePeers', () => { + it('offers the homographs sharing the row’s surface form', () => { + const store = makeSharedStore(); + store.dispatch(writeGloss('tok-3', 'word', 'second')); + + const peers = selectAnalysisMergePeers(store.getState().analysis, 'ta-shared'); + + expect(peers.map((p) => p.gloss?.und)).toEqual(['second']); + }); + + it('offers nothing when the row has no homograph', () => { + const store = makeSharedStore(); + + expect(selectAnalysisMergePeers(store.getState().analysis, 'ta-shared')).toHaveLength(0); + }); + + it('offers nothing for an analysisId that resolves to no payload', () => { + const store = makeSharedStore(); + + expect(selectAnalysisMergePeers(store.getState().analysis, 'nope')).toHaveLength(0); + }); + }); +}); diff --git a/src/store/analysisSlice.ts b/src/store/analysisSlice.ts index 426ef2d7..81fc8ecb 100644 --- a/src/store/analysisSlice.ts +++ b/src/store/analysisSlice.ts @@ -11,7 +11,7 @@ import type { TokenSnapshot, } from 'interlinearizer'; import { emptyAnalysis } from '../types/empty-factories'; -import { analysesAreIdentical } from '../utils/analysis-identity'; +import { analysesAreIdentical, normalizeSurfaceForm } from '../utils/analysis-identity'; import { buildCatalogRows } from '../utils/analysis-query'; import { isEmptyMultiString } from '../utils/multi-string'; import { @@ -353,6 +353,18 @@ function mergeIntoIdenticalPayload(state: AnalysisState, analysis: TokenAnalysis state.analysis.tokenAnalyses = state.analysis.tokenAnalyses.filter((ta) => ta !== analysis); } +/** + * Drops a `TokenAnalysis` and every link pointing at it, addressed by id alone — so the record goes + * on its own terms and takes every token with it, rather than being retired as one token lets go of + * it. A no-op when the id resolves to no payload. + */ +function removeAnalysisAndLinks(state: AnalysisState, analysisId: string): void { + state.analysis.tokenAnalyses = state.analysis.tokenAnalyses.filter((ta) => ta.id !== analysisId); + state.analysis.tokenAnalysisLinks = state.analysis.tokenAnalysisLinks.filter( + (l) => l.analysisId !== analysisId, + ); +} + /** * 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 @@ -677,6 +689,174 @@ const analysisSlice = createSlice({ mergeIntoIdenticalPayload(state, target); }, }, + // The reducers below are keyed by `analysisId` rather than `tokenRef`, and the key is the whole + // of the scope distinction: a `tokenRef` edit changes what one token means and forks a shared + // payload to do it, an `analysisId` edit changes what the record says everywhere. Neither + // family takes a scope flag, because the address the caller can supply already says which act + // it is. + /** + * Writes a gloss onto a `TokenAnalysis` addressed by its own id, changing what that record says + * for every token linked to it. + * + * A blank `value` clears the active language's gloss, and an edit that empties the record + * removes it and every link to it. An edit that makes the record identical to a sibling + * collapses it into that sibling, so the edited row disappears from the catalog. + */ + writeAnalysisGloss: { + /** Reads the clock before the action reaches the reducer, keeping the reducer pure. */ + prepare(arg: { analysisId: string; value: string }) { + return { payload: { ...arg, now: nowIso() } }; + }, + reducer(state, action: PayloadAction<{ analysisId: string; value: string; now: string }>) { + const { analysisId, value, now } = action.payload; + const lang = state.analysisLanguage; + + const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === analysisId); + if (!analysis) return; + + if (value.trim() === '') { + if (analysis.gloss) { + delete analysis.gloss[lang]; + if (Object.keys(analysis.gloss).length === 0) delete analysis.gloss; + } + } else { + if (!analysis.gloss) analysis.gloss = {}; + analysis.gloss[lang] = value; + } + analysis.updatedAt = now; + + // Removed outright rather than left as an empty payload the pool would still carry. + if (isEmptyTokenAnalysis(analysis)) { + removeAnalysisAndLinks(state, analysisId); + return; + } + mergeIntoIdenticalPayload(state, analysis); + }, + }, + /** + * Replaces the morpheme breakdown on a `TokenAnalysis` addressed by its own id, for every token + * linked to it, so one correction fixes a mis-split word across all its occurrences. + * + * The breakdown is replaced rather than reconciled: morphemes are rebuilt from `forms` with + * fresh ids, dropping any glosses the old morphemes carried, which the caller is expected to + * have warned about. An empty `forms` removes the breakdown, and removes the record when + * nothing else remains on it. + */ + writeAnalysisMorphemes: { + /** + * Mints the new morphemes' ids and reads the clock before the action reaches the reducer, + * keeping the reducer pure. + */ + prepare(arg: { analysisId: string; forms: readonly string[]; writingSystem: string }) { + return { + payload: { + analysisId: arg.analysisId, + writingSystem: arg.writingSystem, + morphemes: arg.forms.map((form) => ({ id: crypto.randomUUID(), form })), + now: nowIso(), + }, + }; + }, + reducer( + state, + action: PayloadAction<{ + analysisId: string; + writingSystem: string; + morphemes: readonly { id: string; form: string }[]; + now: string; + }>, + ) { + const { analysisId, writingSystem, morphemes, now } = action.payload; + + const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === analysisId); + if (!analysis) return; + + if (morphemes.length === 0) delete analysis.morphemes; + else analysis.morphemes = morphemes.map(({ id, form }) => ({ id, form, writingSystem })); + analysis.updatedAt = now; + + if (isEmptyTokenAnalysis(analysis)) { + removeAnalysisAndLinks(state, analysisId); + return; + } + mergeIntoIdenticalPayload(state, analysis); + }, + }, + /** + * Writes a gloss onto one morpheme of a `TokenAnalysis` addressed by its own id, for every + * token linked to it. Clearing the gloss keeps the morpheme, a breakdown being content in its + * own right, so this never empties the enclosing record. + */ + writeAnalysisMorphemeGloss: { + /** Reads the clock before the action reaches the reducer, keeping the reducer pure. */ + prepare(arg: { analysisId: string; morphemeId: string; value: string }) { + return { payload: { ...arg, now: nowIso() } }; + }, + reducer( + state, + action: PayloadAction<{ + analysisId: string; + morphemeId: string; + value: string; + now: string; + }>, + ) { + const { analysisId, morphemeId, value, now } = action.payload; + const lang = state.analysisLanguage; + + const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === analysisId); + const morpheme = analysis?.morphemes?.find((m) => m.id === morphemeId); + if (!analysis || !morpheme) return; + + if (value.trim() === '') { + if (morpheme.gloss) { + delete morpheme.gloss[lang]; + if (Object.keys(morpheme.gloss).length === 0) delete morpheme.gloss; + } + } else { + if (!morpheme.gloss) morpheme.gloss = {}; + morpheme.gloss[lang] = value; + } + analysis.updatedAt = now; + // A morpheme gloss is part of analysis identity, so this edit can collapse onto a sibling. + mergeIntoIdenticalPayload(state, analysis); + }, + }, + /** + * Removes a `TokenAnalysis` and every link to it. Its tokens fall back to whatever the + * suggestion pool still offers for their surface form — a surviving homograph, or nothing, in + * which case they read as blank; {@link selectAnalysisDeletionOutcome} reports which. + * + * Irreversible, and the only reducer that drops a record the user never emptied. + */ + deleteAnalysis(state, action: PayloadAction<{ analysisId: string }>) { + removeAnalysisAndLinks(state, action.payload.analysisId); + }, + /** + * Moves every link on one `TokenAnalysis` to another and drops the source, so the target's + * usage count becomes the sum of the two and the source's tokens end up analyzed as the target + * rather than stranded with nothing. + * + * Only the links move: no write is aimed at what the target says, so neither it nor the moved + * links are re-stamped. No-ops when either id resolves to no payload, or when both name the + * same record. + */ + mergeAnalysisInto( + state, + action: PayloadAction<{ sourceAnalysisId: string; targetAnalysisId: string }>, + ) { + const { sourceAnalysisId, targetAnalysisId } = action.payload; + if (sourceAnalysisId === targetAnalysisId) return; + const has = (id: string) => state.analysis.tokenAnalyses.some((ta) => ta.id === id); + if (!has(sourceAnalysisId) || !has(targetAnalysisId)) return; + + state.analysis.tokenAnalysisLinks.forEach((l) => { + if (l.analysisId === sourceAnalysisId) l.analysisId = targetAnalysisId; + }); + state.analysis.tokenAnalyses = state.analysis.tokenAnalyses.filter( + (ta) => ta.id !== sourceAnalysisId, + ); + }, /** * Approves a shared `TokenAnalysis` payload for a token — the persisted half of accepting a * suggestion or promoting a candidate (see {@link selectResolvedTokenAnalysis}). No new payload @@ -941,6 +1121,11 @@ export const { writeMorphemes, deleteMorphemes, writeMorphemeGloss, + writeAnalysisGloss, + writeAnalysisMorphemes, + writeAnalysisMorphemeGloss, + deleteAnalysis, + mergeAnalysisInto, approveAnalysisForToken, createPhrase, updatePhrase, @@ -955,6 +1140,12 @@ export default analysisSlice.reducer; // #region Selectors +/** + * Shared empty result for a row with no merge peers, so a subscriber reading it under `Object.is` + * sees a stable reference rather than a fresh array each call. + */ +const NO_MERGE_PEERS: readonly TokenAnalysis[] = []; + /** Projects `tokenAnalyses` out of `AnalysisState` for use as a `createSelector` input. */ const selectTokenAnalyses = (state: AnalysisState) => state.analysis.tokenAnalyses; @@ -1054,6 +1245,72 @@ export const selectCatalogRows = createSelector( buildCatalogRows({ tokenAnalyses, tokenAnalysisLinks }, { analysisLanguage, currentBook }), ); +/** + * What deleting a `TokenAnalysis` would do to the tokens that approve it, so an irreversible delete + * can be confirmed with its concrete consequence rather than a generic "are you sure". + */ +export interface AnalysisDeletionOutcome { + /** + * `'blank'` when the affected tokens are left reading as unanalyzed, `'fallback'` when a + * surviving homograph takes over and they read as that instead. + */ + kind: 'blank' | 'fallback'; + /** How many tokens the deletion affects. */ + usageCount: number; + /** + * What the affected tokens will read once the deletion commits. Absent when the surviving peer + * carries no gloss in the active analysis language, leaving no word to quote at the user. + */ + fallbackGloss?: string; +} + +/** + * Reports what {@link deleteAnalysis} would do to the given row, for the confirmation to name. + * Returns `undefined` when the id resolves to no payload, so a stale row cannot open a confirmation + * for a record that is already gone. + */ +export function selectAnalysisDeletionOutcome( + state: AnalysisState, + analysisId: string, +): AnalysisDeletionOutcome | undefined { + const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === analysisId); + if (!analysis) return undefined; + + const usageCount = state.analysis.tokenAnalysisLinks.filter( + (l) => l.analysisId === analysisId && l.status === 'approved', + ).length; + + // Ask the engine, so the confirmation names the peer that actually wins. The payload is dropped + // from the pool outright rather than discounted by one approval: a deletion removes all of its + // approvals at once, and a discounted multi-token payload would compete to replace itself. + const survivingPool = buildPoolIndex( + selectAnalysisById(state), + new Map([...selectApprovedTokenCountByAnalysisId(state)].filter(([id]) => id !== analysisId)), + ); + const fallback = deriveTokenSuggestion(survivingPool, analysis.surfaceText); + if (!fallback) return { kind: 'blank', usageCount }; + + const gloss = fallback.suggested.gloss?.[state.analysisLanguage]; + return { kind: 'fallback', usageCount, ...(gloss ? { fallbackGloss: gloss } : {}) }; +} + +/** + * The other analyses a row may be merged into: those sharing its normalized surface form, so merge + * is offered only among genuine homographs and a row with no peers offers none. Ordered best-first, + * putting the most-used peer at the head. + */ +export function selectAnalysisMergePeers( + state: AnalysisState, + analysisId: string, +): readonly TokenAnalysis[] { + const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === analysisId); + if (!analysis) return NO_MERGE_PEERS; + const bucket = selectPoolIndex(state).get(normalizeSurfaceForm(analysis.surfaceText)); + if (!bucket) return NO_MERGE_PEERS; + const peers = bucket.filter((e) => e.analysis.id !== analysisId).map((e) => e.analysis); + return peers.length > 0 ? peers : NO_MERGE_PEERS; +} + /** * Returns the merged analysis the renderer shows for a token: its approved decision when one * exists, otherwise the engine's suggestion derived live from the approved-analysis pool, or From fb56e8adf904841efae8c4ecd5432960aec56924 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 1 Sep 2026 16:43:44 -0600 Subject: [PATCH 02/14] Hold the catalog's filters and breakdowns to what the data offers Reconcile the filter selections against the facets each render, so a selection cannot outlive the choice it names: an edit beside the panel that empties a facet no longer strands the listing behind a control that is off screen. Distinguish a placeholder label from a real value reading the same, the platform combo box resolving a selection by its label alone. Carry an unchanged morpheme across a re-split whole, keeping the id its references depend on along with its gloss and lexicon refs. Count the deletion preview's usages off the approved-token index the catalog row counts by, as its own doc already promised. --- .../components/AnalysisCatalogPanel.test.tsx | 68 +++++++ src/__tests__/store/analysisSlice.test.ts | 173 ++++++++++++++++++ src/__tests__/utils/analysis-query.test.ts | 84 +++++++++ src/components/AnalysisCatalogPanel.tsx | 30 ++- src/components/CatalogFilterPopover.tsx | 23 ++- src/components/CatalogQueryControls.tsx | 6 +- src/store/analysisSlice.ts | 74 +++++--- src/utils/analysis-query.ts | 77 ++++++++ 8 files changed, 494 insertions(+), 41 deletions(-) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 9855ddd4..4cb51b17 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -873,6 +873,74 @@ describe('AnalysisCatalogPanel', () => { expect(listedAnalysisIds()).toEqual(['blank']); }); + // The platform combo box resolves a click by matching the label back to its entry, so a real + // value reading exactly as the untagged placeholder would otherwise take that choice's clicks. + /** Two analyses in two books, so the books facet is offered and one edit can collapse it. */ + const TWO_BOOKS: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'in-gen', surfaceText: 'λόγος', gloss: { en: 'word' } }, + { ...FIXTURE_STAMPS, id: 'in-exo', surfaceText: 'ἦν', gloss: { en: 'was' } }, + ], + tokenAnalysisLinks: [link('in-gen', 'GEN 1:1:0'), link('in-exo', 'EXO 1:1:0')], + }; + + // An edit beside the panel can remove the last row carrying a chosen value, which takes that + // facet's control off screen. Holding the choice would narrow the list to nothing with no + // control left to widen it back by. + it('releases a book filter once an edit leaves that facet with nothing to offer', async () => { + renderPanelWithGlossEditing({ analysis: TWO_BOOKS, analysisLanguage: 'en' }); + await openFilters(); + const books = within(screen.getByTestId('catalog-filter-books')); + await userEvent.click(books.getByRole('option', { name: 'EXO' })); + expect(listedAnalysisIds()).toEqual(['in-exo']); + + // Clearing its only gloss empties the payload, which drops the analysis and its link. + act(() => editGloss('EXO 1:1:0', 'ἦν', '')); + + expect(listedAnalysisIds()).toEqual(['in-gen']); + }); + + it('stops counting a filter the facets have withdrawn as active', async () => { + renderPanelWithGlossEditing({ analysis: TWO_BOOKS, analysisLanguage: 'en' }); + await openFilters(); + const books = within(screen.getByTestId('catalog-filter-books')); + await userEvent.click(books.getByRole('option', { name: 'EXO' })); + + act(() => editGloss('EXO 1:1:0', 'ἦν', '')); + + expect(screen.getByTestId('catalog-filters-button')).toHaveTextContent( + '%interlinearizer_analysisCatalog_filters%', + ); + }); + + it('tells the untagged choice apart from a value that reads the same', async () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { + ...FIXTURE_STAMPS, + id: 'named', + surfaceText: 'λόγος', + pos: '%interlinearizer_analysisCatalog_filter_untagged%', + }, + { ...FIXTURE_STAMPS, id: 'untagged', surfaceText: 'ἦν' }, + ], + tokenAnalysisLinks: [link('named', 'GEN 1:1:0'), link('untagged', 'GEN 1:2:0')], + }; + renderPanel({ analysis }); + await openFilters(); + + // The untagged choice keeps the plain label and the value reading the same is marked as a + // recorded value, so an exact match on the label reaches the choice rather than the value. + const pos = within(screen.getByTestId('catalog-filter-pos')); + await userEvent.click( + pos.getByRole('option', { name: '%interlinearizer_analysisCatalog_filter_untagged%' }), + ); + + expect(listedAnalysisIds()).toEqual(['untagged']); + }); + it('offers no book choice for a draft confined to one book', async () => { const analysis: TextAnalysis = { ...emptyAnalysis(), diff --git a/src/__tests__/store/analysisSlice.test.ts b/src/__tests__/store/analysisSlice.test.ts index c5d9b6e4..1fa9026f 100644 --- a/src/__tests__/store/analysisSlice.test.ts +++ b/src/__tests__/store/analysisSlice.test.ts @@ -2702,6 +2702,115 @@ describe('analysis-keyed reducers', () => { expect(store.getState().analysis.analysis.tokenAnalyses[0].morphemes).toBeUndefined(); }); + + /** A shared payload broken down into a prefix and a stem, each carrying what it was analyzed as. */ + function makeAnnotatedStore() { + return makeSharedStore({ + surfaceText: 'unhappy', + morphemes: [ + { + id: 'm-un', + form: 'un', + writingSystem: 'en', + gloss: { und: 'NEG' }, + entryRef: { authority: 'lexicon', entryId: 'e-un' }, + }, + { id: 'm-happy', form: 'happy', writingSystem: 'en', gloss: { und: 'glad' } }, + ], + }); + } + + it('keeps an unchanged morpheme whole when a neighbor is re-split', () => { + const store = makeAnnotatedStore(); + + store.dispatch( + writeAnalysisMorphemes({ + analysisId: 'ta-shared', + forms: ['un', 'happi'], + writingSystem: 'en', + }), + ); + + const [kept] = store.getState().analysis.analysis.tokenAnalyses[0].morphemes ?? []; + expect(kept).toEqual({ + id: 'm-un', + form: 'un', + writingSystem: 'en', + gloss: { und: 'NEG' }, + entryRef: { authority: 'lexicon', entryId: 'e-un' }, + }); + }); + + it('mints a morpheme for a form the old breakdown cannot account for', () => { + const store = makeAnnotatedStore(); + + store.dispatch( + writeAnalysisMorphemes({ + analysisId: 'ta-shared', + forms: ['un', 'happi'], + writingSystem: 'en', + }), + ); + + const morphemes = store.getState().analysis.analysis.tokenAnalyses[0].morphemes ?? []; + expect(morphemes[1]).toEqual({ + id: expect.any(String), + form: 'happi', + writingSystem: 'en', + }); + expect(morphemes[1].id).not.toBe('m-happy'); + }); + + it('matches a repeated form against a distinct morpheme each time', () => { + const store = makeSharedStore({ + surfaceText: 'ba ba', + morphemes: [ + { id: 'm-1', form: 'ba', writingSystem: 'en', gloss: { und: 'first' } }, + { id: 'm-2', form: 'ba', writingSystem: 'en', gloss: { und: 'second' } }, + ], + }); + + store.dispatch( + writeAnalysisMorphemes({ + analysisId: 'ta-shared', + forms: ['ba', 'ba'], + writingSystem: 'en', + }), + ); + + const morphemes = store.getState().analysis.analysis.tokenAnalyses[0].morphemes ?? []; + expect(morphemes.map((m) => m.id)).toEqual(['m-1', 'm-2']); + }); + + it('refreshes the writing system on a morpheme it keeps', () => { + const store = makeAnnotatedStore(); + + store.dispatch( + writeAnalysisMorphemes({ + analysisId: 'ta-shared', + forms: ['un', 'happy'], + writingSystem: 'fr', + }), + ); + + const morphemes = store.getState().analysis.analysis.tokenAnalyses[0].morphemes ?? []; + expect(morphemes.map((m) => m.writingSystem)).toEqual(['fr', 'fr']); + }); + + it('drops what a form carried when the re-split leaves no morpheme holding it', () => { + const store = makeAnnotatedStore(); + + store.dispatch( + writeAnalysisMorphemes({ + analysisId: 'ta-shared', + forms: ['unhappy'], + writingSystem: 'en', + }), + ); + + const morphemes = store.getState().analysis.analysis.tokenAnalyses[0].morphemes ?? []; + expect(morphemes).toEqual([{ id: expect.any(String), form: 'unhappy', writingSystem: 'en' }]); + }); }); describe('writeAnalysisMorphemeGloss', () => { @@ -2885,6 +2994,70 @@ describe('analysis-keyed reducers', () => { expect(selectAnalysisDeletionOutcome(store.getState().analysis, 'nope')).toBeUndefined(); }); + + // The catalog offers a zero-usages filter for exactly this row, so the confirmation it opens + // has to have a number for one: an analysis nothing approves affects no token at all. + it('reports no usages for an analysis no token approves', () => { + const unapproved: TokenAnalysis = { + ...FIXTURE_STAMPS, + id: 'ta-unused', + surfaceText: 'word', + gloss: { und: 'first' }, + }; + const store = createAnalysisStore({ + analysis: { + analysis: { + ...emptyAnalysis(), + tokenAnalyses: [unapproved], + tokenAnalysisLinks: [ + { + ...FIXTURE_STAMPS, + analysisId: unapproved.id, + status: 'candidate', + token: { tokenRef: 'tok-1', surfaceText: 'word' }, + }, + ], + }, + analysisLanguage: 'und', + }, + }); + + const outcome = selectAnalysisDeletionOutcome(store.getState().analysis, 'ta-unused'); + + expect(outcome).toEqual({ kind: 'blank', usageCount: 0 }); + }); + + // The confirmation opens from a catalog row, which counts a token once however many approved + // links carry it to the same analysis. No write path builds a duplicate, so this is the shape + // imported or hand-edited data arrives in; the two numbers must still agree. + it('counts a token carrying the same approval twice as one usage', () => { + const shared: TokenAnalysis = { + ...FIXTURE_STAMPS, + id: 'ta-shared', + surfaceText: 'word', + gloss: { und: 'first' }, + }; + const duplicated: TokenAnalysisLink[] = ['tok-1', 'tok-1', 'tok-2'].map((tokenRef) => ({ + ...FIXTURE_STAMPS, + analysisId: shared.id, + status: 'approved', + token: { tokenRef, surfaceText: 'word' }, + })); + const store = createAnalysisStore({ + analysis: { + analysis: { + ...emptyAnalysis(), + tokenAnalyses: [shared], + tokenAnalysisLinks: duplicated, + }, + analysisLanguage: 'und', + }, + }); + + const outcome = selectAnalysisDeletionOutcome(store.getState().analysis, 'ta-shared'); + + expect(outcome).toEqual({ kind: 'blank', usageCount: 2 }); + }); }); describe('selectAnalysisMergePeers', () => { diff --git a/src/__tests__/utils/analysis-query.test.ts b/src/__tests__/utils/analysis-query.test.ts index 0689d492..0b7a2b1b 100644 --- a/src/__tests__/utils/analysis-query.test.ts +++ b/src/__tests__/utils/analysis-query.test.ts @@ -8,6 +8,8 @@ import { applyCatalogQuery, buildCatalogRows, deriveFacets, + reconcileFilters, + type CatalogFilters, type CatalogQuery, type CatalogScope, } from '../../utils/analysis-query'; @@ -833,3 +835,85 @@ describe('deriveFacets', () => { expect(facets.books).toEqual(['GEN', 'JHN']); }); }); + +describe('reconcileFilters', () => { + it('drops a selection whose facet is no longer offered', () => { + const filters = reconcileFilters({ books: ['EXO'] }, { books: undefined }); + + expect(filters.books).toBeUndefined(); + }); + + it('drops only the chosen values the facet stopped offering', () => { + const filters = reconcileFilters({ books: ['GEN', 'EXO'] }, { books: ['GEN', 'JHN'] }); + + expect(filters.books).toEqual(['GEN']); + }); + + it('drops a selection whose every choice the facet stopped offering', () => { + const filters = reconcileFilters({ books: ['EXO'] }, { books: ['GEN', 'JHN'] }); + + expect(filters.books).toBeUndefined(); + }); + + it('keeps a selection the facet still offers whole', () => { + const filters = reconcileFilters({ books: ['GEN'] }, { books: ['GEN', 'JHN'] }); + + expect(filters.books).toEqual(['GEN']); + }); + + it('returns the same object when every selection survives', () => { + const chosen: CatalogFilters = { books: ['GEN'], missingGloss: true }; + + expect(reconcileFilters(chosen, { books: ['GEN', 'JHN'] })).toBe(chosen); + }); + + it('leaves the filters no facet governs untouched', () => { + const filters = reconcileFilters( + { books: ['EXO'], missingGloss: true, zeroUsages: true, morphemes: 'has' }, + { books: undefined }, + ); + + expect(filters.missingGloss).toBe(true); + expect(filters.zeroUsages).toBe(true); + expect(filters.morphemes).toBe('has'); + }); + + it('drops a feature selection whose name lost its facet', () => { + const filters = reconcileFilters( + { features: { case: ['nom'], number: ['sg'] } }, + { features: { number: ['sg', 'pl'] } }, + ); + + expect(filters.features).toEqual({ number: ['sg'] }); + }); + + it('drops the feature filter entirely once no name survives', () => { + const filters = reconcileFilters({ features: { case: ['nom'] } }, { features: undefined }); + + expect(filters.features).toBeUndefined(); + }); + + it('keeps the untagged choice while its facet still offers it', () => { + const filters = reconcileFilters({ pos: [undefined] }, { pos: ['noun', undefined] }); + + expect(filters.pos).toEqual([undefined]); + }); + + // Reconciliation withdraws choices; it does not re-interpret the ones it keeps. + it('leaves a surviving selection narrowing the same rows', () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'a' }, + { ...FIXTURE_STAMPS, id: 'ta-2', surfaceText: 'b' }, + ], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0'), link('ta-2', 'JHN 1:1:0')], + }; + const rows = buildCatalogRows(analysis, scope); + const filters = reconcileFilters({ books: ['GEN'] }, deriveFacets(rows)); + + expect(applyCatalogQuery(rows, makeQuery({ filters })).map((row) => row.analysisId)).toEqual([ + 'ta-1', + ]); + }); +}); diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index 48d5c5cd..99a1c858 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -12,6 +12,7 @@ import useRowWindow from '../hooks/useRowWindow'; import { applyCatalogQuery, deriveFacets, + reconcileFilters, type CatalogFilters, type CatalogQuery, type CatalogSort, @@ -75,14 +76,34 @@ export default function AnalysisCatalogPanel({ /** How the listing is ordered. Most-used first, the question the catalog is opened to answer. */ const [sort, setSort] = useState('usageCount'); - /** Which rows the listing keeps. Nothing narrowed until the reader chooses something. */ - const [filters, setFilters] = useState({}); + /** + * Which rows the listing keeps, as the reader last chose them. A choice here can be withdrawn by + * an edit made beside the panel, so it is the reconciled set below that narrows the listing — + * while the controls are given these, a withdrawn choice needing to stay on screen to be + * cleared. + */ + const [chosenFilters, setFilters] = useState({}); // Each rebuilt only when its own tag changes: the query around them turns over on every keystroke // in the search box, and a collator is expensive enough to be worth not rebuilding that often. const surfaceCollator = useMemo(() => collatorForTag(sourceLanguageTag), [sourceLanguageTag]); const glossCollator = useMemo(() => collatorForTag(analysisLanguage), [analysisLanguage]); + /** + * The choices worth offering as filters, taken against every row the draft holds rather than the + * rows a filter left standing: a facet judged against its own selection's survivors would + * collapse to that selection, leaving no choice on screen to widen it back by. + */ + const facets = useMemo(() => deriveFacets(catalogRows), [catalogRows]); + + /** + * The filters actually narrowing the listing: the reader's choices less any the facets have since + * withdrawn. An edit beside the panel can remove the last row carrying a chosen value, which + * takes that facet's control off screen; keeping the choice would narrow the list to nothing with + * no control left to widen it back by. + */ + const filters = useMemo(() => reconcileFilters(chosenFilters, facets), [chosenFilters, facets]); + /** How the listing is narrowed and ordered, from the controls above the list. */ const query = useMemo( () => ({ search, sort, filters, surfaceCollator, glossCollator }), @@ -129,8 +150,6 @@ export default function AnalysisCatalogPanel({ [localizedStrings, currentBookName], ); - const facets = useMemo(() => deriveFacets(catalogRows), [catalogRows]); - const [interfaceLanguages] = useSetting('platform.interfaceLanguage', ['und']); /** @@ -223,10 +242,11 @@ export default function AnalysisCatalogPanel({ */} {catalogRows.length > 0 && ( void; /** Whether this project breaks words into morphemes, which the breakdown filter is offered for. */ @@ -272,6 +278,7 @@ type CatalogFilterPopoverProps = Readonly<{ export default function CatalogFilterPopover({ facets, filters, + activeFilters, onFiltersChange, showMorphology, analysisLanguageName, @@ -297,16 +304,18 @@ export default function CatalogFilterPopover({ /** * How many of the filter groups are narrowing anything. Each named feature counts on its own, as * each is chosen and cleared on its own; an emptied selection counts for nothing, matching the - * query core's reading of it as no filter rather than as one nothing satisfies. + * query core's reading of it as no filter rather than as one nothing satisfies. Taken against the + * filters that survive reconciliation, a choice narrowing nothing being no narrower of the list. */ const activeCount = [ - filters.books, - filters.pos, - filters.confidence, - ...Object.values(filters.features ?? {}), + activeFilters.books, + activeFilters.pos, + activeFilters.confidence, + ...Object.values(activeFilters.features ?? {}), ].filter((selected) => selected?.length).length + - [filters.missingGloss, filters.morphemes, filters.zeroUsages].filter(Boolean).length; + [activeFilters.missingGloss, activeFilters.morphemes, activeFilters.zeroUsages].filter(Boolean) + .length; return ( diff --git a/src/components/CatalogQueryControls.tsx b/src/components/CatalogQueryControls.tsx index 30a1d3b2..78bbd047 100644 --- a/src/components/CatalogQueryControls.tsx +++ b/src/components/CatalogQueryControls.tsx @@ -44,8 +44,10 @@ type CatalogQueryControlsProps = Readonly<{ onSortChange: (sort: CatalogSort) => void; /** The choices worth offering as filters. */ facets: CatalogFacets; - /** The filters currently narrowing the listing. */ + /** The filters as the reader chose them, which is what each control shows selected. */ filters: CatalogFilters; + /** The filters actually narrowing the listing, which the filter trigger counts. */ + activeFilters: CatalogFilters; /** Records a new set of filters. */ onFiltersChange: (filters: CatalogFilters) => void; /** Whether this project breaks words into morphemes, which the breakdown filter is offered for. */ @@ -73,6 +75,7 @@ export default function CatalogQueryControls({ onSortChange, facets, filters, + activeFilters, onFiltersChange, showMorphology, analysisLanguageName, @@ -119,6 +122,7 @@ export default function CatalogQueryControls({ (); + (old ?? []).forEach((m) => { + const bucket = oldByForm.get(m.form); + if (bucket) bucket.push(m); + else oldByForm.set(m.form, [m]); + }); + return morphemes.map(({ id, form }) => { + const kept = oldByForm.get(form)?.shift(); + return kept ? { ...kept, writingSystem } : { id, form, writingSystem }; + }); +} + const analysisSlice = createSlice({ name: 'analysis', initialState: defaultState, @@ -541,21 +568,7 @@ const analysisSlice = createSlice({ target.updatedAt = now; link.token.surfaceText = surfaceText; link.updatedAt = now; - // 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(); - (target.morphemes ?? []).forEach((m) => { - const bucket = oldByForm.get(m.form); - if (bucket) bucket.push(m); - else oldByForm.set(m.form, [m]); - }); - target.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 }; - }); + target.morphemes = reconcileMorphemes(target.morphemes, morphemes, writingSystem); // An in-place breakdown edit can make this payload identical to an existing one (e.g. a // homograph re-segmented to match a sibling); re-converge so the dedupe the create path // guarantees on first write also holds after morpheme edits (mirrors writeGloss). @@ -734,18 +747,20 @@ const analysisSlice = createSlice({ }, }, /** - * Replaces the morpheme breakdown on a `TokenAnalysis` addressed by its own id, for every token - * linked to it, so one correction fixes a mis-split word across all its occurrences. + * Re-segments the morpheme breakdown on a `TokenAnalysis` addressed by its own id, for every + * token linked to it, so one correction fixes a mis-split word across all its occurrences. * - * The breakdown is replaced rather than reconciled: morphemes are rebuilt from `forms` with - * fresh ids, dropping any glosses the old morphemes carried, which the caller is expected to - * have warned about. An empty `forms` removes the breakdown, and removes the record when - * nothing else remains on it. + * A form the breakdown already carried keeps its morpheme whole — its id, so + * `MorphemeLink.morphemeId` stays valid, along with its gloss and lexicon references — while a + * form with no counterpart is minted fresh. A re-split that drops a form drops what it carried + * with it, there being no morpheme left to hold it. An empty `forms` removes the breakdown, and + * removes the record when nothing else remains on it. */ writeAnalysisMorphemes: { /** - * Mints the new morphemes' ids and reads the clock before the action reaches the reducer, - * keeping the reducer pure. + * Mints an id per form and reads the clock before the action reaches the reducer, keeping the + * reducer pure. Only a form the breakdown cannot already account for spends the id offered + * for it. */ prepare(arg: { analysisId: string; forms: readonly string[]; writingSystem: string }) { return { @@ -772,7 +787,7 @@ const analysisSlice = createSlice({ if (!analysis) return; if (morphemes.length === 0) delete analysis.morphemes; - else analysis.morphemes = morphemes.map(({ id, form }) => ({ id, form, writingSystem })); + else analysis.morphemes = reconcileMorphemes(analysis.morphemes, morphemes, writingSystem); analysis.updatedAt = now; if (isEmptyTokenAnalysis(analysis)) { @@ -1276,16 +1291,19 @@ export function selectAnalysisDeletionOutcome( const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === analysisId); if (!analysis) return undefined; - const usageCount = state.analysis.tokenAnalysisLinks.filter( - (l) => l.analysisId === analysisId && l.status === 'approved', - ).length; + const approvedTokenCounts = selectApprovedTokenCountByAnalysisId(state); + + // Counted off the same index the catalog row counts by, so the confirmation and the row it opened + // from cannot name two different numbers: both count the tokens an approval sits on rather than + // the approvals themselves. + const usageCount = approvedTokenCounts.get(analysisId) ?? 0; // Ask the engine, so the confirmation names the peer that actually wins. The payload is dropped // from the pool outright rather than discounted by one approval: a deletion removes all of its // approvals at once, and a discounted multi-token payload would compete to replace itself. const survivingPool = buildPoolIndex( selectAnalysisById(state), - new Map([...selectApprovedTokenCountByAnalysisId(state)].filter(([id]) => id !== analysisId)), + new Map([...approvedTokenCounts].filter(([id]) => id !== analysisId)), ); const fallback = deriveTokenSuggestion(survivingPool, analysis.surfaceText); if (!fallback) return { kind: 'blank', usageCount }; diff --git a/src/utils/analysis-query.ts b/src/utils/analysis-query.ts index d80d3d82..af6e3ebd 100644 --- a/src/utils/analysis-query.ts +++ b/src/utils/analysis-query.ts @@ -400,6 +400,83 @@ function isActive(selected: readonly T[] | undefined): selected is readonly T return selected !== undefined && selected.length > 0; } +/** + * Drops from a selection every choice its facet no longer offers, or the whole selection when the + * facet is gone. + * + * @returns The selection itself when every choice survives; `undefined` where nothing is left to + * keep, so an emptied selection reads as no filter rather than as one nothing satisfies. + */ +function retainOffered( + selected: readonly T[] | undefined, + offered: readonly T[] | undefined, +): readonly T[] | undefined { + if (!isActive(selected)) return undefined; + if (!offered) return undefined; + const kept = selected.filter((choice) => offered.includes(choice)); + if (kept.length === selected.length) return selected; + return kept.length === 0 ? undefined : kept; +} + +/** Whether two feature selections name the same values for the same features. */ +function sameFeatureSelections( + a: CatalogFilters['features'], + b: CatalogFilters['features'], +): boolean { + const namesA = Object.keys(a ?? {}); + const namesB = Object.keys(b ?? {}); + if (namesA.length !== namesB.length) return false; + return namesA.every((name) => a?.[name] === b?.[name]); +} + +/** + * Narrows a set of filters to the choices the facets still offer, so a selection cannot outlive the + * choice it names. + * + * A facet collapses as the rows behind it change — an edit that removes the last row in the only + * other book leaves {@link deriveFacets} offering no books facet at all — while the selection naming + * that book lives on. Left alone the pair strands the reader: the listing is narrowed to nothing by + * a filter whose control is no longer on screen to widen it back by. + * + * @returns The given filters unchanged when every selection is still offered, so storing the result + * back over them settles rather than looping. + */ +export function reconcileFilters(filters: CatalogFilters, facets: CatalogFacets): CatalogFilters { + const features = Object.entries(filters.features ?? {}).reduce< + Record + >((acc, [name, values]) => { + const kept = retainOffered(values, facets.features?.[name]); + if (kept) acc[name] = kept; + return acc; + }, {}); + + const books = retainOffered(filters.books, facets.books); + const pos = retainOffered(filters.pos, facets.pos); + const confidence = retainOffered(filters.confidence, facets.confidence); + const keptFeatures = Object.keys(features).length === 0 ? undefined : features; + + // Built field by field rather than by overriding a spread of `filters`: a withdrawn selection is + // absent rather than `undefined`, and spreading the original first would keep the stale key. + const reconciled: CatalogFilters = { + ...(filters.zeroUsages !== undefined && { zeroUsages: filters.zeroUsages }), + ...(filters.missingGloss !== undefined && { missingGloss: filters.missingGloss }), + ...(filters.morphemes !== undefined && { morphemes: filters.morphemes }), + ...(books && { books }), + ...(pos && { pos }), + ...(confidence && { confidence }), + ...(keptFeatures && { features: keptFeatures }), + }; + + // The value filters are the only ones a facet can withdraw: the remaining three are offered + // unconditionally, so nothing can strand them. + const isUnchanged = + books === filters.books && + pos === filters.pos && + confidence === filters.confidence && + sameFeatureSelections(keptFeatures, filters.features); + return isUnchanged ? filters : reconciled; +} + /** * Whether a field holding one value at a time is in a state its selection accepts, carrying no * value being the state the selection's `undefined` choice accepts. From e488e3517b3c394c509e90ce9fd10031c1f9b4de Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 31 Aug 2026 14:30:16 -0600 Subject: [PATCH 03/14] Offer merge peers to an analysis no token has approved Merge peers came off the suggestion pool, which admits approved analyses alone, so an imported homograph was listed as a catalog row that could neither be merged into nor merged away. Index every payload by surface form instead, matching what mergeAnalysisInto already accepts. --- src/__tests__/store/analysisSlice.test.ts | 86 +++++++++++++++++++++++ src/components/CatalogFilterPopover.tsx | 3 + src/store/analysisSlice.ts | 28 +++++++- 3 files changed, 115 insertions(+), 2 deletions(-) diff --git a/src/__tests__/store/analysisSlice.test.ts b/src/__tests__/store/analysisSlice.test.ts index 1fa9026f..1f348514 100644 --- a/src/__tests__/store/analysisSlice.test.ts +++ b/src/__tests__/store/analysisSlice.test.ts @@ -2560,6 +2560,50 @@ describe('analysis-keyed reducers', () => { }); } + /** + * Two payloads for one surface form where the second is linked only as a candidate, the shape a + * Paratext 9 import lands: a homograph the catalog lists but the suggestion pool never admits. + * Withholding approval from the shared payload as well leaves neither of them in the pool. + */ + function makeUnapprovedHomographStore({ approveShared = true } = {}) { + const shared: TokenAnalysis = { + ...FIXTURE_STAMPS, + id: 'ta-shared', + surfaceText: 'word', + gloss: { und: 'first' }, + }; + const candidate: TokenAnalysis = { + ...FIXTURE_STAMPS, + id: 'ta-candidate', + surfaceText: 'word', + gloss: { und: 'candidate' }, + }; + const links: TokenAnalysisLink[] = [ + { + ...FIXTURE_STAMPS, + analysisId: shared.id, + status: approveShared ? 'approved' : 'candidate', + token: { tokenRef: 'tok-1', surfaceText: 'word' }, + }, + { + ...FIXTURE_STAMPS, + analysisId: candidate.id, + status: 'candidate', + token: { tokenRef: 'tok-2', surfaceText: 'word' }, + }, + ]; + return createAnalysisStore({ + analysis: { + analysis: { + ...emptyAnalysis(), + tokenAnalyses: [shared, candidate], + tokenAnalysisLinks: links, + }, + analysisLanguage: 'und', + }, + }); + } + describe('writeAnalysisGloss', () => { it('rewrites the gloss for every token linked to the payload', () => { const store = makeSharedStore(); @@ -3081,5 +3125,47 @@ describe('analysis-keyed reducers', () => { expect(selectAnalysisMergePeers(store.getState().analysis, 'nope')).toHaveLength(0); }); + + it('offers a homograph no token has approved', () => { + const store = makeUnapprovedHomographStore(); + + const peers = selectAnalysisMergePeers(store.getState().analysis, 'ta-shared'); + + expect(peers.map((p) => p.gloss?.und)).toEqual(['candidate']); + }); + + it('offers peers to a row no token has approved', () => { + const store = makeUnapprovedHomographStore(); + + const peers = selectAnalysisMergePeers(store.getState().analysis, 'ta-candidate'); + + expect(peers.map((p) => p.gloss?.und)).toEqual(['first']); + }); + + it('pairs two homographs that both went unapproved', () => { + const store = makeUnapprovedHomographStore({ approveShared: false }); + + const peers = selectAnalysisMergePeers(store.getState().analysis, 'ta-candidate'); + + expect(peers.map((p) => p.gloss?.und)).toEqual(['first']); + }); + + it('ranks a peer by approvals, leaving an unapproved one last', () => { + const store = makeUnapprovedHomographStore(); + // A second approval puts 'first' ahead of the newly-approved payload on frequency, so the + // ordering under test is the frequency rank rather than the id tiebreak behind it. + store.dispatch( + approveAnalysisForToken({ + tokenRef: 'tok-4', + surfaceText: 'word', + analysisId: 'ta-shared', + }), + ); + store.dispatch(writeGloss('tok-3', 'word', 'third')); + + const peers = selectAnalysisMergePeers(store.getState().analysis, 'ta-candidate'); + + expect(peers.map((p) => p.gloss?.und)).toEqual(['first', 'third']); + }); }); }); diff --git a/src/components/CatalogFilterPopover.tsx b/src/components/CatalogFilterPopover.tsx index a1baa9cd..fe58f1a3 100644 --- a/src/components/CatalogFilterPopover.tsx +++ b/src/components/CatalogFilterPopover.tsx @@ -274,6 +274,9 @@ type CatalogFilterPopoverProps = Readonly<{ * * A facet offering fewer than two choices raises no control at all: {@link CatalogFacets} omits it, * because a lone choice is the state every row is already in, so choosing it would narrow nothing. + * That covers the facet-backed filters alone. The controls below them stand on a state every row + * has rather than on a facet, so there is nothing for {@link CatalogFacets} to omit and they are + * always raised — inert though one is against data that is uniform in what it asks about. */ export default function CatalogFilterPopover({ facets, diff --git a/src/store/analysisSlice.ts b/src/store/analysisSlice.ts index 3cd17fcd..f8d4a00d 100644 --- a/src/store/analysisSlice.ts +++ b/src/store/analysisSlice.ts @@ -1241,6 +1241,26 @@ export const selectPoolIndex = createSelector( buildPoolIndex, ); +/** + * Which payloads name the same word, keyed by normalized surface form and ordered most-approved + * first. Every payload is filed, an unused one ranking last rather than being left out. + * + * Distinct from {@link selectPoolIndex}, which admits only approved analyses because it answers what + * to suggest for an unanalyzed token, and an unapproved payload is no answer. Which payloads name + * one word is a separate question that an unused payload does answer: a Paratext 9 import can land + * homographs no token has approved, and those are the rows a reader opens the catalog to + * reconcile. + */ +const selectHomographIndex = createSelector( + selectAnalysisById, + selectApprovedTokenCountByAnalysisId, + (analysisById, approvedCounts) => + buildPoolIndex( + analysisById, + new Map([...analysisById.keys()].map((id) => [id, approvedCounts.get(id) ?? 0])), + ), +); + /** * Memoized selector building the Analysis Catalog's rows — one per distinct token analysis, with * its usage counts and locations — against the book named as the second argument. Only a change to @@ -1316,6 +1336,10 @@ export function selectAnalysisDeletionOutcome( * The other analyses a row may be merged into: those sharing its normalized surface form, so merge * is offered only among genuine homographs and a row with no peers offers none. Ordered best-first, * putting the most-used peer at the head. + * + * An unused payload is both offered as a target and given peers of its own, matching what + * {@link mergeAnalysisInto} accepts: it moves links whatever their status, so approval is no + * condition of merging. Suggestion is where approval matters, and that is a separate question. */ export function selectAnalysisMergePeers( state: AnalysisState, @@ -1323,8 +1347,8 @@ export function selectAnalysisMergePeers( ): readonly TokenAnalysis[] { const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === analysisId); if (!analysis) return NO_MERGE_PEERS; - const bucket = selectPoolIndex(state).get(normalizeSurfaceForm(analysis.surfaceText)); - if (!bucket) return NO_MERGE_PEERS; + /* v8 ignore next -- unreachable: every payload is filed, so a resolved row is in its own bucket */ + const bucket = selectHomographIndex(state).get(normalizeSurfaceForm(analysis.surfaceText)) ?? []; const peers = bucket.filter((e) => e.analysis.id !== analysisId).map((e) => e.analysis); return peers.length > 0 ? peers : NO_MERGE_PEERS; } From da3957c38f8b695031b3086212028c23dd3fdbce Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 1 Sep 2026 16:46:05 -0600 Subject: [PATCH 04/14] Spend a filter choice the facets have withdrawn Commit the reconciled set back over the reader's choices, so a value dropped from a facet is gone rather than merely unused. Held, it returned the moment its facet did: an edit that withdrew the last row carrying a chosen value, undone, left the listing narrowed by a filter the reader had watched release, with the restored row the only one it kept. --- .../components/AnalysisCatalogPanel.test.tsx | 35 ++++++-- src/components/AnalysisCatalogPanel.tsx | 22 +++-- src/components/CatalogFilterPopover.tsx | 85 ++++++------------- src/components/CatalogQueryControls.tsx | 6 +- 4 files changed, 68 insertions(+), 80 deletions(-) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 4cb51b17..7aab3c3d 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -560,7 +560,7 @@ describe('AnalysisCatalogPanel', () => { expect(listedAnalysisIds()).toEqual(['in-gen', 'in-exo']); }); - it('keeps offering a chosen book the draft has since stopped using', async () => { + it('releases a chosen book the draft has since stopped using', async () => { const analysis: TextAnalysis = { ...PER_BOOK, tokenAnalyses: [ @@ -576,12 +576,12 @@ describe('AnalysisCatalogPanel', () => { act(() => editGloss('EXO 3:14:0', 'λόγος', '')); - // The books facet is down to one choice and would rightly offer none of its own, but the - // selection still narrows the list — so the choice that clears it has to stay on screen. - expect(screen.getByRole('option', { name: 'EXO' })).toBeInTheDocument(); + // The books facet is down to one choice and offers none of its own, so the selection it was + // narrowing by is spent rather than left on screen to be cleared by hand. + expect(screen.queryByRole('option', { name: 'EXO' })).not.toBeInTheDocument(); }); - it('restores the list when a chosen book the draft stopped using is deselected', async () => { + it('restores the list when a chosen book the draft stopped using is withdrawn', async () => { const analysis: TextAnalysis = { ...PER_BOOK, tokenAnalyses: [ @@ -592,12 +592,11 @@ describe('AnalysisCatalogPanel', () => { renderPanelWithGlossEditing({ analysis }); await openFilters(); await userEvent.click(screen.getByRole('option', { name: 'EXO' })); - act(() => editGloss('EXO 3:14:0', 'λόγος', '')); - await userEvent.click(screen.getByRole('option', { name: 'EXO' })); + act(() => editGloss('EXO 3:14:0', 'λόγος', '')); - // Deselecting has to actually clear the filter rather than merely unmount its control, or the - // reader is left with an empty list and no way back to the draft. + // Releasing the filter has to widen the list on its own, or the reader is left with an empty + // list and no way back to the draft. expect(listedAnalysisIds()).toEqual(['in-gen']); }); @@ -901,6 +900,24 @@ describe('AnalysisCatalogPanel', () => { expect(listedAnalysisIds()).toEqual(['in-gen']); }); + // A withdrawn choice is spent, not merely unused: held, it would come back with its facet and + // narrow the listing by a filter the reader had already watched release. + it('leaves a released book filter released once the edge that withdrew it is undone', async () => { + renderPanelWithGlossEditing({ analysis: TWO_BOOKS, analysisLanguage: 'en' }); + await openFilters(); + const books = within(screen.getByTestId('catalog-filter-books')); + await userEvent.click(books.getByRole('option', { name: 'EXO' })); + act(() => editGloss('EXO 1:1:0', 'ἦν', '')); + expect(listedAnalysisIds()).toEqual(['in-gen']); + + // Glossing it again analyzes the token afresh — a new payload under a new id, so the restored + // row is matched on count rather than named — and raises the books facet that offers EXO. + act(() => editGloss('EXO 1:1:0', 'ἦν', 'was')); + + expect(listedAnalysisIds()).toHaveLength(2); + expect(listedAnalysisIds()).toContain('in-gen'); + }); + it('stops counting a filter the facets have withdrawn as active', async () => { renderPanelWithGlossEditing({ analysis: TWO_BOOKS, analysisLanguage: 'en' }); await openFilters(); diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index 99a1c858..cbc008c4 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -3,7 +3,7 @@ import { Canon } from '@sillsdev/scripture'; import { X } from 'lucide-react'; import { Button, EmptyState, TooltipProvider } from 'platform-bible-react'; import { formatReplacementString, isPlatformError } from 'platform-bible-utils'; -import { useCallback, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useState } from 'react'; import { useAnalysisLanguage, useCatalogRows } from './AnalysisStore'; import CatalogQueryControls, { QUERY_CONTROL_STRING_KEYS } from './CatalogQueryControls'; import CatalogRowView, { ROW_STRING_KEYS } from './CatalogRowView'; @@ -78,9 +78,8 @@ export default function AnalysisCatalogPanel({ /** * Which rows the listing keeps, as the reader last chose them. A choice here can be withdrawn by - * an edit made beside the panel, so it is the reconciled set below that narrows the listing — - * while the controls are given these, a withdrawn choice needing to stay on screen to be - * cleared. + * an edit made beside the panel, so it is the reconciled set below that narrows the listing until + * the withdrawal is committed back over these. */ const [chosenFilters, setFilters] = useState({}); @@ -104,6 +103,18 @@ export default function AnalysisCatalogPanel({ */ const filters = useMemo(() => reconcileFilters(chosenFilters, facets), [chosenFilters, facets]); + /** + * Commits a withdrawal back over the choices it narrowed, so a value the facets dropped is spent + * rather than merely unused. Left recorded it would return with its facet, narrowing the listing + * by a filter the reader had watched release. + * + * Settles after one withdrawal, {@link reconcileFilters} yielding the very set it was given once + * every choice survives. + */ + useEffect(() => { + if (filters !== chosenFilters) setFilters(filters); + }, [filters, chosenFilters]); + /** How the listing is narrowed and ordered, from the controls above the list. */ const query = useMemo( () => ({ search, sort, filters, surfaceCollator, glossCollator }), @@ -242,11 +253,10 @@ export default function AnalysisCatalogPanel({ */} {catalogRows.length > 0 && ( 0; -} - /** * The breakdown filter's inactive choice, spelled out because {@link CatalogFilters} spells it * `undefined` while the platform select needs a value for every choice it offers. @@ -98,10 +85,10 @@ function morphemeChoice(value: string): CatalogFilters['morphemes'] { /** Props for {@link FacetFilter}. */ type FacetFilterProps = Readonly<{ /** - * The choices the rows offer for this field, `undefined` standing for carrying no value. Absent - * where the rows have stopped offering any, which leaves only what is still selected to offer. + * The choices the rows offer for this field, `undefined` among them standing for carrying no + * value. Raised only for a field the rows still offer choices for. */ - choices: readonly (T | undefined)[] | undefined; + choices: readonly (T | undefined)[]; /** The choices currently selected; absent or empty means the field narrows nothing. */ selected: readonly (T | undefined)[] | undefined; /** Records a new selection, in the field's own terms rather than the control's. */ @@ -141,19 +128,13 @@ function FacetFilter({ localizedStrings['%interlinearizer_analysisCatalog_filter_untagged%'].trim(); const emptyLabel = localizedStrings['%interlinearizer_analysisCatalog_filter_empty%'].trim(); - /** The choices to offer: the field's own, plus any still selected that it no longer lists. */ - const offered = [ - ...(choices ?? []), - ...(selected ?? []).filter((choice) => !(choices ?? []).includes(choice)), - ]; - /** * Each control value read back to the choice it stands for. Held as a map rather than compared * against the sentinels so that a choice the field spells as absence or as the empty string is * recovered as the choice it is. */ const choiceByValue = new Map( - offered.map((choice) => [valueOf(choice), choice]), + choices.map((choice) => [valueOf(choice), choice]), ); /** @@ -165,16 +146,19 @@ function FacetFilter({ const nameOfValue = (choice: T) => (labelFor ? labelFor(choice) : choice).trim() || emptyLabel; /** - * What each offered choice is called, no two alike: the platform control cannot tell two options - * sharing a label apart, so only a distinctly named choice can be filtered by. + * What each choice is called, no two alike: the platform control cannot tell two options sharing + * a label apart, so only a distinctly named choice can be filtered by. + * + * A value reading as a label another choice holds is marked as recorded repeatedly, since a value + * can be spelled like the marking itself. */ const labelByChoice = new Map(); const claimed = new Set(); - offered.forEach((choice) => { + choices.forEach((choice) => { if (choice === undefined) claimed.add(untaggedLabel); if (choice === '') claimed.add(emptyLabel); }); - offered.forEach((choice) => { + choices.forEach((choice) => { if (choice === undefined) return labelByChoice.set(choice, untaggedLabel); if (choice === '') return labelByChoice.set(choice, emptyLabel); let name = nameOfValue(choice); @@ -249,14 +233,8 @@ function FilterToggle({ type CatalogFilterPopoverProps = Readonly<{ /** The choices worth offering as filters. */ facets: CatalogFacets; - /** The filters as the reader chose them, which is what each control shows selected. */ + /** The filters currently narrowing the listing. */ filters: CatalogFilters; - /** - * The filters that are actually narrowing the listing, which the trigger counts. Parts company - * with `filters` over a choice the facets have withdrawn: still the reader's, so still on screen - * to be cleared, but narrowing nothing and so counting for nothing. - */ - activeFilters: CatalogFilters; /** Records a new set of filters. */ onFiltersChange: (filters: CatalogFilters) => void; /** Whether this project breaks words into morphemes, which the breakdown filter is offered for. */ @@ -281,7 +259,6 @@ type CatalogFilterPopoverProps = Readonly<{ export default function CatalogFilterPopover({ facets, filters, - activeFilters, onFiltersChange, showMorphology, analysisLanguageName, @@ -291,34 +268,22 @@ export default function CatalogFilterPopover({ const filtersLabel = localizedStrings['%interlinearizer_analysisCatalog_filters%']; - /** - * The feature names to raise a control for: those the rows offer choices for, and any a selection - * still narrows by. - */ - const featureNames = [ - ...new Set([ - ...Object.keys(facets.features ?? {}), - ...Object.entries(filters.features ?? {}) - .filter(([, values]) => values?.length) - .map(([name]) => name), - ]), - ]; + /** Each feature to raise a control for with its choices, being those the rows offer. */ + const featureFacets = Object.entries(facets.features ?? {}); /** * How many of the filter groups are narrowing anything. Each named feature counts on its own, as * each is chosen and cleared on its own; an emptied selection counts for nothing, matching the - * query core's reading of it as no filter rather than as one nothing satisfies. Taken against the - * filters that survive reconciliation, a choice narrowing nothing being no narrower of the list. + * query core's reading of it as no filter rather than as one nothing satisfies. */ const activeCount = [ - activeFilters.books, - activeFilters.pos, - activeFilters.confidence, - ...Object.values(activeFilters.features ?? {}), + filters.books, + filters.pos, + filters.confidence, + ...Object.values(filters.features ?? {}), ].filter((selected) => selected?.length).length + - [activeFilters.missingGloss, activeFilters.morphemes, activeFilters.zeroUsages].filter(Boolean) - .length; + [filters.missingGloss, filters.morphemes, filters.zeroUsages].filter(Boolean).length; return ( @@ -347,7 +312,7 @@ export default function CatalogFilterPopover({ className="tw:flex tw:w-auto tw:min-w-56 tw:flex-col tw:gap-3" data-testid="catalog-filters-panel" > - {isFilterable(facets.books, filters.books) && ( + {facets.books !== undefined && ( )} - {isFilterable(facets.pos, filters.pos) && ( + {facets.pos !== undefined && ( )} - {isFilterable(facets.confidence, filters.confidence) && ( + {facets.confidence !== undefined && ( ( + {featureFacets.map(([name, choices]) => ( void; /** The choices worth offering as filters. */ facets: CatalogFacets; - /** The filters as the reader chose them, which is what each control shows selected. */ + /** The filters currently narrowing the listing. */ filters: CatalogFilters; - /** The filters actually narrowing the listing, which the filter trigger counts. */ - activeFilters: CatalogFilters; /** Records a new set of filters. */ onFiltersChange: (filters: CatalogFilters) => void; /** Whether this project breaks words into morphemes, which the breakdown filter is offered for. */ @@ -75,7 +73,6 @@ export default function CatalogQueryControls({ onSortChange, facets, filters, - activeFilters, onFiltersChange, showMorphology, analysisLanguageName, @@ -122,7 +119,6 @@ export default function CatalogQueryControls({ Date: Mon, 31 Aug 2026 15:39:36 -0600 Subject: [PATCH 05/14] Wire the analysis catalog's rows to the analysis-keyed reducers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The catalog listed what the draft records without offering any way to correct it, while the reducers that would do so had no caller. A row is a shared analysis with no token in context, so editing one is an edit to every token linked to it — the correction a mis-split word repeated a hundred times needs. Expanding a row now reveals its gloss, its morpheme breakdown and each morpheme's gloss, and the controls for merging or deleting it. Fields commit on blur and Enter and revert on Escape, so an edit is not written across every token of an analysis mid-word. Deleting is confirmed by a modal naming the concrete consequence — how many uses go blank, or which surviving homograph they fall back to. Delete ships before undo, so that copy is the only guard, and a confirmation that promised a fallback that does not exist would be worse than none. Merging is offered only to a row with pool peers, since there is otherwise nothing to reassign its tokens to, and preselects no target: it moves every use of one analysis onto another and drops the source. An edit that makes a row identical to a sibling collapses the two, which leaves the edited row gone from the listing and another's count grown. A banner above the list now names where the edit went and what the survivor counts, and the listing scrolls to it, so the pair reads as the convergence it is rather than as lost work. Nothing the write reports says a collapse happened, so the dispatch hook reads the store either side of it and follows the old links to see where they now point. Merge peers are bucketed by the normalized surface form the pool buckets by; grouping by the raw text would offer the control to homographs differing only in case and then open an empty picker. The platform-bible-react stub's Input dropped onBlur, which left every commit-on-blur field silently uncommitted in tests. --- __mocks__/platform-bible-react.tsx | 4 + contributions/localizedStrings.json | 30 + .../components/AnalysisCatalogPanel.test.tsx | 588 +++++++++++++++++- src/__tests__/store/analysisSlice.test.ts | 27 + src/components/AnalysisCatalogPanel.tsx | 223 ++++++- src/components/AnalysisStore.tsx | 229 +++++++ src/components/CatalogDeleteModal.tsx | 132 ++++ src/components/CatalogMergeModal.tsx | 118 ++++ src/components/CatalogMergeNotice.tsx | 82 +++ src/components/CatalogRowEditor.tsx | 305 +++++++++ src/components/CatalogRowView.tsx | 99 ++- 11 files changed, 1814 insertions(+), 23 deletions(-) create mode 100644 src/components/CatalogDeleteModal.tsx create mode 100644 src/components/CatalogMergeModal.tsx create mode 100644 src/components/CatalogMergeNotice.tsx create mode 100644 src/components/CatalogRowEditor.tsx diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index bb6ae609..dcd96656 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -20,6 +20,7 @@ import { } from 'react'; import type { ChangeEventHandler, + FocusEventHandler, CSSProperties, KeyboardEventHandler, MouseEventHandler, @@ -420,6 +421,7 @@ export const Input = forwardRef< className?: string; style?: CSSProperties; disabled?: boolean; + onBlur?: FocusEventHandler; onChange?: ChangeEventHandler; onKeyDown?: KeyboardEventHandler; 'aria-label'?: string; @@ -434,6 +436,7 @@ export const Input = forwardRef< className, style, disabled, + onBlur, onChange, onKeyDown, 'aria-label': ariaLabel, @@ -451,6 +454,7 @@ export const Input = forwardRef< className={className} style={style} disabled={disabled} + onBlur={onBlur} onChange={onChange} onKeyDown={onKeyDown} aria-label={ariaLabel} diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index 5f2918ac..53636bc1 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -52,6 +52,36 @@ "%interlinearizer_analysisCatalog_noUsages%": "Not used anywhere", "%interlinearizer_analysisCatalog_showAllUsages%": "Show {count} more", + "%interlinearizer_analysisCatalog_editGloss%": "Gloss", + "%interlinearizer_analysisCatalog_editMorphemes%": "Split into morphemes", + "%interlinearizer_analysisCatalog_editMorphemesHint%": "Enter morpheme forms separated by spaces", + "%interlinearizer_analysisCatalog_editMorphemesSave%": "Save breakdown", + "%interlinearizer_analysisCatalog_editMorphemesCancel%": "Cancel", + "%interlinearizer_analysisCatalog_editMorphemesOpen%": "Edit breakdown for {form}", + "%interlinearizer_analysisCatalog_morphemeGloss%": "Gloss for morpheme {form}", + "%interlinearizer_analysisCatalog_appliesToAll%": "Edits here apply to every use of this analysis.", + "%interlinearizer_analysisCatalog_merge%": "Merge…", + "%interlinearizer_analysisCatalog_mergeTitle%": "Merge {form} into another analysis", + "%interlinearizer_analysisCatalog_mergePrompt%": "Every use of this analysis becomes the analysis you choose. This analysis is then removed.", + "%interlinearizer_analysisCatalog_mergePeerUsageCount%": "{count} uses", + "%interlinearizer_analysisCatalog_mergeCancel%": "Cancel", + "%interlinearizer_analysisCatalog_mergeConfirm%": "Merge", + "%interlinearizer_analysisCatalog_merged%": "Merged into {gloss} — now {count} uses.", + "%interlinearizer_analysisCatalog_mergedNoGloss%": "Merged into {form} — now {count} uses.", + "%interlinearizer_analysisCatalog_mergedDismiss%": "Dismiss", + "%interlinearizer_analysisCatalog_delete%": "Delete", + "%interlinearizer_analysisCatalog_deleteTitle%": "Delete the analysis of {form}?", + "%interlinearizer_analysisCatalog_deleteBlank%": "{count} uses will be left with no analysis.", + "%interlinearizer_analysisCatalog_deleteBlankOne%": "1 use will be left with no analysis.", + "%interlinearizer_analysisCatalog_deleteBlankNone%": "This analysis is used nowhere, so nothing else changes.", + "%interlinearizer_analysisCatalog_deleteFallback%": "{count} uses will fall back to {gloss}.", + "%interlinearizer_analysisCatalog_deleteFallbackOne%": "1 use will fall back to {gloss}.", + "%interlinearizer_analysisCatalog_deleteFallbackNoGloss%": "{count} uses will fall back to another analysis of the same form.", + "%interlinearizer_analysisCatalog_deleteFallbackNoGlossOne%": "1 use will fall back to another analysis of the same form.", + "%interlinearizer_analysisCatalog_deleteUndoWarning%": "This cannot be undone.", + "%interlinearizer_analysisCatalog_deleteCancel%": "Cancel", + "%interlinearizer_analysisCatalog_deleteConfirm%": "Delete", + "%interlinearizer_projectSettings_title%": "Interlinearizer", "%interlinearizer_projectSettings_continuousScroll%": "Continuous Scroll", "%interlinearizer_projectSettings_continuousScrollDescription%": "Display words in a continuous horizontal scroll strip instead of chapter-segmented rows", diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 7aab3c3d..9cf4be93 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -70,6 +70,8 @@ type PanelOptions = Partial<{ currentBook: string; analysis: TextAnalysis; analysisLanguage: string; + /** Receives the analysis after every store write, so a test can assert on what was persisted. */ + onSave: (analysis: TextAnalysis) => void; /** Records every reference the panel navigates to, through the host scroll-group hook. */ setScrRef: (ref: SerializedVerseRef) => void; /** Reference the host scroll group reports, i.e. where the view already sits. */ @@ -101,6 +103,7 @@ function PanelProviders({ {children} @@ -1317,8 +1320,14 @@ describe('AnalysisCatalogPanel', () => { await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + // Each morpheme's gloss is an editable field, so it reads off the input rather than the text. const morphemes = within(rowFor('ta-1')).getAllByTestId('catalog-row-morpheme'); - expect(morphemes.map((m) => m.textContent)).toEqual(['λογword', 'οςNOM.SG']); + expect(morphemes.map((m) => m.textContent)).toEqual(['λογ', 'ος']); + expect( + within(rowFor('ta-1')) + .getAllByTestId('catalog-row-morpheme-gloss-input') + .map((i) => i.getAttribute('value')), + ).toEqual(['word', 'NOM.SG']); }); it('shows a morpheme with no gloss in the active language as its form alone', async () => { @@ -1580,4 +1589,581 @@ describe('AnalysisCatalogPanel', () => { expect(onClose).toHaveBeenCalled(); }); + + describe('editing a row', () => { + /** One analysis, shared by two tokens, so an edit here is visibly an edit to both. */ + const SHARED: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'λόγος', gloss: { en: 'word' } }, + ], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0'), link('ta-1', 'GEN 1:3:4')], + }; + + /** Expands the row and returns its detail, where the edit controls live. */ + async function expandRow(analysisId: string): Promise { + await userEvent.click(within(rowFor(analysisId)).getByTestId('catalog-row-toggle')); + return rowFor(analysisId); + } + + it('rewrites the gloss for every token linked to the analysis', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: SHARED, onSave }); + + const row = await expandRow('ta-1'); + const input = within(row).getByTestId('catalog-row-gloss-input'); + await userEvent.clear(input); + await userEvent.type(input, 'message'); + await userEvent.tab(); + + // One payload holding both links, so the single write reached both tokens without forking. + const saved: TextAnalysis = onSave.mock.calls.at(-1)[0]; + expect(saved.tokenAnalyses).toHaveLength(1); + expect(saved.tokenAnalyses[0].gloss).toEqual({ en: 'message' }); + expect(saved.tokenAnalysisLinks.map((l) => l.analysisId)).toEqual(['ta-1', 'ta-1']); + }); + + it('rewrites the morpheme breakdown for every token linked to the analysis', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: SHARED, onSave }); + + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + const input = within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input'); + await userEvent.clear(input); + await userEvent.type(input, 'λογ ος'); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-save')); + + const saved: TextAnalysis = onSave.mock.calls.at(-1)[0]; + expect(saved.tokenAnalyses).toHaveLength(1); + expect(saved.tokenAnalyses[0].morphemes?.map((m) => m.form)).toEqual(['λογ', 'ος']); + expect(saved.tokenAnalysisLinks.map((l) => l.analysisId)).toEqual(['ta-1', 'ta-1']); + }); + + it('leaves the breakdown alone when the editor is canceled', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: SHARED, onSave }); + + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + await userEvent.type( + within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input'), + 'λογ ος', + ); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-cancel')); + + expect(onSave).not.toHaveBeenCalled(); + }); + + it('commits a gloss edit on Enter', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: SHARED, onSave }); + + const row = await expandRow('ta-1'); + const input = within(row).getByTestId('catalog-row-gloss-input'); + await userEvent.clear(input); + await userEvent.type(input, 'message{Enter}'); + + const saved: TextAnalysis = onSave.mock.calls.at(-1)[0]; + expect(saved.tokenAnalyses[0].gloss).toEqual({ en: 'message' }); + }); + + it('reverts a gloss edit on Escape', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: SHARED, onSave }); + + const row = await expandRow('ta-1'); + const input = within(row).getByTestId('catalog-row-gloss-input'); + await userEvent.clear(input); + await userEvent.type(input, 'message{Escape}'); + + // Reverted to the committed text, so the blur that follows has nothing left to write. + expect(input).toHaveAttribute('value', 'word'); + await userEvent.tab(); + expect(onSave).not.toHaveBeenCalled(); + }); + + it('commits a breakdown edit on Enter', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: SHARED, onSave }); + + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + const input = within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input'); + await userEvent.clear(input); + await userEvent.type(input, 'λογ ος{Enter}'); + + const saved: TextAnalysis = onSave.mock.calls.at(-1)[0]; + expect(saved.tokenAnalyses[0].morphemes?.map((m) => m.form)).toEqual(['λογ', 'ος']); + }); + + it('abandons a breakdown edit on Escape', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: SHARED, onSave }); + + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + await userEvent.type( + within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input'), + 'λογ ος{Escape}', + ); + + expect(onSave).not.toHaveBeenCalled(); + expect( + within(rowFor('ta-1')).queryByTestId('catalog-row-breakdown-input'), + ).not.toBeInTheDocument(); + }); + + it('removes the breakdown when the editor is emptied', async () => { + const analysis: TextAnalysis = { + ...SHARED, + tokenAnalyses: [ + { + ...SHARED.tokenAnalyses[0], + morphemes: [{ ...FIXTURE_STAMPS, id: 'm-1', form: 'λογ', writingSystem: 'el' }], + }, + ], + }; + const onSave = jest.fn(); + renderPanel({ analysis, onSave }); + + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + await userEvent.clear(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input')); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-save')); + + const saved: TextAnalysis = onSave.mock.calls.at(-1)[0]; + expect(saved.tokenAnalyses[0].morphemes).toBeUndefined(); + }); + + it('removes the breakdown when the editor is given the whole word back', async () => { + const analysis: TextAnalysis = { + ...SHARED, + tokenAnalyses: [ + { + ...SHARED.tokenAnalyses[0], + morphemes: [ + { ...FIXTURE_STAMPS, id: 'm-1', form: 'λογ', writingSystem: 'el' }, + { ...FIXTURE_STAMPS, id: 'm-2', form: 'ος', writingSystem: 'el' }, + ], + }, + ], + }; + const onSave = jest.fn(); + renderPanel({ analysis, onSave }); + + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + const input = within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input'); + await userEvent.clear(input); + // A lone morpheme equal to the whole word records no segmentation, so asking for it is a + // request for the unsegmented state rather than a one-morpheme breakdown. + await userEvent.type(input, 'λόγος'); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-save')); + + const saved: TextAnalysis = onSave.mock.calls.at(-1)[0]; + expect(saved.tokenAnalyses[0].morphemes).toBeUndefined(); + }); + + it('rewrites a morpheme gloss for every token linked to the analysis', async () => { + const analysis: TextAnalysis = { + ...SHARED, + tokenAnalyses: [ + { + ...SHARED.tokenAnalyses[0], + morphemes: [{ ...FIXTURE_STAMPS, id: 'm-1', form: 'λογ', writingSystem: 'el' }], + }, + ], + }; + const onSave = jest.fn(); + renderPanel({ analysis, onSave }); + + const row = await expandRow('ta-1'); + await userEvent.type( + within(row).getByTestId('catalog-row-morpheme-gloss-input'), + 'word-stem', + ); + await userEvent.tab(); + + const saved: TextAnalysis = onSave.mock.calls.at(-1)[0]; + expect(saved.tokenAnalyses[0].morphemes?.[0].gloss).toEqual({ en: 'word-stem' }); + }); + }); + + describe('merging on edit', () => { + /** Two homographs whose glosses differ, so editing one into the other's collapses them. */ + const TWO_HOMOGRAPHS: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'ἀρχῇ', gloss: { en: 'start' } }, + { ...FIXTURE_STAMPS, id: 'ta-2', surfaceText: 'ἀρχῇ', gloss: { en: 'beginning' } }, + ], + tokenAnalysisLinks: [ + link('ta-1', 'GEN 1:1:0'), + link('ta-2', 'GEN 1:3:4'), + link('ta-2', 'GEN 2:7:2'), + ], + }; + + /** Edits `ta-1`'s gloss to match `ta-2`'s, which collapses the two onto `ta-2`. */ + async function editIntoEquality(): Promise { + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + const input = within(rowFor('ta-1')).getByTestId('catalog-row-gloss-input'); + await userEvent.clear(input); + await userEvent.type(input, 'beginning'); + await userEvent.tab(); + } + + it('drops the edited row and moves its usages onto the surviving one', async () => { + renderPanel({ analysis: TWO_HOMOGRAPHS }); + + await editIntoEquality(); + + expect(listedAnalysisIds()).toEqual(['ta-2']); + expect(within(rowFor('ta-2')).getByTestId('catalog-row-usage-count')).toHaveTextContent('3'); + }); + + it('announces where the edited row went', async () => { + renderPanel({ analysis: TWO_HOMOGRAPHS }); + + await editIntoEquality(); + + // The notice names the surviving gloss and its new count, so a row vanishing while another's + // count jumps reads as the convergence it is rather than as lost work. + expect(screen.getByTestId('catalog-merge-notice')).toHaveTextContent( + '%interlinearizer_analysisCatalog_merged%', + ); + }); + + it('names the survivor by its form when it carries no gloss', async () => { + const morphemes = [{ ...FIXTURE_STAMPS, id: 'm-1', form: 'ἀρχ', writingSystem: 'el' }]; + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + // A breakdown apiece, so clearing ta-1's gloss leaves a record with content rather than + // an empty one — and one identical to ta-2, which collapses the two onto a survivor + // there is no gloss to name. + { + ...FIXTURE_STAMPS, + id: 'ta-1', + surfaceText: 'ἀρχῇ', + gloss: { en: 'start' }, + morphemes, + }, + { ...FIXTURE_STAMPS, id: 'ta-2', surfaceText: 'ἀρχῇ', morphemes }, + ], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0'), link('ta-2', 'GEN 1:3:4')], + }; + renderPanel({ analysis }); + + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + await userEvent.clear(within(rowFor('ta-1')).getByTestId('catalog-row-gloss-input')); + await userEvent.tab(); + + expect(screen.getByTestId('catalog-merge-notice')).toHaveTextContent( + '%interlinearizer_analysisCatalog_mergedNoGloss%', + ); + }); + + it('leaves no notice when an edit empties the record away', async () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + // Gloss and nothing else, so clearing it leaves an empty record, which is removed outright + // rather than collapsed onto anything — there is no survivor to send the reader to. + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'λόγος', gloss: { en: 'word' } }, + ], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0')], + }; + renderPanel({ analysis }); + + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + await userEvent.clear(within(rowFor('ta-1')).getByTestId('catalog-row-gloss-input')); + await userEvent.tab(); + + expect(screen.queryByTestId('catalog-merge-notice')).not.toBeInTheDocument(); + expect(screen.queryAllByTestId('catalog-row')).toHaveLength(0); + }); + + it('leaves no notice when an edit collapses nothing', async () => { + renderPanel({ analysis: TWO_HOMOGRAPHS }); + + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + const input = within(rowFor('ta-1')).getByTestId('catalog-row-gloss-input'); + await userEvent.clear(input); + await userEvent.type(input, 'origin'); + await userEvent.tab(); + + expect(screen.queryByTestId('catalog-merge-notice')).not.toBeInTheDocument(); + }); + + it('dismisses the notice from its own control', async () => { + renderPanel({ analysis: TWO_HOMOGRAPHS }); + await editIntoEquality(); + + await userEvent.click(screen.getByTestId('catalog-merge-notice-dismiss')); + + expect(screen.queryByTestId('catalog-merge-notice')).not.toBeInTheDocument(); + }); + }); + + describe('merging into another row', () => { + const TWO_HOMOGRAPHS: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'ἀρχῇ', gloss: { en: 'start' } }, + { ...FIXTURE_STAMPS, id: 'ta-2', surfaceText: 'ἀρχῇ', gloss: { en: 'beginning' } }, + ], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0'), link('ta-2', 'GEN 1:3:4')], + }; + + it('offers the merge control to a row with pool peers', async () => { + renderPanel({ analysis: TWO_HOMOGRAPHS }); + + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + + expect(within(rowFor('ta-1')).getByTestId('catalog-row-merge')).toBeInTheDocument(); + }); + + it('withholds the merge control from a row with no pool peers', async () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [{ ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'λόγος' }], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0')], + }; + renderPanel({ analysis }); + + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + + expect(within(rowFor('ta-1')).queryByTestId('catalog-row-merge')).not.toBeInTheDocument(); + }); + + it('moves every usage onto the chosen target', async () => { + renderPanel({ analysis: TWO_HOMOGRAPHS }); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-merge')); + await userEvent.click(screen.getByTestId('catalog-merge-peer')); + await userEvent.click(screen.getByTestId('catalog-merge-confirm')); + + expect(listedAnalysisIds()).toEqual(['ta-2']); + expect(within(rowFor('ta-2')).getByTestId('catalog-row-usage-count')).toHaveTextContent('2'); + }); + + it('leaves both analyses alone when the picker is canceled', async () => { + renderPanel({ analysis: TWO_HOMOGRAPHS }); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-merge')); + await userEvent.click(screen.getByTestId('catalog-merge-cancel')); + + // Both still listed; the order is the default most-used-first, which the two tie on. + expect(listedAnalysisIds()).toHaveLength(2); + expect(listedAnalysisIds()).toContain('ta-1'); + expect(listedAnalysisIds()).toContain('ta-2'); + }); + + it('labels a peer that carries no gloss rather than leaving it nameless', async () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'ἀρχῇ', gloss: { en: 'start' } }, + { + ...FIXTURE_STAMPS, + id: 'ta-2', + surfaceText: 'ἀρχῇ', + morphemes: [{ ...FIXTURE_STAMPS, id: 'm-1', form: 'ἀρχ', writingSystem: 'el' }], + }, + ], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0'), link('ta-2', 'GEN 1:3:4')], + }; + renderPanel({ analysis }); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-merge')); + + // The stub leaves every key unresolved, which stands in for the lookup not having landed — + // so the peer falls back to the em dash rather than being offered as a blank choice. + expect(screen.getByTestId('catalog-merge-peer')).toHaveTextContent('—'); + }); + + it('refuses to merge until a target is chosen', async () => { + renderPanel({ analysis: TWO_HOMOGRAPHS }); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-merge')); + + expect(screen.getByTestId('catalog-merge-confirm')).toBeDisabled(); + }); + }); + + describe('deleting a row', () => { + /** One analysis nothing else shares a form with, so deleting it leaves its token blank. */ + const LONE: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'λόγος', gloss: { en: 'word' } }, + ], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0'), link('ta-1', 'GEN 1:3:4')], + }; + + /** Expands the row and opens its delete confirmation. */ + async function openDeleteConfirm(analysisId: string): Promise { + await userEvent.click(within(rowFor(analysisId)).getByTestId('catalog-row-toggle')); + await userEvent.click(within(rowFor(analysisId)).getByTestId('catalog-row-delete')); + } + + it('states that the uses are left blank when no homograph survives', async () => { + renderPanel({ analysis: LONE }); + + await openDeleteConfirm('ta-1'); + + expect(screen.getByTestId('catalog-delete-outcome')).toHaveTextContent( + '%interlinearizer_analysisCatalog_deleteBlank%', + ); + }); + + it('states the fallback the uses take when a homograph survives', async () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'ἀρχῇ', gloss: { en: 'start' } }, + { ...FIXTURE_STAMPS, id: 'ta-2', surfaceText: 'ἀρχῇ', gloss: { en: 'beginning' } }, + ], + tokenAnalysisLinks: [ + link('ta-1', 'GEN 1:1:0'), + link('ta-1', 'GEN 1:3:4'), + link('ta-2', 'GEN 2:7:2'), + ], + }; + renderPanel({ analysis }); + + await openDeleteConfirm('ta-1'); + + // The two outcomes must be told apart: this copy is the only guard before an irreversible + // delete, and promising a fallback that does not exist is worse than no confirmation at all. + expect(screen.getByTestId('catalog-delete-outcome')).toHaveTextContent( + '%interlinearizer_analysisCatalog_deleteFallback%', + ); + }); + + it('states a lone blanked use in the singular', async () => { + const analysis: TextAnalysis = { + ...LONE, + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0')], + }; + renderPanel({ analysis }); + + await openDeleteConfirm('ta-1'); + + // "1 uses will be left with no analysis" reads as a bug in the sentence that has to carry an + // irreversible decision, so the singular is a message of its own. + expect(screen.getByTestId('catalog-delete-outcome')).toHaveTextContent( + '%interlinearizer_analysisCatalog_deleteBlankOne%', + ); + }); + + it('states that nothing else changes when the analysis is used nowhere', async () => { + const analysis: TextAnalysis = { ...LONE, tokenAnalysisLinks: [] }; + renderPanel({ analysis }); + + await openDeleteConfirm('ta-1'); + + expect(screen.getByTestId('catalog-delete-outcome')).toHaveTextContent( + '%interlinearizer_analysisCatalog_deleteBlankNone%', + ); + }); + + it('states a lone falling-back use in the singular', async () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'ἀρχῇ', gloss: { en: 'start' } }, + { ...FIXTURE_STAMPS, id: 'ta-2', surfaceText: 'ἀρχῇ', gloss: { en: 'beginning' } }, + ], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0'), link('ta-2', 'GEN 2:7:2')], + }; + renderPanel({ analysis }); + + await openDeleteConfirm('ta-1'); + + expect(screen.getByTestId('catalog-delete-outcome')).toHaveTextContent( + '%interlinearizer_analysisCatalog_deleteFallbackOne%', + ); + }); + + it('describes a fallback that carries no gloss rather than naming it', async () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'ἀρχῇ', gloss: { en: 'start' } }, + // A breakdown but no gloss: analyzed enough to win the fallback, with no word to quote. + { + ...FIXTURE_STAMPS, + id: 'ta-2', + surfaceText: 'ἀρχῇ', + morphemes: [{ ...FIXTURE_STAMPS, id: 'm-1', form: 'ἀρχ', writingSystem: 'el' }], + }, + ], + tokenAnalysisLinks: [ + link('ta-1', 'GEN 1:1:0'), + link('ta-1', 'GEN 1:3:4'), + link('ta-2', 'GEN 2:7:2'), + ], + }; + renderPanel({ analysis }); + + await openDeleteConfirm('ta-1'); + + expect(screen.getByTestId('catalog-delete-outcome')).toHaveTextContent( + '%interlinearizer_analysisCatalog_deleteFallbackNoGloss%', + ); + }); + + it('describes a lone use falling back to a glossless analysis in the singular', async () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'ἀρχῇ', gloss: { en: 'start' } }, + { + ...FIXTURE_STAMPS, + id: 'ta-2', + surfaceText: 'ἀρχῇ', + morphemes: [{ ...FIXTURE_STAMPS, id: 'm-1', form: 'ἀρχ', writingSystem: 'el' }], + }, + ], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0'), link('ta-2', 'GEN 2:7:2')], + }; + renderPanel({ analysis }); + + await openDeleteConfirm('ta-1'); + + expect(screen.getByTestId('catalog-delete-outcome')).toHaveTextContent( + '%interlinearizer_analysisCatalog_deleteFallbackNoGlossOne%', + ); + }); + + it('removes the analysis and its links when confirmed', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: LONE, onSave }); + await openDeleteConfirm('ta-1'); + + await userEvent.click(screen.getByTestId('catalog-delete-confirm')); + + const saved: TextAnalysis = onSave.mock.calls.at(-1)[0]; + expect(saved.tokenAnalyses).toEqual([]); + expect(saved.tokenAnalysisLinks).toEqual([]); + }); + + it('leaves the analysis untouched when the confirmation is canceled', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: LONE, onSave }); + await openDeleteConfirm('ta-1'); + + await userEvent.click(screen.getByTestId('catalog-delete-cancel')); + + expect(onSave).not.toHaveBeenCalled(); + expect(listedAnalysisIds()).toEqual(['ta-1']); + }); + }); }); diff --git a/src/__tests__/store/analysisSlice.test.ts b/src/__tests__/store/analysisSlice.test.ts index 1f348514..b3a01875 100644 --- a/src/__tests__/store/analysisSlice.test.ts +++ b/src/__tests__/store/analysisSlice.test.ts @@ -2645,6 +2645,33 @@ describe('analysis-keyed reducers', () => { expect(tokenAnalyses[0].gloss).toBeUndefined(); }); + it('glosses a record that carried none, a breakdown having been entered first', () => { + const store = makeSharedStore({ + gloss: undefined, + morphemes: [{ id: 'm-1', form: 'word', writingSystem: 'en' }], + }); + + store.dispatch(writeAnalysisGloss({ analysisId: 'ta-shared', value: 'first' })); + + expect(store.getState().analysis.analysis.tokenAnalyses[0].gloss).toEqual({ und: 'first' }); + }); + + it('leaves a record that never carried a gloss alone when the gloss is cleared', () => { + // Analyzed by its breakdown alone, which is the state a breakdown entered before its glosses + // sits in — so clearing the gloss it does not have must not disturb the record. + const store = makeSharedStore({ + gloss: undefined, + morphemes: [{ id: 'm-1', form: 'word', writingSystem: 'en' }], + }); + + store.dispatch(writeAnalysisGloss({ analysisId: 'ta-shared', value: '' })); + + const { tokenAnalyses } = store.getState().analysis.analysis; + expect(tokenAnalyses).toHaveLength(1); + expect(tokenAnalyses[0].gloss).toBeUndefined(); + expect(tokenAnalyses[0].morphemes).toHaveLength(1); + }); + it('collapses onto a content-identical sibling, leaving the sibling as the survivor', () => { const store = makeSharedStore(); // A second payload for the same word, glossed differently — a homograph the edit will match. diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index cbc008c4..1b6c76d0 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -4,11 +4,26 @@ import { X } from 'lucide-react'; import { Button, EmptyState, TooltipProvider } from 'platform-bible-react'; import { formatReplacementString, isPlatformError } from 'platform-bible-utils'; import { useCallback, useEffect, useMemo, useState } from 'react'; -import { useAnalysisLanguage, useCatalogRows } from './AnalysisStore'; +import { + useAnalysisDeletionOutcome, + useAnalysisLanguage, + useAnalysisMergePeers, + useAnalysisRowDispatch, + useCatalogRows, + type AnalysisEditOutcome, +} from './AnalysisStore'; +import CatalogDeleteModal, { DELETE_STRING_KEYS } from './CatalogDeleteModal'; +import CatalogMergeModal, { MERGE_STRING_KEYS } from './CatalogMergeModal'; +import CatalogMergeNotice, { + MERGE_NOTICE_STRING_KEYS, + type MergeNotice, +} from './CatalogMergeNotice'; import CatalogQueryControls, { QUERY_CONTROL_STRING_KEYS } from './CatalogQueryControls'; import CatalogRowView, { ROW_STRING_KEYS } from './CatalogRowView'; import { useInterlinearNav } from './InterlinearNavContext'; import useRowWindow from '../hooks/useRowWindow'; +import type { AnalysisDeletionOutcome } from '../store/analysisSlice'; +import { normalizeSurfaceForm } from '../utils/analysis-identity'; import { applyCatalogQuery, deriveFacets, @@ -35,6 +50,9 @@ const STRING_KEYS = [ '%interlinearizer_analysisCatalog_noMatches%', ...QUERY_CONTROL_STRING_KEYS, ...ROW_STRING_KEYS, + ...MERGE_NOTICE_STRING_KEYS, + ...MERGE_STRING_KEYS, + ...DELETE_STRING_KEYS, ] as const satisfies `%${string}%`[]; /** Props for {@link AnalysisCatalogPanel}. */ @@ -51,7 +69,12 @@ type AnalysisCatalogPanelProps = Readonly<{ /** * The analysis catalog: every analysis the draft records, listed with the usage data the catalog - * lists it by. Read-only — nothing here writes to the analysis. + * lists it by, and editable in place. + * + * Every write from here is keyed by the analysis rather than by a token, so it changes what a + * record says for every token linked to it — one correction fixes a mis-split word across all its + * occurrences. That is the opposite of an edit made in the interlinear view, which forks a shared + * payload to keep itself local to one token. * * Sits beside the interlinear view rather than over it, so a jump to a usage can move the view * while the list the jump came from stays on screen. @@ -221,6 +244,159 @@ export default function AnalysisCatalogPanel({ [navigate, requestFocusToken], ); + const rowDispatch = useAnalysisRowDispatch(); + const readDeletionOutcome = useAnalysisDeletionOutcome(); + + /** + * The row whose merge picker or delete confirmation is open, or `undefined` when neither is. Held + * as an id rather than as a row, so a listing that turns over beneath an open modal cannot leave + * it holding a stale copy of what it is about to act on. + */ + const [mergeSourceId, setMergeSourceId] = useState(undefined); + const [deletingId, setDeletingId] = useState(undefined); + + /** + * What the last edit's collapse left standing, or `undefined` when no edit has collapsed one. + * Kept until dismissed or superseded: the reader may be looking anywhere in the list when an edit + * commits, and a row that vanishes unexplained reads as data loss. + */ + const [mergeNotice, setMergeNotice] = useState(undefined); + + /** + * Records what an edit did, so a collapse is reported rather than left to look like a vanished + * row. An ordinary edit clears whatever the last one said, the notice naming the edit just made + * rather than an older one. + */ + const reportEditOutcome = useCallback((outcome: AnalysisEditOutcome, surfaceText: string) => { + setMergeNotice( + outcome.kind === 'merged' + ? { + survivingAnalysisId: outcome.survivingAnalysisId, + survivingGloss: outcome.survivingGloss, + surfaceText, + usageCount: outcome.survivingUsageCount, + } + : undefined, + ); + }, []); + + /** + * The surface form of the row an edit came from, for a merge notice to name the survivor by when + * it carries no gloss. The two share a form — a collapse only ever happens between homographs — + * so the edited row's own is the survivor's too. + */ + const surfaceTextOf = useCallback( + (analysisId: string) => + /* v8 ignore next -- the id came from a row of this very listing, so it always resolves */ + catalogRows.find((r) => r.analysisId === analysisId)?.surfaceText ?? '', + [catalogRows], + ); + + const handleGlossCommit = useCallback( + (analysisId: string, value: string) => { + reportEditOutcome(rowDispatch.writeGloss(analysisId, value), surfaceTextOf(analysisId)); + }, + [reportEditOutcome, rowDispatch, surfaceTextOf], + ); + + const handleMorphemesCommit = useCallback( + (analysisId: string, forms: readonly string[]) => { + reportEditOutcome( + rowDispatch.writeMorphemes(analysisId, forms, sourceLanguageTag), + surfaceTextOf(analysisId), + ); + }, + [reportEditOutcome, rowDispatch, sourceLanguageTag, surfaceTextOf], + ); + + const handleMorphemeGlossCommit = useCallback( + (analysisId: string, morphemeId: string, value: string) => { + reportEditOutcome( + rowDispatch.writeMorphemeGloss(analysisId, morphemeId, value), + surfaceTextOf(analysisId), + ); + }, + [reportEditOutcome, rowDispatch, surfaceTextOf], + ); + + /** + * The outcome the open confirmation is stating, read once as it opens rather than subscribed: + * quoting a fallback that changed under the reader mid-decision would be worse than quoting the + * one they opened on. + */ + const [deletionOutcome, setDeletionOutcome] = useState( + undefined, + ); + + const handleDeleteRequest = useCallback( + (analysisId: string) => { + const outcome = readDeletionOutcome(analysisId); + // No outcome means the record is already gone, so there is nothing left to confirm deleting. + /* v8 ignore next -- the id came from a row of this very listing, so it always resolves */ + if (!outcome) return; + setDeletionOutcome(outcome); + setDeletingId(analysisId); + }, + [readDeletionOutcome], + ); + + const handleDeleteConfirm = useCallback(() => { + if (deletingId) rowDispatch.deleteAnalysis(deletingId); + setDeletingId(undefined); + // A deleted row cannot be the one a merge notice points at, and leaving the notice up would + // send the reader to a row that is no longer there. + setMergeNotice(undefined); + }, [deletingId, rowDispatch]); + + const handleMergeConfirm = useCallback( + (targetAnalysisId: string) => { + if (mergeSourceId) rowDispatch.mergeInto(mergeSourceId, targetAnalysisId); + setMergeSourceId(undefined); + }, + [mergeSourceId, rowDispatch], + ); + + const mergeSourceRow = useMemo( + () => catalogRows.find((row) => row.analysisId === mergeSourceId), + [catalogRows, mergeSourceId], + ); + const deletingRow = useMemo( + () => catalogRows.find((row) => row.analysisId === deletingId), + [catalogRows, deletingId], + ); + + const mergePeers = useAnalysisMergePeers(mergeSourceId ?? ''); + + /** + * How many tokens approve each analysis, so the merge picker can rank its choices. Taken off the + * rows the panel already holds rather than derived again, the catalog's usage count being that + * same number. + */ + const usageCountByAnalysisId = useMemo( + () => new Map(catalogRows.map((row) => [row.analysisId, row.usageCount])), + [catalogRows], + ); + + /** + * Which rows have a peer to merge into, so each row's merge control is offered only where it + * leads somewhere. Derived once for the listing rather than subscribed per row, which would be a + * pool lookup per analysis in the draft on every store change. + * + * Bucketed by the same normalized form the pool buckets by, so a row offered the control always + * finds peers in the picker: grouping by the raw text instead would withhold it from homographs + * differing only in case or Unicode form, which are peers as far as the store is concerned. + */ + const idsWithMergePeers = useMemo(() => { + const byForm = new Map(); + catalogRows.forEach((row) => { + const key = normalizeSurfaceForm(row.surfaceText); + const bucket = byForm.get(key) ?? []; + bucket.push(row.analysisId); + byForm.set(key, bucket); + }); + return new Set([...byForm.values()].filter((bucket) => bucket.length > 1).flat()); + }, [catalogRows]); + return ( // The panel sits beside the interlinear view rather than within it, so the row tooltips have no // enclosing provider to inherit, and a Tooltip without one throws. The delay is irrelevant here: @@ -267,6 +443,14 @@ export default function AnalysisCatalogPanel({ /> )} + {mergeNotice && ( + setMergeNotice(undefined)} + /> + )} + {rows.length === 0 ? ( // Two ways to have nothing to list, and they call for different answers: a draft that has // recorded nothing yet, and a query that kept none of what it did. Telling a reader the @@ -291,8 +475,16 @@ export default function AnalysisCatalogPanel({ analysisLanguage={analysisLanguage} isSelected={row.analysisId === selectedAnalysisId} localizedStrings={localizedStrings} + onDeleteRequest={handleDeleteRequest} + onGlossCommit={handleGlossCommit} + onMergeRequest={ + idsWithMergePeers.has(row.analysisId) ? setMergeSourceId : undefined + } + onMorphemeGlossCommit={handleMorphemeGlossCommit} + onMorphemesCommit={handleMorphemesCommit} onUsageSelect={handleUsageSelect} row={row} + shouldRevealSelf={row.analysisId === mergeNotice?.survivingAnalysisId} usageCountInBookLabel={usageCountInBookLabel} /> ))} @@ -304,6 +496,33 @@ export default function AnalysisCatalogPanel({
  • )} + + {/* + Both modals are mounted against the row they were opened on rather than against the id + alone, so a listing that turns over beneath one — an edit made in the view beside the + panel — closes it instead of leaving it acting on a record that is no longer there. + */} + {mergeSourceRow && ( + setMergeSourceId(undefined)} + onConfirm={handleMergeConfirm} + peers={mergePeers} + surfaceText={mergeSourceRow.surfaceText} + usageCountByAnalysisId={usageCountByAnalysisId} + /> + )} + + {deletingRow && deletionOutcome && ( + setDeletingId(undefined)} + onConfirm={handleDeleteConfirm} + outcome={deletionOutcome} + surfaceText={deletingRow.surfaceText} + /> + )} ); diff --git a/src/components/AnalysisStore.tsx b/src/components/AnalysisStore.tsx index a304618d..fbd6b5e1 100644 --- a/src/components/AnalysisStore.tsx +++ b/src/components/AnalysisStore.tsx @@ -2,6 +2,7 @@ import type { MorphemeAnalysis, PhraseAnalysisLink, TextAnalysis, + TokenAnalysis, TokenSnapshot, } from 'interlinearizer'; import { createContext, useCallback, useContext, useEffect, useMemo, useRef } from 'react'; @@ -11,11 +12,15 @@ import { createAnalysisStore, type AnalysisDispatch, type AnalysisRootState } fr import { approveAnalysisForToken, createPhrase, + deleteAnalysis, deleteMorphemes, deletePhrase, + mergeAnalysisInto, mergePhrases, selectAnalysis, selectAnalysisLanguage, + selectAnalysisDeletionOutcome, + selectAnalysisMergePeers, selectApprovedGloss, selectApprovedMorphemes, selectCatalogRows, @@ -27,11 +32,15 @@ import { selectSuggestionAfterClearing, selectSegmentFreeTranslation, updatePhrase, + writeAnalysisGloss, + writeAnalysisMorphemeGloss, + writeAnalysisMorphemes, writeGloss, writeMorphemeGloss, writeMorphemes, writePhraseGloss, writeSegmentFreeTranslation, + type AnalysisDeletionOutcome, } from '../store/analysisSlice'; import { emptyAnalysis } from '../types/empty-factories'; import type { CatalogRow } from '../utils/analysis-query'; @@ -407,6 +416,226 @@ export function useCatalogRows(currentBook: string): readonly CatalogRow[] { return useSelector((state: AnalysisRootState) => selectCatalogRows(state.analysis, currentBook)); } +/** + * Where a catalog row's edit left the record it was aimed at. + * + * A write that makes a row content-identical to a sibling collapses the two, so the row the reader + * edited is gone from the listing and another's usage count has grown by its tokens. That reads as + * data loss unless it is reported, hence this rather than a bare `void`. + */ +export type AnalysisEditOutcome = + | { + /** The record survived the edit under its own id, which is the ordinary case. */ + kind: 'edited'; + } + | { + /** The record collapsed onto a content-identical sibling and no longer exists. */ + kind: 'merged'; + /** The surviving record's id, which the listing should now point the reader at. */ + survivingAnalysisId: string; + /** What the survivor now reads as in the active analysis language, `''` when it has none. */ + survivingGloss: string; + /** The survivor's usage count once the collapsed record's tokens joined it. */ + survivingUsageCount: number; + } + | { + /** The edit emptied the record, which removed it and every link to it. */ + kind: 'removed'; + }; + +/** + * Reports where an edit left the record `analysisId` named, by reading the store either side of the + * write. Nothing the write itself reports says this: a collapse repoints the links and drops the + * record silently, so where it went is recoverable only by comparing the two states. + * + * A record that is simply gone was emptied by its own edit; one whose links moved elsewhere + * collapsed onto the record they moved to. The two are told apart by following the links, since + * only a collapse leaves them pointing somewhere. + */ +function readEditOutcome( + before: TextAnalysis, + after: TextAnalysis, + analysisId: string, + analysisLanguage: string, +): AnalysisEditOutcome { + if (after.tokenAnalyses.some((ta) => ta.id === analysisId)) return { kind: 'edited' }; + + // The links as they stood before the write name the tokens the record held; where those same + // tokens point now is where the record went. + const movedTokenRefs = new Set( + before.tokenAnalysisLinks + .filter((l) => l.analysisId === analysisId) + .map((l) => l.token.tokenRef), + ); + const survivor = after.tokenAnalysisLinks.find((l) => movedTokenRefs.has(l.token.tokenRef)); + if (!survivor) return { kind: 'removed' }; + + const survivingAnalysis = after.tokenAnalyses.find((ta) => ta.id === survivor.analysisId); + return { + kind: 'merged', + survivingAnalysisId: survivor.analysisId, + /* v8 ignore next -- a link the store holds always resolves to a record it holds */ + survivingGloss: survivingAnalysis?.gloss?.[analysisLanguage] ?? '', + survivingUsageCount: new Set( + after.tokenAnalysisLinks + .filter((l) => l.analysisId === survivor.analysisId && l.status === 'approved') + .map((l) => l.token.tokenRef), + ).size, + }; +} + +/** The catalog's write callbacks, each keyed by `analysisId` and so global to that record. */ +export type AnalysisRowDispatch = { + /** + * Writes a gloss onto the record itself, changing what it says for every token linked to it. A + * blank value clears it. + */ + writeGloss: (analysisId: string, value: string) => AnalysisEditOutcome; + /** + * Replaces the record's morpheme breakdown for every token linked to it. The breakdown is + * replaced rather than reconciled, so the old morphemes' glosses go with it. + */ + writeMorphemes: ( + analysisId: string, + forms: readonly string[], + writingSystem: string, + ) => AnalysisEditOutcome; + /** Writes a gloss onto one morpheme of the record, for every token linked to it. */ + writeMorphemeGloss: ( + analysisId: string, + morphemeId: string, + value: string, + ) => AnalysisEditOutcome; + /** + * Removes the record and every link to it, leaving its tokens on whatever the suggestion pool + * still offers. Irreversible — see {@link useAnalysisDeletionOutcome} for what it will cost. + */ + deleteAnalysis: (analysisId: string) => void; + /** Moves every link on one record to another and drops the source. */ + mergeInto: (sourceAnalysisId: string, targetAnalysisId: string) => void; +}; + +/** + * Returns stable callbacks for editing the analysis records the catalog lists. Every callback here + * is keyed by `analysisId` and so rewrites the record for all its tokens at once — the opposite of + * the `tokenRef`-keyed hooks above, which fork a shared payload to keep an edit local to one token. + * The key is the whole of the distinction; neither family takes a scope flag. + * + * Each write reports where it left the record, so the caller can tell the reader that an edit + * collapsed the row onto a sibling rather than letting it vanish unexplained. + * + * @throws When called outside an {@link AnalysisStoreProvider}. + */ +export function useAnalysisRowDispatch(): AnalysisRowDispatch { + const { dispatch, save } = useAnalysisSave('useAnalysisRowDispatch'); + const store = useStore(); + + /** + * Dispatches one write and reports where it left the record, persisting once afterwards. Every + * write path shares it, so they cannot disagree about what counts as a merge. + */ + const writeAndReport = useCallback( + (analysisId: string, action: Parameters[0]): AnalysisEditOutcome => { + const before = store.getState().analysis.analysis; + dispatch(action); + const { analysis: after, analysisLanguage } = store.getState().analysis; + save(); + return readEditOutcome(before, after, analysisId, analysisLanguage); + }, + [dispatch, save, store], + ); + + const handleWriteGloss = useCallback( + (analysisId: string, value: string) => + writeAndReport(analysisId, writeAnalysisGloss({ analysisId, value })), + [writeAndReport], + ); + + const handleWriteMorphemes = useCallback( + (analysisId: string, forms: readonly string[], writingSystem: string) => + writeAndReport(analysisId, writeAnalysisMorphemes({ analysisId, forms, writingSystem })), + [writeAndReport], + ); + + const handleWriteMorphemeGloss = useCallback( + (analysisId: string, morphemeId: string, value: string) => + writeAndReport(analysisId, writeAnalysisMorphemeGloss({ analysisId, morphemeId, value })), + [writeAndReport], + ); + + const handleDelete = useCallback( + (analysisId: string) => { + dispatch(deleteAnalysis({ analysisId })); + save(); + }, + [dispatch, save], + ); + + const handleMergeInto = useCallback( + (sourceAnalysisId: string, targetAnalysisId: string) => { + dispatch(mergeAnalysisInto({ sourceAnalysisId, targetAnalysisId })); + save(); + }, + [dispatch, save], + ); + + return useMemo( + () => ({ + writeGloss: handleWriteGloss, + writeMorphemes: handleWriteMorphemes, + writeMorphemeGloss: handleWriteMorphemeGloss, + deleteAnalysis: handleDelete, + mergeInto: handleMergeInto, + }), + [ + handleWriteGloss, + handleWriteMorphemes, + handleWriteMorphemeGloss, + handleDelete, + handleMergeInto, + ], + ); +} + +/** + * Returns a stable getter for what deleting a record would do to the tokens that approve it — left + * blank, or falling back to a surviving homograph — so a confirmation can name the concrete + * consequence. Returns `undefined` for an id that resolves to no record. + * + * A getter rather than a subscription: the outcome is read once, when the confirmation opens, and + * subscribing every row to it would recompute the suggestion pool per row on every store change. + * + * @throws When called outside an {@link AnalysisStoreProvider}. + */ +export function useAnalysisDeletionOutcome(): ( + analysisId: string, +) => AnalysisDeletionOutcome | undefined { + useRequiredCallbacks('useAnalysisDeletionOutcome'); + const store = useStore(); + + return useCallback( + (analysisId: string) => selectAnalysisDeletionOutcome(store.getState().analysis, analysisId), + [store], + ); +} + +/** + * Returns the records the given row may be merged into: those sharing its normalized surface form, + * most-used first, so merging is offered only among genuine homographs. + * + * Subscribed rather than read on demand, because whether a row has peers at all decides whether its + * merge control is offered, which has to follow an edit made beside the panel. + * + * @throws When called outside an {@link AnalysisStoreProvider}. + */ +export function useAnalysisMergePeers(analysisId: string): readonly TokenAnalysis[] { + useRequiredCallbacks('useAnalysisMergePeers'); + + return useSelector((state: AnalysisRootState) => + selectAnalysisMergePeers(state.analysis, analysisId), + ); +} + /** * Returns the active BCP 47 analysis-language tag from the nearest {@link AnalysisStoreProvider}. * diff --git a/src/components/CatalogDeleteModal.tsx b/src/components/CatalogDeleteModal.tsx new file mode 100644 index 00000000..c94ac119 --- /dev/null +++ b/src/components/CatalogDeleteModal.tsx @@ -0,0 +1,132 @@ +import { Button } from 'platform-bible-react'; +import { formatReplacementString, type LanguageStrings } from 'platform-bible-utils'; +import { ModalShell } from './modals/ModalShell'; +import type { AnalysisDeletionOutcome } from '../store/analysisSlice'; + +/** Localized string keys the delete confirmation renders. */ +export const DELETE_STRING_KEYS = [ + '%interlinearizer_analysisCatalog_deleteTitle%', + '%interlinearizer_analysisCatalog_deleteBlank%', + '%interlinearizer_analysisCatalog_deleteBlankOne%', + '%interlinearizer_analysisCatalog_deleteBlankNone%', + '%interlinearizer_analysisCatalog_deleteFallback%', + '%interlinearizer_analysisCatalog_deleteFallbackOne%', + '%interlinearizer_analysisCatalog_deleteFallbackNoGloss%', + '%interlinearizer_analysisCatalog_deleteFallbackNoGlossOne%', + '%interlinearizer_analysisCatalog_deleteUndoWarning%', + '%interlinearizer_analysisCatalog_deleteCancel%', + '%interlinearizer_analysisCatalog_deleteConfirm%', +] as const satisfies `%${string}%`[]; + +/** + * States the concrete consequence of the deletion in the reader's own terms — how many uses are + * affected and what they will read as afterwards — rather than asking a generic "are you sure". + * + * The cases are the outcomes the store distinguishes crossed with whether there is a word to quote: + * a fallback whose peer carries no gloss in the active language can only be described, not named. + * Each has a singular form, because "1 uses" reads as a bug in the sentence that has to carry an + * irreversible decision. + * + * Zero uses is separated from the plural rather than left to say "0 uses will be left with no + * analysis", which invites the reader to wonder which nothing it means. Only the blank outcome can + * be reached with no uses: a fallback is derived from the pool, which an unused record has no + * approvals in. + */ +function outcomeMessage( + outcome: AnalysisDeletionOutcome, + localizedStrings: LanguageStrings, +): string { + const { kind, usageCount, fallbackGloss } = outcome; + + if (kind === 'blank') { + if (usageCount === 0) + return localizedStrings['%interlinearizer_analysisCatalog_deleteBlankNone%']; + if (usageCount === 1) + return localizedStrings['%interlinearizer_analysisCatalog_deleteBlankOne%']; + return formatReplacementString( + localizedStrings['%interlinearizer_analysisCatalog_deleteBlank%'], + { count: usageCount }, + ); + } + + if (!fallbackGloss) { + if (usageCount === 1) + return localizedStrings['%interlinearizer_analysisCatalog_deleteFallbackNoGlossOne%']; + return formatReplacementString( + localizedStrings['%interlinearizer_analysisCatalog_deleteFallbackNoGloss%'], + { count: usageCount }, + ); + } + + if (usageCount === 1) + return formatReplacementString( + localizedStrings['%interlinearizer_analysisCatalog_deleteFallbackOne%'], + { gloss: fallbackGloss }, + ); + return formatReplacementString( + localizedStrings['%interlinearizer_analysisCatalog_deleteFallback%'], + { count: usageCount, gloss: fallbackGloss }, + ); +} + +/** Props for {@link CatalogDeleteModal}. */ +type CatalogDeleteModalProps = Readonly<{ + /** Surface form of the analysis being deleted, named in the title. */ + surfaceText: string; + /** What the deletion will do to the tokens that approve the analysis. */ + outcome: AnalysisDeletionOutcome; + /** Commits the deletion. */ + onConfirm: () => void; + /** Backs out, leaving the analysis untouched. */ + onCancel: () => void; + /** Resolved localizations covering at least {@link DELETE_STRING_KEYS}. */ + localizedStrings: LanguageStrings; +}>; + +/** + * Confirms deleting an analysis, naming what the deletion costs. + * + * Delete ships before undo, so this copy is the only guard: the analysis and every link to it go at + * once, and the tokens that carried it are left on whatever the suggestion pool still offers. The + * modal is dismissable by Escape and by clicking outside — nothing is in flight to abandon, and a + * confirmation that traps the reader is worse than one they can back out of. + */ +export default function CatalogDeleteModal({ + surfaceText, + outcome, + onConfirm, + onCancel, + localizedStrings, +}: CatalogDeleteModalProps) { + return ( + +

    + {outcomeMessage(outcome, localizedStrings)} +

    +

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

    +
    + + +
    +
    + ); +} diff --git a/src/components/CatalogMergeModal.tsx b/src/components/CatalogMergeModal.tsx new file mode 100644 index 00000000..2ff277fe --- /dev/null +++ b/src/components/CatalogMergeModal.tsx @@ -0,0 +1,118 @@ +import type { TokenAnalysis } from 'interlinearizer'; +import { Button } from 'platform-bible-react'; +import { formatReplacementString, type LanguageStrings } from 'platform-bible-utils'; +import { useState } from 'react'; +import { ModalShell } from './modals/ModalShell'; +import { resolvedOrEmpty } from '../utils/localized-strings'; + +/** Localized string keys the merge picker renders. */ +export const MERGE_STRING_KEYS = [ + '%interlinearizer_analysisCatalog_mergeTitle%', + '%interlinearizer_analysisCatalog_mergePrompt%', + '%interlinearizer_analysisCatalog_mergePeerUsageCount%', + '%interlinearizer_analysisCatalog_mergeCancel%', + '%interlinearizer_analysisCatalog_mergeConfirm%', + '%interlinearizer_analysisCatalog_noGloss%', +] as const satisfies `%${string}%`[]; + +/** Props for {@link CatalogMergeModal}. */ +type CatalogMergeModalProps = Readonly<{ + /** Surface form of the analysis being merged away, named in the title. */ + surfaceText: string; + /** The analyses this one may be merged into: its pool peers, most-used first. */ + peers: readonly TokenAnalysis[]; + /** How many tokens approve each peer, keyed by analysis id, for the choices to be ranked by. */ + usageCountByAnalysisId: ReadonlyMap; + /** BCP 47 tag the peers' glosses are read under. */ + analysisLanguage: string; + /** Commits the merge into the chosen target. */ + onConfirm: (targetAnalysisId: string) => void; + /** Backs out, leaving both analyses untouched. */ + onCancel: () => void; + /** Resolved localizations covering at least {@link MERGE_STRING_KEYS}. */ + localizedStrings: LanguageStrings; +}>; + +/** + * Picks which analysis to merge a row into, from its pool peers alone — the records sharing its + * surface form, which are the only ones a merge could sensibly reassign its tokens to. + * + * Nothing is preselected: a merge moves every use of one analysis onto another and drops the + * source, so the target is a decision to make rather than one to default into. Confirm stays + * disabled until a choice is made. + */ +export default function CatalogMergeModal({ + surfaceText, + peers, + usageCountByAnalysisId, + analysisLanguage, + onConfirm, + onCancel, + localizedStrings, +}: CatalogMergeModalProps) { + const [targetId, setTargetId] = useState(undefined); + + // Visible cell text, so an unresolved key would leave a peer nameless in a list the reader + // chooses from. The em dash reads as "no gloss" in any language, as it does in the listing. + const noGloss = + resolvedOrEmpty(localizedStrings['%interlinearizer_analysisCatalog_noGloss%']) || '—'; + + return ( + +

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

    + +
      + {peers.map((peer) => ( +
    • + +
    • + ))} +
    + +
    + + +
    +
    + ); +} diff --git a/src/components/CatalogMergeNotice.tsx b/src/components/CatalogMergeNotice.tsx new file mode 100644 index 00000000..96834b15 --- /dev/null +++ b/src/components/CatalogMergeNotice.tsx @@ -0,0 +1,82 @@ +import { X } from 'lucide-react'; +import { Button } from 'platform-bible-react'; +import { formatReplacementString, type LanguageStrings } from 'platform-bible-utils'; + +/** Localized string keys the merge notice renders. */ +export const MERGE_NOTICE_STRING_KEYS = [ + '%interlinearizer_analysisCatalog_merged%', + '%interlinearizer_analysisCatalog_mergedNoGloss%', + '%interlinearizer_analysisCatalog_mergedDismiss%', +] as const satisfies `%${string}%`[]; + +/** What a merge-on-edit left standing, for the notice to name. */ +export interface MergeNotice { + /** The surviving analysis's id, which the listing scrolls to. */ + survivingAnalysisId: string; + /** What the survivor reads as, `''` when it carries no gloss in the active language. */ + survivingGloss: string; + /** The survivor's surface form, named in place of the gloss when it has none. */ + surfaceText: string; + /** The survivor's usage count once the collapsed record's tokens joined it. */ + usageCount: number; +} + +/** Props for {@link CatalogMergeNotice}. */ +type CatalogMergeNoticeProps = Readonly<{ + notice: MergeNotice; + /** Dismisses the notice. */ + onDismiss: () => void; + /** Resolved localizations covering at least {@link MERGE_NOTICE_STRING_KEYS}. */ + localizedStrings: LanguageStrings; +}>; + +/** + * Reports that an edit collapsed one row into another, naming where the edit went and what the + * surviving row now counts. + * + * Without this the row the reader was editing simply disappears while an unrelated row's count + * jumps, which reads as data loss rather than as the convergence it is. It persists until dismissed + * or superseded rather than fading, because the reader may be looking anywhere in the list when + * their edit commits. + * + * Reaches a screen reader after the edit it explains rather than interrupting the field the reader + * is still in. + */ +export default function CatalogMergeNotice({ + notice, + onDismiss, + localizedStrings, +}: CatalogMergeNoticeProps) { + const { survivingGloss, surfaceText, usageCount } = notice; + + // Named by its gloss where it has one, since that is what the reader was editing toward; by its + // form otherwise, a notice that named neither leaving nothing to recognize the row by. + const message = survivingGloss + ? formatReplacementString(localizedStrings['%interlinearizer_analysisCatalog_merged%'], { + gloss: survivingGloss, + count: usageCount, + }) + : formatReplacementString(localizedStrings['%interlinearizer_analysisCatalog_mergedNoGloss%'], { + form: surfaceText, + count: usageCount, + }); + + return ( +
    +

    {message}

    + +
    + ); +} diff --git a/src/components/CatalogRowEditor.tsx b/src/components/CatalogRowEditor.tsx new file mode 100644 index 00000000..de9ffa4a --- /dev/null +++ b/src/components/CatalogRowEditor.tsx @@ -0,0 +1,305 @@ +import type { MorphemeAnalysis } from 'interlinearizer'; +import { Button, Input, Label } from 'platform-bible-react'; +import { formatReplacementString, type LanguageStrings } from 'platform-bible-utils'; +import { useId, useState } from 'react'; +import { resolvedOrEmpty } from '../utils/localized-strings'; + +/** Localized string keys the row editor renders. */ +export const ROW_EDITOR_STRING_KEYS = [ + '%interlinearizer_analysisCatalog_editGloss%', + '%interlinearizer_analysisCatalog_editMorphemes%', + '%interlinearizer_analysisCatalog_editMorphemesHint%', + '%interlinearizer_analysisCatalog_editMorphemesSave%', + '%interlinearizer_analysisCatalog_editMorphemesCancel%', + '%interlinearizer_analysisCatalog_editMorphemesOpen%', + '%interlinearizer_analysisCatalog_morphemeGloss%', + '%interlinearizer_analysisCatalog_appliesToAll%', + '%interlinearizer_analysisCatalog_merge%', + '%interlinearizer_analysisCatalog_delete%', +] as const satisfies `%${string}%`[]; + +/** Props for {@link CatalogRowEditor}. */ +type CatalogRowEditorProps = Readonly<{ + analysisId: string; + /** Surface form of the analysis, which the breakdown editor pre-fills from when there is none. */ + surfaceText: string; + /** The analysis's gloss in the active language, `''` when it has none. */ + gloss: string; + morphemes: readonly MorphemeAnalysis[]; + /** BCP 47 tag the morpheme glosses are read and written under. */ + analysisLanguage: string; + /** Writes the analysis's gloss for every token linked to it. */ + onGlossCommit: (value: string) => void; + /** Replaces the analysis's morpheme breakdown for every token linked to it. */ + onMorphemesCommit: (forms: readonly string[]) => void; + /** Writes one morpheme's gloss for every token linked to the analysis. */ + onMorphemeGlossCommit: (morphemeId: string, value: string) => void; + /** Opens the merge picker. Absent when the analysis has no pool peers to merge into. */ + onMergeRequest?: () => void; + /** Opens the delete confirmation. */ + onDeleteRequest: () => void; + /** Resolved localizations covering at least {@link ROW_EDITOR_STRING_KEYS}. */ + localizedStrings: LanguageStrings; +}>; + +/** Collapses leading, trailing, and repeated internal whitespace to single spaces. */ +function normalize(value: string): string { + return value.trim().replace(/\s+/g, ' '); +} + +/** + * A text field whose edit commits on blur and on Enter, and reverts on Escape, holding its draft + * locally until then. + * + * Committing on blur rather than per keystroke is what the gloss inputs in the interlinear view do, + * and it matters more here: every keystroke would be a write across every token the analysis holds, + * and an edit passing through a state identical to a sibling would collapse the row mid-word. + * + * The draft is keyed on the committed value, so a row re-rendered under a different analysis — the + * listing reorders under an edit — refills rather than showing the previous row's text. + */ +function CommitOnBlurInput({ + ariaLabel, + committedValue, + id, + onCommit, + testId, +}: Readonly<{ + ariaLabel?: string; + committedValue: string; + id?: string; + onCommit: (value: string) => void; + testId: string; +}>) { + const [draft, setDraft] = useState(committedValue); + const [draftOf, setDraftOf] = useState(committedValue); + + // Adjusted during render rather than in an effect, so the field never paints one frame holding + // the previous value. + if (committedValue !== draftOf) { + setDraftOf(committedValue); + setDraft(committedValue); + } + + const commit = () => { + if (draft !== committedValue) onCommit(draft); + }; + + return ( + setDraft(e.target.value)} + onKeyDown={(e) => { + if (e.key === 'Enter') { + e.preventDefault(); + commit(); + } else if (e.key === 'Escape') { + e.preventDefault(); + setDraft(committedValue); + } + }} + type="text" + value={draft} + /> + ); +} + +/** + * The editable half of an expanded catalog row: the analysis's gloss, its morpheme breakdown and + * each morpheme's gloss, and the merge and delete controls. + * + * Every write here is keyed by the analysis rather than by a token, so it changes what the record + * says everywhere it is used — which the note above the fields says outright, because a row gives + * no other clue how many tokens an edit is about to rewrite. + * + * The breakdown is edited as a line of space-separated forms, as the interlinear view's morpheme + * editor does, so the same input reads the same in both places. It is behind its own toggle because + * committing it discards the old morphemes' glosses, which is not an edit to make by tabbing past. + */ +export default function CatalogRowEditor({ + analysisId, + surfaceText, + gloss, + morphemes, + analysisLanguage, + onGlossCommit, + onMorphemesCommit, + onMorphemeGlossCommit, + onMergeRequest, + onDeleteRequest, + localizedStrings, +}: CatalogRowEditorProps) { + const glossFieldId = useId(); + const breakdownFieldId = useId(); + + /** The breakdown draft while the editor is open, or `undefined` when it is closed. */ + const [breakdownDraft, setBreakdownDraft] = useState(undefined); + + const morphemeForms = morphemes.map((m) => m.form).join(' '); + + const commitBreakdown = () => { + /* v8 ignore next -- only the open editor calls this, and it is open only with a draft held */ + if (breakdownDraft === undefined) return; + const normalized = normalize(breakdownDraft); + // An empty draft has no reading as a breakdown, and a lone form equal to the whole word records + // no segmentation — both are a request for the unsegmented state, which is an empty form list. + const forms = + normalized === '' || normalized === normalize(surfaceText) ? [] : normalized.split(' '); + if (forms.join(' ') !== morphemeForms) onMorphemesCommit(forms); + setBreakdownDraft(undefined); + }; + + return ( +
    +

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

    + +
    + +
    + +
    +
    + + {breakdownDraft === undefined ? ( +
    + + {localizedStrings['%interlinearizer_analysisCatalog_editMorphemes%']} + + +
    + ) : ( +
    + + setBreakdownDraft(e.target.value)} + onKeyDown={(e) => { + if (e.key === 'Enter') { + e.preventDefault(); + commitBreakdown(); + } else if (e.key === 'Escape') { + e.preventDefault(); + setBreakdownDraft(undefined); + } + }} + type="text" + value={breakdownDraft} + /> +

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

    +
    + + +
    +
    + )} + + {morphemes.length > 0 && breakdownDraft === undefined && ( +
    + {morphemes.map((morpheme) => ( + // Form above gloss, as the interlinear view arranges them, so a breakdown reads the + // same in both places. +
    + {morpheme.form} +
    + onMorphemeGlossCommit(morpheme.id, value)} + testId="catalog-row-morpheme-gloss-input" + /> +
    +
    + ))} +
    + )} + +
    + {/* + Offered only when the analysis has pool peers: with none there is nothing a merge could + reassign its tokens to, and a control that opens an empty picker is worse than no control. + */} + {onMergeRequest && ( + + )} + +
    +
    + ); +} diff --git a/src/components/CatalogRowView.tsx b/src/components/CatalogRowView.tsx index bd77dfe9..087d2ce8 100644 --- a/src/components/CatalogRowView.tsx +++ b/src/components/CatalogRowView.tsx @@ -8,6 +8,7 @@ import { } from 'platform-bible-react'; import { formatReplacementString, formatScrRef, type LanguageStrings } from 'platform-bible-utils'; import { memo, useCallback, useState } from 'react'; +import CatalogRowEditor, { ROW_EDITOR_STRING_KEYS } from './CatalogRowEditor'; import type { CatalogRow, CatalogUsage } from '../utils/analysis-query'; import { resolvedOrEmpty } from '../utils/localized-strings'; @@ -21,6 +22,7 @@ export const ROW_STRING_KEYS = [ '%interlinearizer_analysisCatalog_usageCount%', '%interlinearizer_analysisCatalog_noUsages%', '%interlinearizer_analysisCatalog_showAllUsages%', + ...ROW_EDITOR_STRING_KEYS, ] as const satisfies `%${string}%`[]; /** @@ -43,6 +45,24 @@ type CatalogRowViewProps = Readonly<{ localizedStrings: LanguageStrings; /** BCP 47 tag the morpheme glosses are read under. */ analysisLanguage: string; + /** Writes this row's gloss for every token linked to it. */ + onGlossCommit: (analysisId: string, value: string) => void; + /** Replaces this row's morpheme breakdown for every token linked to it. */ + onMorphemesCommit: (analysisId: string, forms: readonly string[]) => void; + /** Writes one of this row's morpheme glosses for every token linked to it. */ + onMorphemeGlossCommit: (analysisId: string, morphemeId: string, value: string) => void; + /** + * Opens the merge picker for this row. Absent when the analysis has no pool peers, which is how + * the merge control is withheld from a row with nothing to merge into. + */ + onMergeRequest?: (analysisId: string) => void; + /** Opens the delete confirmation for this row. */ + onDeleteRequest: (analysisId: string) => void; + /** + * Whether the row should be scrolled into view. Set on the row a merge-on-edit left standing, so + * the reader is taken to where their edit went rather than left where it vanished from. + */ + shouldRevealSelf?: boolean; }>; /** Renders a usage's location the way scripture references are written, e.g. `GEN 1:1`. */ @@ -56,8 +76,8 @@ function usageLabel(usage: CatalogUsage): string { /** * One analysis in the catalog: its surface form and gloss, and how much of the draft it accounts - * for — the whole draft's usage count beside the current book's. Expanding it reveals the morpheme - * breakdown and the places the analysis is applied. + * for — the whole draft's usage count beside the current book's. Expanding it reveals the controls + * for editing the analysis and the places it is applied. * * Each row owns its own layout so that its detail can be nested inside it. */ @@ -68,6 +88,12 @@ function CatalogRowView({ onUsageSelect, localizedStrings, analysisLanguage, + onGlossCommit, + onMorphemesCommit, + onMorphemeGlossCommit, + onMergeRequest, + onDeleteRequest, + shouldRevealSelf = false, }: CatalogRowViewProps) { const [isExpanded, setIsExpanded] = useState(false); @@ -81,6 +107,30 @@ function CatalogRowView({ setShowsAllUsages(false); }, []); + // The editor is handed callbacks already carrying this row's id, so it never needs the id itself + // to report an edit. + const { analysisId } = row; + const handleGlossCommit = useCallback( + (value: string) => onGlossCommit(analysisId, value), + [analysisId, onGlossCommit], + ); + const handleMorphemesCommit = useCallback( + (forms: readonly string[]) => onMorphemesCommit(analysisId, forms), + [analysisId, onMorphemesCommit], + ); + const handleMorphemeGlossCommit = useCallback( + (morphemeId: string, value: string) => onMorphemeGlossCommit(analysisId, morphemeId, value), + [analysisId, onMorphemeGlossCommit], + ); + const handleMergeRequest = useCallback( + () => onMergeRequest?.(analysisId), + [analysisId, onMergeRequest], + ); + const handleDeleteRequest = useCallback( + () => onDeleteRequest(analysisId), + [analysisId, onDeleteRequest], + ); + const visibleUsages = showsAllUsages ? row.usages : row.usages.slice(0, INLINE_USAGE_LIMIT); const hiddenUsageCount = row.usages.length - visibleUsages.length; @@ -98,8 +148,22 @@ function CatalogRowView({ const surfaceTooltip = useTruncationTooltip(); const glossTooltip = useTruncationTooltip(); + /** + * Brings the row into view once the panel asks for it, which it does for the row a merge-on-edit + * left standing. Runs on the flag turning true rather than on every render, so a reader who then + * scrolls away is not dragged back by an unrelated re-render. + */ + const revealRef = useCallback( + (el: HTMLLIElement | null) => { + /* v8 ignore next -- jsdom implements no layout, so scrollIntoView is absent on the element */ + if (shouldRevealSelf) el?.scrollIntoView?.({ block: 'nearest' }); + }, + [shouldRevealSelf], + ); + return (
  • - {row.morphemes.length > 0 && ( -
    - {row.morphemes.map((morpheme) => ( - // Form above gloss, as the interlinear view arranges them, so a breakdown reads the - // same in both places. -
    - {morpheme.form} - - {morpheme.gloss?.[analysisLanguage] ?? ''} - -
    - ))} -
    - )} + {row.usages.length === 0 ? (

    From 6ce45a7c0492f4c54dc2b830dae187571c1199d8 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 1 Sep 2026 08:44:42 -0600 Subject: [PATCH 06/14] Distinguish a removed analysis from a merged one by its links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit readEditOutcome treated any link surviving on the edited record's tokens as the survivor of a collapse. A token may hold several links at once — the one-per-token invariant covers only approved ones — so emptying a record whose token kept an unrelated suggested or candidate link reported a merge into a record that had gained nothing. Identify the survivor instead by a link naming a record its token was not already linked to, which only a collapse produces. Register the catalog row editor's drafts with the pending-edit tracker, as the interlinear view's inputs already do. The gloss fields commit on blur, so this only lights the unsaved indicator while typing; the breakdown editor commits on Enter or Save alone, and its draft was discarded by a project switch with no confirmation. Compare the draft as forms rather than as text so the whole word pre-filled for an unsegmented breakdown does not register as unsaved work. --- .../components/AnalysisCatalogPanel.test.tsx | 99 +++++++++++++++++++ src/components/AnalysisStore.tsx | 22 ++++- src/components/CatalogRowEditor.tsx | 27 ++++- 3 files changed, 138 insertions(+), 10 deletions(-) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 9cf4be93..cb333103 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -72,6 +72,8 @@ type PanelOptions = Partial<{ analysisLanguage: string; /** Receives the analysis after every store write, so a test can assert on what was persisted. */ onSave: (analysis: TextAnalysis) => void; + /** Receives whether any edit is in progress, so a test can assert on the unsaved indicator. */ + onPendingEditsChange: (pending: boolean) => void; /** Records every reference the panel navigates to, through the host scroll-group hook. */ setScrRef: (ref: SerializedVerseRef) => void; /** Reference the host scroll group reports, i.e. where the view already sits. */ @@ -103,6 +105,7 @@ function PanelProviders({ @@ -1788,6 +1791,81 @@ describe('AnalysisCatalogPanel', () => { const saved: TextAnalysis = onSave.mock.calls.at(-1)[0]; expect(saved.tokenAnalyses[0].morphemes?.[0].gloss).toEqual({ en: 'word-stem' }); }); + + describe('reporting uncommitted text', () => { + /** One analysis with a breakdown, so the breakdown editor opens onto committed forms. */ + const SEGMENTED: TextAnalysis = { + ...SHARED, + tokenAnalyses: [ + { + ...SHARED.tokenAnalyses[0], + morphemes: [{ ...FIXTURE_STAMPS, id: 'm-1', form: 'λογ', writingSystem: 'el' }], + }, + ], + }; + + it('reports a gloss held uncommitted, so the unsaved indicator lights while typing', async () => { + const onPendingEditsChange = jest.fn(); + renderPanel({ analysis: SHARED, onPendingEditsChange }); + + const row = await expandRow('ta-1'); + await userEvent.type(within(row).getByTestId('catalog-row-gloss-input'), '!'); + + expect(onPendingEditsChange).toHaveBeenLastCalledWith(true); + }); + + it('stops reporting a gloss once it commits on blur', async () => { + const onPendingEditsChange = jest.fn(); + renderPanel({ analysis: SHARED, onPendingEditsChange }); + + const row = await expandRow('ta-1'); + await userEvent.type(within(row).getByTestId('catalog-row-gloss-input'), '!'); + await userEvent.tab(); + + expect(onPendingEditsChange).toHaveBeenLastCalledWith(false); + }); + + it('reports a breakdown held uncommitted', async () => { + const onPendingEditsChange = jest.fn(); + renderPanel({ analysis: SEGMENTED, onPendingEditsChange }); + + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + await userEvent.type( + within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input'), + '-ος', + ); + + expect(onPendingEditsChange).toHaveBeenLastCalledWith(true); + }); + + it('stops reporting a breakdown once it is canceled', async () => { + const onPendingEditsChange = jest.fn(); + renderPanel({ analysis: SEGMENTED, onPendingEditsChange }); + + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + await userEvent.type( + within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input'), + '-ος', + ); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-cancel')); + + expect(onPendingEditsChange).toHaveBeenLastCalledWith(false); + }); + + // Opening the editor on an unsegmented word pre-fills the whole word, which commits as the + // unsegmented state it already holds — nothing is at stake until the reader changes it. + it('reports nothing for a breakdown draft that would commit as a no-op', async () => { + const onPendingEditsChange = jest.fn(); + renderPanel({ analysis: SHARED, onPendingEditsChange }); + + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + + expect(onPendingEditsChange).not.toHaveBeenCalled(); + }); + }); }); describe('merging on edit', () => { @@ -1885,6 +1963,27 @@ describe('AnalysisCatalogPanel', () => { expect(screen.queryAllByTestId('catalog-row')).toHaveLength(0); }); + it('leaves no notice when the emptied record’s token keeps an unrelated candidate', async () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + // A token may carry several links at once, only the approved one being unique. Clearing the + // approved record's gloss empties it away, leaving the candidate behind untouched — which + // is not a collapse onto it, however much the surviving link looks like one. + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'λόγος', gloss: { en: 'word' } }, + { ...FIXTURE_STAMPS, id: 'ta-2', surfaceText: 'λόγος', gloss: { en: 'reason' } }, + ], + tokenAnalysisLinks: [link('ta-1', 'GEN 1:1:0'), link('ta-2', 'GEN 1:1:0', 'candidate')], + }; + renderPanel({ analysis }); + + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + await userEvent.clear(within(rowFor('ta-1')).getByTestId('catalog-row-gloss-input')); + await userEvent.tab(); + + expect(screen.queryByTestId('catalog-merge-notice')).not.toBeInTheDocument(); + }); + it('leaves no notice when an edit collapses nothing', async () => { renderPanel({ analysis: TWO_HOMOGRAPHS }); diff --git a/src/components/AnalysisStore.tsx b/src/components/AnalysisStore.tsx index fbd6b5e1..c3348e32 100644 --- a/src/components/AnalysisStore.tsx +++ b/src/components/AnalysisStore.tsx @@ -449,8 +449,10 @@ export type AnalysisEditOutcome = * record silently, so where it went is recoverable only by comparing the two states. * * A record that is simply gone was emptied by its own edit; one whose links moved elsewhere - * collapsed onto the record they moved to. The two are told apart by following the links, since - * only a collapse leaves them pointing somewhere. + * collapsed onto the record they moved to. The two are told apart by a link naming a record its + * token was not already linked to, since only a collapse repoints one — a token may hold several + * links at once, the one-per-token invariant covering only `approved` ones, so the links a removal + * leaves behind are no evidence of a survivor. */ function readEditOutcome( before: TextAnalysis, @@ -460,14 +462,24 @@ function readEditOutcome( ): AnalysisEditOutcome { if (after.tokenAnalyses.some((ta) => ta.id === analysisId)) return { kind: 'edited' }; - // The links as they stood before the write name the tokens the record held; where those same - // tokens point now is where the record went. + // The links as they stood before the write name the tokens the record held, each against the + // records that token was already linked to; a link naming anything else is one the write moved. + const heldAnalysisIdsByToken = new Map>(); + before.tokenAnalysisLinks.forEach((l) => { + const held = heldAnalysisIdsByToken.get(l.token.tokenRef); + if (held) held.add(l.analysisId); + else heldAnalysisIdsByToken.set(l.token.tokenRef, new Set([l.analysisId])); + }); const movedTokenRefs = new Set( before.tokenAnalysisLinks .filter((l) => l.analysisId === analysisId) .map((l) => l.token.tokenRef), ); - const survivor = after.tokenAnalysisLinks.find((l) => movedTokenRefs.has(l.token.tokenRef)); + const survivor = after.tokenAnalysisLinks.find( + (l) => + movedTokenRefs.has(l.token.tokenRef) && + !heldAnalysisIdsByToken.get(l.token.tokenRef)?.has(l.analysisId), + ); if (!survivor) return { kind: 'removed' }; const survivingAnalysis = after.tokenAnalyses.find((ta) => ta.id === survivor.analysisId); diff --git a/src/components/CatalogRowEditor.tsx b/src/components/CatalogRowEditor.tsx index de9ffa4a..ff658965 100644 --- a/src/components/CatalogRowEditor.tsx +++ b/src/components/CatalogRowEditor.tsx @@ -3,6 +3,7 @@ import { Button, Input, Label } from 'platform-bible-react'; import { formatReplacementString, type LanguageStrings } from 'platform-bible-utils'; import { useId, useState } from 'react'; import { resolvedOrEmpty } from '../utils/localized-strings'; +import { useReportGlossEditing } from './AnalysisStore'; /** Localized string keys the row editor renders. */ export const ROW_EDITOR_STRING_KEYS = [ @@ -81,6 +82,9 @@ function CommitOnBlurInput({ setDraft(committedValue); } + // Surface uncommitted typing to the unsaved indicator before the edit commits on blur. + useReportGlossEditing(draft !== committedValue); + const commit = () => { if (draft !== committedValue) onCommit(draft); }; @@ -141,14 +145,27 @@ export default function CatalogRowEditor({ const morphemeForms = morphemes.map((m) => m.form).join(' '); + /** + * The forms a draft reads as. An empty draft has no reading as a breakdown, and a lone form equal + * to the whole word records no segmentation — both are a request for the unsegmented state, which + * is an empty form list. + */ + const draftForms = (value: string): string[] => { + const normalized = normalize(value); + return normalized === '' || normalized === normalize(surfaceText) ? [] : normalized.split(' '); + }; + + // The breakdown commits only on Enter or Save, never on blur, so this is the only thing standing + // between a typed re-segmentation and a project switch. Compared as forms rather than as text, so + // the whole word the editor pre-fills for an unsegmented breakdown is not unsaved work. + useReportGlossEditing( + breakdownDraft !== undefined && draftForms(breakdownDraft).join(' ') !== morphemeForms, + ); + const commitBreakdown = () => { /* v8 ignore next -- only the open editor calls this, and it is open only with a draft held */ if (breakdownDraft === undefined) return; - const normalized = normalize(breakdownDraft); - // An empty draft has no reading as a breakdown, and a lone form equal to the whole word records - // no segmentation — both are a request for the unsegmented state, which is an empty form list. - const forms = - normalized === '' || normalized === normalize(surfaceText) ? [] : normalized.split(' '); + const forms = draftForms(breakdownDraft); if (forms.join(' ') !== morphemeForms) onMorphemesCommit(forms); setBreakdownDraft(undefined); }; From 3d521413d02375b7a85b636321c67d38d37f4f2a Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 1 Sep 2026 10:08:22 -0600 Subject: [PATCH 07/14] Report a collapse from the write that performed it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit readEditOutcome inferred a merge from links the edited record's tokens had moved onto. A record no token links to moves none, so collapsing one left exactly the state its removal would leave and was reported as a removal — the row vanished with no notice, for the unused records a Paratext 9 import lands by the hundred. Record the survivor in the reducer that collapses the record, where the payload deciding merge-from-removal is still in hand, and read it back through transient state that never reaches storage. Compare useAnalysisMergePeers shallowly. Its selector filters per row and so allocates a fresh array on every call, re-rendering every row holding peers on every store change. Cover the analysis-keyed hooks directly rather than through the panel, where their write outcomes were asserted only as the absence of a notice element — three distinct paths sharing one negative DOM fact. --- .../components/AnalysisCatalogPanel.test.tsx | 21 ++ .../components/AnalysisStore.test.tsx | 209 ++++++++++++++++++ src/components/AnalysisStore.tsx | 63 ++---- src/store/analysisSlice.ts | 12 + 4 files changed, 265 insertions(+), 40 deletions(-) diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index cb333103..cff5744a 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -1984,6 +1984,27 @@ describe('AnalysisCatalogPanel', () => { expect(screen.queryByTestId('catalog-merge-notice')).not.toBeInTheDocument(); }); + it('announces where an unused row went', async () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + // ta-1 is unlinked, as an imported wordform inventory arrives. No usage count moves when it + // collapses, so the notice is all the reader gets. + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'ἀρχῇ', gloss: { en: 'start' } }, + { ...FIXTURE_STAMPS, id: 'ta-2', surfaceText: 'ἀρχῇ', gloss: { en: 'beginning' } }, + ], + tokenAnalysisLinks: [link('ta-2', 'GEN 1:3:4'), link('ta-2', 'GEN 2:7:2')], + }; + renderPanel({ analysis }); + + await editIntoEquality(); + + expect(listedAnalysisIds()).toEqual(['ta-2']); + expect(screen.getByTestId('catalog-merge-notice')).toHaveTextContent( + '%interlinearizer_analysisCatalog_merged%', + ); + }); + it('leaves no notice when an edit collapses nothing', async () => { renderPanel({ analysis: TWO_HOMOGRAPHS }); diff --git a/src/__tests__/components/AnalysisStore.test.tsx b/src/__tests__/components/AnalysisStore.test.tsx index a4d3129c..388558d9 100644 --- a/src/__tests__/components/AnalysisStore.test.tsx +++ b/src/__tests__/components/AnalysisStore.test.tsx @@ -5,11 +5,16 @@ import { act, render, renderHook, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { TextAnalysis, TokenAnalysis, TokenAnalysisLink } from 'interlinearizer'; import type { ReactNode } from 'react'; +import { emptyAnalysis } from '../../types/empty-factories'; import { FIXTURE_STAMPS } from '../test-helpers'; +import type { AnalysisEditOutcome } from '../../components/AnalysisStore'; import { AnalysisStoreProvider, useAnalysis, + useAnalysisDeletionOutcome, useAnalysisLanguage, + useAnalysisMergePeers, + useAnalysisRowDispatch, useApproveAnalysisDispatch, useGloss, useGlossDispatch, @@ -1475,3 +1480,207 @@ describe('useApproveAnalysisDispatch', () => { ); }); }); + +function approvedLink(analysisId: string, tokenRef: string): TokenAnalysisLink { + return { + ...FIXTURE_STAMPS, + analysisId, + token: { tokenRef, surfaceText: 'ἀρχῇ' }, + status: 'approved', + }; +} + +/** Two homographs glossed differently, so editing `ta-1` into `ta-2`'s gloss collapses them. */ +function twoHomographs(links: readonly TokenAnalysisLink[]): TextAnalysis { + return { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'ἀρχῇ', gloss: { und: 'start' } }, + { ...FIXTURE_STAMPS, id: 'ta-2', surfaceText: 'ἀρχῇ', gloss: { und: 'beginning' } }, + ], + tokenAnalysisLinks: [...links], + }; +} + +describe('useAnalysisRowDispatch', () => { + it('reports an ordinary edit as leaving the record standing', () => { + const { result } = renderStoreHook(() => useAnalysisRowDispatch(), { + initialAnalysis: twoHomographs([approvedLink('ta-1', 'tok-1')]), + }); + + let outcome: AnalysisEditOutcome | undefined; + act(() => { + outcome = result.current.writeGloss('ta-1', 'origin'); + }); + + expect(outcome).toStrictEqual({ kind: 'edited' }); + }); + + it('reports a collapse onto a sibling, naming the survivor', () => { + const { result } = renderStoreHook(() => useAnalysisRowDispatch(), { + initialAnalysis: twoHomographs([ + approvedLink('ta-1', 'tok-1'), + approvedLink('ta-2', 'tok-2'), + ]), + }); + + let outcome: AnalysisEditOutcome | undefined; + act(() => { + outcome = result.current.writeGloss('ta-1', 'beginning'); + }); + + expect(outcome).toStrictEqual({ + kind: 'merged', + survivingAnalysisId: 'ta-2', + survivingGloss: 'beginning', + survivingUsageCount: 2, + }); + }); + + // The case links cannot report: an unlinked record repoints nothing when it collapses. + it('reports a collapse of a record no token links to', () => { + const { result } = renderStoreHook(() => useAnalysisRowDispatch(), { + initialAnalysis: twoHomographs([approvedLink('ta-2', 'tok-2')]), + }); + + let outcome: AnalysisEditOutcome | undefined; + act(() => { + outcome = result.current.writeGloss('ta-1', 'beginning'); + }); + + expect(outcome).toStrictEqual({ + kind: 'merged', + survivingAnalysisId: 'ta-2', + survivingGloss: 'beginning', + survivingUsageCount: 1, + }); + }); + + it('reports an edit that empties the record as a removal', () => { + const { result } = renderStoreHook(() => useAnalysisRowDispatch(), { + initialAnalysis: twoHomographs([approvedLink('ta-1', 'tok-1')]), + }); + + act(() => { + result.current.writeGloss('ta-1', 'beginning'); + }); + + // ta-2 is the survivor the first write recorded, so this checks a stale one is not reported. + let outcome: AnalysisEditOutcome | undefined; + act(() => { + outcome = result.current.writeGloss('ta-2', ''); + }); + + expect(outcome).toStrictEqual({ kind: 'removed' }); + }); + + it('reports a collapse driven by a morpheme breakdown', () => { + const analysis = twoHomographs([approvedLink('ta-2', 'tok-2')]); + const { result } = renderStoreHook(() => useAnalysisRowDispatch(), { + initialAnalysis: { + ...analysis, + // Same gloss apiece, so the records differ only by breakdown. + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'ἀρχῇ', gloss: { und: 'beginning' } }, + { + ...FIXTURE_STAMPS, + id: 'ta-2', + surfaceText: 'ἀρχῇ', + gloss: { und: 'beginning' }, + morphemes: [{ ...FIXTURE_STAMPS, id: 'm-1', form: 'ἀρχ', writingSystem: 'el' }], + }, + ], + }, + }); + + let outcome: AnalysisEditOutcome | undefined; + act(() => { + outcome = result.current.writeMorphemes('ta-1', ['ἀρχ'], 'el'); + }); + + expect(outcome?.kind).toBe('merged'); + }); + + it('throws when called outside an AnalysisStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => renderHook(() => useAnalysisRowDispatch())).toThrow( + 'useAnalysisRowDispatch must be used inside an AnalysisStoreProvider', + ); + }); +}); + +describe('useAnalysisDeletionOutcome', () => { + it('reports the surviving homograph the affected tokens fall back to', () => { + const { result } = renderStoreHook(() => useAnalysisDeletionOutcome(), { + initialAnalysis: twoHomographs([ + approvedLink('ta-1', 'tok-1'), + approvedLink('ta-2', 'tok-2'), + ]), + }); + + expect(result.current('ta-1')).toStrictEqual({ + kind: 'fallback', + usageCount: 1, + fallbackGloss: 'beginning', + }); + }); + + it('reports a blank outcome when no homograph survives', () => { + const { result } = renderStoreHook(() => useAnalysisDeletionOutcome(), { + initialAnalysis: makeAnalysisWithGloss('tok-1', 'hello'), + }); + + expect(result.current('tok-1-analysis')).toStrictEqual({ kind: 'blank', usageCount: 1 }); + }); + + it('returns undefined for an id that resolves to no record', () => { + const { result } = renderStoreHook(() => useAnalysisDeletionOutcome()); + + expect(result.current('ta-missing')).toBeUndefined(); + }); + + it('throws when called outside an AnalysisStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => renderHook(() => useAnalysisDeletionOutcome())).toThrow( + 'useAnalysisDeletionOutcome must be used inside an AnalysisStoreProvider', + ); + }); +}); + +describe('useAnalysisMergePeers', () => { + it('offers the homographs sharing the row’s surface form', () => { + const { result } = renderStoreHook(() => useAnalysisMergePeers('ta-1'), { + initialAnalysis: twoHomographs([approvedLink('ta-2', 'tok-2')]), + }); + + expect(result.current.map((ta) => ta.id)).toStrictEqual(['ta-2']); + }); + + // The written token shares no surface form with either homograph, so ta-1's peers are unaffected. + it('holds its peers steady across an unrelated write', () => { + const { result } = renderStoreHook( + () => ({ peers: useAnalysisMergePeers('ta-1'), write: useGlossDispatch() }), + { initialAnalysis: twoHomographs([approvedLink('ta-2', 'tok-2')]) }, + ); + const first = result.current.peers; + + act(() => result.current.write('tok-9', 'other', 'unrelated')); + + expect(result.current.peers).toBe(first); + }); + + it('offers nothing to a record with no homograph', () => { + const { result } = renderStoreHook(() => useAnalysisMergePeers('tok-1-analysis'), { + initialAnalysis: makeAnalysisWithGloss('tok-1', 'hello'), + }); + + expect(result.current).toStrictEqual([]); + }); + + it('throws when called outside an AnalysisStoreProvider', () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + expect(() => renderHook(() => useAnalysisMergePeers('ta-1'))).toThrow( + 'useAnalysisMergePeers must be used inside an AnalysisStoreProvider', + ); + }); +}); diff --git a/src/components/AnalysisStore.tsx b/src/components/AnalysisStore.tsx index c3348e32..6f409e80 100644 --- a/src/components/AnalysisStore.tsx +++ b/src/components/AnalysisStore.tsx @@ -7,7 +7,13 @@ import type { } from 'interlinearizer'; import { createContext, useCallback, useContext, useEffect, useMemo, useRef } from 'react'; import type { ReactNode } from 'react'; -import { Provider as ReduxProvider, useDispatch, useSelector, useStore } from 'react-redux'; +import { + Provider as ReduxProvider, + shallowEqual, + useDispatch, + useSelector, + useStore, +} from 'react-redux'; import { createAnalysisStore, type AnalysisDispatch, type AnalysisRootState } from '../store'; import { approveAnalysisForToken, @@ -444,53 +450,28 @@ export type AnalysisEditOutcome = }; /** - * Reports where an edit left the record `analysisId` named, by reading the store either side of the - * write. Nothing the write itself reports says this: a collapse repoints the links and drops the - * record silently, so where it went is recoverable only by comparing the two states. - * - * A record that is simply gone was emptied by its own edit; one whose links moved elsewhere - * collapsed onto the record they moved to. The two are told apart by a link naming a record its - * token was not already linked to, since only a collapse repoints one — a token may hold several - * links at once, the one-per-token invariant covering only `approved` ones, so the links a removal - * leaves behind are no evidence of a survivor. + * Reports where an edit left the record `analysisId` named, so a row that vanishes can be explained + * rather than read as lost work. Only the survivor the write recorded tells a collapse from a + * removal; the state they leave is the same. */ function readEditOutcome( - before: TextAnalysis, after: TextAnalysis, + survivingAnalysisId: string | undefined, analysisId: string, analysisLanguage: string, ): AnalysisEditOutcome { if (after.tokenAnalyses.some((ta) => ta.id === analysisId)) return { kind: 'edited' }; + if (survivingAnalysisId === undefined) return { kind: 'removed' }; - // The links as they stood before the write name the tokens the record held, each against the - // records that token was already linked to; a link naming anything else is one the write moved. - const heldAnalysisIdsByToken = new Map>(); - before.tokenAnalysisLinks.forEach((l) => { - const held = heldAnalysisIdsByToken.get(l.token.tokenRef); - if (held) held.add(l.analysisId); - else heldAnalysisIdsByToken.set(l.token.tokenRef, new Set([l.analysisId])); - }); - const movedTokenRefs = new Set( - before.tokenAnalysisLinks - .filter((l) => l.analysisId === analysisId) - .map((l) => l.token.tokenRef), - ); - const survivor = after.tokenAnalysisLinks.find( - (l) => - movedTokenRefs.has(l.token.tokenRef) && - !heldAnalysisIdsByToken.get(l.token.tokenRef)?.has(l.analysisId), - ); - if (!survivor) return { kind: 'removed' }; - - const survivingAnalysis = after.tokenAnalyses.find((ta) => ta.id === survivor.analysisId); + const survivingAnalysis = after.tokenAnalyses.find((ta) => ta.id === survivingAnalysisId); return { kind: 'merged', - survivingAnalysisId: survivor.analysisId, - /* v8 ignore next -- a link the store holds always resolves to a record it holds */ + survivingAnalysisId, + /* v8 ignore next -- the survivor was just written into the state this reads */ survivingGloss: survivingAnalysis?.gloss?.[analysisLanguage] ?? '', survivingUsageCount: new Set( after.tokenAnalysisLinks - .filter((l) => l.analysisId === survivor.analysisId && l.status === 'approved') + .filter((l) => l.analysisId === survivingAnalysisId && l.status === 'approved') .map((l) => l.token.tokenRef), ).size, }; @@ -548,11 +529,10 @@ export function useAnalysisRowDispatch(): AnalysisRowDispatch { */ const writeAndReport = useCallback( (analysisId: string, action: Parameters[0]): AnalysisEditOutcome => { - const before = store.getState().analysis.analysis; dispatch(action); - const { analysis: after, analysisLanguage } = store.getState().analysis; + const { analysis, analysisLanguage, lastCollapseSurvivorId } = store.getState().analysis; save(); - return readEditOutcome(before, after, analysisId, analysisLanguage); + return readEditOutcome(analysis, lastCollapseSurvivorId, analysisId, analysisLanguage); }, [dispatch, save, store], ); @@ -638,13 +618,16 @@ export function useAnalysisDeletionOutcome(): ( * Subscribed rather than read on demand, because whether a row has peers at all decides whether its * merge control is offered, which has to follow an edit made beside the panel. * + * A row's peers hold steady across an edit that does not alter them. + * * @throws When called outside an {@link AnalysisStoreProvider}. */ export function useAnalysisMergePeers(analysisId: string): readonly TokenAnalysis[] { useRequiredCallbacks('useAnalysisMergePeers'); - return useSelector((state: AnalysisRootState) => - selectAnalysisMergePeers(state.analysis, analysisId), + return useSelector( + (state: AnalysisRootState) => selectAnalysisMergePeers(state.analysis, analysisId), + shallowEqual, ); } diff --git a/src/store/analysisSlice.ts b/src/store/analysisSlice.ts index f8d4a00d..7712ae4c 100644 --- a/src/store/analysisSlice.ts +++ b/src/store/analysisSlice.ts @@ -28,6 +28,12 @@ export type AnalysisState = { analysis: TextAnalysis; /** BCP 47 tag identifying the language used when reading and writing gloss values. */ analysisLanguage: string; + /** + * The record the last write's collapse left standing. Reports a collapse the state cannot show: a + * record holding no links — how an imported wordform inventory arrives — repoints nothing when it + * collapses, leaving what its removal would leave. Never reaches storage. + */ + lastCollapseSurvivorId?: string; }; /** Payload for the {@link writeGloss} action, extended with a pre-generated UUID. */ @@ -341,6 +347,8 @@ function forkSharedAnalysis( * * The surviving payload keeps its own timestamps and the repointed links keep theirs: no write was * aimed at the survivor or at any token's annotation, only at which record holds the content. + * + * Leaves the survivor in {@link AnalysisState.lastCollapseSurvivorId}. */ function mergeIntoIdenticalPayload(state: AnalysisState, analysis: TokenAnalysis): void { const other = state.analysis.tokenAnalyses.find( @@ -351,6 +359,7 @@ function mergeIntoIdenticalPayload(state: AnalysisState, analysis: TokenAnalysis if (l.analysisId === analysis.id) l.analysisId = other.id; }); state.analysis.tokenAnalyses = state.analysis.tokenAnalyses.filter((ta) => ta !== analysis); + state.lastCollapseSurvivorId = other.id; } /** @@ -726,6 +735,7 @@ const analysisSlice = createSlice({ const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === analysisId); if (!analysis) return; + state.lastCollapseSurvivorId = undefined; if (value.trim() === '') { if (analysis.gloss) { @@ -785,6 +795,7 @@ const analysisSlice = createSlice({ const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === analysisId); if (!analysis) return; + state.lastCollapseSurvivorId = undefined; if (morphemes.length === 0) delete analysis.morphemes; else analysis.morphemes = reconcileMorphemes(analysis.morphemes, morphemes, writingSystem); @@ -822,6 +833,7 @@ const analysisSlice = createSlice({ const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === analysisId); const morpheme = analysis?.morphemes?.find((m) => m.id === morphemeId); if (!analysis || !morpheme) return; + state.lastCollapseSurvivorId = undefined; if (value.trim() === '') { if (morpheme.gloss) { From dbba9628de6ac9a4dd791ef9fb2a2821ee1ad365 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 1 Sep 2026 16:47:21 -0600 Subject: [PATCH 08/14] Keep a catalog edit's aftermath in reach of the reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three ways the catalog dropped work or the thread of it. A merge notice names the row an edit collapsed onto and scrolls to it, but the reveal only fires for a row the window has mounted. An unlinked record inherits no usages to carry its survivor up the listing, so the survivor stays where its gloss sorts it — past the first chunk, and the notice pointed at nothing on screen. Let a caller name a row the window must cover, applied over the count so a new listing still resets. The breakdown editor held its draft itself, and the breakdown commits on neither blur nor unmount, so collapsing the row or a query that stopped listing it took a typed re-segmentation with it. Hold the drafts in the panel, which outlives all three routes, and report them from there — the editor unmounts while the draft it was reporting for is still owed. Clearing the breakdown drops every morpheme gloss on the record, for every token it holds, and nothing forks a copy first. Confirm it, as the token editor confirms its own reset. --- contributions/localizedStrings.json | 2 + .../components/AnalysisCatalogPanel.test.tsx | 194 ++++++++++++++++++ src/components/AnalysisCatalogPanel.tsx | 58 +++++- src/components/CatalogRowEditor.tsx | 89 +++++--- src/components/CatalogRowView.tsx | 12 ++ src/hooks/useRowWindow.ts | 16 +- 6 files changed, 338 insertions(+), 33 deletions(-) diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index 53636bc1..3389c869 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -58,6 +58,8 @@ "%interlinearizer_analysisCatalog_editMorphemesSave%": "Save breakdown", "%interlinearizer_analysisCatalog_editMorphemesCancel%": "Cancel", "%interlinearizer_analysisCatalog_editMorphemesOpen%": "Edit breakdown for {form}", + "%interlinearizer_analysisCatalog_confirmResetPrompt%": "Discard this breakdown and its glosses everywhere {form} is used?", + "%interlinearizer_analysisCatalog_confirmResetAction%": "Discard breakdown", "%interlinearizer_analysisCatalog_morphemeGloss%": "Gloss for morpheme {form}", "%interlinearizer_analysisCatalog_appliesToAll%": "Edits here apply to every use of this analysis.", "%interlinearizer_analysisCatalog_merge%": "Merge…", diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index cff5744a..9d6fb037 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -1658,6 +1658,156 @@ describe('AnalysisCatalogPanel', () => { expect(onSave).not.toHaveBeenCalled(); }); + /** A record with a breakdown whose morphemes carry no glosses of their own. */ + const SEGMENTED_NO_GLOSSES: TextAnalysis = { + ...SHARED, + tokenAnalyses: [ + { + ...SHARED.tokenAnalyses[0], + morphemes: [{ ...FIXTURE_STAMPS, id: 'm-1', form: 'λογ', writingSystem: 'el' }], + }, + ], + }; + + /** A record whose morphemes carry glosses, so clearing its breakdown destroys them. */ + const GLOSSED_MORPHEMES: TextAnalysis = { + ...SHARED, + tokenAnalyses: [ + { + ...SHARED.tokenAnalyses[0], + morphemes: [ + { + ...FIXTURE_STAMPS, + id: 'm-1', + form: 'λογ', + writingSystem: 'el', + gloss: { en: 'word' }, + }, + { ...FIXTURE_STAMPS, id: 'm-2', form: 'ος', writingSystem: 'el' }, + ], + }, + ], + }; + + /** Opens the breakdown editor on `ta-1` and empties it, which asks for the unsegmented state. */ + async function clearBreakdown(): Promise { + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + await userEvent.clear(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input')); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-save')); + } + + it('confirms before clearing a breakdown whose morphemes carry glosses', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: GLOSSED_MORPHEMES, onSave }); + + await clearBreakdown(); + + expect(within(rowFor('ta-1')).getByTestId('catalog-row-editor')).toHaveTextContent( + '%interlinearizer_analysisCatalog_confirmResetPrompt%', + ); + expect(onSave).not.toHaveBeenCalled(); + }); + + it('clears the breakdown once the reset is confirmed', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: GLOSSED_MORPHEMES, onSave }); + + await clearBreakdown(); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-save')); + + const saved: TextAnalysis = onSave.mock.calls.at(-1)[0]; + expect(saved.tokenAnalyses[0].morphemes).toBeUndefined(); + }); + + it('returns to the draft when Escape declines the reset', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: GLOSSED_MORPHEMES, onSave }); + + await clearBreakdown(); + await userEvent.type( + within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input'), + '{Escape}', + ); + + expect(within(rowFor('ta-1')).getByTestId('catalog-row-editor')).not.toHaveTextContent( + '%interlinearizer_analysisCatalog_confirmResetPrompt%', + ); + expect(onSave).not.toHaveBeenCalled(); + }); + + it('keeps the breakdown when the reset is declined', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: GLOSSED_MORPHEMES, onSave }); + + await clearBreakdown(); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-cancel')); + + expect(onSave).not.toHaveBeenCalled(); + expect(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input')).toBeInTheDocument(); + }); + + it('clears a breakdown carrying no morpheme glosses without asking', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: SEGMENTED_NO_GLOSSES, onSave }); + + await clearBreakdown(); + + const saved: TextAnalysis = onSave.mock.calls.at(-1)[0]; + expect(saved.tokenAnalyses[0].morphemes).toBeUndefined(); + }); + + it('keeps a breakdown draft across collapsing the row', async () => { + renderPanel({ analysis: SHARED }); + + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + const input = within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input'); + await userEvent.clear(input); + await userEvent.type(input, 'λογ ος'); + + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + + expect(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input')).toHaveValue( + 'λογ ος', + ); + }); + + it('keeps a breakdown draft across a search that stops listing the row', async () => { + renderPanel({ analysis: SHARED }); + + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + const input = within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input'); + await userEvent.clear(input); + await userEvent.type(input, 'λογ ος'); + + await userEvent.type(searchBox(), 'zzz'); + expect(screen.queryAllByTestId('catalog-row')).toHaveLength(0); + await userEvent.clear(searchBox()); + + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + expect(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input')).toHaveValue( + 'λογ ος', + ); + }); + + it('reports a held breakdown draft while its row is unmounted', async () => { + const onPendingEditsChange = jest.fn(); + renderPanel({ analysis: SHARED, onPendingEditsChange }); + + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + await userEvent.type( + within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input'), + '-ος', + ); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-toggle')); + + expect(onPendingEditsChange).toHaveBeenLastCalledWith(true); + }); + it('commits a gloss edit on Enter', async () => { const onSave = jest.fn(); renderPanel({ analysis: SHARED, onSave }); @@ -2025,6 +2175,50 @@ describe('AnalysisCatalogPanel', () => { expect(screen.queryByTestId('catalog-merge-notice')).not.toBeInTheDocument(); }); + + // Neither homograph is linked, so the survivor inherits no usages to carry it up the listing + // and stays wherever its gloss sorts it — here past the end of the window's first chunk. + it('mounts a survivor the window would otherwise leave off', async () => { + const analysis: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'ta-src', surfaceText: 'ἀρχῇ', gloss: { en: 'aaa' } }, + ...Array.from({ length: 100 }, (_unused, index) => ({ + ...FIXTURE_STAMPS, + id: `filler-${index}`, + surfaceText: `word${index}`, + gloss: { en: `g${String(index).padStart(3, '0')}` }, + })), + { ...FIXTURE_STAMPS, id: 'ta-dst', surfaceText: 'ἀρχῇ', gloss: { en: 'zzz' } }, + ], + tokenAnalysisLinks: [], + }; + renderPanel({ analysis }); + + await userEvent.click(screen.getByTestId('catalog-sort-gloss')); + expect(listedAnalysisIds()).not.toContain('ta-dst'); + + await userEvent.click(within(rowFor('ta-src')).getByTestId('catalog-row-toggle')); + const input = within(rowFor('ta-src')).getByTestId('catalog-row-gloss-input'); + await userEvent.clear(input); + await userEvent.type(input, 'zzz'); + await userEvent.tab(); + + expect(screen.getByTestId('catalog-merge-notice')).toBeInTheDocument(); + expect(listedAnalysisIds()).toContain('ta-dst'); + }); + + // The notice outlives the listing it was raised against, so a search narrowing the survivor + // away leaves it naming a row that is nowhere to be mounted. + it('holds the notice when a search excludes the survivor', async () => { + renderPanel({ analysis: TWO_HOMOGRAPHS }); + await editIntoEquality(); + + await userEvent.type(searchBox(), 'zzz'); + + expect(screen.queryAllByTestId('catalog-row')).toHaveLength(0); + expect(screen.getByTestId('catalog-merge-notice')).toBeInTheDocument(); + }); }); describe('merging into another row', () => { diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index 1b6c76d0..4fb075c6 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -10,8 +10,10 @@ import { useAnalysisMergePeers, useAnalysisRowDispatch, useCatalogRows, + useReportGlossEditing, type AnalysisEditOutcome, } from './AnalysisStore'; +import { breakdownDraftForms } from './CatalogRowEditor'; import CatalogDeleteModal, { DELETE_STRING_KEYS } from './CatalogDeleteModal'; import CatalogMergeModal, { MERGE_STRING_KEYS } from './CatalogMergeModal'; import CatalogMergeNotice, { @@ -208,12 +210,30 @@ export default function AnalysisCatalogPanel({ */ const listing = useMemo(() => ({ query, currentBook }), [query, currentBook]); + /** + * What the last edit's collapse left standing, or `undefined` when no edit has collapsed one. + * Kept until dismissed or superseded: the reader may be looking anywhere in the list when an edit + * commits, and a row that vanishes unexplained reads as data loss. + */ + const [mergeNotice, setMergeNotice] = useState(undefined); + + /** + * Where the row a merge notice names sits in the listing, or `undefined` when no notice stands. A + * collapse can leave the survivor anywhere — an unused record inherits no usages to carry it up a + * listing ordered by them — so it is not otherwise guaranteed to be within the mounted window. + */ + const noticedRowIndex = useMemo(() => { + if (!mergeNotice) return undefined; + const index = rows.findIndex((r) => r.analysisId === mergeNotice.survivingAnalysisId); + return index === -1 ? undefined : index; + }, [rows, mergeNotice]); + /** * The slice of the listing that is actually mounted. A draft accumulates analyses without bound * and every row carries its own expander and usage list, so the list grows as it is scrolled * rather than rendering whole. */ - const { windowRows, scrollRef, sentinelRef } = useRowWindow(rows, listing); + const { windowRows, scrollRef, sentinelRef } = useRowWindow(rows, listing, noticedRowIndex); const { navigate, requestFocusToken } = useInterlinearNav(); @@ -256,11 +276,37 @@ export default function AnalysisCatalogPanel({ const [deletingId, setDeletingId] = useState(undefined); /** - * What the last edit's collapse left standing, or `undefined` when no edit has collapsed one. - * Kept until dismissed or superseded: the reader may be looking anywhere in the list when an edit - * commits, and a row that vanishes unexplained reads as data loss. + * The breakdown draft each row is holding, keyed by analysis id, for the rows holding one. Kept + * here rather than in the row because a row unmounts whenever it is collapsed or a query stops + * listing it, and the breakdown commits on neither blur nor unmount — so a draft left in the row + * would go with it, taking a re-segmentation the reader typed but had not saved. */ - const [mergeNotice, setMergeNotice] = useState(undefined); + const [breakdownDrafts, setBreakdownDrafts] = useState>(new Map()); + + const handleBreakdownDraftChange = useCallback( + (analysisId: string, draft: string | undefined) => { + setBreakdownDrafts((drafts) => { + const next = new Map(drafts); + if (draft === undefined) next.delete(analysisId); + else next.set(analysisId, draft); + return next; + }); + }, + [], + ); + + // Compared as forms rather than as text, so the whole word the editor pre-fills for an + // unsegmented breakdown is not unsaved work. + useReportGlossEditing( + catalogRows.some((r) => { + const draft = breakdownDrafts.get(r.analysisId); + if (draft === undefined) return false; + return ( + breakdownDraftForms(draft, r.surfaceText).join(' ') !== + r.morphemes.map((m) => m.form).join(' ') + ); + }), + ); /** * Records what an edit did, so a collapse is reported rather than left to look like a vanished @@ -473,8 +519,10 @@ export default function AnalysisCatalogPanel({ void; /** Opens the delete confirmation. */ onDeleteRequest: () => void; + /** + * The breakdown draft, or `undefined` while the breakdown editor is closed. Held by the caller, + * which outlives this editor's own mounting. + */ + breakdownDraft: string | undefined; + /** Records the breakdown draft, `undefined` closing the breakdown editor. */ + onBreakdownDraftChange: (draft: string | undefined) => void; /** Resolved localizations covering at least {@link ROW_EDITOR_STRING_KEYS}. */ localizedStrings: LanguageStrings; }>; @@ -48,6 +57,16 @@ function normalize(value: string): string { return value.trim().replace(/\s+/g, ' '); } +/** + * The forms a breakdown draft reads as, against the surface form it segments. An empty draft has no + * reading as a breakdown, and a lone form equal to the whole word records no segmentation — both + * are a request for the unsegmented state, which is an empty form list. + */ +export function breakdownDraftForms(draft: string, surfaceText: string): string[] { + const normalized = normalize(draft); + return normalized === '' || normalized === normalize(surfaceText) ? [] : normalized.split(' '); +} + /** * A text field whose edit commits on blur and on Enter, and reverts on Escape, holding its draft * locally until then. @@ -135,39 +154,40 @@ export default function CatalogRowEditor({ onMorphemeGlossCommit, onMergeRequest, onDeleteRequest, + breakdownDraft, + onBreakdownDraftChange, localizedStrings, }: CatalogRowEditorProps) { const glossFieldId = useId(); const breakdownFieldId = useId(); - /** The breakdown draft while the editor is open, or `undefined` when it is closed. */ - const [breakdownDraft, setBreakdownDraft] = useState(undefined); + /** Whether the reader is being asked to confirm a breakdown they have asked to clear. */ + const [confirmingReset, setConfirmingReset] = useState(false); const morphemeForms = morphemes.map((m) => m.form).join(' '); - /** - * The forms a draft reads as. An empty draft has no reading as a breakdown, and a lone form equal - * to the whole word records no segmentation — both are a request for the unsegmented state, which - * is an empty form list. - */ - const draftForms = (value: string): string[] => { - const normalized = normalize(value); - return normalized === '' || normalized === normalize(surfaceText) ? [] : normalized.split(' '); - }; + const draftForms = (value: string): string[] => breakdownDraftForms(value, surfaceText); - // The breakdown commits only on Enter or Save, never on blur, so this is the only thing standing - // between a typed re-segmentation and a project switch. Compared as forms rather than as text, so - // the whole word the editor pre-fills for an unsegmented breakdown is not unsaved work. - useReportGlossEditing( - breakdownDraft !== undefined && draftForms(breakdownDraft).join(' ') !== morphemeForms, - ); + const isLosingReset = + breakdownDraft !== undefined && + draftForms(breakdownDraft).length === 0 && + morphemes.some((m) => m.gloss !== undefined); - const commitBreakdown = () => { + const writeBreakdown = () => { /* v8 ignore next -- only the open editor calls this, and it is open only with a draft held */ if (breakdownDraft === undefined) return; const forms = draftForms(breakdownDraft); if (forms.join(' ') !== morphemeForms) onMorphemesCommit(forms); - setBreakdownDraft(undefined); + onBreakdownDraftChange(undefined); + setConfirmingReset(false); + }; + + // A re-split carries its glosses across to the forms it keeps, but clearing the breakdown drops + // every one of them, for every token the record holds. The record is never forked here, so there + // is no other copy to fall back on. + const commitBreakdown = () => { + if (isLosingReset) setConfirmingReset(true); + else writeBreakdown(); }; return ( @@ -203,7 +223,7 @@ export default function CatalogRowEditor({ )} className="tw:h-auto tw:px-1 tw:py-0 tw:text-xs" data-testid="catalog-row-breakdown-open" - onClick={() => setBreakdownDraft(morphemeForms || surfaceText)} + onClick={() => onBreakdownDraftChange(morphemeForms || surfaceText)} size="sm" type="button" variant="link" @@ -220,26 +240,40 @@ export default function CatalogRowEditor({ className="tw:h-7 tw:font-mono tw:text-sm" data-testid="catalog-row-breakdown-input" id={breakdownFieldId} - onChange={(e) => setBreakdownDraft(e.target.value)} + onChange={(e) => { + // Typing on past a prompt is an answer to it: the draft it was asked about is no + // longer the draft in hand. + setConfirmingReset(false); + onBreakdownDraftChange(e.target.value); + }} onKeyDown={(e) => { if (e.key === 'Enter') { e.preventDefault(); commitBreakdown(); } else if (e.key === 'Escape') { e.preventDefault(); - setBreakdownDraft(undefined); + if (confirmingReset) setConfirmingReset(false); + else onBreakdownDraftChange(undefined); } }} type="text" value={breakdownDraft} />

    - {localizedStrings['%interlinearizer_analysisCatalog_editMorphemesHint%']} + {confirmingReset + ? formatReplacementString( + localizedStrings['%interlinearizer_analysisCatalog_confirmResetPrompt%'], + { form: surfaceText }, + ) + : localizedStrings['%interlinearizer_analysisCatalog_editMorphemesHint%']}

    diff --git a/src/components/CatalogRowView.tsx b/src/components/CatalogRowView.tsx index 087d2ce8..3cd269ec 100644 --- a/src/components/CatalogRowView.tsx +++ b/src/components/CatalogRowView.tsx @@ -63,6 +63,10 @@ type CatalogRowViewProps = Readonly<{ * the reader is taken to where their edit went rather than left where it vanished from. */ shouldRevealSelf?: boolean; + /** This row's breakdown draft, or `undefined` while its breakdown editor is closed. */ + breakdownDraft: string | undefined; + /** Records this row's breakdown draft, `undefined` closing its breakdown editor. */ + onBreakdownDraftChange: (analysisId: string, draft: string | undefined) => void; }>; /** Renders a usage's location the way scripture references are written, e.g. `GEN 1:1`. */ @@ -94,6 +98,8 @@ function CatalogRowView({ onMergeRequest, onDeleteRequest, shouldRevealSelf = false, + breakdownDraft, + onBreakdownDraftChange, }: CatalogRowViewProps) { const [isExpanded, setIsExpanded] = useState(false); @@ -130,6 +136,10 @@ function CatalogRowView({ () => onDeleteRequest(analysisId), [analysisId, onDeleteRequest], ); + const handleBreakdownDraftChange = useCallback( + (draft: string | undefined) => onBreakdownDraftChange(analysisId, draft), + [analysisId, onBreakdownDraftChange], + ); const visibleUsages = showsAllUsages ? row.usages : row.usages.slice(0, INLINE_USAGE_LIMIT); const hiddenUsageCount = row.usages.length - visibleUsages.length; @@ -249,9 +259,11 @@ function CatalogRowView({ { * compared by reference: a reader who has scrolled deep into one listing and then narrows it is * looking at a new list, not further down the old one. A caller passes everything that decides * which listing it is showing, and one whose listing never changes passes nothing. Keyed on that - * rather than on `rows`, which turns over on any edit to the underlying analysis as well. + * rather than on `rows`, which turns over on any edit to the underlying analysis as well — a gloss + * approved in the view beside an open catalog would otherwise collapse a deeply scrolled list back + * to its first chunk. + * + * A caller with a row it must be able to point the reader at passes that row's index as + * `mustMount`, which the window covers however far down the listing it falls. */ export default function useRowWindow( rows: readonly T[], listing?: unknown, + mustMount?: number, ): UseRowWindowResult { const [count, setCount] = useState(INITIAL_ROW_COUNT); @@ -91,7 +97,13 @@ export default function useRowWindow( return () => observer.disconnect(); }, [scrollEl, sentinelEl, count, rows.length]); - const windowRows = useMemo(() => rows.slice(0, count), [rows, count]); + // Applied over the count rather than into it, so the window still shrinks back to its first chunk + // when the listing changes: a row held mounted for one notice must not raise the floor for the + // listing after it. + const windowRows = useMemo( + () => rows.slice(0, mustMount === undefined ? count : Math.max(count, mustMount + 1)), + [rows, count, mustMount], + ); return { windowRows, scrollRef, sentinelRef }; } From 0e62c327bf0634584e8b453f880ba67558c6e6bb Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 1 Sep 2026 13:23:44 -0600 Subject: [PATCH 09/14] Ask before closing the catalog over an unsaved breakdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The breakdown editor commits on neither blur nor unmount, so a re-segmentation survives a row collapsing or a query that stops listing it. Closing the panel ended that silently: the draft went with the unmounted panel, and the unsaved marker cleared with it. Gate the close control on the predicate the unsaved marker already reported, and offer discard alone — saving would commit an edit that drops the old morphemes' glosses for every token the record holds. The question is mounted on the draft still standing as well as on the ask, so saving or canceling it from the row beneath withdraws the question instead of asking about work that is no longer unsaved. --- contributions/localizedStrings.json | 4 ++ .../components/AnalysisCatalogPanel.test.tsx | 72 +++++++++++++++++++ .../components/CatalogCloseModal.test.tsx | 60 ++++++++++++++++ src/components/AnalysisCatalogPanel.tsx | 61 ++++++++++++---- src/components/CatalogCloseModal.tsx | 59 +++++++++++++++ 5 files changed, 244 insertions(+), 12 deletions(-) create mode 100644 src/__tests__/components/CatalogCloseModal.test.tsx create mode 100644 src/components/CatalogCloseModal.tsx diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index 3389c869..181af642 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -83,6 +83,10 @@ "%interlinearizer_analysisCatalog_deleteUndoWarning%": "This cannot be undone.", "%interlinearizer_analysisCatalog_deleteCancel%": "Cancel", "%interlinearizer_analysisCatalog_deleteConfirm%": "Delete", + "%interlinearizer_analysisCatalog_closeConfirmTitle%": "Discard the unsaved breakdown?", + "%interlinearizer_analysisCatalog_closeConfirmPrompt%": "You have typed a morpheme breakdown but not saved it. Closing the catalog discards it.", + "%interlinearizer_analysisCatalog_closeConfirmCancel%": "Keep editing", + "%interlinearizer_analysisCatalog_closeConfirmDiscard%": "Discard and close", "%interlinearizer_projectSettings_title%": "Interlinearizer", "%interlinearizer_projectSettings_continuousScroll%": "Continuous Scroll", diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 9d6fb037..b68cee73 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -1808,6 +1808,78 @@ describe('AnalysisCatalogPanel', () => { expect(onPendingEditsChange).toHaveBeenLastCalledWith(true); }); + describe('closing over an unsaved breakdown', () => { + /** Opens the breakdown editor on `ta-1` and types a re-segmentation without saving it. */ + async function typeUnsavedBreakdown() { + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + const input = within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input'); + await userEvent.clear(input); + await userEvent.type(input, 'λογ ος'); + } + + it('asks before closing over a breakdown draft', async () => { + const onClose = jest.fn(); + renderPanel({ analysis: SHARED, onClose }); + await typeUnsavedBreakdown(); + + await userEvent.click(screen.getByTestId('analysis-catalog-close')); + + expect(screen.getByTestId('catalog-close-title')).toBeInTheDocument(); + expect(onClose).not.toHaveBeenCalled(); + }); + + it('keeps the draft in hand when the close is declined', async () => { + const onClose = jest.fn(); + renderPanel({ analysis: SHARED, onClose }); + await typeUnsavedBreakdown(); + await userEvent.click(screen.getByTestId('analysis-catalog-close')); + + await userEvent.click(screen.getByTestId('catalog-close-cancel')); + + expect(onClose).not.toHaveBeenCalled(); + expect(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input')).toHaveValue( + 'λογ ος', + ); + }); + + it('closes without asking once the breakdown is saved', async () => { + const onClose = jest.fn(); + renderPanel({ analysis: SHARED, onClose }); + await typeUnsavedBreakdown(); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-save')); + + await userEvent.click(screen.getByTestId('analysis-catalog-close')); + + expect(onClose).toHaveBeenCalled(); + expect(screen.queryByTestId('catalog-close-title')).not.toBeInTheDocument(); + }); + + it('closes without asking when the draft only re-states the current breakdown', async () => { + const onClose = jest.fn(); + renderPanel({ analysis: SHARED, onClose }); + // The editor pre-fills the whole word, which is not typed work. + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + + await userEvent.click(screen.getByTestId('analysis-catalog-close')); + + expect(onClose).toHaveBeenCalled(); + }); + + it('withdraws the question when the draft is canceled beneath it', async () => { + const onClose = jest.fn(); + renderPanel({ analysis: SHARED, onClose }); + await typeUnsavedBreakdown(); + await userEvent.click(screen.getByTestId('analysis-catalog-close')); + + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-cancel')); + + expect(screen.queryByTestId('catalog-close-title')).not.toBeInTheDocument(); + expect(onClose).not.toHaveBeenCalled(); + }); + }); + it('commits a gloss edit on Enter', async () => { const onSave = jest.fn(); renderPanel({ analysis: SHARED, onSave }); diff --git a/src/__tests__/components/CatalogCloseModal.test.tsx b/src/__tests__/components/CatalogCloseModal.test.tsx new file mode 100644 index 00000000..2ebd0be0 --- /dev/null +++ b/src/__tests__/components/CatalogCloseModal.test.tsx @@ -0,0 +1,60 @@ +/// +/// + +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import CatalogCloseModal, { CLOSE_STRING_KEYS } from '../../components/CatalogCloseModal'; + +/** Each key resolving to itself: the text arrives as a prop, so only key placement is assertable. */ +const STRINGS = Object.fromEntries(CLOSE_STRING_KEYS.map((k) => [k, k])); + +/** The modal with both callbacks stubbed, so a test asserts on which one the click reached. */ +function renderModal(overrides: { onConfirm?: jest.Mock; onCancel?: jest.Mock } = {}) { + const onConfirm = overrides.onConfirm ?? jest.fn(); + const onCancel = overrides.onCancel ?? jest.fn(); + render( + , + ); + return { onConfirm, onCancel }; +} + +describe('CatalogCloseModal', () => { + it('names what closing would discard', () => { + renderModal(); + + expect(screen.getByTestId('catalog-close-title')).toHaveTextContent( + '%interlinearizer_analysisCatalog_closeConfirmTitle%', + ); + expect(screen.getByTestId('catalog-close-prompt')).toHaveTextContent( + '%interlinearizer_analysisCatalog_closeConfirmPrompt%', + ); + }); + + it('closes the panel when the discard is confirmed', async () => { + const { onConfirm, onCancel } = renderModal(); + + await userEvent.click(screen.getByTestId('catalog-close-discard')); + + expect(onConfirm).toHaveBeenCalled(); + expect(onCancel).not.toHaveBeenCalled(); + }); + + it('backs out to the panel when the close is declined', async () => { + const { onConfirm, onCancel } = renderModal(); + + await userEvent.click(screen.getByTestId('catalog-close-cancel')); + + expect(onCancel).toHaveBeenCalled(); + expect(onConfirm).not.toHaveBeenCalled(); + }); + + // Escape resolves to cancel, not discard, so the reflex that dismisses a dialog keeps the draft. + it('keeps the draft when dismissed by Escape', async () => { + const { onConfirm, onCancel } = renderModal(); + + await userEvent.keyboard('{Escape}'); + + expect(onCancel).toHaveBeenCalled(); + expect(onConfirm).not.toHaveBeenCalled(); + }); +}); diff --git a/src/components/AnalysisCatalogPanel.tsx b/src/components/AnalysisCatalogPanel.tsx index 4fb075c6..ea82fcfd 100644 --- a/src/components/AnalysisCatalogPanel.tsx +++ b/src/components/AnalysisCatalogPanel.tsx @@ -14,6 +14,7 @@ import { type AnalysisEditOutcome, } from './AnalysisStore'; import { breakdownDraftForms } from './CatalogRowEditor'; +import CatalogCloseModal, { CLOSE_STRING_KEYS } from './CatalogCloseModal'; import CatalogDeleteModal, { DELETE_STRING_KEYS } from './CatalogDeleteModal'; import CatalogMergeModal, { MERGE_STRING_KEYS } from './CatalogMergeModal'; import CatalogMergeNotice, { @@ -55,6 +56,7 @@ const STRING_KEYS = [ ...MERGE_NOTICE_STRING_KEYS, ...MERGE_STRING_KEYS, ...DELETE_STRING_KEYS, + ...CLOSE_STRING_KEYS, ] as const satisfies `%${string}%`[]; /** Props for {@link AnalysisCatalogPanel}. */ @@ -295,19 +297,41 @@ export default function AnalysisCatalogPanel({ [], ); - // Compared as forms rather than as text, so the whole word the editor pre-fills for an - // unsegmented breakdown is not unsaved work. - useReportGlossEditing( - catalogRows.some((r) => { - const draft = breakdownDrafts.get(r.analysisId); - if (draft === undefined) return false; - return ( - breakdownDraftForms(draft, r.surfaceText).join(' ') !== - r.morphemes.map((m) => m.form).join(' ') - ); - }), + /** + * Whether any row is holding a breakdown the reader has changed but not saved. + * + * Compared as forms rather than as text, so the whole word the editor pre-fills for an + * unsegmented breakdown is not unsaved work. + */ + const hasUnsavedBreakdown = useMemo( + () => + catalogRows.some((r) => { + const draft = breakdownDrafts.get(r.analysisId); + if (draft === undefined) return false; + return ( + breakdownDraftForms(draft, r.surfaceText).join(' ') !== + r.morphemes.map((m) => m.form).join(' ') + ); + }), + [catalogRows, breakdownDrafts], ); + useReportGlossEditing(hasUnsavedBreakdown); + + /** Whether the reader is being asked to confirm closing over a breakdown they have not saved. */ + const [confirmingClose, setConfirmingClose] = useState(false); + + /** + * Closes the panel, or asks first when a breakdown draft would go with it. + * + * A breakdown commits on neither blur nor unmount, so closing is the one route that can drop + * typed text the reader never asked to discard. + */ + const handleCloseRequest = useCallback(() => { + if (hasUnsavedBreakdown) setConfirmingClose(true); + else onClose(); + }, [hasUnsavedBreakdown, onClose]); + /** * Records what an edit did, so a collapse is reported rather than left to look like a vanished * row. An ordinary edit clears whatever the last one said, the notice naming the edit just made @@ -459,7 +483,7 @@ export default function AnalysisCatalogPanel({ + + + + ); +} From 34b65d8325b1f2a9041946d3580bb9115e9845b0 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 1 Sep 2026 15:31:27 -0600 Subject: [PATCH 10/14] Ask before a re-split strands a glossed morpheme Both breakdown editors now name the forms whose glosses a re-split would drop and confirm first, matching the prompt each already showed for clearing a breakdown outright. The token editor skips the prompt for a shared payload, which the write forks rather than re-segmenting in place. --- contributions/localizedStrings.json | 4 + .../components/AnalysisCatalogPanel.test.tsx | 57 +++++++++ .../components/MorphemeEditor.test.tsx | 113 ++++++++++++++++++ src/__tests__/components/TokenChip.test.tsx | 44 ++++++- src/__tests__/store/analysisSlice.test.ts | 70 +++++++++++ src/components/AnalysisStore.tsx | 16 +++ src/components/CatalogRowEditor.tsx | 76 ++++++++---- src/components/MorphemeEditor.tsx | 90 ++++++++++---- src/components/TokenChip.tsx | 6 + src/components/__mocks__/AnalysisStore.tsx | 8 ++ src/store/analysisSlice.ts | 46 +++++++ 11 files changed, 485 insertions(+), 45 deletions(-) diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index 181af642..b97a3006 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -60,6 +60,8 @@ "%interlinearizer_analysisCatalog_editMorphemesOpen%": "Edit breakdown for {form}", "%interlinearizer_analysisCatalog_confirmResetPrompt%": "Discard this breakdown and its glosses everywhere {form} is used?", "%interlinearizer_analysisCatalog_confirmResetAction%": "Discard breakdown", + "%interlinearizer_analysisCatalog_confirmResplitPrompt%": "This breakdown drops {forms}, discarding the glosses on it everywhere this analysis is used. Save anyway?", + "%interlinearizer_analysisCatalog_confirmResplitAction%": "Save and discard", "%interlinearizer_analysisCatalog_morphemeGloss%": "Gloss for morpheme {form}", "%interlinearizer_analysisCatalog_appliesToAll%": "Edits here apply to every use of this analysis.", "%interlinearizer_analysisCatalog_merge%": "Merge…", @@ -121,6 +123,8 @@ "%interlinearizer_morphemeEditor_emptyHint%": "Enter morpheme forms separated by spaces", "%interlinearizer_morphemeEditor_confirmResetPrompt%": "Discard this breakdown and its glosses?", "%interlinearizer_morphemeEditor_confirmResetAction%": "Reset", + "%interlinearizer_morphemeEditor_confirmResplitPrompt%": "This breakdown drops {forms}, discarding the glosses on it. Save anyway?", + "%interlinearizer_morphemeEditor_confirmResplitAction%": "Save and discard", "%interlinearizer_morphemeGloss_label%": "Gloss for morpheme {form}", "%interlinearizer_tokenChip_editMorphemes%": "Edit morpheme breakdown for {token}", "%interlinearizer_tokenChip_defineMorphemes%": "Define morpheme breakdown for {token}", diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index b68cee73..48e1932f 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -1757,6 +1757,63 @@ describe('AnalysisCatalogPanel', () => { expect(saved.tokenAnalyses[0].morphemes).toBeUndefined(); }); + /** Opens the breakdown editor on `ta-1` and re-splits it to `forms`, then saves. */ + async function resplitBreakdown(forms: string): Promise { + const row = await expandRow('ta-1'); + await userEvent.click(within(row).getByTestId('catalog-row-breakdown-open')); + const input = within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input'); + await userEvent.clear(input); + await userEvent.type(input, forms); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-save')); + } + + it('confirms before a re-split that strands a glossed morpheme', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: GLOSSED_MORPHEMES, onSave }); + + await resplitBreakdown('λογος'); + + expect(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-confirm')).toHaveTextContent( + '%interlinearizer_analysisCatalog_confirmResplitPrompt%', + ); + expect(onSave).not.toHaveBeenCalled(); + }); + + it('re-splits once the loss is confirmed', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: GLOSSED_MORPHEMES, onSave }); + + await resplitBreakdown('λογος'); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-save')); + + const saved: TextAnalysis = onSave.mock.calls.at(-1)[0]; + expect(saved.tokenAnalyses[0].morphemes?.map((m) => m.form)).toEqual(['λογος']); + }); + + it('keeps the breakdown when the re-split is declined', async () => { + const onSave = jest.fn(); + renderPanel({ analysis: GLOSSED_MORPHEMES, onSave }); + + await resplitBreakdown('λογος'); + await userEvent.click(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-cancel')); + + expect(onSave).not.toHaveBeenCalled(); + expect(within(rowFor('ta-1')).getByTestId('catalog-row-breakdown-input')).toHaveValue( + 'λογος', + ); + }); + + it('re-splits without asking when every glossed morpheme survives', async () => { + // Only the unglossed "ος" is dropped, and bare segmentation is cheap to retype. + const onSave = jest.fn(); + renderPanel({ analysis: GLOSSED_MORPHEMES, onSave }); + + await resplitBreakdown('λογ'); + + const saved: TextAnalysis = onSave.mock.calls.at(-1)[0]; + expect(saved.tokenAnalyses[0].morphemes?.map((m) => m.form)).toEqual(['λογ']); + }); + it('keeps a breakdown draft across collapsing the row', async () => { renderPanel({ analysis: SHARED }); diff --git a/src/__tests__/components/MorphemeEditor.test.tsx b/src/__tests__/components/MorphemeEditor.test.tsx index 177eff18..95aa282d 100644 --- a/src/__tests__/components/MorphemeEditor.test.tsx +++ b/src/__tests__/components/MorphemeEditor.test.tsx @@ -4,6 +4,7 @@ import { useLocalizedStrings } from '@papi/frontend/react'; import { fireEvent, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import type { MorphemeAnalysis } from 'interlinearizer'; import type { ComponentProps } from 'react'; import { MorphemeBreakdownPopover } from '../../components/MorphemeEditor'; @@ -17,9 +18,17 @@ const LOCALIZED = { '%interlinearizer_morphemeEditor_emptyHint%': 'Enter morpheme forms separated by spaces', '%interlinearizer_morphemeEditor_confirmResetPrompt%': 'Discard this breakdown and its glosses?', '%interlinearizer_morphemeEditor_confirmResetAction%': 'Reset', + '%interlinearizer_morphemeEditor_confirmResplitPrompt%': + 'This breakdown drops {forms}, discarding the glosses on it. Save anyway?', + '%interlinearizer_morphemeEditor_confirmResplitAction%': 'Save and discard', '%interlinearizer_morphemeGloss_label%': 'Gloss for morpheme {form}', }; +/** A morpheme carrying a gloss, so dropping it is the loss a re-split confirms over. */ +function glossed(id: string, form: string): MorphemeAnalysis { + return { id, form, writingSystem: 'und', gloss: { und: form } }; +} + beforeEach(() => { jest.mocked(useLocalizedStrings).mockReturnValue([LOCALIZED, false]); }); @@ -450,6 +459,110 @@ describe('MorphemeBreakdownPopover', () => { }); }); + describe('re-split confirmation', () => { + /** + * Renders the popover over a glossed breakdown of "unbelievable" that this token solely owns, + * so a re-split dropping any of its forms destroys that form's gloss outright. + */ + function renderResplitting( + props: Partial> = {}, + ) { + return renderPopover({ + initialValue: 'un- believ -able', + morphemes: [glossed('m-1', 'un-'), glossed('m-2', 'believ'), glossed('m-3', '-able')], + onReset: jest.fn(), + surfaceText: 'unbelievable', + ...props, + }); + } + + /** Replaces the draft with `value` and commits it. */ + async function commit(value: string) { + await userEvent.clear(screen.getByRole('textbox')); + await userEvent.type(screen.getByRole('textbox'), value); + await userEvent.keyboard('{Enter}'); + } + + it('asks before a re-split that strands a glossed form', async () => { + const onSave = jest.fn(); + const onClose = jest.fn(); + renderResplitting({ onSave, onClose }); + await commit('un- believe'); + expect(screen.getByTestId('morpheme-split-confirm')).toBeInTheDocument(); + expect(onSave).not.toHaveBeenCalled(); + expect(onClose).not.toHaveBeenCalled(); + }); + + it('names the stranded forms in the prompt', async () => { + renderResplitting(); + await commit('un- believe'); + expect(screen.getByTestId('morpheme-split-confirm')).toHaveTextContent( + 'This breakdown drops believ, -able, discarding the glosses on it. Save anyway?', + ); + }); + + it('saves and closes when the confirmation is accepted', async () => { + const onSave = jest.fn(); + const onClose = jest.fn(); + renderResplitting({ onSave, onClose }); + await commit('un- believe'); + await userEvent.click(screen.getByTestId('morpheme-split-confirm-action')); + expect(onSave).toHaveBeenCalledWith('un- believe'); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('returns to the draft when the confirmation is canceled', async () => { + const onSave = jest.fn(); + renderResplitting({ onSave }); + await commit('un- believe'); + await userEvent.click(screen.getByRole('button', { name: 'Cancel' })); + expect(screen.queryByTestId('morpheme-split-confirm')).not.toBeInTheDocument(); + expect(screen.getByRole('textbox')).toHaveValue('un- believe'); + expect(onSave).not.toHaveBeenCalled(); + }); + + it('leaves a pending re-split unwritten when the user presses outside the panel', async () => { + // The same reasoning as the reset confirmation: the loss is irreversible, so a stray click + // must not answer the prompt, even though an outside press on an edited draft normally saves. + const onSave = jest.fn(); + const onClose = jest.fn(); + renderResplitting({ onSave, onClose }); + await commit('un- believe'); + await userEvent.click(screen.getByTestId('popover-outside')); + expect(onSave).not.toHaveBeenCalled(); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('saves without asking when the re-split keeps every glossed form', async () => { + const onSave = jest.fn(); + renderResplitting({ onSave }); + await commit('un- believ -able -ness'); + expect(screen.queryByTestId('morpheme-split-confirm')).not.toBeInTheDocument(); + expect(onSave).toHaveBeenCalledWith('un- believ -able -ness'); + }); + + it('saves without asking when the stranded form carried no gloss', async () => { + const onSave = jest.fn(); + renderResplitting({ + morphemes: [glossed('m-1', 'un-'), { id: 'm-2', form: 'believ', writingSystem: 'und' }], + onSave, + }); + await commit('un- believe'); + expect(screen.queryByTestId('morpheme-split-confirm')).not.toBeInTheDocument(); + expect(onSave).toHaveBeenCalledWith('un- believe'); + }); + + it('saves without asking when the payload is shared, its morphemes withheld', async () => { + // A shared payload is forked rather than re-segmented in place, so the co-linked tokens keep + // the glosses this token drops and there is nothing to confirm. + const onSave = jest.fn(); + renderResplitting({ morphemes: undefined, onSave }); + await commit('un- believe'); + expect(screen.queryByTestId('morpheme-split-confirm')).not.toBeInTheDocument(); + expect(onSave).toHaveBeenCalledWith('un- believe'); + }); + }); + it('falls back to the token gloss input on close when the chip has no morpheme gloss field', async () => { render(