From 8e65f2ce4b11b7f76369a3c8b1f61a8fb4114b5a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 17 Jun 2026 19:32:29 +0000 Subject: [PATCH] Refactor: extract annotation undo/redo into CanvasHistory EditorView held the annotation undo/redo as two loose @State arrays plus inline push/cap/clear-redo and pop/apply logic. Move the stack ownership and rules into a testable CanvasHistory value type; the view records snapshots and applies the state CanvasHistory returns. Behavior-preserving (image-level undo stacks are unchanged). Adds CanvasHistoryTests (7 cases). --- Sources/Stag/Views/Editor/CanvasHistory.swift | 42 ++++++++++++ Sources/Stag/Views/Editor/EditorView.swift | 21 +++--- Tests/StagTests/CanvasHistoryTests.swift | 67 +++++++++++++++++++ 3 files changed, 117 insertions(+), 13 deletions(-) create mode 100644 Sources/Stag/Views/Editor/CanvasHistory.swift create mode 100644 Tests/StagTests/CanvasHistoryTests.swift diff --git a/Sources/Stag/Views/Editor/CanvasHistory.swift b/Sources/Stag/Views/Editor/CanvasHistory.swift new file mode 100644 index 0000000..d402398 --- /dev/null +++ b/Sources/Stag/Views/Editor/CanvasHistory.swift @@ -0,0 +1,42 @@ +import Foundation + +/// Bounded undo/redo history for the editor's annotation canvas. Owns the two +/// `CanvasState` stacks that previously lived as loose `@State` arrays inside +/// `EditorView`, along with the push/cap/clear-redo rule. Recording a new state +/// clears the redo stack; `undo`/`redo` swap the supplied "current" state across +/// the stacks and return the state the caller should apply (or `nil`). +struct CanvasHistory { + private(set) var undoStack: [CanvasState] = [] + private(set) var redoStack: [CanvasState] = [] + let limit: Int + + init(limit: Int = 100) { self.limit = limit } + + var canUndo: Bool { !undoStack.isEmpty } + var canRedo: Bool { !redoStack.isEmpty } + + /// Pushes a snapshot onto the undo stack (capped at `limit`) and clears redo. + mutating func record(_ state: CanvasState) { + undoStack.append(state) + if undoStack.count > limit { + undoStack.removeFirst(undoStack.count - limit) + } + redoStack.removeAll() + } + + /// Pops the previous state, pushing `current` onto the redo stack. Returns the + /// state to apply, or `nil` when there is nothing to undo. + mutating func undo(current: CanvasState) -> CanvasState? { + guard let previous = undoStack.popLast() else { return nil } + redoStack.append(current) + return previous + } + + /// Pops the next redo state, pushing `current` onto the undo stack. Returns the + /// state to apply, or `nil` when there is nothing to redo. + mutating func redo(current: CanvasState) -> CanvasState? { + guard let next = redoStack.popLast() else { return nil } + undoStack.append(current) + return next + } +} diff --git a/Sources/Stag/Views/Editor/EditorView.swift b/Sources/Stag/Views/Editor/EditorView.swift index 52f1fbb..cddc76e 100644 --- a/Sources/Stag/Views/Editor/EditorView.swift +++ b/Sources/Stag/Views/Editor/EditorView.swift @@ -18,8 +18,7 @@ struct EditorView: View { } // Undo / Redo - @State private var undoStack: [CanvasState] = [] - @State private var redoStack: [CanvasState] = [] + @State private var canvasHistory = CanvasHistory() // Canvas state @State private var annotations: [Annotation] = [] @@ -1227,9 +1226,7 @@ struct EditorView: View { // MARK: - Undo / Redo / Delete private func pushUndo() { - undoStack.append(CanvasState(annotations: annotations, currentTool: currentTool, selectedAnnotationId: selectedAnnotationId, rotation: rotation)) - if undoStack.count > 100 { undoStack.removeFirst() } - redoStack.removeAll() + canvasHistory.record(CanvasState(annotations: annotations, currentTool: currentTool, selectedAnnotationId: selectedAnnotationId, rotation: rotation)) } private func pushUndoImage() { @@ -1244,9 +1241,8 @@ struct EditorView: View { imageRedoStack.append(workingImage) workingImage = imageUndoStack.removeLast() } - guard !undoStack.isEmpty else { return } - redoStack.append(CanvasState(annotations: annotations, currentTool: currentTool, selectedAnnotationId: selectedAnnotationId, rotation: rotation)) - let state = undoStack.removeLast() + let current = CanvasState(annotations: annotations, currentTool: currentTool, selectedAnnotationId: selectedAnnotationId, rotation: rotation) + guard let state = canvasHistory.undo(current: current) else { return } annotations = state.annotations currentTool = state.currentTool selectedAnnotationId = state.selectedAnnotationId @@ -1258,9 +1254,8 @@ struct EditorView: View { imageUndoStack.append(workingImage) workingImage = imageRedoStack.removeLast() } - guard !redoStack.isEmpty else { return } - undoStack.append(CanvasState(annotations: annotations, currentTool: currentTool, selectedAnnotationId: selectedAnnotationId, rotation: rotation)) - let state = redoStack.removeLast() + let current = CanvasState(annotations: annotations, currentTool: currentTool, selectedAnnotationId: selectedAnnotationId, rotation: rotation) + guard let state = canvasHistory.redo(current: current) else { return } annotations = state.annotations currentTool = state.currentTool selectedAnnotationId = state.selectedAnnotationId @@ -1709,8 +1704,8 @@ struct EditorView: View { private var actionButtons: some View { HStack(spacing: 1) { - actionButton("arrow.uturn.backward", "Undo (⌘Z)", undo, !undoStack.isEmpty) - actionButton("arrow.uturn.forward", "Redo (⇧⌘Z)", redo, !redoStack.isEmpty) + actionButton("arrow.uturn.backward", "Undo (⌘Z)", undo, canvasHistory.canUndo) + actionButton("arrow.uturn.forward", "Redo (⇧⌘Z)", redo, canvasHistory.canRedo) actionButton("trash", "Delete (⌫)", deleteSelected, selectedAnnotationId != nil) toolbarDivider actionButton("doc.on.clipboard", "Copy (⌘C)", exportAndCopy, true) diff --git a/Tests/StagTests/CanvasHistoryTests.swift b/Tests/StagTests/CanvasHistoryTests.swift new file mode 100644 index 0000000..b86d8e5 --- /dev/null +++ b/Tests/StagTests/CanvasHistoryTests.swift @@ -0,0 +1,67 @@ +import XCTest +@testable import Stag + +/// Bounded undo/redo history extracted from EditorView. `rotation` is used as a +/// per-state tag (CanvasState's own `==` only compares annotation ids, so the +/// tests assert on the returned state's fields instead). +final class CanvasHistoryTests: XCTestCase { + + private func state(_ tag: CGFloat) -> CanvasState { + CanvasState(annotations: [], currentTool: .arrow, selectedAnnotationId: nil, rotation: tag) + } + + func testStartsEmpty() { + let h = CanvasHistory() + XCTAssertFalse(h.canUndo) + XCTAssertFalse(h.canRedo) + } + + func testRecordEnablesUndoAndClearsRedoState() { + var h = CanvasHistory() + h.record(state(1)) + XCTAssertTrue(h.canUndo) + XCTAssertFalse(h.canRedo) + } + + func testUndoReturnsPreviousAndEnablesRedo() { + var h = CanvasHistory() + h.record(state(1)) + let applied = h.undo(current: state(2)) + XCTAssertEqual(applied?.rotation, 1) + XCTAssertTrue(h.canRedo) + XCTAssertFalse(h.canUndo) + } + + func testRedoReturnsSwappedState() { + var h = CanvasHistory() + h.record(state(1)) + _ = h.undo(current: state(2)) // redo now holds the "2" state + let redone = h.redo(current: state(1)) + XCTAssertEqual(redone?.rotation, 2) + XCTAssertTrue(h.canUndo) + } + + func testRecordingClearsRedoStack() { + var h = CanvasHistory() + h.record(state(1)) + _ = h.undo(current: state(2)) + XCTAssertTrue(h.canRedo) + h.record(state(3)) + XCTAssertFalse(h.canRedo) + } + + func testUndoRedoOnEmptyReturnNil() { + var h = CanvasHistory() + XCTAssertNil(h.undo(current: state(1))) + XCTAssertNil(h.redo(current: state(1))) + } + + func testCapDropsOldestStates() { + var h = CanvasHistory(limit: 3) + for i in 1...5 { h.record(state(CGFloat(i))) } // retains 3,4,5 + XCTAssertEqual(h.undo(current: state(99))?.rotation, 5) + XCTAssertEqual(h.undo(current: state(98))?.rotation, 4) + XCTAssertEqual(h.undo(current: state(97))?.rotation, 3) + XCTAssertNil(h.undo(current: state(0))) // 1 and 2 were dropped + } +}