fix(visual-editor): keep dragged element at new position after rearrange (FT-1910)#28
Open
joalves wants to merge 4 commits into
Open
fix(visual-editor): keep dragged element at new position after rearrange (FT-1910)#28joalves wants to merge 4 commits into
joalves wants to merge 4 commits into
Conversation
…nge (FT-1910) trackMoveChange generated both element and target selectors against the post-drop DOM and only then reverted the live drop. PreviewManager runs removePreviewChanges before replaying changes in replace mode, so it queried selectors against the reverted DOM where a path like `#zone > div:nth-of-type(1)` resolved to a different sibling — the SDK either moved the wrong element or no element, and the user-visible symptom was the dragged element snapping back to its original position. Capture the destination as Element references (anchor + position semantic) at post-drop, revert, then generate both elementSelector and targetSelector against the reverted DOM. The Element refs stay valid across the move; the selectors now resolve to the same nodes the SDK sees on replay. The vestigial originalTargetSelector / originalPosition locals were never written into the change object and have been dropped. Other changes: * Button.tsx: drop disabled:pointer-events-none in favor of disabled:cursor-not-allowed so existing title tooltips on disabled buttons fire on hover. The HTML disabled attribute already blocks clicks, making pointer-events-none belt-and-suspenders that broke tooltip discoverability — most visibly on the JSON-editor button when the Visual Editor is active. * tests/e2e/helpers/ve-rearrange.ts: new helper that drives a real HTML5 drag/drop on class-less siblings under #rearrange-zone via page.evaluate. Playwright's mouse-based dragTo doesn't reliably trigger HTML5 drag events in headless Chromium, so the events are dispatched directly. Polls for the expected order after the SDK postMessage round-trip. * visual-editor-complete.spec.ts: wires the rearrange step in after testAllVisualEditorActions. * tests/test-pages/visual-editor-test.html: adds #rearrange-zone with three id-less, class-less divs. id-less is required because the selector generator returns the id directly when present, bypassing the parent-context path that surfaces the bug. * tests/e2e/helpers/ve-url-filter.ts: read the JSON editor's full doc by scrolling cm-scroller and accumulating cm-line text. CodeMirror v6 only renders visible lines, so reading cm-content.textContent missed urlFilter once the saved variant config grew past the viewport — the rearrange change pushed it over. * tests/e2e/visual-editor-context-menu.spec.ts: deleted. Zero expect() calls, posted a window message the codebase never handled, every step gated behind `if (await contextMenu.isVisible())`. The same context-menu actions are covered with real assertions in visual-editor-complete.spec.ts via testAllVisualEditorActions. Verification: 5 sequential passes of `npx playwright test tests/e2e/visual-editor-complete.spec.ts` (~12s each).
…-1910) `tests/` was ignoring the whole tree, which forced every new file under tests/e2e/ to be added with `git add -f`. Add `!tests/e2e/**` so the e2e helpers, specs, and fixtures are tracked without that workaround. Subsequent rules (`*.png` etc.) keep test-run artifacts out. Also add `.claude/worktrees/` so the per-feature worktree directory this repo uses doesn't show up as untracked in `git status` on main.
WalkthroughThis pull request refactors the visual-editor rearrange flow to compute a drop anchor (nested vs sibling before/after), updates dragover/drop handlers to insert according to that anchor, reverts the DOM before generating selectors in trackMoveChange, and ensures Escape cancels rearrange via a capture keydown. It adds injected CSS for drop indicators, tweaks Button disabled cursor, adds Playwright rearrange helpers and tests (with CodeMirror v6 reading fix), introduces rearrange fixtures in the test page, updates the Visual Editor complete test to run the scenarios, adjusts .gitignore to track tests/e2e/**, and bumps version to 1.5.2. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
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: 4
🧹 Nitpick comments (1)
tests/e2e/helpers/ve-rearrange.ts (1)
40-41: ⚡ Quick winAvoid the text-coupled locator for the dragged card.
This helper is validating sibling order, not fixture copy, so
hasText: "Gamma"makes it fail on harmless label edits. Prefer a structural locator anchored under#rearrange-zone, for examplezone.locator(":scope > div").nth(2). Playwright scopes nested locators to the outer locator, and its CSS examples use full relative selectors rather than a bare leading combinator. (playwright.dev)♻️ Suggested change
- const gamma = zone.locator("> div", { hasText: "Gamma" }) + const gamma = zone.locator(":scope > div").nth(2)As per coding guidelines "Always add
idattributes to elements and use ID selectors (e.g.,#id) in tests, never use text-based selectors likegetByText()orhas-text()".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/helpers/ve-rearrange.ts` around lines 40 - 41, Replace the text-coupled locator used to find the "Gamma" card (the `gamma` variable created from `zone.locator(...)`) with a structural, index-based locator anchored to the `zone` root; e.g., use a scoped child selector like `:scope > div` with `.nth(2)` on the same `zone.locator` call so the test targets the third sibling structurally instead of matching by text, complying with the ID/text-less selector guideline in ve-rearrange helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitignore:
- Around line 62-65: The negation pattern is ineffective because the file
currently ignores the directory with "tests/" which prevents Git from descending
into subdirs and honoring "!tests/e2e/**"; update the .gitignore by replacing
the "tests/" ignore entry and the "!tests/e2e/**" negation with the recommended
pair: use "tests/*" to ignore direct children of tests and add "!tests/e2e/" to
re-include the e2e directory (so files under tests/e2e/ are tracked); locate the
existing "tests/" and "!tests/e2e/**" lines and make this replacement.
In `@src/visual-editor/core/edit-modes.ts`:
- Around line 563-576: The rollback catch block around
originalParent.insertBefore/appendChild can fail and, if it does, the function
continues and emits selectors from the post-drop DOM; update the catch in the
block that uses originalParent, originalNextSibling and element (the try that
calls insertBefore/appendChild and logs via debugWarn) to return immediately
after logging (or otherwise abort tracking/throw) so we do not persist a
corrupted move change or emit selectors from the wrong tree.
In `@tests/e2e/helpers/ve-url-filter.ts`:
- Around line 96-103: The helper currently queries CodeMirror by brittle class
selectors (scroller and content) — update the queries to use stable ID
selectors: change the document.querySelector(".cm-scroller") and
document.querySelector(".cm-content") calls in this helper (references:
scroller, content, collect, seen) to use document.getElementById or
document.querySelector("#your-id") and ensure the corresponding DOM where
CodeMirror is instantiated has id attributes (e.g., id="editor-scroller",
id="editor-content"); keep the null-check and early return logic the same and
only replace the selector strings so collect() still iterates
content.querySelectorAll(".cm-line") unchanged.
In `@tests/e2e/visual-editor-complete.spec.ts`:
- Around line 97-100: The testRearrangeDragDrop(testPage) step adds a new "move"
change, so the later assertion still using verifySidebarHasChanges(sidebar, 4)
is stale; update the spec to reflect the extra saved change by either having
testRearrangeDragDrop undo its change or increment the expected count in the
later verifySidebarHasChanges call (e.g., change the expected value from 4 to 5)
so the sidebar change count matches the new move entry.
---
Nitpick comments:
In `@tests/e2e/helpers/ve-rearrange.ts`:
- Around line 40-41: Replace the text-coupled locator used to find the "Gamma"
card (the `gamma` variable created from `zone.locator(...)`) with a structural,
index-based locator anchored to the `zone` root; e.g., use a scoped child
selector like `:scope > div` with `.nth(2)` on the same `zone.locator` call so
the test targets the third sibling structurally instead of matching by text,
complying with the ID/text-less selector guideline in ve-rearrange helpers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aeae7006-5646-48dc-9157-6f8a0edc5bc2
📒 Files selected for processing (8)
.gitignoresrc/components/ui/Button.tsxsrc/visual-editor/core/edit-modes.tstests/e2e/helpers/ve-rearrange.tstests/e2e/helpers/ve-url-filter.tstests/e2e/visual-editor-complete.spec.tstests/e2e/visual-editor-context-menu.spec.tstests/test-pages/visual-editor-test.html
💤 Files with no reviewable changes (1)
- tests/e2e/visual-editor-context-menu.spec.ts
Comment on lines
62
to
+65
| tests/ | ||
| # …but track the e2e test sources / helpers / fixtures. Without this exception | ||
| # every new file under tests/e2e/ has to be added with `git add -f`. | ||
| !tests/e2e/** |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Test whether the gitignore pattern correctly allows tests/e2e/** files
# Create a temporary test file under tests/e2e/
TEST_FILE="tests/e2e/gitignore-pattern-test-temp.txt"
mkdir -p tests/e2e
echo "test" > "$TEST_FILE"
# Check if Git considers this file for tracking (should show as untracked, not ignored)
if git check-ignore -v "$TEST_FILE" > /dev/null 2>&1; then
echo "❌ FAIL: $TEST_FILE is ignored by .gitignore (pattern may not work correctly)"
git check-ignore -v "$TEST_FILE"
RESULT=1
else
echo "✅ PASS: $TEST_FILE is not ignored and can be tracked"
RESULT=0
fi
# Clean up
rm -f "$TEST_FILE"
exit $RESULTRepository: absmartly/browser-extension
Length of output: 242
Fix the gitignore negation pattern for tests/e2e/ directory.
The current pattern tests/ on line 62 followed by !tests/e2e/** on line 65 does not work as intended. When a directory is ignored with a trailing slash, Git does not descend into it to evaluate negation patterns for subdirectories. A test confirms that new files under tests/e2e/ are currently being ignored.
To fix this, change the pattern to:
Recommended pattern
tests/*
!tests/e2e/This ignores all direct children of tests/ but re-includes the tests/e2e/ directory (and implicitly all files beneath it).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.gitignore around lines 62 - 65, The negation pattern is ineffective because
the file currently ignores the directory with "tests/" which prevents Git from
descending into subdirs and honoring "!tests/e2e/**"; update the .gitignore by
replacing the "tests/" ignore entry and the "!tests/e2e/**" negation with the
recommended pair: use "tests/*" to ignore direct children of tests and add
"!tests/e2e/" to re-include the e2e directory (so files under tests/e2e/ are
tracked); locate the existing "tests/" and "!tests/e2e/**" lines and make this
replacement.
Comment on lines
+96
to
+103
| const scroller = document.querySelector(".cm-scroller") as HTMLElement | null | ||
| const content = document.querySelector(".cm-content") | ||
| if (!scroller || !content) return "" | ||
|
|
||
| const seen = new Set<string>() | ||
| const collect = () => { | ||
| content.querySelectorAll(".cm-line").forEach((line) => { | ||
| seen.add(line.textContent ?? "") |
There was a problem hiding this comment.
Use stable ID selectors for the CodeMirror query path.
Line 96 and Line 97 currently rely on library class selectors (.cm-scroller, .cm-content), which makes this helper brittle and breaks the test selector rule.
Suggested update in this helper
- const scroller = document.querySelector(".cm-scroller") as HTMLElement | null
- const content = document.querySelector(".cm-content")
+ const scroller = document.querySelector("#json-editor-scroller") as HTMLElement | null
+ const content = document.querySelector("#json-editor-content") as HTMLElement | null
if (!scroller || !content) return ""
const seen = new Set<string>()
const collect = () => {
- content.querySelectorAll(".cm-line").forEach((line) => {
- seen.add(line.textContent ?? "")
- })
+ seen.add(content.textContent ?? "")
}As per coding guidelines, Always add id attributes to elements and use ID selectors (e.g., #id) in tests, never use text-based selectors like getByText() or has-text().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/e2e/helpers/ve-url-filter.ts` around lines 96 - 103, The helper
currently queries CodeMirror by brittle class selectors (scroller and content) —
update the queries to use stable ID selectors: change the
document.querySelector(".cm-scroller") and document.querySelector(".cm-content")
calls in this helper (references: scroller, content, collect, seen) to use
document.getElementById or document.querySelector("#your-id") and ensure the
corresponding DOM where CodeMirror is instantiated has id attributes (e.g.,
id="editor-scroller", id="editor-content"); keep the null-check and early return
logic the same and only replace the selector strings so collect() still iterates
content.querySelectorAll(".cm-line") unchanged.
…e cancel (FT-1910)
Iterates on the snap-back fix to make rearrange usable on real pages.
Behaviour:
* Block elements default to *sibling-only* placement. Walking up from
the cursor's e.target, the algorithm prefers an ancestor whose parent
IS the dragged element's parent (true sibling), and falls back to the
innermost block-in-block ancestor (re-parent to a new container at
the right hierarchy level). Without this, dropping the cursor over a
card's <h3> nested the dragged element inside the card — exactly the
precision problem the user hit on the launchpad grid.
* Inline elements (computed display === "inline") default to *free /
nested* placement: the dragged element nests directly into the
cursor's element. That's the natural "drop a span into a paragraph"
outcome.
* Alt SWAPS the strategy — block + Alt → free, inline + Alt → sibling.
No new key per element type; one mental model.
* Escape cancels an in-progress drag without recording a move. Capture-
phase keydown listener so it always reaches us before any page-level
handler. Cleans up listeners and class state via the same handleDragEnd
path the native dragend uses.
* The 10s auto-exit setTimeout is gone. It was kicking the user out of
rearrange mode mid-decision and the native dragend cleanup makes it
redundant for the orphan-listener case it was guarding.
getSmartDraggableElement now returns named inline elements (a / button
/ span) directly instead of walking past them to a block ancestor — a
span-with-text drag should drag the span, not its containing card.
trackMoveChange's only-element-child position picker now prefers
"lastChild" over "firstChild". The drop runs appendChild, so the SDK
needs to reproduce that on replay; "firstChild" would call
insertBefore(target.firstChild) and land the element BEFORE any text
content of the container, which doesn't match what the user just saw.
Drop visualization (styles.ts):
* Sibling-mode candidate now shows a 3px coloured line on its top or
bottom edge instead of a fill highlight, so the insertion point is
unambiguous between cards.
* Free / nested mode shows a dashed outline + light fill on the
container the dragged element will nest into, plus a different colour
from sibling mode so the user can see at a glance which strategy is
active.
Tests:
* tests/test-pages/visual-editor-test.html: #rearrange-zone-deep block
with id-less, class-less divs each containing inner h3 + p. id-less
is required because the selector generator returns the id directly
when present, bypassing the parent-context path. The inner content
is required because the precision case is "cursor over inner child".
Plus a #rearrange-inline <p> with three <span> children for the
inline-default and inline+Alt scenarios.
* tests/e2e/helpers/ve-rearrange.ts: split into scenario functions
driven by a dispatchDrag helper that emits a full HTML5 drag-event
sequence with a configurable altKey. testRearrangeAllScenarios runs
the snap-back regression, block-snaps-to-sibling, inline-default-
nests, inline-Alt-sibling, and Escape-cancels in sequence.
* testBlockAltDropNests is exported but NOT wired into the runner: it
exposes a marker-cleanup edge case where nesting a block into another
card's <p> leaves a production-experiment-named [data-absmartly-
experiment] attribute that VE.stop's removePreviewChanges("__visual_
editor__") doesn't sweep. The drop logic itself works (manual smoke-
testing confirms Alt-on-block correctly nests); the marker-cleanup
path is a separate fix outside this PR.
Live reposition preview (the dragged element rearranging in real time
as the cursor moves) is deferred to a follow-up ticket — that's a
fundamentally different drag loop and deserves its own PR.
Verification: npx playwright test tests/e2e/visual-editor-complete.spec.ts
passes locally in ~12s. typecheck clean.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/visual-editor/core/edit-modes.ts (1)
725-738:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAbort tracking if the rollback fails.
The catch block logs the error but does not return early. If
insertBefore/appendChildthrows, the method continues and emits selectors from the post-drop tree, which recreates the same mismatch this fix is trying to remove.🛡️ Suggested fix
} catch (err) { debugWarn("[ABSmartly] Failed to revert drop pre-replay:", err) + return }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/visual-editor/core/edit-modes.ts` around lines 725 - 738, The catch block around DOM reattachment (where originalParent.insertBefore/appendChild is called for element with originalNextSibling) currently only logs via debugWarn and allows execution to continue; change it to abort further tracking by returning early (or otherwise stopping the function) when the reattachment fails so the function does not proceed to emit selectors from the post-drop tree. Locate the try/catch that references originalParent, originalNextSibling, element, insertBefore and appendChild and update the catch to return (or throw) after calling debugWarn to prevent subsequent selector emission.
🧹 Nitpick comments (2)
tests/e2e/helpers/ve-rearrange.ts (1)
358-373: ⚡ Quick winRemove duplicate JSDoc comment block.
The JSDoc comment explaining why
testBlockAltDropNestsis excluded appears twice (lines 358-365 and 362-373). Remove the first occurrence.🧹 Suggested fix
-/** - * Run every rearrange scenario in sequence. Order matters — each scenario - * leaves the DOM in a known state that the next one builds on. - */ /** * Run every rearrange scenario in sequence. Order matters — each scenario * leaves the DOM in a known state that the next one builds on.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/helpers/ve-rearrange.ts` around lines 358 - 373, Delete the duplicated JSDoc block that explains why testBlockAltDropNests is excluded; keep a single copy of the explanatory comment and remove the first occurrence so only one JSDoc describing testBlockAltDropNests remains adjacent to the exported function name testBlockAltDropNests (ensure surrounding blank lines remain consistent after removal).src/visual-editor/core/edit-modes.ts (1)
84-99: 💤 Low valueRemove the unused
nearestBlockAncestorfunction.The function defined at line 84 is not called anywhere in the codebase and should be deleted to eliminate dead code.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/visual-editor/core/edit-modes.ts` around lines 84 - 99, Delete the unused helper function declaration named nearestBlockAncestor from src/visual-editor/core/edit-modes.ts by removing the entire const nearestBlockAncestor = (el: Element): Element | null => { ... } block; ensure you only remove that function and its inner logic (do not alter smartElement or isBlockLevel usages elsewhere), and run a quick project-wide search to confirm there are no remaining references to nearestBlockAncestor so the codebase remains clean.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/visual-editor/core/edit-modes.ts`:
- Around line 725-738: The catch block around DOM reattachment (where
originalParent.insertBefore/appendChild is called for element with
originalNextSibling) currently only logs via debugWarn and allows execution to
continue; change it to abort further tracking by returning early (or otherwise
stopping the function) when the reattachment fails so the function does not
proceed to emit selectors from the post-drop tree. Locate the try/catch that
references originalParent, originalNextSibling, element, insertBefore and
appendChild and update the catch to return (or throw) after calling debugWarn to
prevent subsequent selector emission.
---
Nitpick comments:
In `@src/visual-editor/core/edit-modes.ts`:
- Around line 84-99: Delete the unused helper function declaration named
nearestBlockAncestor from src/visual-editor/core/edit-modes.ts by removing the
entire const nearestBlockAncestor = (el: Element): Element | null => { ... }
block; ensure you only remove that function and its inner logic (do not alter
smartElement or isBlockLevel usages elsewhere), and run a quick project-wide
search to confirm there are no remaining references to nearestBlockAncestor so
the codebase remains clean.
In `@tests/e2e/helpers/ve-rearrange.ts`:
- Around line 358-373: Delete the duplicated JSDoc block that explains why
testBlockAltDropNests is excluded; keep a single copy of the explanatory comment
and remove the first occurrence so only one JSDoc describing
testBlockAltDropNests remains adjacent to the exported function name
testBlockAltDropNests (ensure surrounding blank lines remain consistent after
removal).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ca3f3e43-d240-42cd-b273-4aaf6bf238b0
📒 Files selected for processing (5)
src/visual-editor/core/edit-modes.tssrc/visual-editor/ui/styles.tstests/e2e/helpers/ve-rearrange.tstests/e2e/visual-editor-complete.spec.tstests/test-pages/visual-editor-test.html
✅ Files skipped from review due to trivial changes (1)
- tests/e2e/visual-editor-complete.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two-part fix for the visual editor's rearrange/drag flow.
Part 1 — selector mismatch on the dragged element ("snap-back" bug)
trackMoveChangewas generating both element and target selectors against the post-drop DOM, then reverting the live drop.PreviewManagerrunsremovePreviewChangesbefore replaying changes in replace mode, so it queried selectors against the reverted DOM where a path like#zone > div:nth-of-type(1)resolved to a different sibling — the SDK either moved the wrong element or no element, and the dragged element appeared to snap back. Captured the destination as Element references at post-drop, revert, then generate selectors against the reverted DOM.Part 2 — block-vs-inline-aware drop with Alt swap and Escape cancel
After Part 1 the move committed but the drop targeting was still aggressive: cursor over a card's inner
<h3>nested the dragged element inside the card. New algorithm:display: blocketc.)display: inline)Alt swaps the strategy regardless of element type — one mental model.
Other behaviour changes:
dragendcleanup makes it redundant.getSmartDraggableElementnow returns named inline elements (a/button/span) directly instead of walking past them to a block ancestor — span-with-text drags drag the span, not the containing card.trackMoveChangeonly-child position picker now prefers"lastChild"over"firstChild"so the SDK reproduces whatappendChilddid instead of dropping the element before any text content.Button.tsxno longer suppresses pointer events when disabled, so the existing "Cannot edit JSON while Visual Editor is active" tooltip fires on hover..gitignore:!tests/e2e/**so new e2e files don't needgit add -f;.claude/worktrees/so per-feature worktree dirs don't show as untracked on main.Deferred to a follow-up
testBlockAltDropNestse2e coverage — the function is implemented and exported, but pulling it intotestRearrangeAllScenariosexposes a marker-cleanup quirk where nesting a block into another card's<p>leaves a production-experiment-named[data-absmartly-experiment]attribute thatVE.stop'sremovePreviewChanges("__visual_editor__")doesn't sweep. The drop logic itself works (manual smoke-testing confirms Alt-on-block correctly nests); the marker-cleanup path is a separate fix.Test plan
npm run build:devnpx playwright test tests/e2e/visual-editor-complete.spec.ts— passes locally in ~12sbun run typecheck<p>(deferred from e2e per note above)JIRA: FT-1910
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores