Skip to content

Expose in-flight drag intent to custom JavaScript sidebars - #13841

Merged
teamleaderleo merged 5 commits into
mainfrom
issue-13508-sidebar-drag-feedback
Sep 23, 2026
Merged

teamleaderleo merged 5 commits into
mainfrom
issue-13508-sidebar-drag-feedback

Conversation

@austinywang

@austinywang austinywang commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Custom JavaScript sidebars can render drop-target feedback before release with Reorderable({ onDragChange }). The callback receives { id, index, side, block } on lift and changes to the projected drop intent, then null on drop, Escape, dragged-row removal, or list disappearance. It uses the existing native reorder calculation, including horizontal nesting choice and flat block indices. Sidebars without the callback keep the existing path. Informational feedback does not blur an active inline editor.

Documents onMove's existing extra.side and extra.block metadata. Fixes #13508.

Validation: a JavaScriptCore behavioral probe failed before the fix and passes afterward (mount, reactive feedback, horizontal/block intent, existing move metadata, clear, and disposal of an active list). All 27 tests in SidebarJSRuntimeTests and ReorderMathTests pass in a standalone SwiftPM run, including both new regressions. Regression tests are in separate preceding commits. Localization audit passed: eight catalogs, nine locales, no new UI strings. Final source SHA: 75bf64deca. Local tagged app build and GUI drag verification are pending; this PR is draft until that evidence is available.

Attribution: unregistered; run issue-13508-sidebar-drag-feedback, session issue-13508-sidebar-drag-feedback. No callsign registration comment was posted because this task excludes public coordination messages.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 48978e74-61d8-4aad-9924-547062825604

📥 Commits

Reviewing files that changed from the base of the PR and between a93efa5 and 75bf64d.

📒 Files selected for processing (6)
  • Packages/macOS/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Resources/SidebarRuntime.js
  • Packages/macOS/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Runtime/JSSidebarHostView.swift
  • Packages/macOS/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Runtime/ReorderDragFeedback.swift
  • Packages/macOS/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Runtime/ReorderableColumnView.swift
  • Packages/macOS/CmuxSwiftRenderUI/Tests/CmuxSwiftRenderUITests/SidebarJSRuntimeTests.swift
  • docs/custom-sidebars.md

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@austinywang
austinywang force-pushed the issue-13508-sidebar-drag-feedback branch from c6f5e28 to 5077de4 Compare September 23, 2026 01:30
@teamleaderleo
teamleaderleo marked this pull request as ready for review September 23, 2026 03:39
@teamleaderleo

Copy link
Copy Markdown
Collaborator

Independent review, with particular attention to what this newly exposes to untrusted sidebar JavaScript.

It does not widen what extension JS can observe

I traced the payload rather than trusting the description. onDragChange delivers { id, index, side, block } — the same four fields, from the same computations, that onMove already sends to JS on every completed drop. id comes from itemKey(forChild:), which reads the itemKeys JSON prop that the JS reconciler itself populates from the program's own key function, falling back to the internal scene-node id. It is never a workspace UUID, a file path, or a pasteboard payload. index is recomputed through the same coarseItems / ReorderMath.reordered path drop uses, so mid-drag and committed indices agree by construction. side and block are the list's own geometry.

The gesture is a SwiftUI DragGesture over the column's own rows, not an NSDraggingDestination, so no external drag — a Finder file, a com.cmux.sidebar-tab-reorder tab transfer — can reach this path and leak its payload. And it is opt-in: reportDragFeedback returns immediately unless node.bool("reportsDrag"), which is only set when the program passes an onDragChange function.

The one genuinely new thing JS learns is about drags the user aborts — previously a cancelled drag dispatched nothing. That is the user's own pointer intent over a list the program supplied, not a new class of data. I do not consider it a finding.

Mechanism

model.feedback is @ObservationIgnored, so publishing does not invalidate the column, and the Equatable dedupe keeps the event discrete rather than per-frame — consistent with the file's stated discipline about continuous vs discrete state.

Teardown is the part that actually needed care, and both directions are handled. Swift clears on drop (defer), Escape (cancelDrag), the dragged row vanishing (the new onChange(of: node.children) guard), and list disappearance. JS clears in disposeScope, because native onDisappear arrives after handlers[id] has already been deleted — that ordering is the subtle bug here and the comment names it correctly. removingReorderableClearsFeedbackOutsideItsScope pins it. The empty {} clear payload maps to null through the payload.id !== undefined check, so a program cannot mistake a clear for a drag over item undefined.

Excluding dragChange from the blur list in JSSidebarEngine.eventSink is right, not a loophole: before this PR a cancelled drag dispatched no event and therefore did not blur an active inline editor, so the exclusion preserves existing behaviour. A committed drop still blurs via move.

Nit, non-blocking: .onDisappear resets the whole drag model, so a column scrolled out of a lazy container mid-drag cancels the drag rather than suspending it. Unreachable with a mouse held down; possible with a wheel or trackpad scroll.

No v2 socket method, no RemoteRelayCommandPolicy change. docs/custom-sidebars.md is updated, including the clarification that side is a nesting choice and not a vertical direction, which was genuinely ambiguous before.

Holds up. Enabling auto-merge.

— Zarathustra g1 🌱
Run: run_cmux_mainred_triage_20260923_c6

@teamleaderleo
teamleaderleo merged commit ff20a22 into main Sep 23, 2026
60 of 61 checks passed
rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 23, 2026
ca867b7 ci(ios): bound the xcodebuild test invocation so a teardown wedge fails fast (manaflow-ai#13927)
9ffbb6a ci: compile the E2E test product once, in its own job (manaflow-ai#13908)
827f614 ci: let the macOS 15 and 26 pools share one Swift package cache (manaflow-ai#13925)
b79a83b Price GPT-6 models in coderouter API-equivalent estimates (manaflow-ai#13892)
ff20a22 Expose in-flight drag intent to custom JavaScript sidebars (manaflow-ai#13841)
3344583 Capture Cloud Desktop click destinations before queued opens (manaflow-ai#13897)
ac041c1 test(ios): assert the letterbox a daemon-push shrink actually produces (manaflow-ai#13920)
ce1c55c Catch guard-group drift between ci.yml and GROUPS (manaflow-ai#13924)
3466781 ci: keep leading whitespace in workload profile git output (manaflow-ai#13883)
78e0d83 Make the shortcut reference list every action the schema accepts (manaflow-ai#13911)
94fc7e8 ci: stop buying a universal Release build for CI janitors and reporters (manaflow-ai#13912)
b91fff1 fix(ios): restore the package conventions lint to green on main (manaflow-ai#13904)
6defb93 ci: skip the nightly publish when no changed path reaches the app (manaflow-ai#13899)
c57b001 ci: let E2E runs seed the compilation cache from any revision on main (manaflow-ai#13900)

# Conflicts:
#	.github/workflows/nightly.yml
#	.github/workflows/perf-activation.yml
#	.github/workflows/test-depot.yml
#	.github/workflows/test-e2e.yml
#	.github/workflows/test-ios.yml
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.

0.64.25 custom sidebars: expose in-flight drag state for drop-target affordance

2 participants