Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions contributions/localizedStrings.json
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@
"%interlinearizer_modal_saveAs_overwrite_confirm_body%": "Overwrite this project? Its saved analysis will be replaced with the current draft.",
"%interlinearizer_modal_saveAs_overwrite_confirm_ok%": "Overwrite",
"%interlinearizer_modal_saveAs_overwrite_confirm_cancel%": "Cancel",
"%interlinearizer_modal_saveAs_save_active%": "Save",
"%interlinearizer_modal_saveAs_save_active_clean%": "No unsaved changes to save.",
"%interlinearizer_modal_saveAs_cancel%": "Cancel",

"%interlinearizer_wipe_modal_title%": "Wipe draft analysis",
Expand Down
152 changes: 152 additions & 0 deletions src/__tests__/components/modals/SaveAsProjectModal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ const LOCALIZED: Record<string, string> = {
'%interlinearizer_modal_saveAs_overwrite_confirm_body%': 'Overwrite this project with the draft?',
'%interlinearizer_modal_saveAs_overwrite_confirm_ok%': 'Overwrite',
'%interlinearizer_modal_saveAs_overwrite_confirm_cancel%': 'Keep project',
'%interlinearizer_modal_saveAs_save_active%': 'Save',
'%interlinearizer_modal_saveAs_save_active_clean%': 'No unsaved changes to save.',
'%interlinearizer_modal_saveAs_cancel%': 'Cancel',
'%interlinearizer_modal_select_name_unnamed%': 'Unnamed',
'%interlinearizer_modal_select_active_badge%': 'Active',
Expand All @@ -43,6 +45,7 @@ const STUB_PROJECT_2 = makeProjectSummary({

const defaultProps = {
sourceProjectId: 'src-proj',
hasUnsavedWork: true,
onSaveNew: jest.fn(),
onOverwrite: jest.fn(),
onClose: jest.fn(),
Expand Down Expand Up @@ -314,6 +317,155 @@ describe('SaveAsProjectModal', () => {
expect(screen.queryByText('Overwrite this project with the draft?')).not.toBeInTheDocument();
});

it('labels the active row Save rather than Overwrite', async () => {
mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT, STUB_PROJECT_2]));
render(<SaveAsProjectModal {...defaultProps} activeProjectId={STUB_PROJECT_2.id} />);

await waitFor(() => expect(screen.getByText('French glosses')).toBeInTheDocument());
const activeRow = screen.getByText('French glosses').closest('li');
if (!activeRow) throw new Error('expected the active project row to be present');
expect(within(activeRow).getByRole('button', { name: 'Save' })).toBeInTheDocument();
expect(within(activeRow).queryByRole('button', { name: 'Overwrite' })).not.toBeInTheDocument();
});

it('keeps the Overwrite label on rows that are not the active project', async () => {
mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT, STUB_PROJECT_2]));
render(<SaveAsProjectModal {...defaultProps} activeProjectId={STUB_PROJECT_2.id} />);

await waitFor(() => expect(screen.getByText('Unnamed')).toBeInTheDocument());
const otherRow = screen.getByText('Unnamed').closest('li');
if (!otherRow) throw new Error('expected the non-active project row to be present');
expect(within(otherRow).getByRole('button', { name: 'Overwrite' })).toBeInTheDocument();
});

it('saves the active project on the first press, with no confirmation step', async () => {
const onOverwrite = jest.fn();
mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT_2]));
render(
<SaveAsProjectModal
{...defaultProps}
activeProjectId={STUB_PROJECT_2.id}
onOverwrite={onOverwrite}
/>,
);

await waitFor(() => expect(screen.getByText('French glosses')).toBeInTheDocument());
const activeRow = screen.getByText('French glosses').closest('li');
if (!activeRow) throw new Error('expected the active project row to be present');
await userEvent.click(within(activeRow).getByRole('button', { name: 'Save' }));

expect(onOverwrite).toHaveBeenCalledWith(STUB_PROJECT_2);
expect(screen.queryByTestId('save-as-overwrite-confirm')).not.toBeInTheDocument();
});

it('disables the active row Save while its write is in flight to block duplicate submits', async () => {
let resolveSave: () => void = () => {};
const onOverwrite = jest.fn(
() =>
new Promise<void>((resolve) => {
resolveSave = resolve;
}),
);
mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT_2]));
render(
<SaveAsProjectModal
{...defaultProps}
activeProjectId={STUB_PROJECT_2.id}
onOverwrite={onOverwrite}
/>,
);

await waitFor(() => expect(screen.getByText('French glosses')).toBeInTheDocument());
const saveButton = screen.getByRole('button', { name: 'Save' });
await userEvent.click(saveButton);

