Skip to content

Commit 91c13e6

Browse files
Refactor modal layer, remove dead code, and fix review findings
Quality cleanup stacked on the persisted-draft work, deferred out of that PR. Modal layer: - Add ModalShell, ConfirmDialog, useSubmitGuard, and parseLanguageTags and use them across the six project modals: shared overlay/dialog/title chrome, WipeConfirm and DiscardDraftConfirm collapsed into ConfirmDialog, and the duplicated comma-separated language parsing and double-submit guards unified. Dead code: - Remove the never-dispatched setAnalysis reducer and the unused selectPhraseAnalysisById selector (and their tests); drop needless exports on ProjectMetadataModalProps, ARC_CORNER_RADIUS, and RECENTER_FADE_EASING. Correctness: - isPhraseAnalysisLink rejects an empty tokens array (fail-safe at the load gate); resolveApprovedAnalysis uses findLast so it mutates the same approved link the read selectors surface under duplicate-link corruption. Tailwind: - Extract modal-actions, modal-form-label, and modal-error-box utilities for the repeated footer-row, form-label, and destructive-box class strings across the modals, and reuse the existing section-label utility in SegmentView instead of re-listing its classes. No unused custom utilities found. Docs/simplification: - Correct useGlossDispatch, ActiveProject (updateAnalysis), and firstIndex JSDoc; hoist ScriptureNavControls string keys to a module const; hoist the duplicated verseKey(liveScrRef) in InterlinearNavContext; extract closeCurrentVerse in usjBookExtractor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 39fa413 commit 91c13e6

23 files changed

Lines changed: 674 additions & 616 deletions

src/__tests__/store/analysisSlice.test.ts

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,14 @@ import { makePhraseLink } from '../test-helpers';
1515
import { emptyAnalysis } from '../../types/empty-factories';
1616
import {
1717
createPhrase,
18-
defaultState,
1918
deleteMorphemes,
2019
deletePhrase,
2120
mergePhrases,
2221
selectApprovedGloss,
2322
selectApprovedMorphemes,
2423
selectPhraseLinkByTokenRef,
25-
selectPhraseAnalysisById,
2624
selectPhraseGloss,
2725
selectPhraseLinks,
28-
setAnalysis,
2926
selectSegmentFreeTranslation,
3027
updatePhrase,
3128
writeGloss,
@@ -63,23 +60,6 @@ function makeAnalysis(ta: TokenAnalysis): TextAnalysis {
6360
};
6461
}
6562

