Skip to content

Commit c97dcdf

Browse files
Fire onGlossChange only when TokenChip input is blurred, adjust onGlossChange to keep stored analyses minimal for now
1 parent 9de91fd commit c97dcdf

4 files changed

Lines changed: 99 additions & 34 deletions

File tree

src/__tests__/components/AnalysisStore.test.tsx

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ describe('useAnalysis', () => {
270270
});
271271

272272
describe('useGlossDispatch', () => {
273-
it('creates a new approved TokenAnalysis on each write', async () => {
273+
it('replaces the existing approved analysis on subsequent writes for the same token', async () => {
274274
render(
275275
<AnalysisStoreProvider analysisLanguage="und">
276276
<AnalysisReader />
@@ -280,9 +280,24 @@ describe('useGlossDispatch', () => {
280280
await userEvent.click(screen.getByRole('button', { name: 'write' }));
281281
await userEvent.click(screen.getByRole('button', { name: 'write' }));
282282
const analysis: TextAnalysis = JSON.parse(screen.getByTestId('analysis').textContent ?? '');
283+
expect(analysis.tokenAnalyses).toHaveLength(1);
284+
expect(analysis.tokenAnalysisLinks).toHaveLength(1);
285+
expect(analysis.tokenAnalysisLinks[0].status).toBe('approved');
286+
});
287+
288+
it('creates a new approved analysis when writing to a different token', async () => {
289+
render(
290+
<AnalysisStoreProvider analysisLanguage="und">
291+
<AnalysisReader />
292+
<GlossWriter tokenRef="tok-1" surfaceText="word" value="hi" />
293+
<GlossWriter tokenRef="tok-2" surfaceText="other" value="bye" />
294+
</AnalysisStoreProvider>,
295+
);
296+
await userEvent.click(screen.getAllByRole('button', { name: 'write' })[0]);
297+
await userEvent.click(screen.getAllByRole('button', { name: 'write' })[1]);
298+
const analysis: TextAnalysis = JSON.parse(screen.getByTestId('analysis').textContent ?? '');
283299
expect(analysis.tokenAnalyses).toHaveLength(2);
284300
expect(analysis.tokenAnalysisLinks).toHaveLength(2);
285-
analysis.tokenAnalysisLinks.forEach((link) => expect(link.status).toBe('approved'));
286301
});
287302

288303
it('does not touch existing suggested analyses on write', async () => {

src/__tests__/components/TokenChip.test.tsx

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,17 +179,31 @@ describe('TokenChip', () => {
179179
expect(screen.getByRole('textbox', { name: 'Gloss for hello' })).toHaveValue('');
180180
});
181181

182-
it('calls the store onGlossChange spy with tokenId and value for each keystroke', async () => {
182+
it('calls the store onGlossChange spy once on blur with the final value', async () => {
183183
const spy = jest.fn();
184184
render(
185185
<AnalysisStoreProvider analysisLanguage="und" onGlossChange={spy}>
186186
<TokenChip {...requiredProps()} />
187187
</AnalysisStoreProvider>,
188188
);
189189
await userEvent.type(screen.getByRole('textbox', { name: 'Gloss for hello' }), 'in');
190-
expect(spy).toHaveBeenCalledTimes(2);
191-
expect(spy).toHaveBeenNthCalledWith(1, 'GEN 1:1:0', 'i');
192-
expect(spy).toHaveBeenNthCalledWith(2, 'GEN 1:1:0', 'in');
190+
expect(spy).not.toHaveBeenCalled();
191+
await userEvent.tab();
192+
expect(spy).toHaveBeenCalledTimes(1);
193+
expect(spy).toHaveBeenCalledWith('GEN 1:1:0', 'in');
194+
});
195+
196+
it('does not call the store onGlossChange spy when blurring without typing', async () => {
197+
const spy = jest.fn();
198+
render(
199+
<AnalysisStoreProvider analysisLanguage="und" onGlossChange={spy}>
200+
<TokenChip {...requiredProps()} />
201+
</AnalysisStoreProvider>,
202+
);
203+
await userEvent.click(screen.getByRole('textbox', { name: 'Gloss for hello' }));
204+
await userEvent.tab();
205+
expect(spy).toHaveBeenCalledTimes(1);
206+
expect(spy).toHaveBeenCalledWith('GEN 1:1:0', '');
193207
});
194208

195209
it('calls onFocus when the input is focused', async () => {

src/components/AnalysisStore.tsx

Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -170,42 +170,69 @@ export function AnalysisStoreProvider({
170170
const getAnalysis = useCallback(() => analysisRef.current, []);
171171

172172
/**
173-
* Creates a new approved `TokenAnalysis` + `TokenAnalysisLink` for `tokenRef` with the given
174-
* gloss string (under `analysisLanguage`), replaces the analysis snapshot, notifies subscribers,
175-
* calls `onSave`, and calls the optional `spy` prop for test observability.
176-
*
177-
* The new `TokenAnalysis` gets a UUID (`crypto.randomUUID()`) as its id to ensure global
178-
* uniqueness. Existing analyses for the token are left untouched — this follows the data model's
179-
* "multiple competing analyses" design; the UI manages selection and deletion separately.
173+
* Writes an approved gloss for `tokenRef`. If an approved `TokenAnalysis` already exists for the
174+
* token it is updated in-place; otherwise a new `TokenAnalysis` + `TokenAnalysisLink` are
175+
* appended. Non-approved analyses for the token are left untouched. Replaces the analysis
176+
* snapshot, notifies subscribers, calls `onSave`, and calls the optional `spy` prop for test
177+
* observability.
180178
*
181179
* @param tokenRef - The `Token.ref` of the token being glossed.
182180
* @param surfaceText - The surface text of the token (stored as `Analysis.surfaceText`).
183181
* @param value - The new gloss string.
184182
*/
185183
const onGlossChange = useCallback(
186184
(tokenRef: string, surfaceText: string, value: string) => {
187-
const id = crypto.randomUUID();
188-
const newAnalysis: TokenAnalysis = {
189-
id,
190-
surfaceText,
191-
gloss: { [analysisLanguage]: value },
192-
};
193-
const newLink: TokenAnalysisLink = {
194-
analysisId: id,
195-
status: 'approved',
196-
token: { tokenRef, surfaceText },
197-
};
185+
// eslint-disable-next-line no-type-assertion/no-type-assertion -- ??= above guarantees non-null; TS can't see through the closure boundary
186+
const existingApprovedId = approvedAnalysisIdByTokenRef.current!.get(tokenRef);
187+
188+
let nextAnalyses: TokenAnalysis[];
189+
let nextLinks: TokenAnalysisLink[];
190+
let nextById: Map<string, TokenAnalysis>;
191+
192+
if (existingApprovedId === undefined) {
193+
const id = crypto.randomUUID();
194+
const newAnalysis: TokenAnalysis = {
195+
id,
196+
surfaceText,
197+
gloss: { [analysisLanguage]: value },
198+
};
199+
const newLink: TokenAnalysisLink = {
200+
analysisId: id,
201+
status: 'approved',
202+
token: { tokenRef, surfaceText },
203+
};
204+
nextAnalyses = [...analysisRef.current.tokenAnalyses, newAnalysis];
205+
nextLinks = [...analysisRef.current.tokenAnalysisLinks, newLink];
206+
// eslint-disable-next-line no-type-assertion/no-type-assertion -- ??= above guarantees non-null
207+
nextById = new Map([...analysisByIdRef.current!, [id, newAnalysis]]);
208+
} else {
209+
// Update the gloss on the existing approved analysis; preserve all other fields.
210+
// eslint-disable-next-line no-type-assertion/no-type-assertion -- ??= above guarantees non-null; index only stores ids present in analysisByIdRef
211+
const existing = analysisByIdRef.current!.get(existingApprovedId)!;
212+
const updated: TokenAnalysis = {
213+
...existing,
214+
surfaceText,
215+
gloss: { ...existing.gloss, [analysisLanguage]: value },
216+
};
217+
nextAnalyses = analysisRef.current.tokenAnalyses.map(
218+
/* v8 ignore next -- passthrough branch for non-matching tokens is structurally unreachable in tests */
219+
(ta) => (ta.id === existingApprovedId ? updated : ta),
220+
);
221+
// Links are unchanged — the same link already points to existingApprovedId.
222+
nextLinks = analysisRef.current.tokenAnalysisLinks;
223+
// eslint-disable-next-line no-type-assertion/no-type-assertion -- ??= above guarantees non-null
224+
nextById = new Map([...analysisByIdRef.current!, [existingApprovedId, updated]]);
225+
}
198226

199227
const next: TextAnalysis = {
200228
...analysisRef.current,
201-
tokenAnalyses: [...analysisRef.current.tokenAnalyses, newAnalysis],
202-
tokenAnalysisLinks: [...analysisRef.current.tokenAnalysisLinks, newLink],
229+
tokenAnalyses: nextAnalyses,
230+
tokenAnalysisLinks: nextLinks,
203231
};
204232

205233
analysisRef.current = next;
206-
// eslint-disable-next-line no-type-assertion/no-type-assertion -- ??= above guarantees non-null; TS can't see through the closure boundary
207-
analysisByIdRef.current = new Map([...analysisByIdRef.current!, [id, newAnalysis]]);
208-
approvedAnalysisIdByTokenRef.current = buildApprovedGlossIndex(next, analysisByIdRef.current);
234+
analysisByIdRef.current = nextById;
235+
approvedAnalysisIdByTokenRef.current = buildApprovedGlossIndex(next, nextById);
209236

210237
listenersRef.current.forEach((l) => l());
211238
onSave?.(next);

src/components/TokenChip.tsx

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import type { Token } from 'interlinearizer';
2-
import { memo } from 'react';
2+
import { memo, useEffect, useState } from 'react';
33
import { useGloss, useGlossDispatch } from './AnalysisStore';
44

55
/**
66
* Renders a single word token as an inline chip with an editable gloss input below the surface
77
* text. Gloss value and dispatch are read from {@link AnalysisStoreProvider} context via
8-
* {@link useGloss} and {@link useGlossDispatch}.
8+
* {@link useGloss} and {@link useGlossDispatch}. The gloss is written to the store only on blur to
9+
* avoid creating a new analysis entry on every keystroke.
910
*
1011
* @param props - Component props
1112
* @param props.token - The word token to render.
@@ -16,8 +17,15 @@ export function TokenChip({
1617
token,
1718
onFocus,
1819
}: Readonly<{ token: Token & { type: 'word' }; onFocus: () => void }>) {
19-
const gloss = useGloss(token.ref);
20+
const committedGloss = useGloss(token.ref);
2021
const onGlossChange = useGlossDispatch();
22+
const [draft, setDraft] = useState(committedGloss);
23+
24+
// Keep local draft in sync when the committed value changes externally (e.g. project switch).
25+
useEffect(() => {
26+
setDraft(committedGloss);
27+
}, [committedGloss]);
28+
2129
return (
2230
<label className="tw:inline-flex tw:shrink-0 tw:flex-col tw:items-center tw:rounded tw:border tw:border-border tw:bg-muted tw:px-1.5 tw:py-0.5">
2331
<span className="tw:whitespace-nowrap tw:font-mono tw:text-sm tw:text-foreground tw:cursor-text">
@@ -27,8 +35,9 @@ export function TokenChip({
2735
aria-label={`Gloss for ${token.surfaceText}`}
2836
className="tw:mt-0.5 tw:rounded tw:border tw:border-border tw:bg-background tw:px-1 tw:text-center tw:text-sm tw:text-foreground tw:outline-none tw:focus:border-ring tw:focus:ring-1 tw:focus:ring-ring"
2937
style={{ fieldSizing: 'content', minWidth: '5ch' }}
30-
value={gloss}
31-
onChange={(e) => onGlossChange(token.ref, token.surfaceText, e.target.value)}
38+
value={draft}
39+
onChange={(e) => setDraft(e.target.value)}
40+
onBlur={() => onGlossChange(token.ref, token.surfaceText, draft)}
3241
onFocus={onFocus}
3342
type="text"
3443
/>

0 commit comments

Comments
 (0)