// Nothing stands between this button and a duplicate write but its own disabled state: the
// active row writes on the first press, so no confirmation intercepts a double-click.
expect(saveButton).toBeDisabled();
expect(onOverwrite).toHaveBeenCalledTimes(1);

resolveSave();
await waitFor(() => expect(saveButton).not.toBeDisabled());
});

it('still confirms before overwriting a project that is not the active one', async () => {
const onOverwrite = jest.fn();
mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT, STUB_PROJECT_2]));
render(
<SaveAsProjectModal
{...defaultProps}
activeProjectId={STUB_PROJECT_2.id}
onOverwrite={onOverwrite}
/>,
);

await waitFor(() => expect(screen.getByText('Unnamed')).toBeInTheDocument());
const otherRow = screen.getByText('Unnamed').closest('li');
if (!otherRow) throw new Error('expected the non-active project row to be present');
await userEvent.click(within(otherRow).getByRole('button', { name: 'Overwrite' }));

expect(onOverwrite).not.toHaveBeenCalled();
expect(screen.getByText('Overwrite this project with the draft?')).toBeInTheDocument();
});

it('reports nothing to save on the active row when the draft holds no unsaved work', async () => {
mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT_2]));
render(
<SaveAsProjectModal
{...defaultProps}
activeProjectId={STUB_PROJECT_2.id}
hasUnsavedWork={false}
/>,
);

await waitFor(() => expect(screen.getByText('French glosses')).toBeInTheDocument());
const activeRow = screen.getByText('French glosses').closest('li');
if (!activeRow) throw new Error('expected the active project row to be present');
expect(within(activeRow).getByText('No unsaved changes to save.')).toBeInTheDocument();
expect(within(activeRow).queryByRole('button', { name: 'Save' })).not.toBeInTheDocument();
});

it('still offers a write to non-active rows when the draft holds no unsaved work', async () => {
// A clean draft is only a no-op against the project it is already open on; every other project
// holds different content, so overwriting it remains a real write.
mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT, STUB_PROJECT_2]));
render(
<SaveAsProjectModal
{...defaultProps}
activeProjectId={STUB_PROJECT_2.id}
hasUnsavedWork={false}
/>,
);

await waitFor(() => expect(screen.getByText('Unnamed')).toBeInTheDocument());
const otherRow = screen.getByText('Unnamed').closest('li');
if (!otherRow) throw new Error('expected the non-active project row to be present');
expect(within(otherRow).getByRole('button', { name: 'Overwrite' })).toBeInTheDocument();
});

it('withdraws the active row Save when the draft goes clean under the open modal', async () => {
mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT_2]));
const { rerender } = render(
<SaveAsProjectModal {...defaultProps} activeProjectId={STUB_PROJECT_2.id} />,
);

await waitFor(() => expect(screen.getByText('French glosses')).toBeInTheDocument());
expect(screen.getByRole('button', { name: 'Save' })).toBeInTheDocument();

// An autosave landing behind the open modal would otherwise leave a live Save button over a
// write that has nothing left to persist.
rerender(
<SaveAsProjectModal
{...defaultProps}
activeProjectId={STUB_PROJECT_2.id}
hasUnsavedWork={false}
/>,
);

expect(screen.queryByRole('button', { name: 'Save' })).not.toBeInTheDocument();
expect(screen.getByText('No unsaved changes to save.')).toBeInTheDocument();
});

