feat(ui): allow tab reordering

[shantur]

No description provided.

[shantur]

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

[codenomadbot]

Gatekeeper review for latest PR state. Request understood: review-only pass for regressions, implementation quality, and better alternatives. Assumption: no branch changes requested unless explicitly asked.

Verdict

Request changes. The implementation builds and typechecks, but I would not merge this as-is because it regresses touch scrolling of the tab strip.

Blocking Finding

1. packages/ui/src/styles/panels/tabs.css:25 disables touch panning on every draggable tab with touch-action: none.

   The tab strip itself is horizontally scrollable via .tab-scroll { overflow-x: auto; }, and packages/ui/src/components/instance-tabs.tsx:42-45 wraps every tab in .tab-draggable. On touch devices, users normally start horizontal scrolling directly on a tab. With touch-action: none on those elements, the browser is told not to perform native pan/scroll gestures from that region, so overflowing tabs become much harder or impossible to scroll unless the user happens to start on the tiny gaps outside tabs. That is a direct mobile/tablet regression for any workspace with enough tabs to overflow.

   Better implementation: do not put touch-action: none on the whole tab. Use a dedicated drag handle with touch-action: none, keep the tab body scrollable, or otherwise gate touch dragging so horizontal tab scrolling remains native and reliable.

Non-Blocking Quality Notes

1. The reorder affordance is pointer-only. There is no keyboard-accessible reorder path and no announcement of changed order. If tab order is a real UI feature, consider adding keyboard support or at least a focused drag handle pattern with accessible labels.

2. moveAppTab is small and deterministic, but currently untested. A minimal store-level test around moving before/after, ignoring unknown ids, and preserving new tabs would be cheap insurance for future tab-management changes.

3. Adding @thisbeyond/solid-dnd for one tab strip is reasonable only if the team is comfortable with another runtime dependency. The current integration is small, but a native pointer implementation or a reusable internal sortable primitive may be preferable if more reorderable UI is expected.

Checks Run

1. npm ci completed successfully.

2. npm run typecheck --workspace @codenomad/ui passed.

3. npm run build --workspace @codenomad/ui passed. Existing Vite chunk-size warnings and a virtua JSX import-source warning were emitted, but they do not appear introduced by this PR.

Gatekeeper Summary

The state-management approach is simple and mostly clean, and the drag/drop library API usage is valid. The touch scrolling regression is significant enough to block merge until the draggable surface no longer disables native horizontal scrolling across the entire tab.

--
Yours,
CodeNomadBot

[shantur]

@CodeNomadBot

Review updated changes

[codenomadbot]

Updated gatekeeper review for latest PR state. Request understood: re-review the updated changes after the prior blocking feedback. Assumption: this is still review-only.

Verdict

Approved from gatekeeper review. I do not see any blocking regressions in the updated diff.

Prior Blocker Status

Resolved. The previous blocker was touch-action: none on .tab-draggable, which would have disabled native horizontal touch scrolling across the scrollable tab strip. The new commit removes that rule from packages/ui/src/styles/panels/tabs.css, so the tab strip can keep native touch scrolling behavior.

Findings

No blocking findings.

Residual Risks / Suggestions

1. Touch devices may now preserve scrolling at the expense of reliable touch drag initiation, depending on browser pointer-event behavior. That is an acceptable tradeoff versus blocking scroll, but if touch reordering is a hard requirement later, use a dedicated drag handle with an explicit touch-action policy rather than making the whole tab draggable surface consume gestures.

2. Keyboard-accessible reordering is still not implemented. This is not a regression from the update, but it is worth adding if tab ordering becomes a core accessibility requirement.

3. moveAppTab remains untested. A small store-level test for before/after moves and unknown ids would be useful hardening, but I would not block this PR on it.

Checks Run

1. npm run typecheck --workspace @codenomad/ui passed.

2. npm run build --workspace @codenomad/ui passed. Existing Vite chunk-size warnings and the existing virtua JSX import-source warning still appear; they do not look introduced by this PR.

Gatekeeper Summary

The updated implementation fixes the touch-scrolling regression, keeps the state change minimal, and passes the relevant UI verification checks. I would merge this PR if the team is comfortable with the added @thisbeyond/solid-dnd dependency and the current pointer-first reorder UX.

--
Yours,
CodeNomadBot