Skip to content

Commit d4fdf59

Browse files
Prevent stale data race condition, update old TW classes, TW class consolidation
1 parent 3d57a97 commit d4fdf59

8 files changed

Lines changed: 123 additions & 45 deletions

src/__tests__/components/ContinuousView.test.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ describe('ContinuousView fade overlays', () => {
429429
// Prev fade gradient is tw:from-background (prev-to-next gradient)
430430
const gradients = container.querySelectorAll('[aria-hidden="true"]');
431431
const prevFades = Array.from(gradients).filter((el) =>
432-
el.className.includes('tw:bg-gradient-to-e'),
432+
el.className.includes('tw:bg-linear-to-r'),
433433
);
434434
expect(prevFades).toHaveLength(0);
435435
});
@@ -439,7 +439,7 @@ describe('ContinuousView fade overlays', () => {
439439

440440
const gradients = container.querySelectorAll('[aria-hidden="true"]');
441441
const nextFades = Array.from(gradients).filter((el) =>
442-
el.className.includes('tw:bg-gradient-to-s'),
442+
el.className.includes('tw:bg-linear-to-l'),
443443
);
444444
expect(nextFades).toHaveLength(1);
445445
});
@@ -451,7 +451,7 @@ describe('ContinuousView fade overlays', () => {
451451

452452
const gradients = container.querySelectorAll('[aria-hidden="true"]');
453453
const prevFades = Array.from(gradients).filter((el) =>
454-
el.className.includes('tw:bg-gradient-to-e'),
454+
el.className.includes('tw:bg-linear-to-r'),
455455
);
456456
expect(prevFades).toHaveLength(1);
457457
});
@@ -466,7 +466,7 @@ describe('ContinuousView fade overlays', () => {
466466

467467
const gradients = container.querySelectorAll('[aria-hidden="true"]');
468468
const nextFades = Array.from(gradients).filter((el) =>
469-
el.className.includes('tw:bg-gradient-to-s'),
469+
el.className.includes('tw:bg-linear-to-l'),
470470
);
471471
expect(nextFades).toHaveLength(0);
472472
});

src/__tests__/components/SelectInterlinearProjectModal.test.tsx

