Implement Phase 6: undo/redo with command pattern

- UndoableAction interface with AddStrokeAction and DeleteStrokeAction - UndoManager: undo/redo stacks (depth 50), canUndo/canRedo StateFlows - EditorViewModel: stroke operations routed through UndoManager, visual callbacks sync canvas view without full DB reload - Toolbar: undo/redo buttons with enabled state - 9 unit tests for UndoManager Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-24 14:40:24 -07:00
parent 7cf779934d
commit 5eeedff464
9 changed files with 326 additions and 13 deletions
--- a/app/src/main/kotlin/net/metacircular/engpad/ui/editor/EditorScreen.kt
+++ b/app/src/main/kotlin/net/metacircular/engpad/ui/editor/EditorScreen.kt
@@ -13,6 +13,7 @@ import androidx.compose.ui.platform.LocalContext
 import androidx.compose.ui.viewinterop.AndroidView
 import androidx.lifecycle.viewmodel.compose.viewModel
 import net.metacircular.engpad.data.db.EngPadDatabase
+import net.metacircular.engpad.data.db.toFloatArray
 import net.metacircular.engpad.data.model.PageSize
 import net.metacircular.engpad.data.repository.PageRepository

@@ -30,6 +31,8 @@ fun EditorScreen(
    )
    val canvasState by viewModel.canvasState.collectAsState()
    val strokes by viewModel.strokes.collectAsState()
+    val canUndo by viewModel.undoManager.canUndo.collectAsState()
+    val canRedo by viewModel.undoManager.canRedo.collectAsState()

    val context = LocalContext.current
    val canvasView = remember { PadCanvasView(context) }
