feat: add undo/redo for character changes (Closes #89) (#131)

.plan/issue-89-undo-redo.md

@@ -0,0 +1,64 @@
+# Plan: Issue #89 — Undo/Redo for Character Changes
+
+## Summary
+Implement an undo/redo stack in `CharacterViewModel` so that users can revert accidental character changes. The undo/redo buttons will be added to the top app bar. The stack tracks the last 20 character snapshots. Auto-save continues to save the current (post-undo/redo) state only.
+
+## Implementation Steps
+
+### Step 1: Create UndoRedoManager utility class
+**File**: `sharedUI/src/commonMain/kotlin/org/shahondin1624/viewmodel/UndoRedoManager.kt`
+
+Create a generic `UndoRedoManager<T>` class that:
+- Maintains an undo stack (max 20 entries) and a redo stack
+- Exposes `canUndo: StateFlow<Boolean>` and `canRedo: StateFlow<Boolean>`
+- Methods: `pushState(state: T)`, `undo(current: T): T?`, `redo(current: T): T?`, `clear()`
+- When a new state is pushed, the redo stack is cleared
+- When the undo stack exceeds 20 entries, the oldest entry is dropped
+
+### Step 2: Integrate UndoRedoManager into CharacterViewModel
+**File**: `sharedUI/src/commonMain/kotlin/org/shahondin1624/viewmodel/CharacterViewModel.kt`
+
+- Add an `UndoRedoManager<ShadowrunCharacter>` instance
+- In `updateCharacter()` and `setCharacter()`: push the OLD state onto the undo stack before applying the new state
+- Add `undo()` method: calls `undoRedoManager.undo(current)`, sets the result as current character (without pushing to undo stack)
+- Add `redo()` method: calls `undoRedoManager.redo(current)`, sets the result as current character (without pushing to undo stack)
+- Expose `canUndo: StateFlow<Boolean>` and `canRedo: StateFlow<Boolean>` from the manager
+- Auto-save debounce continues to work normally -- it saves whatever the current state is
+
+### Step 3: Add undo/redo buttons to the top app bar
+**File**: `sharedUI/src/commonMain/kotlin/org/shahondin1624/App.kt`
+
+- Add Undo and Redo `IconButton`s in the `actions` block of `TopAppBar`, before the save status indicator
+- Use `Icons.AutoMirrored.Filled.Undo` and `Icons.AutoMirrored.Filled.Redo` (or `Icons.Default.Undo`/`Icons.Default.Redo`)
+- Buttons are enabled/disabled based on `canUndo`/`canRedo` state
+- Only show on the CHARACTER_SHEET route (not on creation or settings)
+
+### Step 4: Add string resources
+**File**: `sharedUI/src/commonMain/composeResources/values/strings.xml`
+
+- Add `undo_content_desc` = "Undo"
+- Add `redo_content_desc` = "Redo"
+
+### Step 5: Add test tags
+**File**: `sharedUI/src/commonMain/kotlin/org/shahondin1624/lib/components/TestTags.kt`
+
+- Add `UNDO_BUTTON = "undo_button"`
+- Add `REDO_BUTTON = "redo_button"`
+
+### Step 6: Write unit tests for UndoRedoManager
+**File**: `sharedUI/src/commonTest/kotlin/org/shahondin1624/viewmodel/UndoRedoManagerTest.kt`
+
+Test cases:
+- Initial state: canUndo=false, canRedo=false
+- Push state -> canUndo=true, canRedo=false
+- Undo returns previous state, canRedo=true
+- Redo returns the state that was undone
+- Push after undo clears redo stack
+- Stack limited to 20 entries
+- Clear resets everything
+
+## AC Verification Checklist
+1. [ ] Undo button available in the top bar (on character sheet route)
+2. [ ] At least the last 10 changes can be undone (we support 20)
+3. [ ] Redo is available after undo
+4. [ ] Auto-save works with the undo stack (saves current state, not undo history)

sharedUI/src/commonMain/composeResources/values/strings.xml

@@ -37,6 +37,10 @@
     <string name="top_bar_create_character">Create Character</string>
     <string name="top_bar_settings">Settings</string>
 
+    <!-- Undo/Redo -->
+    <string name="undo_content_desc">Undo</string>
+    <string name="redo_content_desc">Redo</string>
+
     <!-- Save status -->
     <string name="save_status_saving">Saving...</string>
     <string name="save_status_saved">Saved</string>

sharedUI/src/commonMain/kotlin/org/shahondin1624/App.kt

@@ -10,7 +10,9 @@ import androidx.compose.material.icons.filled.History
 import androidx.compose.material.icons.filled.LightMode
 import androidx.compose.material.icons.filled.Menu
 import androidx.compose.material.icons.filled.Person
+import androidx.compose.material.icons.filled.Redo
 import androidx.compose.material.icons.filled.Settings
+import androidx.compose.material.icons.filled.Undo
 import androidx.compose.material3.*
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.CompositionLocalProvider
@@ -288,6 +290,8 @@ private fun MainScaffold(
     val character by characterViewModel.character.collectAsState()
     val saveStatus by characterViewModel.saveStatus.collectAsState()
     val loadStatus by characterViewModel.loadStatus.collectAsState()
+    val canUndo by characterViewModel.canUndo.collectAsState()
+    val canRedo by characterViewModel.canRedo.collectAsState()
     val snackbarHostState = remember { SnackbarHostState() }
 
     // Show load error dialog when deserialization fails
@@ -366,6 +370,28 @@ private fun MainScaffold(
                     }
                 },
                 actions = {
+                    if (currentRoute == AppRoutes.CHARACTER_SHEET) {
+                        IconButton(
+                            onClick = { characterViewModel.undo() },
+                            enabled = canUndo,
+                            modifier = Modifier.testTag(TestTags.UNDO_BUTTON)
+                        ) {
+                            Icon(
+                                Icons.Default.Undo,
+                                contentDescription = stringResource(Res.string.undo_content_desc)
+                            )
+                        }
+                        IconButton(
+                            onClick = { characterViewModel.redo() },
+                            enabled = canRedo,
+                            modifier = Modifier.testTag(TestTags.REDO_BUTTON)
+                        ) {
+                            Icon(
+                                Icons.Default.Redo,
+                                contentDescription = stringResource(Res.string.redo_content_desc)
+                            )
+                        }
+                    }
                     SaveStatusIndicator(status = saveStatus)
                     IconButton(
                         onClick = { showHistoryDialog = true },

sharedUI/src/commonMain/kotlin/org/shahondin1624/lib/components/TestTags.kt

@@ -144,6 +144,10 @@ object TestTags {
     const val SAVE_STATUS_ERROR = "save_status_error"
     const val SAVE_ERROR_SNACKBAR = "save_error_snackbar"
 
+    // --- Undo/Redo ---
+    const val UNDO_BUTTON = "undo_button"
+    const val REDO_BUTTON = "redo_button"
+
     // --- Top app bar ---
     const val TOP_APP_BAR = "top_app_bar"
     const val THEME_TOGGLE = "theme_toggle"

sharedUI/src/commonMain/kotlin/org/shahondin1624/viewmodel/CharacterViewModel.kt

@@ -36,6 +36,14 @@ class CharacterViewModel : ViewModel() {
     /** Observable load status — UI should check for [LoadStatus.Error] to show recovery dialog. */
     val loadStatus: StateFlow<LoadStatus> = _loadStatus.asStateFlow()
 
+    private val undoRedoManager = UndoRedoManager<ShadowrunCharacter>()
+
+    /** Whether an undo operation is available. */
+    val canUndo: StateFlow<Boolean> = undoRedoManager.canUndo
+
+    /** Whether a redo operation is available. */
+    val canRedo: StateFlow<Boolean> = undoRedoManager.canRedo
+
     private val _saveStatus = MutableStateFlow<SaveStatus>(SaveStatus.Idle)
 
     /** Observable save status for UI display. */
@@ -57,6 +65,7 @@ class CharacterViewModel : ViewModel() {
      */
     fun setCharacter(character: ShadowrunCharacter) {
         Napier.i(tag = TAG) { "Character replaced: ${character.characterData.name}" }
+        undoRedoManager.pushState(_character.value)
         _character.value = character
     }
 
@@ -64,9 +73,32 @@ class CharacterViewModel : ViewModel() {
      * Update the character by applying a transformation function.
      */
     fun updateCharacter(transform: (ShadowrunCharacter) -> ShadowrunCharacter) {
+        undoRedoManager.pushState(_character.value)
         _character.value = transform(_character.value)
     }
 
+    /**
+     * Undo the last character change, restoring the previous state.
+     */
+    fun undo() {
+        val previous = undoRedoManager.undo(_character.value)
+        if (previous != null) {
+            Napier.i(tag = TAG) { "Undo: restored character '${previous.characterData.name}'" }
+            _character.value = previous
+        }
+    }
+
+    /**
+     * Redo the last undone change.
+     */
+    fun redo() {
+        val redone = undoRedoManager.redo(_character.value)
+        if (redone != null) {
+            Napier.i(tag = TAG) { "Redo: restored character '${redone.characterData.name}'" }
+            _character.value = redone
+        }
+    }
+
     /**
      * Save the current character immediately (synchronous).
      * Also updates the save status indicator.

sharedUI/src/commonMain/kotlin/org/shahondin1624/viewmodel/UndoRedoManager.kt

@@ -0,0 +1,89 @@
+package org.shahondin1624.viewmodel
+
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.asStateFlow
+
+/**
+ * Generic undo/redo manager that maintains a bounded history of states.
+ *
+ * @param maxSize Maximum number of undo entries to retain (oldest are evicted when exceeded).
+ */
+class UndoRedoManager<T>(private val maxSize: Int = DEFAULT_MAX_SIZE) {
+
+    private val undoStack = ArrayDeque<T>()
+    private val redoStack = ArrayDeque<T>()
+
+    private val _canUndo = MutableStateFlow(false)
+    val canUndo: StateFlow<Boolean> = _canUndo.asStateFlow()
+
+    private val _canRedo = MutableStateFlow(false)
+    val canRedo: StateFlow<Boolean> = _canRedo.asStateFlow()
+
+    /**
+     * Push the current state onto the undo stack before a new change is applied.
+     * Clears the redo stack since the timeline has diverged.
+     */
+    fun pushState(state: T) {
+        undoStack.addLast(state)
+        if (undoStack.size > maxSize) {
+            undoStack.removeFirst()
+        }
+        redoStack.clear()
+        updateFlows()
+    }
+
+    /**
+     * Undo the last change by popping the most recent state from the undo stack.
+     * Pushes [current] onto the redo stack so it can be restored later.
+     *
+     * @param current The current state (before undo) to push onto redo.
+     * @return The previous state to restore, or null if nothing to undo.
+     */
+    fun undo(current: T): T? {
+        if (undoStack.isEmpty()) return null
+        redoStack.addLast(current)
+        val previous = undoStack.removeLast()
+        updateFlows()
+        return previous
+    }
+
+    /**
+     * Redo the last undone change by popping from the redo stack.
+     * Pushes [current] onto the undo stack.
+     *
+     * @param current The current state (before redo) to push onto undo.
+     * @return The redone state to restore, or null if nothing to redo.
+     */
+    fun redo(current: T): T? {
+        if (redoStack.isEmpty()) return null
+        undoStack.addLast(current)
+        val redone = redoStack.removeLast()
+        updateFlows()
+        return redone
+    }
+
+    /**
+     * Clear all undo and redo history.
+     */
+    fun clear() {
+        undoStack.clear()
+        redoStack.clear()
+        updateFlows()
+    }
+
+    /** Current number of undo entries (exposed for testing). */
+    val undoSize: Int get() = undoStack.size
+
+    /** Current number of redo entries (exposed for testing). */
+    val redoSize: Int get() = redoStack.size
+
+    private fun updateFlows() {
+        _canUndo.value = undoStack.isNotEmpty()
+        _canRedo.value = redoStack.isNotEmpty()
+    }
+
+    companion object {
+        const val DEFAULT_MAX_SIZE = 20
+    }
+}

sharedUI/src/commonTest/kotlin/org/shahondin1624/UndoRedoManagerTest.kt

@@ -0,0 +1,145 @@
+package org.shahondin1624
+
+import org.shahondin1624.viewmodel.UndoRedoManager
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertNull
+import kotlin.test.assertTrue
+
+class UndoRedoManagerTest {
+
+    @Test
+    fun initialState_cannotUndoOrRedo() {
+        val manager = UndoRedoManager<String>()
+        assertFalse(manager.canUndo.value)
+        assertFalse(manager.canRedo.value)
+    }
+
+    @Test
+    fun pushState_enablesUndo() {
+        val manager = UndoRedoManager<String>()
+        manager.pushState("A")
+        assertTrue(manager.canUndo.value)
+        assertFalse(manager.canRedo.value)
+    }
+
+    @Test
+    fun undo_returnsPreviousState() {
+        val manager = UndoRedoManager<String>()
+        manager.pushState("A")
+        val result = manager.undo("B")
+        assertEquals("A", result)
+    }
+
+    @Test
+    fun undo_enablesRedo() {
+        val manager = UndoRedoManager<String>()
+        manager.pushState("A")
+        manager.undo("B")
+        assertTrue(manager.canRedo.value)
+    }
+
+    @Test
+    fun redo_returnsUndoneState() {
+        val manager = UndoRedoManager<String>()
+        manager.pushState("A")
+        manager.undo("B") // undo from B to A, B goes to redo
+        val result = manager.redo("A") // redo from A, should get B
+        assertEquals("B", result)
+    }
+
+    @Test
+    fun pushAfterUndo_clearsRedoStack() {
+        val manager = UndoRedoManager<String>()
+        manager.pushState("A")
+        manager.undo("B")
+        assertTrue(manager.canRedo.value)
+        manager.pushState("C") // new change should clear redo
+        assertFalse(manager.canRedo.value)
+    }
+
+    @Test
+    fun undo_whenEmpty_returnsNull() {
+        val manager = UndoRedoManager<String>()
+        val result = manager.undo("current")
+        assertNull(result)
+    }
+
+    @Test
+    fun redo_whenEmpty_returnsNull() {
+        val manager = UndoRedoManager<String>()
+        val result = manager.redo("current")
+        assertNull(result)
+    }
+
+    @Test
+    fun stackLimitedToMaxSize() {
+        val manager = UndoRedoManager<Int>(maxSize = 3)
+        manager.pushState(1)
+        manager.pushState(2)
+        manager.pushState(3)
+        manager.pushState(4) // should evict 1
+        assertEquals(3, manager.undoSize)
+        // Undo three times: should get 4, 3, 2 (not 1)
+        val third = manager.undo(5) // gets 4
+        assertEquals(4, third)
+        val second = manager.undo(4) // gets 3
+        assertEquals(3, second)
+        val first = manager.undo(3) // gets 2
+        assertEquals(2, first)
+        val empty = manager.undo(2) // nothing left
+        assertNull(empty)
+    }
+
+    @Test
+    fun clear_resetsEverything() {
+        val manager = UndoRedoManager<String>()
+        manager.pushState("A")
+        manager.pushState("B")
+        manager.undo("C")
+        assertTrue(manager.canUndo.value)
+        assertTrue(manager.canRedo.value)
+
+        manager.clear()
+        assertFalse(manager.canUndo.value)
+        assertFalse(manager.canRedo.value)
+        assertEquals(0, manager.undoSize)
+        assertEquals(0, manager.redoSize)
+    }
+
+    @Test
+    fun multipleUndoRedo_maintainsCorrectOrder() {
+        val manager = UndoRedoManager<String>()
+        // Simulate: start with "A", change to "B", then "C"
+        manager.pushState("A") // push A before changing to B
+        manager.pushState("B") // push B before changing to C
+        // Current state is "C"
+
+        // Undo from C -> B
+        val undone1 = manager.undo("C")
+        assertEquals("B", undone1)
+
+        // Undo from B -> A
+        val undone2 = manager.undo("B")
+        assertEquals("A", undone2)
+
+        // Redo from A -> B
+        val redone1 = manager.redo("A")
+        assertEquals("B", redone1)
+
+        // Redo from B -> C
+        val redone2 = manager.redo("B")
+        assertEquals("C", redone2)
+    }
+
+    @Test
+    fun defaultMaxSizeIs20() {
+        val manager = UndoRedoManager<Int>()
+        // Push 25 states
+        for (i in 1..25) {
+            manager.pushState(i)
+        }
+        assertEquals(20, manager.undoSize)
+    }
+}