Lines changed: 85 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,18 @@ describe('SelectInterlinearProjectModal', () => {
5454

5555
it('renders nothing while strings are loading', () => {
5656
jest.mocked(useLocalizedStrings).mockReturnValue([{}, true]);
57+
mockSendCommand.mockReturnValue(new Promise(() => {}));
5758
const { container } = render(<SelectInterlinearProjectModal {...defaultProps} />);
5859
expect(container.firstChild).toBeNull();
5960
});
6061

61-
it('renders the modal heading when strings are loaded', () => {
62+
it('renders the modal heading when strings are loaded', async () => {
6263
render(<SelectInterlinearProjectModal {...defaultProps} />);
63-
expect(
64-
screen.getByRole('heading', { name: /select interlinear project/i }),
65-
).toBeInTheDocument();
64+
await waitFor(() =>
65+
expect(
66+
screen.getByRole('heading', { name: /select interlinear project/i }),
67+
).toBeInTheDocument(),
68+
);
6669
});
6770

6871
it('shows empty-state message when no projects are returned', async () => {
@@ -181,4 +184,82 @@ describe('SelectInterlinearProjectModal', () => {
181184
await waitFor(() => expect(screen.getByText('No projects yet.')).toBeInTheDocument());
182185
expect(screen.queryByRole('listitem')).not.toBeInTheDocument();
183186
});
187+
188+
it('disables Cancel and Create New while the project list is loading', async () => {
189+
let resolveLoad: (v: string) => void = () => {};
190+
mockSendCommand.mockImplementation(
191+
() =>
192+
new Promise((resolve) => {
193+
resolveLoad = resolve;
194+
}),
195+
);
196+
render(<SelectInterlinearProjectModal {...defaultProps} />);
197+
198+
expect(screen.getByRole('button', { name: /^cancel$/i })).toBeDisabled();
199+
expect(screen.getByRole('button', { name: /create new/i })).toBeDisabled();
200+
201+
resolveLoad('[]');
202+
await waitFor(() =>
203+
expect(screen.getByRole('button', { name: /^cancel$/i })).not.toBeDisabled(),
204+
);
205+
});
206+
207+
it('re-enables Cancel and Create New after loading finishes', async () => {
208+
mockSendCommand.mockResolvedValue('[]');
209+
render(<SelectInterlinearProjectModal {...defaultProps} />);
210+
211+
await waitFor(() =>
212+
expect(screen.getByRole('button', { name: /^cancel$/i })).not.toBeDisabled(),
213+
);
214+
expect(screen.getByRole('button', { name: /create new/i })).not.toBeDisabled();
215+
});
216+
217+
it('clears the project list immediately when a new load begins', async () => {
218+
mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT]));
219+
const { rerender } = render(<SelectInterlinearProjectModal {...defaultProps} />);
220+
await waitFor(() => expect(screen.getByText('Unnamed')).toBeInTheDocument());
221+
222+
// Hold the second load in-flight so we can assert the list cleared before it resolves.
223+
let resolveSecond: (v: string) => void = () => {};
224+
mockSendCommand.mockImplementation(
225+
() =>
226+
new Promise((resolve) => {
227+
resolveSecond = resolve;
228+
}),
229+
);
230+
231+
rerender(<SelectInterlinearProjectModal {...defaultProps} sourceProjectId="src-proj-2" />);
232+
233+
await waitFor(() => expect(screen.queryByText('Unnamed')).not.toBeInTheDocument());
234+
expect(screen.queryByRole('listitem')).not.toBeInTheDocument();
235+
236+
resolveSecond('[]');
237+
await waitFor(() =>
238+
expect(screen.getByRole('button', { name: /^cancel$/i })).not.toBeDisabled(),
239+
);
240+
});
241+
242+
it('ignores a response that arrives after a newer load has started', async () => {
243+
let resolveFirst: (v: string) => void = () => {};
244+
mockSendCommand
245+
.mockImplementationOnce(
246+
() =>
247+
new Promise((resolve) => {
248+
resolveFirst = resolve;
249+
}),
250+
)
251+
.mockResolvedValue(JSON.stringify([STUB_PROJECT_2]));
252+
253+
const { rerender } = render(<SelectInterlinearProjectModal {...defaultProps} />);
254+
255+
// Start a second load by changing sourceProjectId before the first resolves.
256+
rerender(<SelectInterlinearProjectModal {...defaultProps} sourceProjectId="src-proj-2" />);
257+
await waitFor(() => expect(screen.getByText('French glosses')).toBeInTheDocument());
258+
259+
// Now deliver the stale first response — it must not replace the current list.
260+
resolveFirst(JSON.stringify([STUB_PROJECT]));
261+
262+
await waitFor(() => expect(screen.queryByText('Unnamed')).not.toBeInTheDocument());
263+
expect(screen.getByText('French glosses')).toBeInTheDocument();
264+
});
184265
});

src/components/ContinuousView.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ export default function ContinuousView({
318318
{/* Previous navigation arrow */}
319319
<button
320320
aria-label="Previous token"
321-
className="tw:z-10 tw:shrink-0 tw:rounded tw:p-1 tw:text-foreground tw:disabled:opacity-30 tw:hover:bg-muted/50"
321+
className="tw:icon-button"
322322
disabled={atStart}
323323
onClick={stepPrev}
324324
type="button"
@@ -332,15 +332,15 @@ export default function ContinuousView({
332332
{!atStart && (
333333
<div
334334
aria-hidden="true"
335-
className="tw:pointer-events-none tw:absolute tw:inset-y-0 tw:inset-s-0 tw:z-10 tw:w-8 tw:bg-gradient-to-e tw:from-background tw:to-transparent"
335+
className="tw:pointer-events-none tw:absolute tw:inset-y-0 tw:left-0 tw:z-10 tw:w-8 tw:bg-linear-to-r tw:from-background tw:to-transparent"
336336
/>
337337
)}
338338

339339
{/* Next fade overlay — only rendered when the next arrow is enabled */}
340340
{!atEnd && (
341341
<div
342342
aria-hidden="true"
343-
className="tw:pointer-events-none tw:absolute tw:inset-y-0 tw:inset-e-0 tw:z-10 tw:w-8 tw:bg-gradient-to-s tw:from-background tw:to-transparent"
343+
className="tw:pointer-events-none tw:absolute tw:inset-y-0 tw:right-0 tw:z-10 tw:w-8 tw:bg-linear-to-l tw:from-background tw:to-transparent"
344344
/>
345345
)}
346346

@@ -379,7 +379,7 @@ export default function ContinuousView({
379379
{/* Next navigation arrow */}
380380
<button
381381
aria-label="Next token"
382-
className="tw:z-10 tw:shrink-0 tw:rounded tw:p-1 tw:text-foreground tw:disabled:opacity-30 tw:hover:bg-muted/50"
382+
className="tw:icon-button"
383383
disabled={atEnd}
384384
onClick={stepNext}
385385
type="button"

src/components/CreateProjectModal.tsx

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,7 @@ export function CreateProjectModal({
122122
className="tw:modal-dialog tw:rounded tw:w-96"
123123
open
124124
>
125-
<h2
126-
id="create-project-modal-title"
127-
className="tw:text-base tw:font-semibold tw:text-foreground tw:mb-4"
128-
>
125+
<h2 id="create-project-modal-title" className="tw:modal-title">
129126
{localizedStrings['%interlinearizer_modal_create_title%']}
130127
</h2>
131128
<label className="tw:block tw:text-sm tw:mb-1" htmlFor="project-name">

src/components/ProjectMetadataModal.tsx

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -180,20 +180,14 @@ export function ProjectMetadataModal({
180180
className="tw:modal-dialog tw:rounded-lg tw:w-lg"
181181
open
182182
>
183-
<h2
184-
id="project-metadata-modal-title"
185-
className="tw:text-base tw:font-semibold tw:text-foreground tw:mb-4"
186-
>
183+
<h2 id="project-metadata-modal-title" className="tw:modal-title">
187184
{localizedStrings['%interlinearizer_modal_metadata_title%']}
188185
</h2>
189186

190187
{/* Editable fields */}
191188
<div className="tw:flex tw:flex-col tw:gap-3 tw:mb-4">
192189
<div className="tw:flex tw:flex-col tw:gap-1">
193-
<label
194-
className="tw:text-xs tw:font-medium tw:text-muted-foreground tw:uppercase tw:tracking-wide"
195-
htmlFor="metadata-edit-name"
196-
>
190+
<label className="tw:section-label" htmlFor="metadata-edit-name">
197191
{localizedStrings['%interlinearizer_modal_metadata_name_label%']}
198192
</label>
199193
<input
@@ -206,10 +200,7 @@ export function ProjectMetadataModal({
206200
</div>
207201

208202
<div className="tw:flex tw:flex-col tw:gap-1">
209-
<label
210-
className="tw:text-xs tw:font-medium tw:text-muted-foreground tw:uppercase tw:tracking-wide"
211-
htmlFor="metadata-edit-description"
212-
>
203+
<label className="tw:section-label" htmlFor="metadata-edit-description">
213204
{localizedStrings['%interlinearizer_modal_metadata_description_label%']}
214205
</label>
215206
<textarea
@@ -225,10 +216,7 @@ export function ProjectMetadataModal({
225216
</div>
226217

227218
<div className="tw:flex tw:flex-col tw:gap-1">
228-
<label
229-
className="tw:text-xs tw:font-medium tw:text-muted-foreground tw:uppercase tw:tracking-wide"
230-
htmlFor="metadata-edit-language"
231-
>
219+
<label className="tw:section-label" htmlFor="metadata-edit-language">
232220
{localizedStrings['%interlinearizer_modal_metadata_analysis_language_label%']}
233221
</label>
234222
<input
@@ -339,9 +327,7 @@ function MetadataRow({
339327
}: Readonly<{ label: string; value: string; mono?: boolean }>) {
340328
return (
341329
<div className="tw:flex tw:flex-col tw:gap-0.5">
342-
<dt className="tw:text-xs tw:font-medium tw:text-muted-foreground tw:uppercase tw:tracking-wide">
343-
{label}
344-
</dt>
330+
<dt className="tw:section-label">{label}</dt>
345331
<dd
346332
className={['tw:text-sm tw:break-all tw:text-foreground', mono ? 'tw:font-mono' : '']
347333
.filter(Boolean)

src/components/SegmentView.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,7 @@ export function SegmentView({
4747
onClick={() => onClick?.({ book, chapter, verse })}
4848
type="button"
4949
>
50-
<span className="tw:mb-2 tw:block tw:text-xs tw:font-medium tw:text-muted-foreground tw:uppercase tw:tracking-wide">
51-
{verse}
52-
</span>
50+
<span className="tw:mb-2 tw:block tw:section-label">{verse}</span>
5351
{displayMode === 'baseline-text' ? (
5452
<span className="tw:font-mono tw:text-sm tw:text-foreground">{segment.baselineText}</span>
5553
) : (

src/components/SelectInterlinearProjectModal.tsx

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import papi, { logger } from '@papi/frontend';
22
import { useLocalizedStrings } from '@papi/frontend/react';
33
import { Info } from 'lucide-react';
44
import { Button } from 'platform-bible-react';
5-
import { useCallback, useEffect, useState } from 'react';
5+
import { useCallback, useEffect, useRef, useState } from 'react';
66
import type { InterlinearProject } from 'interlinearizer';
77

88
/** Localized string keys used by {@link SelectInterlinearProjectModal}. */
@@ -98,21 +98,28 @@ export function SelectInterlinearProjectModal({
9898

9999
const [projects, setProjects] = useState<InterlinearProjectSummary[]>([]);
100100
const [isLoading, setIsLoading] = useState(true);
101+
/** Incremented each time a load starts; lets an in-flight response detect it has been superseded. */
102+
const loadGenRef = useRef(0);
101103

102104
/**
103105
* Fetches interlinear projects for `sourceProjectId` and updates the `projects` state. Logs and
104-
* shows a notification on failure.
106+
* shows a notification on failure. Ignores the response if a newer load has started since this
107+
* one was initiated.
105108
*
106109
* @returns A promise that resolves when the project list is loaded or the error notification is
107110
* sent.
108111
*/
109112
const loadProjects = useCallback(async () => {
113+
loadGenRef.current += 1;
114+
const gen = loadGenRef.current;
110115
setIsLoading(true);
116+
setProjects([]);
111117
try {
112118
const json = await papi.commands.sendCommand(
113119
'interlinearizer.getProjectsForSource',
114120
sourceProjectId,
115121
);
122+
if (gen !== loadGenRef.current) return;
116123
const parsed: unknown = JSON.parse(json);
117124
if (!Array.isArray(parsed)) {
118125
logger.warn('Interlinearizer: getProjectsForSource returned non-array', parsed);
@@ -131,7 +138,7 @@ export function SelectInterlinearProjectModal({
131138
.send({ message: '%interlinearizer_error_load_projects_failed%', severity: 'error' })
132139
.catch(() => {});
133140
} finally {
134-
setIsLoading(false);
141+
if (gen === loadGenRef.current) setIsLoading(false);
135142
}
136143
}, [sourceProjectId]);
137144

@@ -148,10 +155,7 @@ export function SelectInterlinearProjectModal({
148155
className="tw:modal-dialog tw:rounded-lg tw:w-lg"
149156
open
150157
>
151-
<h2
152-
id="select-project-modal-title"
153-
className="tw:text-base tw:font-semibold tw:text-foreground tw:mb-4"
154-
>
158+
<h2 id="select-project-modal-title" className="tw:modal-title">
155159
{localizedStrings['%interlinearizer_modal_select_title%']}
156160
</h2>
157161

src/tailwind.css

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,18 @@
3939
@apply tw:static tw:bg-background tw:text-foreground tw:border tw:border-border tw:p-6 tw:shadow-lg;
4040
}
4141

42+
@utility modal-title {
43+
@apply tw:text-base tw:font-semibold tw:text-foreground tw:mb-4;
44+
}
45+
46+
@utility section-label {
47+
@apply tw:text-xs tw:font-medium tw:text-muted-foreground tw:uppercase tw:tracking-wide;
48+
}
49+
50+
@utility icon-button {
51+
@apply tw:z-10 tw:shrink-0 tw:rounded tw:p-1 tw:text-foreground tw:disabled:opacity-30 tw:hover:bg-muted/50;
52+
}
53+
4254
/*
4355
* CUSTOM: Theme variable policy.
4456
*

0 commit comments

Comments
 (0)