Skip to content

feat: expose clearHistory() to reset the undo/redo stack #373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SSShooter merged 6 commits into from
Mar 11, 2026
Merged

feat: expose clearHistory() to reset the undo/redo stack #373

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cb4d6a0
feat: expose clearHistory() method on MindElixir instance
DisciplinedSoftware Mar 10, 2026
e6e688b
feat: add clearHistory() to MindElixirInstance type declaration
DisciplinedSoftware Mar 10, 2026
601dc4b
fix: reset currentSelectedNodes in clearHistory() and add missing rev…
DisciplinedSoftware Mar 10, 2026
15a42c9
fix: mark clearHistory as optional in MindElixirInstance
DisciplinedSoftware Mar 10, 2026
1e50bf0
test: add Playwright tests for clearHistory()
DisciplinedSoftware Mar 10, 2026
90b0e0e
fix: use mei.clearSelection() in clearHistory and fix tests
Mar 11, 2026
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
5 changes: 5 additions & 0 deletions src/plugin/operationHistory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ export default function (mei: MindElixirInstance) {
}
}
}
mei.clearHistory = function () {
history = []
currentIndex = -1
current = mei.getData()

Copilot AI Mar 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clearHistory() clears the history stack but leaves currentSelectedNodes unchanged. Since the intended usage is right after refresh(), selection is typically cleared, and the next operation may incorrectly record the previous diagram’s selected node IDs in currentSelected, which can break selection restoration on undo/redo. Reset currentSelectedNodes inside clearHistory() (e.g., derive it from mei.currentNodes) or update the selection tracking to handle deselection events as well.

Suggested change
current = mei.getData()
current = mei.getData()
// keep cached selection in sync with current diagram state
currentSelectedNodes = mei.currentNodes.map(n => n.nodeObj)

Copilot uses AI. Check for mistakes.

@DisciplinedSoftware DisciplinedSoftware Mar 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 601dc4b — added currentSelectedNodes = [] to clearHistory() so stale node IDs from the previous diagram cannot affect selection restoration after refresh() + clearHistory().

}
Comment on lines +91 to +96

Copilot AI Mar 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No automated test is added for the new clearHistory() behavior. There are existing Playwright specs covering operation history/undo/redo; please add a test that (1) performs some operations, (2) calls refresh(newData) + clearHistory(), then (3) asserts that undo/redo cannot revert into the pre-refresh diagram and that the first undo baseline is the refreshed data.

Copilot uses AI. Check for mistakes.

@DisciplinedSoftware DisciplinedSoftware Mar 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 1e50bf0 — new file tests/clear-history.spec.ts with three Playwright tests:

  1. Undo cannot revert into the pre-refresh diagram — performs ops on diagram A, calls refresh(B) + clearHistory(), then asserts Ctrl+Z/Ctrl+Y are no-ops across the diagram boundary.
  2. Operations after clearHistory() are undoable normally — verifies that new operations on diagram B undo/redo correctly, and that the stack doesn't reach back into diagram A.
  3. First undo baseline is the refreshed state — verifies that undoing the first post-refresh operation lands exactly on the refreshed diagram, not an earlier one.

const handleOperation = function (operation: Operation) {
if (operation.name === 'beginEdit') return
history = history.slice(0, currentIndex + 1)
Expand Down
7 changes: 7 additions & 0 deletions src/types/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,13 @@ export interface MindElixirInstance extends Omit<Required<Options>, 'markdown' |
history: Operation[]
undo: () => void
redo: () => void
/**
* Reset the undo/redo stack and update the internal baseline snapshot to the
* current diagram state. Call this after loading new data into an existing
* instance (e.g. after `refresh()`) to prevent users from undoing back into
* a previously loaded diagram.
*/
clearHistory: () => void

@gemini-code-assist gemini-code-assist Bot Mar 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The clearHistory method is declared as mandatory in the MindElixirInstance interface. However, its implementation in src/plugin/operationHistory.ts is only applied to the instance if the allowUndo option is true (as seen in src/methods.ts, line 94). If allowUndo is false, mei.clearHistory will not be defined at runtime, which could lead to a TypeError if the method is called, despite the type definition indicating its presence. To prevent this type-runtime mismatch, clearHistory should be marked as optional in the interface.

Suggested change
/**
* Reset the undo/redo stack and update the internal baseline snapshot to the
* current diagram state. Call this after loading new data into an existing
* instance (e.g. after `refresh()`) to prevent users from undoing back into
* a previously loaded diagram.
*/
clearHistory: () => void
/**
* Reset the undo/redo stack and update the internal baseline snapshot to the
* current diagram state. Call this after loading new data into an existing
* instance (e.g. after `refresh()`) to prevent users from undoing back into
* a previously loaded diagram.
*/
clearHistory?: () => void

@DisciplinedSoftware DisciplinedSoftware Mar 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 15a42c9 — changed to clearHistory?: () => void and updated the JSDoc to note that it is only available when allowUndo is true.

Copilot AI Mar 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clearHistory is declared as a required method on MindElixirInstance, but it’s only assigned inside the operationHistory plugin (and that plugin is skipped when allowUndo is false and in lite mode). This makes the type contract unsafe for consumers because mind.clearHistory() can be undefined at runtime. Consider either (a) making clearHistory optional in the interface, or (b) defining a default no-op implementation on the instance/prototype so it always exists regardless of allowUndo/build mode.

Suggested change
clearHistory: () => void
clearHistory?: () => void

Copilot uses AI. Check for mistakes.

@DisciplinedSoftware DisciplinedSoftware Mar 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 15a42c9 — changed to clearHistory?: () => void and updated the JSDoc to note that it is only available when allowUndo is true.


selection: SelectionArea
dragMoveHelper: ReturnType<typeof createDragMoveHelper>
Expand Down
Loading