diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index dd7c949c..ff70f772 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 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..d2f31cd5 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 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 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 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) => @@ -170,47 +180,66 @@ export function SaveAsProjectModal({ {localizedStrings['%interlinearizer_modal_saveAs_none%']}

) : ( -
    +
      {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; return ( -
    • -
      - - +
    • + + + + {isNoOp ? ( + + {localizedStrings['%interlinearizer_modal_saveAs_save_active_clean%']} + ) : ( - + )} {isConfirming && ( -
      +

      {projectName} {localizedStrings['%interlinearizer_modal_saveAs_overwrite_confirm_body%']}