diff --git a/PROGRESS.md b/PROGRESS.md index a27c910..edd8d2e 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -71,9 +71,18 @@ See PROJECT_PLAN.md for the full step list. processes historical touch points for thorough erasing, deletes from view + Room DB, rebuilds backing bitmap +### Phase 6: Undo/Redo (2026-03-24) + +- [x] 6.1: UndoableAction interface, AddStrokeAction, DeleteStrokeAction +- [x] 6.2: UndoManager with undo/redo stacks (depth 50), StateFlow for canUndo/canRedo +- [x] 6.3: EditorViewModel rewired — stroke add/delete go through UndoManager, + visual callbacks sync canvas view without full reload +- [x] 6.4: 9 unit tests for UndoManager (perform, undo, redo, depth limit, clear, no-ops) +- Toolbar now has undo/redo buttons with enabled state + ## In Progress -Phase 6: Undo/Redo +Phase 7: Selection ## Decisions & Deviations diff --git a/PROJECT_PLAN.md b/PROJECT_PLAN.md index 328d77a..f37bcf1 100644 --- a/PROJECT_PLAN.md +++ b/PROJECT_PLAN.md @@ -78,13 +78,13 @@ completed and log them in PROGRESS.md. ## Phase 6: Undo/Redo -- [ ] 6.1: `UndoableAction` interface + concrete actions - - `undo/UndoableAction.kt` -- [ ] 6.2: `UndoManager` — undo/redo stacks, depth 50 +- [x] 6.1: `UndoableAction` interface + `AddStrokeAction`, `DeleteStrokeAction` + - `undo/UndoableAction.kt`, `undo/StrokeActions.kt` +- [x] 6.2: `UndoManager` — undo/redo stacks, depth 50 - `undo/UndoManager.kt` -- [ ] 6.3: Integrate with `EditorViewModel` + toolbar -- [ ] 6.4: Unit tests — `test/.../undo/UndoManagerTest.kt` -- **Verify:** `./gradlew test` + manual test +- [x] 6.3: Integrated with `EditorViewModel` + toolbar (undo/redo buttons) +- [x] 6.4: Unit tests — 9 tests in `UndoManagerTest.kt` +- **Verify:** `./gradlew build` — PASSED (19 tests total) ## Phase 7: Selection — Move, Copy, Delete diff --git a/app/src/main/kotlin/net/metacircular/engpad/ui/editor/EditorScreen.kt b/app/src/main/kotlin/net/metacircular/engpad/ui/editor/EditorScreen.kt index 84f459f..f3fa405 100644 --- 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( diff --git a/app/src/main/kotlin/net/metacircular/engpad/ui/editor/EditorViewModel.kt b/app/src/main/kotlin/net/metacircular/engpad/ui/editor/EditorViewModel.kt index 424364c..7616e7b 100644 --- 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>(emptyList()) val strokes: StateFlow> = _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, diff --git a/app/src/main/kotlin/net/metacircular/engpad/ui/editor/Toolbar.kt b/app/src/main/kotlin/net/metacircular/engpad/ui/editor/Toolbar.kt index 8eb7a75..dc93d3a 100644 --- 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") + } } } diff --git a/app/src/main/kotlin/net/metacircular/engpad/undo/StrokeActions.kt b/app/src/main/kotlin/net/metacircular/engpad/undo/StrokeActions.kt new file mode 100644 index 0000000..cb24277 --- /dev/null +++ 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)) + } +} diff --git a/app/src/main/kotlin/net/metacircular/engpad/undo/UndoManager.kt b/app/src/main/kotlin/net/metacircular/engpad/undo/UndoManager.kt new file mode 100644 index 0000000..1833ec4 --- /dev/null +++ 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() + private val redoStack = ArrayDeque() + + private val _canUndo = MutableStateFlow(false) + val canUndo: StateFlow = _canUndo + + private val _canRedo = MutableStateFlow(false) + val canRedo: StateFlow = _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 + } +} diff --git a/app/src/main/kotlin/net/metacircular/engpad/undo/UndoableAction.kt b/app/src/main/kotlin/net/metacircular/engpad/undo/UndoableAction.kt new file mode 100644 index 0000000..84355d5 --- /dev/null +++ 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() +} diff --git a/app/src/test/kotlin/net/metacircular/engpad/undo/UndoManagerTest.kt b/app/src/test/kotlin/net/metacircular/engpad/undo/UndoManagerTest.kt new file mode 100644 index 0000000..d03629f --- /dev/null +++ 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() + + @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()) + } +}