fix(macros): commands no longer disappear when reordering in the macro editor#1244
Conversation
…o editor
The macro command list assigned svelte-dnd-action's raw consider/finalize items
(including the library's internal shadow placeholder) straight to `commands`, and
only filtered the placeholder out of the {#each}. With few items, a command could
linger in state as the placeholder and then get filtered out of the render — i.e.
disappear when reordering via drag.
Mirror the fix already applied to the Settings choice list in #883: strip the
SHADOW_PLACEHOLDER_ITEM_ID entry in handleConsider and handleSort before assigning
to `commands`. (CommandList was the only dndzone list missing this.)
Verified by hand in the dev vault: reordering with 2+ commands no longer drops an
item, and order persists. build + svelte-check clean; unit tests pass.
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCommandList.svelte now filters the dnd-action internal shadow placeholder item from the commands array in both the consider and sort event handlers. This prevents the placeholder identifier from persisting in component state during drag-and-drop reordering operations. ChangesDrag-and-drop shadow placeholder filtering
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b87049637
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| commands = (newItems as ICommand[]).filter( | ||
| (c) => c.id !== SHADOW_PLACEHOLDER_ITEM_ID | ||
| ); |
There was a problem hiding this comment.
Preserve the dragged command during consider
In the pointer-drag path, svelte-dnd-action's first consider event replaces the dragged item with the shadow placeholder; filtering it here means commands (and the dndzone's next items config) temporarily no longer contains the dragged command at all. If the user releases before a later over-index event re-inserts a shadow item, such as a tiny drag or dropping outside immediately after drag starts, handleSort receives that shortened list and saveCommands(commands) persists the macro with the command removed. Keep the dragged command represented in state during consider (or restore it before finalize) rather than dropping the placeholder from the dndzone items.
Useful? React with 👍 / 👎.
Bug
In the macro editor, dragging to reorder a command could make another command disappear (most visibly with 2 commands).
Cause
CommandList'shandleConsider/handleSortassigned svelte-dnd-action's rawe.detail.items— including the library's internal shadow placeholder — directly tocommands, and only filtered the placeholder out of the{#each}. With few items, a command could end up retained in state as the placeholder and then be filtered out of the render, so it vanished.Fix
Strip
SHADOW_PLACEHOLDER_ITEM_IDin both handlers before assigning tocommands— the exact fix already applied to the Settings choice list in #883. CommandList was the only dndzone list still missing it (pre-existing; unrelated to the recent CI/type work).Verification
Manually verified in the dev vault: reordering with 2+ commands no longer drops an item and the order persists.
bun run build+bun run check(svelte-check 0/0) clean; unit tests pass.Summary by CodeRabbit