Skip to content

feat(live-preview): add grip-handle drag reorder for list items#132

Open
Harriet-star-glitch wants to merge 3 commits into
floatboatai:mainfrom
Harriet-star-glitch:feat/core-list-drag-reorder
Open

feat(live-preview): add grip-handle drag reorder for list items#132
Harriet-star-glitch wants to merge 3 commits into
floatboatai:mainfrom
Harriet-star-glitch:feat/core-list-drag-reorder

Conversation

@Harriet-star-glitch

@Harriet-star-glitch Harriet-star-glitch commented Jul 6, 2026

Copy link
Copy Markdown

Summary / 摘要

Adds a small grip handle before each list item's bullet/number in live preview; dragging it reorders the item among its siblings (nested sub-lists move with their parent, preserving indentation).

Motivation / 背景与动机

  • Issue:
  • Roadmap (docs/ROADMAP.md): not tracked as its own top-level row; tracked in the OpenSpec change below (task 2.4, P1 Advanced Editing)
  • OpenSpec change: add-missing-features (task 2.4 marked done in this PR)

Changes / 变更内容

  • packages/core:
    • live-preview.ts: grip handle + custom mousedown/mousemove/mouseup drag state machine inside buildListDecorations (no HTML5 DnD, per CLAUDE.md rule 5). New pure, unit-testable getListItemRanges() / computeListReorderChange() helpers so the reorder semantics are covered without needing to simulate pixel-accurate drag coordinates in jsdom.
    • live-preview-table.ts: generalized the table widget's editing-lock counter (previously table-only, isTableEditing) into isStructuralEditing() / acquireStructuralEditingLock() / releaseStructuralEditingLock(), since list drag needs the same "skip live-preview StateField rebuilds mid-interaction" guarantee the table widget already relied on (CLAUDE.md rules 8/12). Internal only — not exported from the package's public index.ts.
  • openspec/: add-missing-features/tasks.md — checked off 2.4.

Testing / 测试

  • pnpm test passes (527/527, including 12 new tests)
  • Affected packages build (pnpm build)
  • New / updated vitest cases: packages/core/test/live-preview-list-drag.test.ts — grip rendering (top-level list, nested sub-list, and the cursor-revealed-raw-markup case), the pure reorder computation (move down / move up / no-op / out-of-range / preserves nested indentation), and a real EditorView round-trip applying the computed change.
  • Manual UI check in electron-demo: built the app and drove the packaged Electron binary with a throwaway Playwright script (not committed) — typed a real list via keyboard, dragged a grip with real mouse events, confirmed the drop indicator rendered mid-drag and the document text reordered correctly (alpha,beta,gammabeta,gamma,alpha for a drag of item 0 past item 2), and confirmed no console/page errors.

Compliance / 合规自检

  • CLA signed — will sign if/when the bot prompts
  • AI disclosure: this PR's functional code is primarily AI-generated. I asked Claude (Anthropic's Claude Code) to survey this repo's open roadmap items, pick an unclaimed one, and design + implement it end-to-end, including the shared editing-lock refactor and the new test file. I chose the task, reviewed the diff, test output, and the manual verification recording, but have not independently rewritten the implementation. I'm disclosing this explicitly per GOVERNANCE.md §6.2 rather than checking "not AI-generated" — happy to walk through and defend any part of it in review, and understand maintainers may decide this doesn't meet the bar in §6.2.
  • New dependencies: none
  • No build artifacts committed
  • No secrets committed

Checklist / 自检清单

  • Title follows Conventional Commits
  • Public API changes update package README — n/a, no public API surface changed (isStructuralEditing etc. stay internal to packages/core/src, not exported from index.ts)
  • Touched live-preview-table.ts → only renamed/generalized the shared lock counter; did not touch any of the 12 Table Widget drag/DOM rules in CLAUDE.md
  • New capability / breaking change → this completes an existing, already-approved OpenSpec item (add-missing-features feat(electron): i18n for renderer UI driven by language setting #2.4) rather than opening a new proposal
  • Change aligns with project scope

Adds a small grip handle before each list item's bullet/number in live
preview. Dragging it (mousedown/mousemove/mouseup, no HTML5 DnD, per
project convention) reorders the item among its siblings; nested items
move with whatever sub-list they contain, so indentation is preserved
automatically since the raw text is relocated verbatim.

Closes OpenSpec add-missing-features task 2.4.

- packages/core/src/live-preview.ts: grip rendering + drag state machine
  in buildListDecorations; new pure, unit-testable
  getListItemRanges()/computeListReorderChange() helpers so the reorder
  semantics don't depend on simulating pixel-accurate drag coordinates.
- packages/core/src/live-preview-table.ts: generalized the table's
  editing-lock counter (previously table-only) into
  isStructuralEditing()/acquireStructuralEditingLock()/
  releaseStructuralEditingLock(), since list drag needs the same
  "skip live-preview rebuilds mid-interaction" guarantee the table
  widget already relied on.
- packages/core/test/live-preview-list-drag.test.ts: 12 new tests
  covering grip rendering (incl. nested lists and cursor-revealed
  raw markup), the pure reorder computation (down/up/no-op/
  nested-indentation-preserving), and a real EditorView round-trip.

Manually verified in the electron-demo build with a real mouse-driven
drag (Playwright driving the packaged Electron binary): the drop
indicator renders during the drag and the document reorders correctly,
with no console errors.
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@Harriet-star-glitch Harriet-star-glitch changed the title 宋锐姿创建 feat(live-preview): add grip-handle drag reorder for list items Jul 6, 2026
…e offsets

view.coordsAtPos() throws "No tile at position N" for an offset outside
CM6's currently-measured viewport (virtualized rendering) — reachable
simply by dragging a grip toward a target that has scrolled off-screen
in a long list, no external interference required. It reproduced
reliably in a test that also mutates the document mid-drag (a host
setDocument() call while the grip is held), which is the other half of
the same underlying issue: the drag's captured item ranges/doc snapshot
can go stale while a drag is in progress.

- gapIndexAtClientY/showIndicatorAtGap now go through a safeCoordsAtPos
  wrapper instead of calling view.coordsAtPos() directly, so a
  measurement failure mid-drag degrades to a coordinate fallback
  instead of an uncaught exception in the mousemove handler.
- onDragEnd now compares the live document against the snapshot taken
  when this list was last built, and bails out instead of dispatching
  a change computed from now-invalid offsets — the failure mode
  without this would be silent corruption (splicing at the wrong
  position), not just a crash.

Added a regression test that mutates the document via setDocument()
between mousedown and mouseup and asserts no uncaught exception and no
corruption of the externally-set document.
…llel counter

Found during a follow-up self-review of floatboatai#132, not from a reported bug.

buildListDecorations tracked each grip's drag index with a counter
incremented once per list.children entry, while the itemRanges array
used to actually perform the reorder is built separately by
getListItemRanges, which skips any item lacking `position` info. Real
markdown parsed by this pipeline always has position, so the two never
observably diverged in this PR's own tests — but a host's custom
remarkPlugin injecting a position-less node into a list would desync
the counter from itemRanges' indices for every subsequent sibling,
sending later grips to reorder the wrong item (or an out-of-range
index, silently absorbed by computeListReorderChange's existing bounds
check — no crash, no corruption, just a drag that silently does
nothing to the wrong item).

Deriving the index directly from itemRanges (matched by the item's own
`from` offset) instead of a separately-maintained counter makes this
class of desync impossible by construction.

All 528 tests still pass; typecheck and build unaffected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants