From d514cd39188a32313c44c8224d9e307144973611 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 21 Aug 2026 13:40:59 -0600 Subject: [PATCH 1/3] Save As: treat the active project as a save, not an overwrite Overwriting the project the draft is already open on is the same write the Save command performs, so the destructive confirmation was warning about a risk that isn't there. That row now reads Save, writes on the first press, and skips the confirmation entirely; the other rows keep the Overwrite label and its red confirm. A draft with nothing unsaved would write nothing at all, so the active row reports that instead of offering the save, and withdraws the button if an autosave lands while the dialog is open. Without a confirmation in front of it, the button's disabled state is the only thing blocking a double-click from writing twice, so that is now covered by a test. --- contributions/localizedStrings.json | 2 + .../modals/SaveAsProjectModal.test.tsx | 152 ++++++++++++++++++ src/components/modals/ProjectModals.tsx | 4 +- src/components/modals/SaveAsProjectModal.tsx | 53 ++++-- 4 files changed, 199 insertions(+), 12 deletions(-) diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index dd7c949c..9cc101dc 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -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", diff --git a/src/__tests__/components/modals/SaveAsProjectModal.test.tsx b/src/__tests__/components/modals/SaveAsProjectModal.test.tsx index f9932ff4..9343ad94 100644 --- a/src/__tests__/components/modals/SaveAsProjectModal.test.tsx +++ b/src/__tests__/components/modals/SaveAsProjectModal.test.tsx @@ -25,6 +25,8 @@ const LOCALIZED: Record = { '%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', @@ -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(), @@ -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(); + + 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(); + + 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( + , + ); + + 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((resolve) => { + resolveSave = resolve; + }), + ); + mockSendCommand.mockResolvedValue(JSON.stringify([STUB_PROJECT_2])); + render( + , + ); + + 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( + , + ); + + 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( + , + ); + + 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( + , + ); + + 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( + , + ); + + 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( + , + ); + + 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); diff --git a/src/components/modals/ProjectModals.tsx b/src/components/modals/ProjectModals.tsx index ecfad359..7d91b062 100644 --- a/src/components/modals/ProjectModals.tsx +++ b/src/components/modals/ProjectModals.tsx @@ -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" @@ -547,6 +548,7 @@ export default function ProjectModals({ void | Promise; @@ -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) => @@ -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; @@ -192,7 +207,7 @@ 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%'] } @@ -200,14 +215,30 @@ export function SaveAsProjectModal({ unnamedLabel={localizedStrings['%interlinearizer_modal_select_name_unnamed%']} /> - + {isNoOp ? ( + + {localizedStrings['%interlinearizer_modal_saveAs_save_active_clean%']} + + ) : ( + + )} {isConfirming && (
From 2b8ab98721e5458dc13dfdf76a265136605c09c7 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 25 Aug 2026 12:30:14 -0600 Subject: [PATCH 2/3] Align the Save As project rows on a shared action column The per-row flex layout sized each row's action independently, so the project boxes ended at different edges depending on whether the row showed Save, Overwrite, or the nothing-to-save note. --- src/components/modals/SaveAsProjectModal.tsx | 92 ++++++++++---------- 1 file changed, 45 insertions(+), 47 deletions(-) diff --git a/src/components/modals/SaveAsProjectModal.tsx b/src/components/modals/SaveAsProjectModal.tsx index 78dce992..9d12b60b 100644 --- a/src/components/modals/SaveAsProjectModal.tsx +++ b/src/components/modals/SaveAsProjectModal.tsx @@ -180,7 +180,7 @@ export function SaveAsProjectModal({ {localizedStrings['%interlinearizer_modal_saveAs_none%']}

) : ( -
    +
      {projects.map((project) => { const projectName = project.name ?? localizedStrings['%interlinearizer_modal_select_name_unnamed%']; @@ -193,55 +193,53 @@ export function SaveAsProjectModal({ // row, so it is unambiguous which project the confirm will replace. const isConfirming = confirmOverwrite?.id === project.id; return ( -
    • -
      - - +
    • + + + + {isNoOp ? ( + + {localizedStrings['%interlinearizer_modal_saveAs_save_active_clean%']} - {isNoOp ? ( - - {localizedStrings['%interlinearizer_modal_saveAs_save_active_clean%']} - - ) : ( - - )} -
+ ? '%interlinearizer_modal_saveAs_save_active%' + : '%interlinearizer_modal_saveAs_overwrite%' + ] + } + + )} {isConfirming && ( -
+

{projectName} {localizedStrings['%interlinearizer_modal_saveAs_overwrite_confirm_body%']} From 89a5abd89689cdeadb6fc78bb25ae6105655556f Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 25 Aug 2026 13:57:55 -0600 Subject: [PATCH 3/3] Shorten the Save As no-changes label to fit its column --- contributions/localizedStrings.json | 2 +- src/__tests__/components/modals/SaveAsProjectModal.test.tsx | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index 9cc101dc..ff70f772 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -125,7 +125,7 @@ "%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_save_active_clean%": "No changes to save.", "%interlinearizer_modal_saveAs_cancel%": "Cancel", "%interlinearizer_wipe_modal_title%": "Wipe draft analysis", diff --git a/src/__tests__/components/modals/SaveAsProjectModal.test.tsx b/src/__tests__/components/modals/SaveAsProjectModal.test.tsx index 9343ad94..d2f31cd5 100644 --- a/src/__tests__/components/modals/SaveAsProjectModal.test.tsx +++ b/src/__tests__/components/modals/SaveAsProjectModal.test.tsx @@ -26,7 +26,7 @@ const LOCALIZED: Record = { '%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_save_active_clean%': 'No changes to save.', '%interlinearizer_modal_saveAs_cancel%': 'Cancel', '%interlinearizer_modal_select_name_unnamed%': 'Unnamed', '%interlinearizer_modal_select_active_badge%': 'Active', @@ -421,7 +421,7 @@ describe('SaveAsProjectModal', () => { 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).getByText('No changes to save.')).toBeInTheDocument(); expect(within(activeRow).queryByRole('button', { name: 'Save' })).not.toBeInTheDocument(); }); @@ -463,7 +463,7 @@ describe('SaveAsProjectModal', () => { ); expect(screen.queryByRole('button', { name: 'Save' })).not.toBeInTheDocument(); - expect(screen.getByText('No unsaved changes to save.')).toBeInTheDocument(); + expect(screen.getByText('No changes to save.')).toBeInTheDocument(); }); it('logs and notifies when loading the project list rejects', async () => {