66-
describe('setAnalysis', () => {
67-
it('replaces the full analysis in state', () => {
68-
const store = createAnalysisStore();
69-
const next = makeAnalysis({ id: 'ta-1', surfaceText: 'word', gloss: { und: 'hi' } });
70-
71-
store.dispatch(setAnalysis(next));
72-
73-
expect(store.getState().analysis.analysis).toStrictEqual(next);
74-
});
75-
76-
it('does not mutate analysisLanguage', () => {
77-
const store = createAnalysisStore({ analysis: { ...defaultState, analysisLanguage: 'fr' } });
78-
store.dispatch(setAnalysis(emptyAnalysis()));
79-
expect(store.getState().analysis.analysisLanguage).toBe('fr');
80-
});
81-
});
82-
8363
describe('writeGloss', () => {
8464
it('removes an orphaned approved link and creates a fresh one', () => {
8565
// Orphaned state: an approved link whose analysisId has no matching TokenAnalysis.
@@ -564,27 +544,6 @@ describe('selectPhraseLinkByTokenRef', () => {
564544
});
565545
});
566546

567-
describe('selectPhraseAnalysisById', () => {
568-
it('returns the PhraseAnalysis for a known id', () => {
569-
const link = makePhraseLink('phrase-1', ['tok-a']);
570-
const store = createAnalysisStore({
571-
analysis: { analysis: makeAnalysisWithPhrase(link), analysisLanguage: 'und' },
572-
});
573-
574-
const result = selectPhraseAnalysisById(store.getState().analysis, 'phrase-1');
575-
576-
expect(result?.id).toBe('phrase-1');
577-
});
578-
579-
it('returns undefined for an unknown id', () => {
580-
const store = createAnalysisStore();
581-
582-
const result = selectPhraseAnalysisById(store.getState().analysis, 'nonexistent');
583-
584-
expect(result).toBeUndefined();
585-
});
586-
});
587-
588547
describe('writePhraseGloss', () => {
589548
it('writes the gloss for the active language on a phrase', () => {
590549
const link = makePhraseLink('phrase-1', ['tok-a']);

src/components/AnalysisStore.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -273,9 +273,9 @@ export function useAnalysis(): TextAnalysis {
273273
}
274274

275275
/**
276-
* Returns the stable `onGlossChange` callback from the nearest {@link AnalysisStoreProvider}. The
277-
* callback creates or updates the approved `TokenAnalysis` for the token on each call, then
278-
* synchronously invokes `onSave` and the `onGlossChange` spy.
276+
* Returns a stable callback that creates or updates the approved `TokenAnalysis` for a token, then
277+
* synchronously invokes `onSave` and the optional `onGlossChange` spy. The `onGlossChange` spy is
278+
* test-only observability and has no effect on store behavior (see {@link AnalysisStoreProvider}).
279279
*
280280
* @returns A function `(tokenRef, surfaceText, value) => void`.
281281
* @throws When called outside an {@link AnalysisStoreProvider}.

src/components/InterlinearNavContext.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,9 @@ export function InterlinearNavProvider({
318318
/** Handle of the in-flight fade-in→idle timer, or `undefined` when none is pending. */
319319
const fadeInTimeoutRef = useRef<ReturnType<typeof setTimeout> | undefined>(undefined);
320320

321+
/** Verse key of the live ref, computed once for the cross-book / mid-reveal render guards below. */
322+
const liveKey = verseKey(liveScrRef);
323+
321324
// Detect a cross-book navigation *during render* and start the fade-out synchronously, so the
322325
// curtain drops in the same commit the book ref changes — never a paint later (an effect-driven
323326
// fade-out would let the mounted views render one frame against the new book's ref before the
@@ -332,8 +335,8 @@ export function InterlinearNavProvider({
332335
setFadePhase('out');
333336
} else if (
334337
fadePhase === 'in' &&
335-
verseKey(liveScrRef) !== verseKey(prevLiveScrRef) &&
336-
!isInternalNavMarkerFresh(pendingInternalNavRef.current.get(verseKey(liveScrRef)))
338+
liveKey !== verseKey(prevLiveScrRef) &&
339+
!isInternalNavMarkerFresh(pendingInternalNavRef.current.get(liveKey))
337340
) {
338341
// A follow-up external navigation landing mid-fade-in: the host resolves one picker selection
339342
// as two navigations (book change, then precise target), so the second routinely arrives while

src/components/SegmentView.tsx

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,7 @@ export function SegmentView({
175175
// it stays a bare verse number (the chapter is carried by SegmentListView's inline header).
176176
const verseLabelText = chapterLabelInVerse ? `${chapter}:${verse}` : `${verse}`;
177177

178-
const segmentHeader = (
179-
<span className="tw:mb-2 tw:block tw:text-xs tw:font-medium tw:text-muted-foreground tw:uppercase tw:tracking-wide">
180-
{verseLabelText}
181-
</span>
182-
);
178+
const segmentHeader = <span className="tw:section-label tw:mb-2 tw:block">{verseLabelText}</span>;
183179

184180
/**
185181
* `false` until just after the first paint, then `true`. Gates the link-slot fade transition: the

src/components/controls/ScriptureNavControls.tsx

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { useLocalizedStrings, useRecentScriptureRefs } from '@papi/frontend/react';
2-
import { useMemo } from 'react';
32
import {
43
BOOK_CHAPTER_CONTROL_STRING_KEYS,
54
BookChapterControl,
@@ -11,6 +10,13 @@ import {
1110
/** Fixed set of scroll-group IDs offered in the selector; `undefined` means "unlinked". */
1211
const AVAILABLE_SCROLL_GROUPS = [undefined, 0, 1, 2, 3, 4];
1312

13+
/**
14+
* Localized string keys for {@link BookChapterControl}, hoisted to module scope so the array
15+
* reference passed to `useLocalizedStrings` is stable across renders (a fresh array each render
16+
* would make the hook re-fetch every render). Mirrors the `STRING_KEYS` pattern in the views.
17+
*/
18+
const STRING_KEYS = [...BOOK_CHAPTER_CONTROL_STRING_KEYS];
19+
1420
/**
1521
* Props for {@link ScriptureNavControls}. Combines the scripture-reference fields from
1622
* `BookChapterControlProps` with the scroll-group fields from `ScrollGroupSelectorProps`.
@@ -35,9 +41,7 @@ export default function ScriptureNavControls({
3541
scrollGroupId,
3642
onChangeScrollGroupId,
3743
}: ScriptureNavControlsProps) {
38-
const [localizedStrings] = useLocalizedStrings(
39-
useMemo(() => [...BOOK_CHAPTER_CONTROL_STRING_KEYS], []),
40-
);
44+
const [localizedStrings] = useLocalizedStrings(STRING_KEYS);
4145
const { recentScriptureRefs: recentRefs, addRecentScriptureRef: onAddRecentRef } =
4246
useRecentScriptureRefs();
4347

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/** @file Shared confirm/cancel dialog for destructive or discard confirmations. */
2+
import { Button } from 'platform-bible-react';
3+
import { ModalShell } from './ModalShell';
4+
5+
/**
6+
* A small modal asking the user to confirm or cancel a single action (a destructive wipe, a discard
7+
* of unsaved work, etc.). Presentational: it renders the shared {@link ModalShell} chrome with a
8+
* body paragraph and a secondary Cancel / destructive Confirm pair, and delegates the decision to
9+
* the caller via {@link onConfirm} / {@link onCancel}.
10+
*
11+
* @param props - Component props
12+
* @param props.titleId - DOM id wired to the dialog's `aria-labelledby` and title.
13+
* @param props.title - Localized title text.
14+
* @param props.body - Localized body text explaining the consequence.
15+
* @param props.confirmLabel - Localized label for the destructive confirm button.
16+
* @param props.cancelLabel - Localized label for the secondary cancel button.
17+
* @param props.confirmTestId - `data-testid` placed on the confirm button so tests can target it.
18+
* @param props.isSubmitting - When `true`, both buttons are disabled to prevent interaction while
19+
* the caller is processing the confirmed action.
20+
* @param props.onConfirm - Called when the user confirms the action.
21+
* @param props.onCancel - Called when the user backs out, leaving state untouched.
22+
* @returns The confirmation overlay.
23+
*/
24+
export function ConfirmDialog({
25+
titleId,
26+
title,
27+
body,
28+
confirmLabel,
29+
cancelLabel,
30+
confirmTestId,
31+
isSubmitting = false,
32+
onConfirm,
33+
onCancel,
34+
}: Readonly<{
35+
titleId: string;
36+
title: string;
37+
body: string;
38+
confirmLabel: string;
39+
cancelLabel: string;
40+
confirmTestId: string;
41+
isSubmitting?: boolean;
42+
onConfirm: () => void;
43+
onCancel: () => void;
44+
}>) {
45+
return (
46+
<ModalShell titleId={titleId} title={title} width="tw:w-96">
47+
<p className="tw:text-sm tw:mb-4">{body}</p>
48+
<div className="tw:modal-actions">
49+
<Button variant="secondary" onClick={onCancel} disabled={isSubmitting}>
50+
{cancelLabel}
51+
</Button>
52+
<Button
53+
variant="destructive"
54+
onClick={onConfirm}
55+
data-testid={confirmTestId}
56+
disabled={isSubmitting}
57+
>
58+
{confirmLabel}
59+
</Button>
60+
</div>
61+
</ModalShell>
62+
);
63+
}

src/components/modals/CreateProjectModal.tsx

Lines changed: 48 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { useLocalizedStrings } from '@papi/frontend/react';
22
import { Button } from 'platform-bible-react';
33
import { useState, useCallback } from 'react';
4+
import { parseLanguageTags } from '../../utils/language-tags';
5+
import { ModalShell } from './ModalShell';
46

57
/** Localized string keys used by {@link CreateProjectModal}. */
68
const CREATE_PROJECT_MODAL_STRING_KEYS: `%${string}%`[] = [
@@ -68,10 +70,7 @@ export function CreateProjectModal({
6870
* {@link onCreateDraft}.
6971
*/
7072
const handleSubmit = useCallback(() => {
71-
const parsedLanguages = analysisLanguages
72-
.split(',')
73-
.map((t) => t.trim())
74-
.filter((t) => t.length > 0);
73+
const parsedLanguages = parseLanguageTags(analysisLanguages);
7574
const normalizedAnalysisLanguages = parsedLanguages.length > 0 ? parsedLanguages : ['und'];
7675
onCreateDraft({
7776
analysisLanguages: normalizedAnalysisLanguages,
@@ -83,56 +82,50 @@ export function CreateProjectModal({
8382
/* v8 ignore next */ if (stringsLoading) return undefined;
8483

8584
return (
86-
<div className="tw:modal-overlay">
87-
<dialog
88-
aria-labelledby="create-project-modal-title"
89-
aria-modal="true"
90-
className="tw:modal-dialog tw:rounded tw:w-96"
91-
open
92-
>
93-
<h2 id="create-project-modal-title" className="tw:modal-title">
94-
{localizedStrings['%interlinearizer_modal_create_title%']}
95-
</h2>
96-
<label className="tw:block tw:text-sm tw:mb-1" htmlFor="project-name">
97-
{localizedStrings['%interlinearizer_modal_create_name_label%']}
98-
</label>
99-
<input
100-
id="project-name"
101-
className="tw:modal-form-input tw:mb-3"
102-
value={name}
103-
onChange={(e) => setName(e.target.value)}
104-
placeholder={localizedStrings['%interlinearizer_modal_create_name_placeholder%']}
105-
/>
106-
<label className="tw:block tw:text-sm tw:mb-1" htmlFor="project-description">
107-
{localizedStrings['%interlinearizer_modal_create_description_label%']}
108-
</label>
109-
<textarea
110-
id="project-description"
111-
className="tw:modal-form-input tw:mb-3 tw:resize-none"
112-
rows={2}
113-
value={description}
114-
onChange={(e) => setDescription(e.target.value)}
115-
placeholder={localizedStrings['%interlinearizer_modal_create_description_placeholder%']}
116-
/>
117-
<label className="tw:block tw:text-sm tw:mb-1" htmlFor="analysis-language">
118-
{localizedStrings['%interlinearizer_modal_create_language_label%']}
119-
</label>
120-
<input
121-
id="analysis-language"
122-
className="tw:modal-form-input tw:mb-4"
123-
value={analysisLanguages}
124-
onChange={(e) => setAnalysisLanguages(e.target.value)}
125-
placeholder={localizedStrings['%interlinearizer_modal_create_language_placeholder%']}
126-
/>
127-
<div className="tw:flex tw:gap-2 tw:justify-end">
128-
<Button variant="secondary" onClick={onClose} disabled={isSubmitting}>
129-
{localizedStrings['%interlinearizer_modal_create_cancel%']}
130-
</Button>
131-
<Button onClick={handleSubmit} disabled={isSubmitting}>
132-
{localizedStrings['%interlinearizer_modal_create_submit%']}
133-
</Button>
134-
</div>
135-
</dialog>
136-
</div>
85+
<ModalShell
86+
titleId="create-project-modal-title"
87+
title={localizedStrings['%interlinearizer_modal_create_title%']}
88+
width="tw:w-96"
89+
>
90+
<label className="tw:modal-form-label" htmlFor="project-name">
91+
{localizedStrings['%interlinearizer_modal_create_name_label%']}
92+
</label>
93+
<input
94+
id="project-name"
95+
className="tw:modal-form-input tw:mb-3"
96+
value={name}
97+
onChange={(e) => setName(e.target.value)}
98+
placeholder={localizedStrings['%interlinearizer_modal_create_name_placeholder%']}
99+
/>
100+
<label className="tw:modal-form-label" htmlFor="project-description">
101+
{localizedStrings['%interlinearizer_modal_create_description_label%']}
102+
</label>
103+
<textarea
104+
id="project-description"
105+
className="tw:modal-form-input tw:mb-3 tw:resize-none"
106+
rows={2}
107+
value={description}
108+
onChange={(e) => setDescription(e.target.value)}
109+
placeholder={localizedStrings['%interlinearizer_modal_create_description_placeholder%']}
110+
/>
111+
<label className="tw:modal-form-label" htmlFor="analysis-language">
112+
{localizedStrings['%interlinearizer_modal_create_language_label%']}
113+
</label>
114+
<input
115+
id="analysis-language"
116+
className="tw:modal-form-input tw:mb-4"
117+
value={analysisLanguages}
118+
onChange={(e) => setAnalysisLanguages(e.target.value)}
119+
placeholder={localizedStrings['%interlinearizer_modal_create_language_placeholder%']}
120+
/>
121+
<div className="tw:modal-actions">
122+
<Button variant="secondary" onClick={onClose} disabled={isSubmitting}>
123+
{localizedStrings['%interlinearizer_modal_create_cancel%']}
124+
</Button>
125+
<Button onClick={handleSubmit} disabled={isSubmitting}>
126+
{localizedStrings['%interlinearizer_modal_create_submit%']}
127+
</Button>
128+
</div>
129+
</ModalShell>
137130
);
138131
}
Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useLocalizedStrings } from '@papi/frontend/react';
2-
import { Button } from 'platform-bible-react';
2+
import { ConfirmDialog } from './ConfirmDialog';
33

44
/** Localized string keys used by {@link DiscardDraftConfirm}. */
55
const DISCARD_DRAFT_CONFIRM_STRING_KEYS: `%${string}%`[] = [
@@ -35,33 +35,16 @@ export function DiscardDraftConfirm({
3535
/* v8 ignore next */ if (stringsLoading) return undefined;
3636

3737
return (
38-
<div className="tw:modal-overlay">
39-
<dialog
40-
aria-labelledby="discard-draft-modal-title"
41-
aria-modal="true"
42-
className="tw:modal-dialog tw:rounded tw:w-96"
43-
open
44-
>
45-
<h2 id="discard-draft-modal-title" className="tw:modal-title">
46-
{localizedStrings['%interlinearizer_confirm_discard_title%']}
47-
</h2>
48-
<p className="tw:text-sm tw:mb-4">
49-
{localizedStrings['%interlinearizer_confirm_discard_body%']}
50-
</p>
51-
<div className="tw:flex tw:gap-2 tw:justify-end">
52-
<Button disabled={isSubmitting} onClick={onCancel} variant="secondary">
53-
{localizedStrings['%interlinearizer_confirm_discard_cancel%']}
54-
</Button>
55-
<Button
56-
data-testid="discard-draft-confirm"
57-
disabled={isSubmitting}
58-
onClick={onConfirm}
59-
variant="destructive"
60-
>
61-
{localizedStrings['%interlinearizer_confirm_discard_ok%']}
62-
</Button>
63-
</div>
64-
</dialog>
65-
</div>
38+
<ConfirmDialog
39+
titleId="discard-draft-modal-title"
40+
title={localizedStrings['%interlinearizer_confirm_discard_title%']}
41+
body={localizedStrings['%interlinearizer_confirm_discard_body%']}
42+
confirmLabel={localizedStrings['%interlinearizer_confirm_discard_ok%']}
43+
cancelLabel={localizedStrings['%interlinearizer_confirm_discard_cancel%']}
44+
confirmTestId="discard-draft-confirm"
45+
isSubmitting={isSubmitting}
46+
onConfirm={onConfirm}
47+
onCancel={onCancel}
48+
/>
6649
);
6750
}

0 commit comments

Comments
 (0)