diff --git a/CLAUDE.md b/CLAUDE.md index 0d98103..8d1fbf4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -62,6 +62,14 @@ docker exec stdout → tokio task → emit("terminal-output-{sessionId}") → li `tabOrder` is user-reorderable (drag, or `Ctrl+Shift+←/→` via `moveActiveTab`) — so **never treat a tab's position as identity**: address tabs by key, and index only through `tabOrder`. `moveTab` deliberately does not activate what it moves. + - **The tab drag is pointer events, not HTML5 drag-and-drop, and must stay that way.** Tauri's + `dragDropEnabled` blocks HTML5 drag inside the webview on Windows, and it cannot simply be + turned off: `TerminalView` needs Tauri's native drag-drop event because it is the only one + that carries dropped *file paths*. An HTML5 drag also carries a `DataTransfer`, which the + default handler types into any text field the drag is released over. + - **A new app-level shortcut must not swallow a text-editing chord.** `useKeyboardShortcuts` + binds on `document` in the capture phase, so `inTextField()` guards the arrow bindings — + excluding xterm's helper textarea, which is an input-method shim rather than a field. - **`hooks/`** — All Tauri IPC calls are encapsulated in hooks (`useTerminal`, `useProjects`, `useDocker`, `useSettings`) - **`lib/tauri-commands.ts`** — Typed `invoke()` wrappers; TypeScript types in `lib/types.ts` must match Rust models - **`components/terminal/TerminalView.tsx`** — xterm.js integration with WebGL rendering, URL detection for OAuth flow diff --git a/HOW-TO-USE.md b/HOW-TO-USE.md index 335f744..f39e058 100644 --- a/HOW-TO-USE.md +++ b/HOW-TO-USE.md @@ -192,10 +192,11 @@ Anthropic-backend project uses that token without its own login. See There is no separate terminal tab bar and no "+" button — tabs appear when you open a project or a terminal. - **Drag a tab to reorder it.** A line shows where it will land, and dropping it does not change - which tab you are looking at — so you can rearrange the strip without pulling focus away from a - terminal that is mid-run. `Ctrl+Shift+←` and `Ctrl+Shift+→` move the *active* tab the same way - without the mouse. The order is per-session: it is not saved when you quit. + **Drag a tab to reorder it.** A line shows where it will land; **Escape** abandons the drag. + Dropping does not change which tab you are looking at — so you can rearrange the strip without + pulling focus away from a terminal that is mid-run. `Ctrl+Shift+←` and `Ctrl+Shift+→` move the + *active* tab the same way without the mouse (they leave text fields alone, where that chord + still selects by word). The order is per-session: it is not saved when you quit. - **Status indicators (top right)** — Docker connection and container image availability. Each pairs a coloured dot with a word, so status is never conveyed by colour alone. The **?** button opens the built-in help. diff --git a/app/src-tauri/src/browser_view/commands.rs b/app/src-tauri/src/browser_view/commands.rs index aa20c38..f52de7b 100644 --- a/app/src-tauri/src/browser_view/commands.rs +++ b/app/src-tauri/src/browser_view/commands.rs @@ -128,23 +128,29 @@ pub async fn open_browser_view_popout( } /// Close the pop-out, putting the view back in the tab. No-op if it is closed. +/// +/// Propagates a failed close rather than reporting success: the pane restores +/// its iframe on success, and doing that with the window still up puts two +/// viewers on one browser. #[tauri::command] pub async fn close_browser_view_popout( project_id: String, app_handle: AppHandle, ) -> Result<(), String> { - popout::close(&app_handle, &project_id); - Ok(()) + popout::close(&app_handle, &project_id) } -/// Whether the pop-out is open — asked on tab open, since a window can outlive -/// the pane that spawned it. +/// Whether the pop-out is open, and whether it is pinned on top. +/// +/// Read on every pane mount: the window outlives the pane — which is unmounted +/// whenever another Project Home sub-tab is selected — so neither fact can be +/// carried in component state. #[tauri::command] -pub async fn is_browser_view_popout_open( +pub async fn get_browser_view_popout_state( project_id: String, app_handle: AppHandle, -) -> Result { - Ok(popout::is_open(&app_handle, &project_id)) +) -> Result { + Ok(popout::state(&app_handle, &project_id)) } /// Pin the pop-out above other windows, so it can be watched while working in diff --git a/app/src-tauri/src/browser_view/mod.rs b/app/src-tauri/src/browser_view/mod.rs index 597b132..ea276ea 100644 --- a/app/src-tauri/src/browser_view/mod.rs +++ b/app/src-tauri/src/browser_view/mod.rs @@ -468,17 +468,33 @@ async fn supervise( let _ = kill_dashboard(&container_id, &cli_entry).await; // Deregister, unless a newer session has already taken this project's slot. - { + let superseded = { let mut map = sessions.lock().await; - if map.get(&project_id).is_some_and(|s| s.epoch == epoch) { - map.remove(&project_id); + match map.get(&project_id) { + Some(session) if session.epoch == epoch => { + map.remove(&project_id); + false + } + // Someone else owns this project now: `stop` removes the session + // from the map *before* awaiting this task, and teardown below is + // seconds of Docker work, so a restart in that window is ordinary. + Some(_) => true, + None => false, } + }; + + // Everything past here speaks for the project as a whole, so a superseded + // supervisor must say nothing: closing the pop-out would destroy the *new* + // session's window, and the off-status would report a running view as + // stopped. + if superseded { + return; } // A pop-out outlives the tab, so nothing else would take it down: the // window would sit there showing a frozen last frame of a viewer that no // longer exists. The session owns it, and this is where the session ends. - popout::close(&app, &project_id); + let _ = popout::close(&app, &project_id); let enabled = manager().is_enabled(&project_id).await; emit(&app, &project_id, &BrowserViewStatus::off(enabled)); diff --git a/app/src-tauri/src/browser_view/popout.rs b/app/src-tauri/src/browser_view/popout.rs index fe9cca1..0ee9992 100644 --- a/app/src-tauri/src/browser_view/popout.rs +++ b/app/src-tauri/src/browser_view/popout.rs @@ -27,15 +27,36 @@ //! dead viewer is worse than no window. The reverse is not true; closing the //! window leaves the view running, and the pane takes it back into the tab. +use serde::Serialize; use tauri::{AppHandle, Emitter, Manager, WebviewUrl, WebviewWindowBuilder, WindowEvent}; -/// Emitted when a pop-out opens or closes. Payload: `{ project_id, open }`. +/// Emitted when a pop-out opens or closes. Payload: [`PopoutState`] plus the +/// project id. /// /// The window can close without the app asking it to — the user hits its X, or /// a teardown takes it — so the pane learns about it the same way it learns /// about everything else here, by listening. const POPOUT_EVENT: &str = "browser-view-popout-changed"; +/// What the pane needs to render its pop-out controls. +/// +/// Both fields are read from the window itself rather than remembered on either +/// side: the pane is unmounted whenever another Project Home sub-tab is +/// selected, so anything it merely *remembers* about the window is gone by the +/// time the user comes back, while the window is still there. +#[derive(Debug, Clone, Copy, Serialize)] +pub struct PopoutState { + pub open: bool, + pub always_on_top: bool, +} + +impl PopoutState { + const CLOSED: Self = Self { + open: false, + always_on_top: false, + }; +} + /// Tauri window labels admit `[a-zA-Z0-9-/:_]` only. Project ids are UUIDs, so /// this never fires in practice; it exists so a hand-edited `projects.json` /// cannot produce a label Tauri rejects at build time. @@ -65,7 +86,7 @@ pub fn open( let _ = window.unminimize(); let _ = window.set_focus(); let _ = window.set_always_on_top(always_on_top); - emit(app, project_id, true); + emit(app, project_id, state(app, project_id)); return Ok(()); } @@ -88,12 +109,12 @@ pub fn open( // to take the view back into the tab. window.on_window_event(move |event| { if matches!(event, WindowEvent::Destroyed) { - emit(&app_for_event, &project_id_owned, false); + emit(&app_for_event, &project_id_owned, PopoutState::CLOSED); } }); log::info!("Browser view: popped out for project {}", project_id); - emit(app, project_id, true); + emit(app, project_id, state(app, project_id)); Ok(()) } @@ -102,23 +123,39 @@ pub fn open( /// `destroy`, not `close`: `close` raises `CloseRequested`, and the app's /// window-event handler treats that as a request to quit for the main window. /// Nothing here should ever be able to be mistaken for that. -pub fn close(app: &AppHandle, project_id: &str) { +/// +/// A failure is **returned, not logged and forgotten**. The pane puts its +/// iframe back the moment it believes the window is gone, so reporting a close +/// that did not happen is how you end up with two viewers driving one browser — +/// the exact state the iframe is dropped to prevent. +pub fn close(app: &AppHandle, project_id: &str) -> Result<(), String> { if let Some(window) = app.get_webview_window(&window_label(project_id)) { - if let Err(e) = window.destroy() { + window.destroy().map_err(|e| { log::warn!( "Browser view: could not close the pop-out for project {}: {}", project_id, e ); - } + format!("Could not close the browser window: {}", e) + })?; } - // Unconditional: `Destroyed` covers the normal path, but a window that was - // already gone still owes the pane an answer. - emit(app, project_id, false); + // `Destroyed` covers the normal path; a window that was already gone still + // owes the pane an answer. + emit(app, project_id, PopoutState::CLOSED); + Ok(()) } -pub fn is_open(app: &AppHandle, project_id: &str) -> bool { - app.get_webview_window(&window_label(project_id)).is_some() +/// Whether the window exists and how it is stacked, read from the window. +pub fn state(app: &AppHandle, project_id: &str) -> PopoutState { + match app.get_webview_window(&window_label(project_id)) { + Some(window) => PopoutState { + open: true, + // A window that cannot answer is not a reason to fail the call; the + // pin is a preference, and "not pinned" is the safe reading. + always_on_top: window.is_always_on_top().unwrap_or(false), + }, + None => PopoutState::CLOSED, + } } /// Pin the pop-out above other windows, or unpin it. No-op when it is closed. @@ -128,13 +165,19 @@ pub fn set_always_on_top(app: &AppHandle, project_id: &str, on_top: bool) -> Res }; window .set_always_on_top(on_top) - .map_err(|e| format!("Could not change the window's stacking: {}", e)) + .map_err(|e| format!("Could not change the window's stacking: {}", e))?; + emit(app, project_id, state(app, project_id)); + Ok(()) } -fn emit(app: &AppHandle, project_id: &str, open: bool) { +fn emit(app: &AppHandle, project_id: &str, state: PopoutState) { let _ = app.emit( POPOUT_EVENT, - serde_json::json!({ "project_id": project_id, "open": open }), + serde_json::json!({ + "project_id": project_id, + "open": state.open, + "always_on_top": state.always_on_top, + }), ); } diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index 634f512..2f22361 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -438,7 +438,7 @@ pub fn run() { browser_view::commands::install_browser_view_browser, browser_view::commands::open_browser_view_popout, browser_view::commands::close_browser_view_popout, - browser_view::commands::is_browser_view_popout_open, + browser_view::commands::get_browser_view_popout_state, browser_view::commands::set_browser_view_popout_always_on_top, // Shared Claude Code auth token commands::auth_token_commands::acquire_claude_token, diff --git a/app/src/components/layout/MainTabs.test.tsx b/app/src/components/layout/MainTabs.test.tsx index d3162b2..0507812 100644 --- a/app/src/components/layout/MainTabs.test.tsx +++ b/app/src/components/layout/MainTabs.test.tsx @@ -45,33 +45,22 @@ const S1 = terminalTabKey("s1"); const S2 = terminalTabKey("s2"); /** - * A stand-in for the DataTransfer jsdom doesn't implement. It only has to - * carry the tab key, which is what a drop falls back to reading. + * A pointer event carrying a real `clientX`. + * + * jsdom implements no `PointerEvent`, so Testing Library's synthesized one has + * no coordinates — and the coordinate is the whole point here, since it decides + * which slot the drop lands in. `MouseEvent` has one, and React dispatches on + * the event's type name either way. */ -function dataTransfer() { - const store: Record = {}; - return { - effectAllowed: "", - dropEffect: "", - setData: (format: string, value: string) => { - store[format] = value; - }, - getData: (format: string) => store[format] ?? "", - }; +function pointer(el: Element, type: string, clientX: number) { + fireEvent(el, new MouseEvent(type, { bubbles: true, cancelable: true, clientX, button: 0 })); } -/** - * A dragover carrying a real `clientX`. - * - * jsdom has no `DragEvent`, so Testing Library's synthesized one is a plain - * `Event` with no pointer coordinates — and the coordinate is the whole point - * here, since it decides which side of a tab the drop lands on. A `MouseEvent` - * has one, and React reads it the same way. - */ -function dragOverAt(el: Element, clientX: number, dt: ReturnType) { - const event = new MouseEvent("dragover", { bubbles: true, cancelable: true, clientX }); - Object.defineProperty(event, "dataTransfer", { value: dt }); - fireEvent(el, event); +/** Press, move past the drag threshold, and release over `endX`. */ +function dragTab(el: Element, fromX: number, endX: number) { + pointer(el, "pointerdown", fromX); + pointer(el, "pointermove", endX); + pointer(el, "pointerup", endX); } /** Pin a tab's geometry so "past the midpoint" means something in jsdom. */ @@ -80,6 +69,13 @@ function place(el: Element, left: number, width = 100) { ({ left, width, right: left + width, top: 0, bottom: 30, height: 30, x: left, y: 0 }) as DOMRect; } +/** Lay the strip out as three 100px tabs starting at x=0. */ +function laidOut() { + const tabs = screen.getAllByRole("tab"); + tabs.forEach((tab, i) => place(tab, i * 100)); + return tabs; +} + const order = () => useAppState.getState().tabOrder; beforeEach(() => { @@ -95,73 +91,124 @@ beforeEach(() => { describe("MainTabs reordering", () => { it("drags a tab to the front", () => { render(); - const tabs = screen.getAllByRole("tab"); - tabs.forEach((tab, i) => place(tab, i * 100)); + const tabs = laidOut(); - const dt = dataTransfer(); - fireEvent.dragStart(tabs[2], { dataTransfer: dt }); - // Left half of the first tab — the marker sits before it. - dragOverAt(tabs[0], 10, dt); - expect(screen.getByTestId("tab-drop-marker")).toBeInTheDocument(); - fireEvent.drop(tabs[0], { dataTransfer: dt }); + // Left half of the first tab — the tab lands before it. + dragTab(tabs[2], 250, 10); expect(order()).toEqual([S2, HOME, S1]); }); it("drops after the tab when the pointer is past its midpoint", () => { render(); - const tabs = screen.getAllByRole("tab"); - tabs.forEach((tab, i) => place(tab, i * 100)); + const tabs = laidOut(); - const dt = dataTransfer(); - fireEvent.dragStart(tabs[0], { dataTransfer: dt }); - dragOverAt(tabs[1], 190, dt); - fireEvent.drop(tabs[1], { dataTransfer: dt }); + dragTab(tabs[0], 50, 190); expect(order()).toEqual([S1, HOME, S2]); }); + it("drops at the end when released past the last tab", () => { + render(); + const tabs = laidOut(); + + dragTab(tabs[0], 50, 800); + + expect(order()).toEqual([S1, S2, HOME]); + }); + it("dragging does not steal the selection", () => { render(); - const tabs = screen.getAllByRole("tab"); - tabs.forEach((tab, i) => place(tab, i * 100)); + const tabs = laidOut(); - const dt = dataTransfer(); - fireEvent.dragStart(tabs[1], { dataTransfer: dt }); - dragOverAt(tabs[2], 290, dt); - fireEvent.drop(tabs[2], { dataTransfer: dt }); + dragTab(tabs[1], 150, 290); expect(order()).toEqual([HOME, S2, S1]); expect(useAppState.getState().activeTabKey).toBe(HOME); }); - it("shows no drop marker until a drag is under way", () => { + it("shows the drop marker only while a drag is under way", () => { render(); + const tabs = laidOut(); + expect(screen.queryByTestId("tab-drop-marker")).toBeNull(); + + pointer(tabs[2], "pointerdown", 250); + pointer(tabs[2], "pointermove", 10); + expect(screen.getByTestId("tab-drop-marker")).toBeInTheDocument(); + + pointer(tabs[2], "pointerup", 10); expect(screen.queryByTestId("tab-drop-marker")).toBeNull(); }); - it("clears the marker when the drag ends without a drop", () => { + it("abandons the drag on Escape, leaving the order alone", () => { render(); - const tabs = screen.getAllByRole("tab"); - tabs.forEach((tab, i) => place(tab, i * 100)); + const tabs = laidOut(); - const dt = dataTransfer(); - fireEvent.dragStart(tabs[2], { dataTransfer: dt }); - dragOverAt(tabs[0], 10, dt); - fireEvent.dragEnd(tabs[2], { dataTransfer: dt }); + pointer(tabs[2], "pointerdown", 250); + pointer(tabs[2], "pointermove", 10); + fireEvent.keyDown(window, { key: "Escape" }); + + expect(screen.queryByTestId("tab-drop-marker")).toBeNull(); + pointer(tabs[2], "pointerup", 10); + expect(order()).toEqual([HOME, S1, S2]); + }); + + it("treats a press that barely moves as a click, not a drag", () => { + render(); + const tabs = laidOut(); + + // Two pixels of tremble, under the threshold. + pointer(tabs[2], "pointerdown", 250); + pointer(tabs[2], "pointermove", 252); + pointer(tabs[2], "pointerup", 252); + fireEvent.click(tabs[2]); + + expect(order()).toEqual([HOME, S1, S2]); + expect(useAppState.getState().activeTabKey).toBe(S2); + }); + + it("does not select the tab it just dropped", () => { + render(); + const tabs = laidOut(); + + dragTab(tabs[2], 250, 10); + // The browser fires a click after the pointerup that ended the drag. + fireEvent.click(tabs[2]); + + expect(order()).toEqual([S2, HOME, S1]); + expect(useAppState.getState().activeTabKey).toBe(HOME); + }); + + it("ignores a press that starts on the close button", () => { + render(); + const tabs = laidOut(); + const close = screen.getByRole("button", { name: "Close shell (bash)" }); + + fireEvent(close, new MouseEvent("pointerdown", { bubbles: true, clientX: 290, button: 0 })); + pointer(tabs[2], "pointermove", 10); expect(screen.queryByTestId("tab-drop-marker")).toBeNull(); expect(order()).toEqual([HOME, S1, S2]); }); - it("leaves a tab being renamed undraggable, so its text stays selectable", () => { + it("does not drag a tab that is being renamed — that drag selects text", () => { render(); - const tabs = screen.getAllByRole("tab"); - expect(tabs[1]).toHaveAttribute("draggable", "true"); - + const tabs = laidOut(); fireEvent.doubleClick(tabs[1]); - expect(screen.getByLabelText("Rename tab")).toBeInTheDocument(); - expect(screen.getAllByRole("tab")[1]).toHaveAttribute("draggable", "false"); + + dragTab(screen.getAllByRole("tab")[1], 150, 10); + + expect(order()).toEqual([HOME, S1, S2]); + }); + + it("carries no drag payload that another element could receive", () => { + // An HTML5 drag would put the tab key in a DataTransfer, and releasing over + // any text field in the app would type `term:…` into it. Pointer events + // have nothing to hand over, and the tabs are not draggable at all. + render(); + for (const tab of screen.getAllByRole("tab")) { + expect(tab).not.toHaveAttribute("draggable", "true"); + } }); }); diff --git a/app/src/components/layout/MainTabs.tsx b/app/src/components/layout/MainTabs.tsx index ab223f2..d0bf25e 100644 --- a/app/src/components/layout/MainTabs.tsx +++ b/app/src/components/layout/MainTabs.tsx @@ -18,6 +18,9 @@ interface ContextMenuState { y: number; } +/** Pixels of horizontal travel before a press becomes a drag rather than a click. */ +const DRAG_THRESHOLD = 4; + const MODE_BADGE: Record = { plan: { text: "plan", className: "bg-[var(--bg-tertiary)] text-[var(--text-secondary)]" }, default: { text: "ask", className: "bg-[var(--bg-tertiary)] text-[var(--text-secondary)]" }, @@ -29,9 +32,8 @@ const MODE_BADGE: Record = * One strip for both main-area tab kinds: Project Home views (⌂) and * terminals (▣). * - * Tabs are draggable. The drag is HTML5's, not a pointer-event - * reimplementation, so the OS supplies the drag image and the Escape-to-cancel - * behaviour for free; the only thing tracked here is where the drop would land. + * Tabs are draggable, on pointer events rather than HTML5 drag-and-drop — see + * `pointerProps` for why neither of the two obvious alternatives works. * `Ctrl+Shift+←/→` does the same thing without a mouse. */ export default function MainTabs() { @@ -53,6 +55,10 @@ export default function MainTabs() { /** The tab being dragged, and the slot it would drop into. */ const [dragKey, setDragKey] = useState(null); const [dropIndex, setDropIndex] = useState(null); + const stripRef = useRef(null); + /** A press that has not yet moved far enough to be a drag. */ + const pending = useRef<{ key: string; startX: number; dragging: boolean } | null>(null); + const suppressClick = useRef(false); useEffect(() => { if (!menu) return; @@ -72,6 +78,20 @@ export default function MainTabs() { } }, [renamingId]); + // Escape abandons a drag — the one affordance a pointer-event drag has to + // supply for itself, since the OS is not running this one. + useEffect(() => { + if (!dragKey) return; + const onKeyDown = (e: KeyboardEvent) => { + if (e.key !== "Escape") return; + pending.current = null; + setDragKey(null); + setDropIndex(null); + }; + window.addEventListener("keydown", onKeyDown); + return () => window.removeEventListener("keydown", onKeyDown); + }, [dragKey]); + if (tabOrder.length === 0) { return (
@@ -152,54 +172,91 @@ export default function MainTabs() { }${dragging ? " opacity-40" : ""}`; const endDrag = () => { + pending.current = null; setDragKey(null); setDropIndex(null); }; /** - * Drag props shared by both tab kinds. + * Which slot the pointer is currently over, as an insertion index into + * `tabOrder`. * - * A tab in rename mode is not draggable: a `draggable` ancestor swallows the - * mouse-drag that selects text inside the input, which would make the rename - * field impossible to select in. + * Measured from the tabs actually on screen rather than from the event's + * target, so the answer is the same whatever the pointer happens to be over — + * including the drop marker itself, and including a `tabOrder` entry whose + * session has already gone and which therefore renders nothing. */ - const dragProps = (key: string, index: number, renaming: boolean) => ({ - draggable: !renaming, - onDragStart: (e: React.DragEvent) => { - e.dataTransfer.effectAllowed = "move"; - // Some platforms refuse to start a drag with an empty payload. - e.dataTransfer.setData("text/plain", key); - setDragKey(key); - setDropIndex(index); + const dropIndexAt = (clientX: number): number => { + const strip = stripRef.current; + if (!strip) return tabOrder.length; + for (const el of strip.querySelectorAll("[data-tab-index]")) { + const rect = el.getBoundingClientRect(); + if (clientX < rect.left + rect.width / 2) return Number(el.dataset.tabIndex); + } + return tabOrder.length; + }; + + /** + * Dragging is done with pointer events, not HTML5 drag-and-drop. + * + * Two reasons, both load-bearing. Tauri's `dragDropEnabled` — which the + * terminal needs left on, because only the native drag-drop event carries + * dropped *file paths* — blocks HTML5 drag inside the webview on Windows, so + * an HTML5 implementation is simply dead there. And an HTML5 drag carries a + * `DataTransfer`: released over any text field in the app, the default + * handler types the payload into it. + */ + const pointerProps = (key: string, renaming: boolean) => ({ + onPointerDown: (e: React.PointerEvent) => { + // Left button only, never from the close button, and never while the + // rename input is up — that drag is a text selection. + if (e.button !== 0 || renaming) return; + if ((e.target as HTMLElement).closest("button, input")) return; + pending.current = { key, startX: e.clientX, dragging: false }; + e.currentTarget.setPointerCapture?.(e.pointerId); }, - onDragOver: (e: React.DragEvent) => { - if (!dragKey) return; // not our drag — a file dropped on the strip isn't one - e.preventDefault(); - e.dataTransfer.dropEffect = "move"; - const rect = e.currentTarget.getBoundingClientRect(); - const after = e.clientX > rect.left + rect.width / 2; - setDropIndex(index + (after ? 1 : 0)); + onPointerMove: (e: React.PointerEvent) => { + const drag = pending.current; + if (!drag) return; + // A few pixels of slop, so a click that trembles stays a click. + if (!drag.dragging && Math.abs(e.clientX - drag.startX) < DRAG_THRESHOLD) return; + drag.dragging = true; + setDragKey(drag.key); + setDropIndex(dropIndexAt(e.clientX)); }, - onDragEnd: endDrag, + onPointerUp: (e: React.PointerEvent) => { + const drag = pending.current; + e.currentTarget.releasePointerCapture?.(e.pointerId); + if (!drag?.dragging) { + pending.current = null; + return; // a plain click: leave it to `onClick` to select the tab + } + const to = dropIndexAt(e.clientX); + const from = tabOrder.indexOf(drag.key); + // `to` is a slot in the strip as it looks *now*; `moveTab` places the tab + // after pulling it out, so every slot past its own shifts down one. + if (from !== -1) moveTab(drag.key, to > from ? to - 1 : to); + // The click that follows this pointerup is the drag's, not a selection. + suppressClick.current = true; + endDrag(); + }, + onPointerCancel: endDrag, }); - /** Drop lands wherever the marker is showing, and only there. */ - const onDrop = (e: React.DragEvent) => { - const key = dragKey ?? e.dataTransfer.getData("text/plain"); - if (!key || dropIndex === null) return endDrag(); - e.preventDefault(); - const from = tabOrder.indexOf(key); - // `dropIndex` is a slot in the strip as it looks *now*; `moveTab` places the - // tab after pulling it out, so every slot past the tab's own shifts down one. - moveTab(key, dropIndex > from ? dropIndex - 1 : dropIndex); - endDrag(); + /** A drag in progress swallows the click it ends with. */ + const activateTab = (key: string) => { + if (suppressClick.current) { + suppressClick.current = false; + return; + } + setActiveTabKey(key); }; const dropMarker = ( - ) : live ? ( + ) : live && poppedOut === false ? (