feat(gradient): add undo/redo functionality with keyboard shortcuts #4
Conversation
✅ Deploy Preview for mesh-magic ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds undo/redo history: new generic history composable, deep-clone utility, integration into mesh state with debounced/pausable recording, keyboard shortcut bindings and UI entries for undo/redo, README update, and unit tests for the history composable. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant KB as Keyboard Shortcuts
participant Mesh as useMeshGradient
participant History as useHistory
participant UI as Toast/UI
User->>KB: Press Ctrl+Z (Undo)
KB->>KB: check isTypingInInput
KB->>Mesh: call undo()
Mesh->>Mesh: cancel debounced record & pause watcher
Mesh->>History: undo()
History->>History: move present -> future, restore previous
History-->>Mesh: provide restored config
Mesh->>Mesh: apply config and resume watcher
Mesh->>UI: show "Undid change" toast
User->>KB: Press Ctrl+Y (Redo)
KB->>KB: check isTypingInInput
KB->>Mesh: call redo()
Mesh->>Mesh: cancel debounced record & pause watcher
Mesh->>History: redo()
History->>History: move present -> past, restore next
History-->>Mesh: provide restored config
Mesh->>Mesh: apply config and resume watcher
Mesh->>UI: show "Redid change" toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/composables/useMeshGradient.ts`:
- Around line 160-171: The debounced history update can still fire after
pauseHistory() is called, causing duplicates; modify debouncedRecordHistory (or
replace useDebounceFn with a small internal debounce) so pending timers are
cancelable and/or no-op when history is paused: either keep the debounce timer
id (e.g., timerId private to the composable) and call clearTimeout(timerId)
inside recordHistory() immediately after pauseHistory(), or add a shared flag
(e.g., isHistoryPaused) that debouncedRecordHistory checks at invocation and
returns early if paused; update references in recordHistory, pauseHistory,
resumeHistory and any mutators like addLayer so they use the cancel/guard to
prevent the scheduled call from pushing a duplicate state.
🧹 Nitpick comments (3)
app/composables/useAppKeyboardShortcuts.ts (2)
52-70: Potentially redundant handler for uppercase 'Z'.The handler for uppercase
"Z"(lines 52-70) may be redundant. The lowercase"z"handler (lines 26-50) already checkse.shiftKeyand handles Ctrl+Shift+Z. Browser key event behavior foronKeyStroke("Z")can be inconsistent—some browsers report lowercasezeven with Shift held.Consider removing this handler or verify via testing that it's necessary for your target browsers.
26-90: Consider extracting repeated focus-check logic.The input/textarea focus check is duplicated across all four keyboard handlers. A small helper function would reduce repetition.
♻️ Optional refactor to reduce duplication
// Add at the top of setupShortcuts or as a module-level function const isTypingInInput = (): boolean => { const activeElement = document.activeElement; return !!activeElement && ( activeElement.tagName.toLowerCase() === "input" || activeElement.tagName.toLowerCase() === "textarea" ); }; // Then use in handlers: onKeyStroke("z", (e: KeyboardEvent) => { if (e.ctrlKey || e.metaKey) { e.preventDefault(); if (isTypingInInput()) return; if (e.shiftKey) { if (canRedo.value) redo(); } else { if (canUndo.value) undo(); } } });app/composables/useHistory.ts (1)
33-42: DuplicatedeepCloneimplementation.This
deepClonefunction is identical to the one inapp/composables/useSavedGradients.ts(lines 50-59). Consider extracting it to a shared utility (e.g.,~/utils/clone.ts) to avoid duplication.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/composables/useMeshGradient.ts (1)
226-234: Shallow copy induplicateLayercauses shared references for nested arrays.The spread operator creates a shallow copy, so
x,y,blur, andopacityarrays are shared between the original and duplicated layer. Modifying one will affect the other.🐛 Proposed fix: deep clone the layer
+import { deepClone } from "~/utils/clone"; const duplicateLayer = (index: number) => { if (config.value.layers.length === maxLayerCount.value) return; const source = config.value.layers[index]; if (!source) return; - const dup = { ...source, id: generateLayerId() }; + const dup = { ...deepClone(source), id: generateLayerId() }; config.value.layers.splice(index + 1, 0, dup); recordHistory(); };app/composables/useAppKeyboardShortcuts.ts (1)
99-140: MissingisTypingInInputguard onA,Backspace, andDeleteshortcuts.These shortcuts could trigger while typing in an input field, causing unintended layer additions or deletions.
🐛 Proposed fix: add typing guard
// Add new layer (A) registerKeyStroke("a", () => { + if (isTypingInInput()) return; + if (config.value.layers.length >= 8) { // ... } // ... }); // Delete last layer (Backspace / Delete) registerKeyStroke("Backspace", () => { + if (isTypingInInput()) return; + if (config.value.layers.length <= 1) { // ... } // ... }); registerKeyStroke("Delete", () => { + if (isTypingInInput()) return; + if (config.value.layers.length <= 1) { // ... } // ... });
🤖 Fix all issues with AI agents
In `@app/composables/useHistory.ts`:
- Around line 113-119: The reset() function currently clones the reactive
initialValue (which may have mutated) so reset restores the current state
instead of the original; modify useHistory to capture and store a non-reactive
snapshot (e.g., clonedInitial or storedInitial) at creation time (right after
receiving initialValue in useHistory), and change reset() to set present.value =
clone(storedInitial) and then call clear(); update references to initialValue in
reset to use the stored snapshot so reset restores the true original state.
🧹 Nitpick comments (3)
app/composables/useHistory.ts (1)
51-67: Consider usingshallowReffor history stacks.The
pastandfuturearrays don't need deep reactivity since their contents are already clones. UsingshallowRefwould reduce reactivity overhead for large history stacks.♻️ Optional optimization
-import { ref, computed } from "vue"; +import { ref, shallowRef, computed } from "vue"; // History stacks -const past = ref<T[]>([]); +const past = shallowRef<T[]>([]); const present = ref<T>(deep ? deepClone(initialValue) : initialValue); -const future = ref<T[]>([]); +const future = shallowRef<T[]>([]);Note: With
shallowRef, you'd need to reassign the array (e.g.,past.value = [...past.value, item]) instead of using.push()to trigger reactivity, or manually calltriggerRef().app/composables/useMeshGradient.ts (1)
193-201: Watch target syntax is unusual but functional.The arrow function
() => config.valuecreates a computed getter that returns the config object. This works but could be simplified to justconfig(the state ref) sincewatchPausablehandles refs directly.♻️ Optional simplification
const { pause: pauseHistory, resume: resumeHistory } = watchPausable( - () => config.value, + config, () => { debouncedRecordHistory(); }, { deep: true, flush: 'sync' } );app/composables/useAppKeyboardShortcuts.ts (1)
19-29: Consider addingcontenteditablecheck toisTypingInInput.The current check misses
contenteditableelements, which are also text input contexts.♻️ Suggested enhancement
const isTypingInInput = (): boolean => { const activeElement = document.activeElement; - return !!activeElement && ( - activeElement.tagName.toLowerCase() === "input" || - activeElement.tagName.toLowerCase() === "textarea" - ); + if (!activeElement) return false; + const tagName = activeElement.tagName.toLowerCase(); + return ( + tagName === "input" || + tagName === "textarea" || + activeElement.getAttribute("contenteditable") === "true" + ); };
…put guards to shortcuts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/composables/useMeshGradient.ts (1)
255-320: Cancel pending debounced history before discrete actions.
randomize,applyTheme, andresetpause the watcher but don’t cancel a pending debounce. If a debounce was queued (e.g., from a slider) it can still fire and create a duplicate history entry after these actions.🔧 Suggested fix
const randomize = ( minLayers = 1, maxLayers = maxLayerCount.value, newBaseColor?: string, ) => { + cancelDebouncedHistory(); // Pause watcher to avoid double recording pauseHistory(); ... }; const applyTheme = (name: keyof typeof themes) => { const t = themes[name]; if (!t) return; + cancelDebouncedHistory(); // Pause watcher to avoid double recording pauseHistory(); ... }; const reset = () => { + cancelDebouncedHistory(); // Pause watcher to avoid double recording pauseHistory(); ... };
🤖 Fix all issues with AI agents
In `@app/composables/useAppKeyboardShortcuts.ts`:
- Around line 36-67: The key handlers for "z" and "y" call e.preventDefault()
before checking isTypingInInput(), which blocks native undo/redo in inputs;
update both onKeyStroke("z", ...) and onKeyStroke("y", ...) to first return
early if isTypingInInput() is true, and only call e.preventDefault() when
handling an app-level undo/redo (i.e., when canUndo.value/canRedo.value leads to
calling undo()/redo()); keep the existing checks for ctrlKey/metaKey and
shiftKey for redo logic.
…nteditable detection
1 participant
Feature: Undo/Redo Functionality
Implemented the ability to undo and redo changes to gradient configurations with keyboard shortcuts.
What's New
Technical Changes
New Files:
app/composables/useHistory.ts- Generic history composable with past/present/future stackstest/unit/useHistory.test.ts- 17 unit tests (all passing)Modified Files:
app/composables/useMeshGradient.ts- Integrated history tracking with watchPausableapp/composables/useAppKeyboardShortcuts.ts- Added undo/redo keyboard shortcutsapp/components/mesh/partials/KeyboardShortcutsDialog.vue- Updated shortcuts documentationREADME.md- Marked feature as completePerformance Optimizations
watchPausablefrom VueUse to prevent double recording during undo/redostructuredClone()for fast deep cloningTesting
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.