Mark/Kimi_Agent_Brother_10_Digitizing_App

Fork 0

Editor Save button is enabled even when the design hasn't been edited since the last save — user clicking Save on an unchanged design gets a misleading 'Updated X' toast, no way to tell 'would do nothing' from 'would update' #243

New issue

Closed

opened 2026-10-04 02:36:49 +00:00 by Mark · 1 comment

Mark commented

2026-10-04 02:36:49 +00:00

Owner

Finding

src/components/editor/EditorHeader.tsx's Save button (line 240+) is disabled only when !canSave:

<button
  data-testid="editor-save"
  onClick={onSave}
  disabled={!canSave}
  ...
>
  <Save size={14} />
  Save
</button>

canSave is computed in the Editor as paths.some((p) => p.visible && p.nodes.length > 0) (per #150). It's a "has content" check, not a "has changes" check.

isDirty is tracked separately (per #161, the beforeunload handler) but is NOT used to disable the Save button.

A user who:

Opens a design
Doesn't edit anything
Clicks Save
The save succeeds (returns { updated: true, design })
The toast says "Updated [filename] in Gallery"
The user wonders "what was updated? I didn't change anything"

The toast is technically correct (the row was re-saved), but the action is a no-op semantically.

Where

Save button: src/components/editor/EditorHeader.tsx:240
canSave: src/pages/Editor.tsx (paths have content)
isDirty: src/pages/Editor.tsx (has unsaved changes)
The two are tracked separately but only canSave is read by the button

Suggested fix shape

Either:

Disable the Save button when !isDirty (so a no-op save is impossible to trigger):

<button
  onClick={onSave}
  disabled={!canSave || !isDirty}
>
  <Save size={14} />
  Save
</button>

Replace the "Updated" toast with a "Nothing to update" message when the design is unchanged:

if (!isDirty) {
  addToast('info', 'No changes to save');
  return;
}

Both (button disabled + toast variant).

The polish move is option 1: the user sees a disabled Save button when there are no changes, the visual cue that "you don't need to save."

The flag should be on the button's "Save" not the dropdown "Save" — when the design is dirty, the dropdown's "Save" and "Save as New" both work; when not dirty, only "Save as New" is meaningful (it forks the unchanged design). The button's "Save" disabled in option 1 handles the "save same content" case.

Severity

Low. The user can ignore the toast. Power users notice the misleading "Updated" message.

Discovered by

Code review (2026-10-02) of editor save flow.

#215 (open) — Editor has no 'last saved' indicator. Same surface; the indicator would also tell the user "nothing to save."
#161 (open) — Editor beforeunload. The isDirty flag is the same state.
The "Save" button text is just "Save" — per #215 the user wants "Save *" when dirty. The disable is the more aggressive fix.

## Finding `src/components/editor/EditorHeader.tsx`'s Save button (line 240+) is disabled only when `!canSave`: ```tsx <button data-testid="editor-save" onClick={onSave} disabled={!canSave} ... > <Save size={14} /> Save </button> ``` `canSave` is computed in the Editor as `paths.some((p) => p.visible && p.nodes.length > 0)` (per #150). It's a "has content" check, not a "has changes" check. `isDirty` is tracked separately (per #161, the `beforeunload` handler) but is NOT used to disable the Save button. A user who: - Opens a design - Doesn't edit anything - Clicks Save - The save succeeds (returns `{ updated: true, design }`) - The toast says "Updated [filename] in Gallery" - The user wonders "what was updated? I didn't change anything" The toast is technically correct (the row was re-saved), but the action is a no-op semantically. ## Where - Save button: `src/components/editor/EditorHeader.tsx:240` - canSave: `src/pages/Editor.tsx` (paths have content) - isDirty: `src/pages/Editor.tsx` (has unsaved changes) - The two are tracked separately but only canSave is read by the button ## Suggested fix shape Either: 1. **Disable the Save button when !isDirty** (so a no-op save is impossible to trigger): ```tsx <button onClick={onSave} disabled={!canSave || !isDirty} > <Save size={14} /> Save </button> ``` 2. **Replace the "Updated" toast with a "Nothing to update" message** when the design is unchanged: ```tsx if (!isDirty) { addToast('info', 'No changes to save'); return; } ``` 3. **Both** (button disabled + toast variant). The polish move is option 1: the user sees a disabled Save button when there are no changes, the visual cue that "you don't need to save." The flag should be on the button's "Save" not the dropdown "Save" — when the design is dirty, the dropdown's "Save" and "Save as New" both work; when not dirty, only "Save as New" is meaningful (it forks the unchanged design). The button's "Save" disabled in option 1 handles the "save same content" case. ## Severity Low. The user can ignore the toast. Power users notice the misleading "Updated" message. ## Discovered by Code review (2026-10-02) of editor save flow. ## Related - #215 (open) — Editor has no 'last saved' indicator. Same surface; the indicator would also tell the user "nothing to save." - #161 (open) — Editor beforeunload. The `isDirty` flag is the same state. - The "Save" button text is just "Save" — per #215 the user wants "Save *" when dirty. The disable is the more aggressive fix.

Mark added the

feature-request

triage-approved

labels

2026-10-04 02:36:49 +00:00

Mark referenced this issue

2026-10-04 03:49:45 +00:00

Editor's session restore (per #161) restores the editor's state from sessionStorage, but the gallery designs list is in localStorage — user with a stale localStorage (e.g., cleared it) gets a 'design not found' error in the editor with no graceful re… #309

Mark referenced this issue

2026-10-04 03:50:03 +00:00

Editor has no 'node' input modal — user wanting to precisely set a node's (x, y) coordinates (vs dragging) has to drag and read the right panel, no in-canvas input field #311

Mark referenced this issue

2026-10-04 03:56:29 +00:00

Editor's Path Properties has a position display but no 'x, y' input for the path — user wanting to nudge a path by exact mm has to drag, no input field #353

Mark referenced this issue from a commit

2026-10-04 09:06:16 +00:00

fix(#243): disable Save button when design has no unsaved edits — prevents misleading 'Updated X' toast on no-op saves

Mark commented

2026-10-04 09:06:35 +00:00

Author

Owner

Closed by commit 35e9e7a on main.

The Save button (EditorHeader.tsx) is now also gated on isDirty, not just canSave. A clean user clicking Save on an unchanged design no longer triggers the misleading "Updated X" toast.

disabled={!canSave || !isDirty}

Save-as-New split stays accessible — forking an unchanged design is meaningful. 3 new tests in EditorHeader.test.tsx (50/50 pass). ESLint clean.

Closed by commit 35e9e7a on main. The Save button (EditorHeader.tsx) is now also gated on `isDirty`, not just `canSave`. A clean user clicking Save on an unchanged design no longer triggers the misleading "Updated X" toast. ```tsx disabled={!canSave || !isDirty} ``` Save-as-New split stays accessible — forking an unchanged design is meaningful. 3 new tests in EditorHeader.test.tsx (50/50 pass). ESLint clean.

Mark closed this issue

2026-10-04 09:06:47 +00:00