Skip to content

Commit 2775bb9

Browse files
Fix draft save mid-edit race and discard-confirm overlay
- markSynced now only clears the dirty flag when the live draft still matches the persisted snapshot, so a gloss edit made during an in-flight Save / Save As no longer clears the unsaved-changes indicator while the project is left stale. The save paths pass the analysis they wrote. - The discard-changes confirmation overlays the active modal instead of replacing it, so canceling returns to the Create dialog with its typed input intact, and confirming Open no longer unmounts and re-fetches the still-open select modal underneath. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9b3f87e commit 2775bb9

5 files changed

Lines changed: 113 additions & 71 deletions

File tree

src/__tests__/components/modals/ProjectModals.test.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -471,9 +471,10 @@ describe('ProjectModals', () => {
471471
render(<ProjectModals {...buildProps({ modal: 'select', dirty: true, loadFromProject })} />);
472472

473473
await userEvent.click(screen.getByTestId('select-select'));
474-
// The discard confirm replaces the select modal; the project is not opened yet.
474+
// The discard confirm overlays the still-mounted select modal (so confirming Open does not
475+
// unmount and re-fetch it); the project is not opened yet.
475476
expect(screen.getByTestId('discard-modal')).toBeInTheDocument();
476-
expect(screen.queryByTestId('select-modal')).toBeNull();
477+
expect(screen.getByTestId('select-modal')).toBeInTheDocument();
477478
expect(loadFromProject).not.toHaveBeenCalled();
478479

479480
await userEvent.click(screen.getByTestId('discard-confirm'));

src/__tests__/hooks/useDraftProject.test.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,20 +320,42 @@ describe('useDraftProject', () => {
320320
it('clears dirty and persists the synced draft without bumping the version', async () => {
321321
const { result } = await renderLoaded();
322322
// Make the draft dirty first so the transition to synced is observable.
323+
const synced = analysisWithToken('tok-sync');
323324
act(() => {
324-
result.current.autosaveAnalysis(analysisWithToken('tok-sync'));
325+
result.current.autosaveAnalysis(synced);
325326
});
326327
expect(result.current.dirty).toBe(true);
327328
const versionBefore = result.current.draftVersion;
328329

329330
act(() => {
330-
result.current.markSynced();
331+
result.current.markSynced(synced);
331332
});
332333

333334
expect(result.current.dirty).toBe(false);
334335
expect(result.current.draftVersion).toBe(versionBefore);
335336
expect(lastSavedDraft().dirty).toBe(false);
336337
});
338+
339+
it('leaves the draft dirty when an edit landed since the saved snapshot', async () => {
340+
const { result } = await renderLoaded();
341+
// The analysis that a Save captured and persisted.
342+
const savedSnapshot = analysisWithToken('tok-saved');
343+
act(() => {
344+
result.current.autosaveAnalysis(savedSnapshot);
345+
});
346+
// A newer edit lands during the save round-trip, replacing the ref's analysis.
347+
act(() => {
348+
result.current.autosaveAnalysis(analysisWithToken('tok-newer'));
349+
});
350+
expect(result.current.dirty).toBe(true);
351+
352+
// Syncing against the now-stale snapshot must not clear the unsaved-changes flag.
353+
act(() => {
354+
result.current.markSynced(savedSnapshot);
355+
});
356+
357+
expect(result.current.dirty).toBe(true);
358+
});
337359
});
338360

339361
describe('getDraftSnapshot', () => {

src/components/InterlinearizerLoader.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ function InterlinearizerLoaderInner({
265265
activeProject.id,
266266
JSON.stringify(snapshot.analysis),
267267
);
268-
markSynced();
268+
markSynced(snapshot.analysis);
269269
} catch (e) {
270270
logger.error('Interlinearizer: failed to save draft to project', e);
271271
}

src/components/modals/ProjectModals.tsx

Lines changed: 58 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { UseWebViewStateHook } from '@papi/core';
22
import papi, { logger } from '@papi/frontend';
3-
import type { DraftProject } from 'interlinearizer';
3+
import type { DraftProject, TextAnalysis } from 'interlinearizer';
44
import { useCallback, useState } from 'react';
55
import type { NewDraftConfig, OpenableProject } from '../../hooks/useDraftProject';
66
import type { InterlinearProjectSummary } from '../../types/interlinear-project-summary';
@@ -23,11 +23,12 @@ type PendingReplace =
2323
| { kind: 'open'; project: InterlinearProjectSummary };
2424

2525
/**
26-
* Single mount point for all project-related dialogs. Renders at most one of
27-
* {@link SelectInterlinearProjectModal}, {@link CreateProjectModal}, {@link ProjectMetadataModal},
28-
* {@link SaveAsProjectModal}, or the {@link DiscardDraftConfirm} guard; manages the shared WebView
29-
* state for the active project; and routes New / Open / Save As through the draft (rather than
30-
* persisting projects directly on every edit).
26+
* Single mount point for all project-related dialogs. Renders the active one of
27+
* {@link SelectInterlinearProjectModal}, {@link CreateProjectModal}, {@link ProjectMetadataModal}, or
28+
* {@link SaveAsProjectModal}, with the {@link DiscardDraftConfirm} guard overlaid on top when a
29+
* draft-replacing action is pending (so canceling returns to the underlying modal with its state
30+
* intact); manages the shared WebView state for the active project; and routes New / Open / Save As
31+
* through the draft (rather than persisting projects directly on every edit).
3132
*
3233
* @param props - Component props
3334
* @param props.activeProject - The currently active interlinear project (the Save target), read
@@ -40,7 +41,8 @@ type PendingReplace =
4041
* on Save As.
4142
* @param props.loadFromProject - Loads a project's analysis + config into the draft (the "Open"
4243
* flow).
43-
* @param props.markSynced - Marks the draft as saved (clears `dirty`) after a successful Save As.
44+
* @param props.markSynced - Marks the draft as saved (clears `dirty`) after a successful Save As,
45+
* given the analysis that was persisted; a no-op if an edit landed during the save.
4446
* @param props.modal - Which modal is currently open.
4547
* @param props.projectId - PAPI source project ID passed from the host.
4648
* @param props.resetDraft - Resets the draft to an empty baseline with the given config (the "New"
@@ -68,7 +70,7 @@ export default function ProjectModals({
6870
dirty: boolean;
6971
getDraftSnapshot: () => DraftProject | undefined;
7072
loadFromProject: (project: OpenableProject) => void;
71-
markSynced: () => void;
73+
markSynced: (savedAnalysis: TextAnalysis) => void;
7274
modal: ModalState;
7375
projectId: string;
7476
resetDraft: (config: NewDraftConfig) => void;
@@ -282,7 +284,7 @@ export default function ProjectModals({
282284
JSON.stringify(snapshot.analysis),
283285
);
284286
setActiveProject(created);
285-
markSynced();
287+
markSynced(snapshot.analysis);
286288
setModal('none');
287289
} catch (e) {
288290
logger.error('Interlinearizer: failed to save draft as new project', e);
@@ -311,7 +313,7 @@ export default function ProjectModals({
311313
JSON.stringify(snapshot.analysis),
312314
);
313315
setActiveProject(project);
314-
markSynced();
316+
markSynced(snapshot.analysis);
315317
setModal('none');
316318
} catch (e) {
317319
logger.error('Interlinearizer: failed to overwrite project with draft', e);
@@ -358,54 +360,55 @@ export default function ProjectModals({
358360

359361
return (
360362
<div>
361-
{pendingReplace ? (
362-
<DiscardDraftConfirm onConfirm={handleConfirmReplace} onCancel={handleCancelReplace} />
363-
) : (
364-
<>
365-
{modal === 'select' && (
366-
<SelectInterlinearProjectModal
367-
sourceProjectId={projectId}
368-
onSelect={handleSelectProject}
369-
onCreateNew={handleSelectCreateNew}
370-
onClose={handleSelectClose}
371-
onViewInfo={handleViewInfo}
372-
/>
373-
)}
363+
{modal === 'select' && (
364+
<SelectInterlinearProjectModal
365+
sourceProjectId={projectId}
366+
onSelect={handleSelectProject}
367+
onCreateNew={handleSelectCreateNew}
368+
onClose={handleSelectClose}
369+
onViewInfo={handleViewInfo}
370+
/>
371+
)}
374372

375-
{modal === 'create' && (
376-
<CreateProjectModal
377-
defaultAnalysisLanguage={defaultAnalysisLanguage}
378-
onClose={handleCreateClose}
379-
onCreateDraft={handleCreateDraft}
380-
/>
381-
)}
373+
{modal === 'create' && (
374+
<CreateProjectModal
375+
defaultAnalysisLanguage={defaultAnalysisLanguage}
376+
onClose={handleCreateClose}
377+
onCreateDraft={handleCreateDraft}
378+
/>
379+
)}
382380

383-
{modal === 'saveAs' && (
384-
<SaveAsProjectModal
385-
sourceProjectId={projectId}
386-
defaultName={draftSnapshot?.suggestedName}
387-
defaultDescription={draftSnapshot?.suggestedDescription}
388-
onSaveNew={handleSaveAsNew}
389-
onOverwrite={handleOverwrite}
390-
onClose={handleSaveAsClose}
391-
/>
392-
)}
381+
{modal === 'saveAs' && (
382+
<SaveAsProjectModal
383+
sourceProjectId={projectId}
384+
defaultName={draftSnapshot?.suggestedName}
385+
defaultDescription={draftSnapshot?.suggestedDescription}
386+
onSaveNew={handleSaveAsNew}
387+
onOverwrite={handleOverwrite}
388+
onClose={handleSaveAsClose}
389+
/>
390+
)}
393391

394-
{modal === 'metadata' && resolvedMetadataProject && (
395-
<ProjectMetadataModal
396-
interlinearProjectId={resolvedMetadataProject.id}
397-
name={resolvedMetadataProject.name}
398-
description={resolvedMetadataProject.description}
399-
sourceProjectId={resolvedMetadataProject.sourceProjectId}
400-
targetProjectId={resolvedMetadataProject.targetProjectId}
401-
analysisLanguages={resolvedMetadataProject.analysisLanguages}
402-
createdAt={resolvedMetadataProject.createdAt}
403-
onClose={handleMetadataClose}
404-
onProjectSaved={handleMetadataProjectSaved}
405-
onProjectDeleted={handleMetadataProjectDeleted}
406-
/>
407-
)}
408-
</>
392+
{modal === 'metadata' && resolvedMetadataProject && (
393+
<ProjectMetadataModal
394+
interlinearProjectId={resolvedMetadataProject.id}
395+
name={resolvedMetadataProject.name}
396+
description={resolvedMetadataProject.description}
397+
sourceProjectId={resolvedMetadataProject.sourceProjectId}
398+
targetProjectId={resolvedMetadataProject.targetProjectId}
399+
analysisLanguages={resolvedMetadataProject.analysisLanguages}
400+
createdAt={resolvedMetadataProject.createdAt}
401+
onClose={handleMetadataClose}
402+
onProjectSaved={handleMetadataProjectSaved}
403+
onProjectDeleted={handleMetadataProjectDeleted}
404+
/>
405+
)}
406+
407+
{/* The discard guard overlays the active modal rather than replacing it, so canceling
408+
returns to that modal with its in-progress input intact, and confirming an Open does not
409+
unmount (and re-fetch) the still-open select modal underneath. */}
410+
{pendingReplace && (
411+
<DiscardDraftConfirm onConfirm={handleConfirmReplace} onCancel={handleCancelReplace} />
409412
)}
410413
</div>
411414
);

src/hooks/useDraftProject.ts

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,16 @@ export type UseDraftProjectResult = {
7979
wipeBook: (bookCode: string) => void;
8080
/** Clears the draft's analysis entirely and marks it dirty. */
8181
wipeAll: () => void;
82-
/** Marks the draft as synced (not dirty) after a successful Save / Save As. */
83-
markSynced: () => void;
82+
/**
83+
* Marks the draft as synced (not dirty) after a successful Save / Save As — but only when the
84+
* draft has not changed since the snapshot that was persisted. Pass the exact analysis that was
85+
* written; if a later auto-save replaced it (an edit made during the save round-trip), the draft
86+
* is left dirty so the unsaved-changes indicator and the next Save reflect that un-persisted edit
87+
* rather than being cleared against a now-stale snapshot.
88+
*
89+
* @param savedAnalysis - The `TextAnalysis` reference that was actually persisted to the project.
90+
*/
91+
markSynced: (savedAnalysis: TextAnalysis) => void;
8492
};
8593

8694
/**
@@ -246,15 +254,23 @@ export default function useDraftProject(
246254
applyReplacement({ ...current, analysis: emptyAnalysis(), dirty: true });
247255
}, [applyReplacement]);
248256

249-
const markSynced = useCallback(() => {
250-
const { current } = draftRef;
251-
/* v8 ignore next -- save is only reachable from the mounted editor */
252-
if (!current) return;
253-
const next: DraftProject = { ...current, dirty: false };
254-
draftRef.current = next;
255-
persist(next);
256-
setDirty(false);
257-
}, [persist]);
257+
const markSynced = useCallback(
258+
(savedAnalysis: TextAnalysis) => {
259+
const { current } = draftRef;
260+
/* v8 ignore next -- save is only reachable from the mounted editor */
261+
if (!current) return;
262+
// If an edit landed during the save round-trip, autosaveAnalysis has already swapped a newer
263+
// analysis (a fresh object) into the ref and marked the draft dirty. Leave it dirty so the
264+
// unsaved indicator and the next Save reflect that un-persisted edit, rather than clearing it
265+
// against the now-stale snapshot we just wrote.
266+
if (current.analysis !== savedAnalysis) return;
267+
const next: DraftProject = { ...current, dirty: false };
268+
draftRef.current = next;
269+
persist(next);
270+
setDirty(false);
271+
},
272+
[persist],
273+
);
258274

259275
return {
260276
isDraftLoading,

0 commit comments

Comments
 (0)