@@ -55,12 +58,26 @@ fun EditorScreen(
        canvasView.onStrokeErased = { strokeId ->
            viewModel.onStrokeErased(strokeId)
        }
+        // Wire undo/redo visual callbacks
+        viewModel.onStrokeAdded = { stroke ->
+            canvasView.addCompletedStroke(
+                stroke.id, stroke.penSize, stroke.color,
+                stroke.pointData.toFloatArray(),
+            )
+        }
+        viewModel.onStrokeRemoved = { id ->
+            canvasView.removeStroke(id)
+        }
    }

    Column(modifier = Modifier.fillMaxSize()) {
        EditorToolbar(
            currentTool = canvasState.tool,
            onToolSelected = { viewModel.setTool(it) },
+            canUndo = canUndo,
+            canRedo = canRedo,
+            onUndo = { viewModel.undo() },
+            onRedo = { viewModel.redo() },
            modifier = Modifier.fillMaxWidth(),
        )
        AndroidView(
--- a/app/src/main/kotlin/net/metacircular/engpad/ui/editor/EditorViewModel.kt
+++ b/app/src/main/kotlin/net/metacircular/engpad/ui/editor/EditorViewModel.kt
@@ -11,6 +11,9 @@ import net.metacircular.engpad.data.db.toBlob
 import net.metacircular.engpad.data.model.PageSize
 import net.metacircular.engpad.data.model.Stroke
 import net.metacircular.engpad.data.repository.PageRepository
+import net.metacircular.engpad.undo.AddStrokeAction
+import net.metacircular.engpad.undo.DeleteStrokeAction
+import net.metacircular.engpad.undo.UndoManager

 class EditorViewModel(
    private val pageId: Long,
@@ -24,6 +27,12 @@ class EditorViewModel(
    private val _strokes = MutableStateFlow<List<Stroke>>(emptyList())
    val strokes: StateFlow<List<Stroke>> = _strokes

+    val undoManager = UndoManager()
+
+    // Callbacks for the canvas view to add/remove strokes visually
+    var onStrokeAdded: ((Stroke) -> Unit)? = null
+    var onStrokeRemoved: ((Long) -> Unit)? = null
+
    init {
        loadStrokes()
    }
@@ -39,9 +48,22 @@ class EditorViewModel(
    }

    fun onStrokeErased(strokeId: Long) {
+        val stroke = _strokes.value.find { it.id == strokeId } ?: return
        viewModelScope.launch {
-            pageRepository.deleteStroke(strokeId)
-            _strokes.value = pageRepository.getStrokes(pageId)
+            undoManager.perform(
+                DeleteStrokeAction(
+                    stroke = stroke,
+                    repository = pageRepository,
+                    onExecute = { id ->
+                        _strokes.value = _strokes.value.filter { it.id != id }
+                        onStrokeRemoved?.invoke(id)
+                    },
+                    onUndo = { restoredStroke ->
+                        _strokes.value = _strokes.value + restoredStroke
+                        onStrokeAdded?.invoke(restoredStroke)
+                    },
+                )
+            )
        }
    }

@@ -60,12 +82,31 @@ class EditorViewModel(
                strokeOrder = order,
                createdAt = System.currentTimeMillis(),
            )
-            val id = pageRepository.addStroke(stroke)
-            // Refresh strokes from DB to get the assigned ID
-            _strokes.value = pageRepository.getStrokes(pageId)
+            undoManager.perform(
+                AddStrokeAction(
+                    stroke = stroke,
+                    repository = pageRepository,
+                    onExecute = { addedStroke ->
+                        _strokes.value = _strokes.value + addedStroke
+                        onStrokeAdded?.invoke(addedStroke)
+                    },
+                    onUndo = { id ->
+                        _strokes.value = _strokes.value.filter { it.id != id }
+                        onStrokeRemoved?.invoke(id)
+                    },
+                )
+            )
        }
    }

+    fun undo() {
+        viewModelScope.launch { undoManager.undo() }
+    }
+
+    fun redo() {
+        viewModelScope.launch { undoManager.redo() }
+    }
+
    class Factory(
        private val pageId: Long,
        private val pageSize: PageSize,
--- a/app/src/main/kotlin/net/metacircular/engpad/ui/editor/Toolbar.kt
+++ b/app/src/main/kotlin/net/metacircular/engpad/ui/editor/Toolbar.kt
@@ -1,10 +1,14 @@
 package net.metacircular.engpad.ui.editor

 import androidx.compose.foundation.layout.Row
+import androidx.compose.foundation.layout.Spacer
 import androidx.compose.foundation.layout.padding
+import androidx.compose.foundation.layout.width
 import androidx.compose.material3.FilterChip
 import androidx.compose.material3.Text
+import androidx.compose.material3.TextButton
 import androidx.compose.runtime.Composable
+import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.unit.dp

@@ -12,9 +16,16 @@ import androidx.compose.ui.unit.dp
 fun EditorToolbar(
    currentTool: Tool,
    onToolSelected: (Tool) -> Unit,
+    canUndo: Boolean,
+    canRedo: Boolean,
+    onUndo: () -> Unit,
+    onRedo: () -> Unit,
    modifier: Modifier = Modifier,
 ) {
-    Row(modifier = modifier.padding(horizontal = 8.dp, vertical = 4.dp)) {
+    Row(
+        modifier = modifier.padding(horizontal = 8.dp, vertical = 4.dp),
+        verticalAlignment = Alignment.CenterVertically,
+    ) {
        FilterChip(
            selected = currentTool == Tool.PEN_FINE,
            onClick = { onToolSelected(Tool.PEN_FINE) },
@@ -33,5 +44,13 @@ fun EditorToolbar(
            label = { Text("Eraser") },
            modifier = Modifier.padding(end = 4.dp),
        )
+        Spacer(modifier = Modifier.weight(1f))
+        TextButton(onClick = onUndo, enabled = canUndo) {
+            Text("Undo")
+        }
+        Spacer(modifier = Modifier.width(4.dp))
+        TextButton(onClick = onRedo, enabled = canRedo) {
+            Text("Redo")
+        }
    }
 }
--- a/app/src/main/kotlin/net/metacircular/engpad/undo/StrokeActions.kt
+++ b/app/src/main/kotlin/net/metacircular/engpad/undo/StrokeActions.kt
@@ -0,0 +1,43 @@
+package net.metacircular.engpad.undo
+
+import net.metacircular.engpad.data.model.Stroke
+import net.metacircular.engpad.data.repository.PageRepository
+
+class AddStrokeAction(
+    private val stroke: Stroke,
+    private val repository: PageRepository,
+    private val onExecute: (Stroke) -> Unit,
+    private val onUndo: (Long) -> Unit,
+) : UndoableAction {
+    override val description = "Add stroke"
+    private var insertedId: Long = 0
+
+    override suspend fun execute() {
+        insertedId = repository.addStroke(stroke)
+        onExecute(stroke.copy(id = insertedId))
+    }
+
+    override suspend fun undo() {
+        repository.deleteStroke(insertedId)
+        onUndo(insertedId)
+    }
+}
+
+class DeleteStrokeAction(
+    private val stroke: Stroke,
+    private val repository: PageRepository,
+    private val onExecute: (Long) -> Unit,
+    private val onUndo: (Stroke) -> Unit,
+) : UndoableAction {
+    override val description = "Delete stroke"
+
+    override suspend fun execute() {
+        repository.deleteStroke(stroke.id)
+        onExecute(stroke.id)
+    }
+
+    override suspend fun undo() {
+        val id = repository.addStroke(stroke)
+        onUndo(stroke.copy(id = id))
+    }
+}
--- a/app/src/main/kotlin/net/metacircular/engpad/undo/UndoManager.kt
+++ b/app/src/main/kotlin/net/metacircular/engpad/undo/UndoManager.kt
@@ -0,0 +1,55 @@
+package net.metacircular.engpad.undo
+
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.StateFlow
+
+class UndoManager(private val maxDepth: Int = MAX_DEPTH) {
+
+    private val undoStack = ArrayDeque<UndoableAction>()
+    private val redoStack = ArrayDeque<UndoableAction>()
+
+    private val _canUndo = MutableStateFlow(false)
+    val canUndo: StateFlow<Boolean> = _canUndo
+
+    private val _canRedo = MutableStateFlow(false)
+    val canRedo: StateFlow<Boolean> = _canRedo
+
+    suspend fun perform(action: UndoableAction) {
+        action.execute()
+        undoStack.addLast(action)
+        if (undoStack.size > maxDepth) {
+            undoStack.removeFirst()
+        }
+        redoStack.clear()
+        updateState()
+    }
+
+    suspend fun undo() {
+        val action = undoStack.removeLastOrNull() ?: return
+        action.undo()
+        redoStack.addLast(action)
+        updateState()
+    }
+
+    suspend fun redo() {
+        val action = redoStack.removeLastOrNull() ?: return
+        action.execute()
+        undoStack.addLast(action)
+        updateState()
+    }
+
+    fun clear() {
+        undoStack.clear()
+        redoStack.clear()
+        updateState()
+    }
+
+    private fun updateState() {
+        _canUndo.value = undoStack.isNotEmpty()
+        _canRedo.value = redoStack.isNotEmpty()
+    }
+
+    companion object {
+        const val MAX_DEPTH = 50
+    }
+}
--- a/app/src/main/kotlin/net/metacircular/engpad/undo/UndoableAction.kt
+++ b/app/src/main/kotlin/net/metacircular/engpad/undo/UndoableAction.kt
@@ -0,0 +1,7 @@
+package net.metacircular.engpad.undo
+
+interface UndoableAction {
+    val description: String
+    suspend fun execute()
+    suspend fun undo()
+}
--- a/app/src/test/kotlin/net/metacircular/engpad/undo/UndoManagerTest.kt
+++ b/app/src/test/kotlin/net/metacircular/engpad/undo/UndoManagerTest.kt
@@ -0,0 +1,122 @@
+package net.metacircular.engpad.undo
+
+import kotlinx.coroutines.test.runTest
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+
+class UndoManagerTest {
+
+    private lateinit var manager: UndoManager
+    private val log = mutableListOf<String>()
+
+    @Before
+    fun setup() {
+        manager = UndoManager()
+        log.clear()
+    }
+
+    private fun action(name: String) = object : UndoableAction {
+        override val description = name
+        override suspend fun execute() { log.add("do:$name") }
+        override suspend fun undo() { log.add("undo:$name") }
+    }
+
+    @Test
+    fun `initial state has no undo or redo`() {
+        assertFalse(manager.canUndo.value)
+        assertFalse(manager.canRedo.value)
+    }
+
+    @Test
+    fun `perform adds to undo stack`() = runTest {
+        manager.perform(action("a"))
+        assertTrue(manager.canUndo.value)
+        assertFalse(manager.canRedo.value)
+        assertEquals(listOf("do:a"), log)
+    }
+
+    @Test
+    fun `undo reverses last action`() = runTest {
+        manager.perform(action("a"))
+        manager.undo()
+        assertFalse(manager.canUndo.value)
+        assertTrue(manager.canRedo.value)
+        assertEquals(listOf("do:a", "undo:a"), log)
+    }
+
+    @Test
+    fun `redo re-executes undone action`() = runTest {
+        manager.perform(action("a"))
+        manager.undo()
+        manager.redo()
+        assertTrue(manager.canUndo.value)
+        assertFalse(manager.canRedo.value)
+        assertEquals(listOf("do:a", "undo:a", "do:a"), log)
+    }
+
+    @Test
+    fun `new action clears redo stack`() = runTest {
+        manager.perform(action("a"))
+        manager.perform(action("b"))
+        manager.undo()
+        assertTrue(manager.canRedo.value)
+        manager.perform(action("c"))
+        assertFalse(manager.canRedo.value)
+    }
+
+    @Test
+    fun `multiple undo and redo`() = runTest {
+        manager.perform(action("a"))
+        manager.perform(action("b"))
+        manager.perform(action("c"))
+        manager.undo()
+        manager.undo()
+        assertEquals(listOf("do:a", "do:b", "do:c", "undo:c", "undo:b"), log)
+        assertTrue(manager.canUndo.value)
+        assertTrue(manager.canRedo.value)
+        manager.redo()
+        assertEquals("do:b", log.last())
+    }
+
+    @Test
+    fun `stack respects max depth`() = runTest {
+        val smallManager = UndoManager(maxDepth = 3)
+        smallManager.perform(action("a"))
+        smallManager.perform(action("b"))
+        smallManager.perform(action("c"))
+        smallManager.perform(action("d"))
+        // "a" should have been evicted
+        smallManager.undo() // undo d
+        smallManager.undo() // undo c
+        smallManager.undo() // undo b
+        smallManager.undo() // nothing — a was evicted
+        assertFalse(smallManager.canUndo.value)
+    }
+
+    @Test
+    fun `clear empties both stacks`() = runTest {
+        manager.perform(action("a"))
+        manager.undo()
+        assertTrue(manager.canRedo.value)
+        manager.clear()
+        assertFalse(manager.canUndo.value)
+        assertFalse(manager.canRedo.value)
+    }
+
+    @Test
+    fun `undo on empty stack is no-op`() = runTest {
+        manager.undo()
+        assertFalse(manager.canUndo.value)
+        assertTrue(log.isEmpty())
+    }
+
+    @Test
+    fun `redo on empty stack is no-op`() = runTest {
+        manager.redo()
+        assertFalse(manager.canRedo.value)
+        assertTrue(log.isEmpty())
+    }
+}