feat(swift-ios): hold-and-drag thread reordering - #11379
Conversation
The native client had no way to rearrange the thread list even though servers already support the fractional order-key protocol used by web drag and React Native mobile's Move up / Move down. Add both menu actions on pinned and active rows, planned against the canonical section across environments and written through thread.pin.reorder / thread.active.reorder, capability-gated and disabled at section edges. Pinning now also takes the top of the arranged run when the server supports it. Generated with Devin (swe-2-high, T3 Code/Cursor harness) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This change adds a substantial production iOS drag-and-drop reordering workflow with persisted cross-environment writes and changes to default sidebar ordering, rather than making a small isolated adjustment. An unresolved medium-severity order-key risk further calls for human validation. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
The move planner treated every enabled environment as writable, so a reorder-capable server that dropped its connection kept stale rows in the canonical section and could receive writes aimed at a dead client. Rows outside the connected set now remain anchors but are never assigned keys, matching the review finding on pingdotgg#11379. Generated with Devin (swe-2-high, T3 Code/Cursor harness) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| .filter { $0.pinnedAt != nil } | ||
| .compactMap(\.pinOrderKey) | ||
| .min() | ||
| orderKey = ThreadOrderPlanner.orderKeyBetween(before: nil, after: firstKey) |
There was a problem hiding this comment.
🟡 Medium App/NativeFeatureClient.swift:1780
When the earliest pinned key is a value such as "ma", orderKeyBetween(before: nil, after: firstKey) returns nil, so setThreadPinned sends no orderKey at line 1782. The newly pinned thread is therefore sorted below all keyed pinned threads instead of being placed at the top; handle this case by generating a valid key before falling back to a keyless pin.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/App/NativeFeatureClient.swift around line 1780:
When the earliest pinned key is a value such as `"ma"`, `orderKeyBetween(before: nil, after: firstKey)` returns `nil`, so `setThreadPinned` sends no `orderKey` at line 1782. The newly pinned thread is therefore sorted below all keyed pinned threads instead of being placed at the top; handle this case by generating a valid key before falling back to a keyless pin.
There was a problem hiding this comment.
Checked against 524f5bd40b42770a011d3254455add1032b32440: this matches the shared clients’ intentional fallback for corrupt stored keys.
packages/client-runtime/src/state/threadSort.tsrejects trailing-akeys, andapps/web/src/components/Sidebar.logic.test.ts:987explicitly assertspinOrderKeyBetween(null, "ma") === null. Both key generators avoid producing trailing-akeys.apps/web/src/hooks/useThreadActions.ts:92-101documents and implements a keyless fallback when key math cannot produce a placement: pinning still succeeds. React Native uses the same fallback inapps/mobile/src/features/home/useThreadListActions.ts:371-379.- SwiftUI mirrors those paths. The 51 passing focused native tests include rejection of corrupt bounds and generation of valid, insertable keys through 2,000 rows.
Keeping this behavior preserves cross-client placement semantics for corrupt data. Repairing arbitrary stored keys would need a shared policy; this comment does not identify a failure for keys the existing generators produce. Resolving as intentional behavior. The PR description now makes the fallback explicit.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
Two review findings on pingdotgg#11379: moveThread now captures the section before awaiting so a mid-flight pin/unpin cannot misapply confirmed keys, and a spread rewrite that fails partway surfaces the confirmed assignments via FeatureThreadMovePartialError so the sidebar reflects the writes that actually landed. Generated with Devin (swe-2-high, T3 Code/Cursor harness) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Addressed the Macroscope findings on the latest push (
|
Generated with Devin (swe-2-high, T3 Code/Cursor harness) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Replace the context-menu Move up/down actions with direct hold-and-drag reordering on the Home thread list, per review feedback. A long-press lifts a row inside its own section (pinned or active); the drop plans fractional order-key writes from the post-drop displayed order — the same input web passes to planPinnedReorder. A stationary hold still opens the context menu. - UICollectionView drag/drop delegates + diffable reorderingHandlers; drop proposals clamp to the dragged row's section so pinned rows can't cross the divider and shelf rows/search results/headers never lift. - planDrop() replaces movePlanner(): arbitrary-position drops with the same writability rules (reorder-capable + connected environments only; disconnected rows anchor but are never written). - pendingReorder holds the dropped order against mid-flight stream updates and converges on the common rows; a failed or partial write snaps back to the true model state. New lifts are blocked while a write is outstanding and no-op drops skip the write. - Stream updates defer while a drag is active (hasActiveDrag) so the lifted row can't shift under the finger. - FeatureThreadMoveDirection/MoveOptions removed; lift gating is a plain writable+connected check. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Note for reviewers on the Macroscope findings re-flagged against the drag-and-drop head (
|
Thread rows in the native SwiftUI client can be held and dragged to reorder within their pinned or active section. A stationary hold opens the context menu. Drops use the existing fractional order-key protocol, reserve hidden rows’ keys, reject writes to unavailable environments, and recover the displayed order when a write fails. When corrupt stored keys prevent placement, new pins retain the shared web/React Native keyless fallback.
Dragging upward used to stall when the finger crossed from the collection view into the Home header. The source drag now keeps scrolling through that area, including a stationary hold, and stops when the drag ends or the list is removed. UIKit still owns in-list scrolling and drop destinations. The app-only drag delegate selector is corrected as well.
The branch includes the existing upstream CI repair #11426: pin
expo-audioto the already-locked57.0.4so fresh release-smoke resolution continues to use its patch. No resolved dependency version changes.Upward scrolling: before and after
Same 70-thread disposable fixture, iPhone 16 Pro Simulator on iOS 26.5, and four-second hold over the project filter. Both builds start at row 41, positioned through the debugger without a preceding pan gesture. The original PR head (
2d2c40b962) stalls near row 39;606c9a4f94reaches row 5. Releasing over the header cancels the drop and preserves the saved order.Comparison video. Real recordings, trimmed to the gesture and result at original speed; GIF exported at 20 fps. Labels are outside the captured UI. Media was captured at
606c9a4f94; final head524f5bd40badds only the upstream CI pin and has identical SwiftUI sources.Current build: downward drag, upward drop, relaunch, and context menu
The downward drop saves thread 03 at position 41. The upward drop saves thread 05 first; it remains first after relaunch, and a stationary hold opens its context menu.
Regression video. Consecutive native tests, original speed, 20 fps GIF; the app relaunches between checks.
Verification
Final head
524f5bd40b: all CI jobs are successful or intentionally skipped; the approvability check is neutral. The corrupt-key comment was checked against the shared client implementation and answered with a source-backed explanation.test_simwith-only-testing:T3CodeTests/HomeThreadSwipeActionTests -only-testing:T3CodeTests/ThreadOrderPlannerTests: 51 passed, zero failed or skipped. Coverage includes stationary header scrolling, inset clamping, cancellation/teardown, wider sidebar boundaries, existing swipe actions, and order planning.node scripts/generate-swift-wire-fixtures.ts --check,git diff --check, andCI=true node scripts/release-smoke.tspassed. The last check passed with the upstream CI prerequisite applied.claude --safe-mode --model claude-fable-5 --effort high --print --output-format json --no-session-persistence --tools Read,Grep,Glob --permission-mode dontAsk. Exit 1, HTTP 429 session limit, zero model tokens; no independent review was produced.Original feature: Devin (swe-2-high, T3 Code/Cursor). Scrolling fix and updated verification/media: GPT-6 Astra, Codex harness.