it('logs and notifies when loading the project list rejects', async () => {
const loadError = new Error('network error');
mockSendCommand.mockRejectedValue(loadError);
Expand Down
4 changes: 3 additions & 1 deletion src/components/modals/ProjectModals.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ type PendingReplace =
* @param props.hasUnsavedWork - Whether the draft has unsaved work — either committed-but-unsaved
* changes or uncommitted text still sitting in a gloss input (matching the tab's unsaved marker).
* When true, New / Open are gated behind the discard confirmation so neither kind of unsaved work
* is silently lost by the draft-replacing swap.
* is silently lost by the draft-replacing swap. When false, Save As reports the active project as
* having nothing to save rather than offering a write that would change nothing.
* @param props.getDraftSnapshot - Returns the latest draft envelope (analysis + config) to persist
* on Save As.
* @param props.loadFromProject - Loads a project's analysis + config into the draft (the "Open"
Expand Down Expand Up @@ -547,6 +548,7 @@ export default function ProjectModals({
<SaveAsProjectModal
sourceProjectId={projectId}
activeProjectId={activeProject?.id}
hasUnsavedWork={hasUnsavedWork}
defaultName={draftSnapshot?.suggestedName}
defaultDescription={draftSnapshot?.suggestedDescription}
onSaveNew={handleSaveAsNew}
Expand Down
53 changes: 42 additions & 11 deletions src/components/modals/SaveAsProjectModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const SAVE_AS_MODAL_STRING_KEYS: `%${string}%`[] = [
'%interlinearizer_modal_saveAs_overwrite_confirm_body%',
'%interlinearizer_modal_saveAs_overwrite_confirm_ok%',
'%interlinearizer_modal_saveAs_overwrite_confirm_cancel%',
'%interlinearizer_modal_saveAs_save_active%',
'%interlinearizer_modal_saveAs_save_active_clean%',
'%interlinearizer_modal_saveAs_cancel%',
'%interlinearizer_modal_select_name_unnamed%',
'%interlinearizer_modal_select_active_badge%',
Expand All @@ -34,11 +36,17 @@ const SAVE_AS_MODAL_STRING_KEYS: `%${string}%`[] = [
* component is presentational: it collects the choice and delegates the actual persistence to the
* caller via {@link onSaveNew} / {@link onOverwrite}.
*
* Writing to the project the draft is already open against is an ordinary save rather than a
* clobber, so that row is presented as a save and needs no confirmation; when the draft holds
* nothing unsaved, that same write would change nothing and is not offered at all.
*
* @param props.sourceProjectId - Source project whose existing interlinear projects to list as
* overwrite targets.
* @param props.activeProjectId - ID of the project currently open as the active Save target, if
* any; the matching overwrite target is badged so the user can tell which project the draft is
* currently working against.
* @param props.hasUnsavedWork - Whether the draft holds work not yet written to the active project;
* only meaningful when {@link activeProjectId} names a project in the list.
* @param props.defaultName - Name prefilled into the new-project field (the draft's suggested
* name).
* @param props.defaultDescription - Description prefilled into the new-project field.
Expand All @@ -52,6 +60,7 @@ const SAVE_AS_MODAL_STRING_KEYS: `%${string}%`[] = [
export function SaveAsProjectModal({
sourceProjectId,
activeProjectId,
hasUnsavedWork,
defaultName,
defaultDescription,
onSaveNew,
Expand All @@ -60,6 +69,7 @@ export function SaveAsProjectModal({
}: Readonly<{
sourceProjectId: string;
activeProjectId?: string;
hasUnsavedWork: boolean;
defaultName?: string;
defaultDescription?: string;
onSaveNew: (name?: string, description?: string) => void | Promise<void>;
Expand Down Expand Up @@ -100,8 +110,8 @@ export function SaveAsProjectModal({
);

/**
* Overwrites the chosen existing project with the draft, blocking re-entry while the save is in
* flight so a double-click cannot fire the overwrite (or another save) twice.
* Writes the draft into the chosen existing project, blocking re-entry while the save is in
* flight so a double-click cannot fire the write (or another save) twice.
*/
const handleConfirmOverwrite = useCallback(
(project: InterlinearProjectSummary) =>
Expand Down Expand Up @@ -174,6 +184,11 @@ export function SaveAsProjectModal({
{projects.map((project) => {
const projectName =
project.name ?? localizedStrings['%interlinearizer_modal_select_name_unnamed%'];
// The active project is the one the draft is already open on, so writing to it is the
// plain Save — nothing is at risk and no confirmation is warranted. A clean draft has
// nothing left to write there at all.
const isActive = project.id === activeProjectId;
const isNoOp = isActive && !hasUnsavedWork;
// Show the confirm inline under the row whose Overwrite was pressed, and highlight that
// row, so it is unambiguous which project the confirm will replace.
const isConfirming = confirmOverwrite?.id === project.id;
Expand All @@ -192,22 +207,38 @@ export function SaveAsProjectModal({
localizedStrings['%interlinearizer_modal_select_active_badge%']
}
className="tw:flex-1"
isActive={project.id === activeProjectId}
isActive={isActive}
modifiedPrefix={
localizedStrings['%interlinearizer_modal_select_modified_prefix%']
}
project={project}
unnamedLabel={localizedStrings['%interlinearizer_modal_select_name_unnamed%']}
/>
</span>
<Button
variant="secondary"
size="sm"
onClick={() => setConfirmOverwrite(project)}
disabled={isSubmitting || isConfirming}
>
{localizedStrings['%interlinearizer_modal_saveAs_overwrite%']}
</Button>
{isNoOp ? (
<span className="tw:text-sm tw:text-muted-foreground">
{localizedStrings['%interlinearizer_modal_saveAs_save_active_clean%']}
</span>
) : (
<Button
variant="secondary"
size="sm"
onClick={
isActive
? () => handleConfirmOverwrite(project)
: () => setConfirmOverwrite(project)
}
disabled={isSubmitting || isConfirming}
>
{
localizedStrings[
isActive
? '%interlinearizer_modal_saveAs_save_active%'
: '%interlinearizer_modal_saveAs_overwrite%'
]
}
</Button>
)}
</div>
{isConfirming && (
<div className="tw:modal-error-box tw:p-3">
Expand Down