Stop the terminal and Files panes refusing drops onto their own chrome
The z-order gate added last round asked `el.contains(elementFromPoint(x, y))` — "is the thing painted here mine?" — and was handed `TerminalView`'s inner xterm host while every overlay in that pane is a *sibling* of it. So any point under the pane's own chrome answered "not mine" and the drop was refused, with no message and no log line. The "▼ Following / ▽ Paused" toggle is rendered unconditionally at `absolute top-2 right-4`, and `ToastHost` is `fixed bottom-4 right-4` 24rem wide with error cards that never time out: two corners of the terminal, and one of the Files pane, that could not accept a file for as long as the app was running. It shipped green because jsdom has no `elementFromPoint`, so not one of the 81 drop tests entered that branch. The tests here install one. The question the gate asks is now "is a *blocking overlay* painted here?". Chrome the pane paints over itself is not one; a dialog backdrop is, and `ui/Modal` marks its own backdrop so the element `elementFromPoint` actually returns is the one carrying the marker. `classifyDrop` also separates "aimed at me and swallowed" from "not my drop", so the first gets a toast and a log line and the second stays silent. Three defects around it: - **A dialog now refuses only the points it covers.** `dropIsBlocked` is document-wide and `ui/Modal` portals to `document.body`, so any open dialog refused every drop in the window. The deeper half of that is that a dialog opened in project A really was still on screen after a tab switch — the pane hides itself with a `hidden` class, which a portal does not inherit — so `PaneVisibility` lets `App` tell a `Modal` its pane stepped aside, and a hidden one paints nothing, traps no focus, answers no Escape and blocks no drop while staying mounted with its state intact. - **`devicePixelRatio` is applied on Windows only.** Only wry's WebView2 backend hands over physical pixels; the macOS and GTK ones deliver logical points and `tauri-runtime-wry` does not rescale them. Halving those was survivable while the test was a bare rect and is a refused drop once z-order joins in. Read from the wry/tauri sources, not verified on a HiDPI Mac or GTK box. - **`isFileExistsError` can no longer be forged by a filename.** It matched `fileexists` anywhere in a normalised error, so uploading a host file called `file-exists.txt` turned *any* failure into a collision — and Replace re-invoked the upload with `overwrite: true`. The marker now has to stand alone in the backend's canonical form, or be a whole discriminant value. - **A refused compaction or cache-clear keeps its dialog.** `reclaim` reports refusals inside `Ok`, so "did it throw" read one as success: the dialog closed, the tick list was dropped, and the explanation appeared in the outcome panel several screens above the row that was clicked. The dialog now stays put and renders the backend's own sentence verbatim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
This commit is contained in:
@@ -1,6 +1,8 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { render, screen, fireEvent, act } from "@testing-library/react";
|
||||
import Modal from "./Modal";
|
||||
import { PaneVisibilityProvider } from "./PaneVisibility";
|
||||
import { dropIsBlocked } from "../../lib/dropTarget";
|
||||
|
||||
/**
|
||||
* Modal focuses asynchronously via rAF so the panel is laid out first; jsdom
|
||||
@@ -102,6 +104,64 @@ describe("Modal", () => {
|
||||
expect(container).toBeTruthy();
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Stepping aside with the pane that owns it
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
it("marks its backdrop as swallowing native file drops", async () => {
|
||||
// The backdrop, not the panel, is what `elementFromPoint` returns for a
|
||||
// drop released beside the dialog — so it is the element that has to carry
|
||||
// the marker `lib/dropTarget` looks for.
|
||||
render(
|
||||
<Modal title="Reset" onClose={vi.fn()}>
|
||||
<p>body</p>
|
||||
</Modal>,
|
||||
);
|
||||
const backdrop = document.querySelector(".fixed.inset-0");
|
||||
expect(backdrop).toHaveAttribute("data-blocks-drop", "true");
|
||||
expect(dropIsBlocked()).toBe(true);
|
||||
});
|
||||
|
||||
it("paints nothing, traps nothing and blocks no drop while its pane is hidden", async () => {
|
||||
// A dialog portals to `document.body`, where the `hidden` class its pane
|
||||
// uses to step aside for another tab cannot reach it. Left to itself it
|
||||
// stayed on screen over the tab the user switched to, kept its Escape
|
||||
// binding, and refused every native file drop in the window.
|
||||
const onClose = vi.fn();
|
||||
const { rerender } = render(
|
||||
<PaneVisibilityProvider visible={false}>
|
||||
<Modal title="Reset" onClose={onClose}>
|
||||
<button>Confirm</button>
|
||||
</Modal>
|
||||
</PaneVisibilityProvider>,
|
||||
);
|
||||
await flushFocus();
|
||||
|
||||
const backdrop = document.querySelector(".fixed.inset-0") as HTMLElement;
|
||||
expect(backdrop.hidden).toBe(true);
|
||||
expect(backdrop.style.display).toBe("none");
|
||||
expect(dropIsBlocked()).toBe(false);
|
||||
expect(backdrop.contains(document.activeElement)).toBe(false);
|
||||
|
||||
fireEvent.keyDown(document, { key: "Escape" });
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
|
||||
// Back on screen: the same dialog, still mounted, resumes everything.
|
||||
rerender(
|
||||
<PaneVisibilityProvider visible={true}>
|
||||
<Modal title="Reset" onClose={onClose}>
|
||||
<button>Confirm</button>
|
||||
</Modal>
|
||||
</PaneVisibilityProvider>,
|
||||
);
|
||||
await flushFocus();
|
||||
expect(backdrop.hidden).toBe(false);
|
||||
expect(dropIsBlocked()).toBe(true);
|
||||
expect(screen.getByRole("dialog").contains(document.activeElement)).toBe(true);
|
||||
fireEvent.keyDown(document, { key: "Escape" });
|
||||
expect(onClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("ignores Escape and overlay clicks when not dismissible", async () => {
|
||||
const onClose = vi.fn();
|
||||
render(
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { useCallback, useEffect, useId, useRef, type ReactNode } from "react";
|
||||
import { createPortal } from "react-dom";
|
||||
import { usePaneVisible } from "./PaneVisibility";
|
||||
|
||||
const FOCUSABLE_SELECTOR = [
|
||||
"a[href]",
|
||||
@@ -66,26 +67,43 @@ export default function Modal({
|
||||
const restoreFocusRef = useRef<HTMLElement | null>(null);
|
||||
const titleId = useId();
|
||||
const descId = useId();
|
||||
// A dialog portals to `document.body`, so the `hidden` class its pane uses to
|
||||
// step aside for another tab cannot reach it. `PaneVisibility` is how it
|
||||
// finds out, and while it is false this dialog paints nothing, traps
|
||||
// nothing, and — via `[hidden]` — blocks no native file drop.
|
||||
const paneVisible = usePaneVisible();
|
||||
const paneVisibleRef = useRef(paneVisible);
|
||||
paneVisibleRef.current = paneVisible;
|
||||
|
||||
// Remember what had focus, move focus inside, restore on unmount.
|
||||
// Remember what had focus, and restore it on unmount — but not if the pane
|
||||
// is hidden by then: a dialog closed while the user is on another tab would
|
||||
// otherwise yank focus back to a control they cannot see.
|
||||
useEffect(() => {
|
||||
restoreFocusRef.current = document.activeElement as HTMLElement | null;
|
||||
const panel = panelRef.current;
|
||||
if (panel) {
|
||||
const target =
|
||||
initialFocusRef?.current ?? focusableWithin(panel)[0] ?? panel;
|
||||
// Defer so the panel is laid out (offsetParent) before we query it.
|
||||
requestAnimationFrame(() => target.focus?.());
|
||||
}
|
||||
return () => {
|
||||
restoreFocusRef.current?.focus?.();
|
||||
if (paneVisibleRef.current) restoreFocusRef.current?.focus?.();
|
||||
};
|
||||
// Mount/unmount only — re-running would steal focus mid-interaction.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, []);
|
||||
|
||||
// Escape closes; Tab is trapped inside the panel.
|
||||
// Move focus inside — on mount, and again whenever the pane comes back.
|
||||
useEffect(() => {
|
||||
if (!paneVisible) return;
|
||||
const panel = panelRef.current;
|
||||
if (!panel) return;
|
||||
const target = initialFocusRef?.current ?? focusableWithin(panel)[0] ?? panel;
|
||||
// Defer so the panel is laid out (offsetParent) before we query it.
|
||||
const frame = requestAnimationFrame(() => target.focus?.());
|
||||
return () => cancelAnimationFrame(frame);
|
||||
// `initialFocusRef` is a ref object; re-running on its identity would steal
|
||||
// focus mid-interaction.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [paneVisible]);
|
||||
|
||||
// Escape closes; Tab is trapped inside the panel. Neither applies while the
|
||||
// pane is hidden — those keystrokes belong to whatever the user is looking
|
||||
// at instead.
|
||||
useEffect(() => {
|
||||
if (!paneVisible) return;
|
||||
const onKeyDown = (e: KeyboardEvent) => {
|
||||
if (e.key === "Escape" && dismissible) {
|
||||
e.stopPropagation();
|
||||
@@ -119,7 +137,7 @@ export default function Modal({
|
||||
};
|
||||
document.addEventListener("keydown", onKeyDown, true);
|
||||
return () => document.removeEventListener("keydown", onKeyDown, true);
|
||||
}, [dismissible, onClose]);
|
||||
}, [dismissible, onClose, paneVisible]);
|
||||
|
||||
const handleOverlayClick = useCallback(
|
||||
(e: React.MouseEvent<HTMLDivElement>) => {
|
||||
@@ -133,6 +151,15 @@ export default function Modal({
|
||||
ref={overlayRef}
|
||||
onClick={handleOverlayClick}
|
||||
className="fixed inset-0 bg-black/60 flex items-center justify-center z-50 p-4"
|
||||
/* The backdrop, not the panel, is what `elementFromPoint` returns for a
|
||||
drop released beside the dialog — so it is the element that has to say
|
||||
"I swallow drops". See `lib/dropTarget.ts`. */
|
||||
data-blocks-drop="true"
|
||||
hidden={!paneVisible}
|
||||
aria-hidden={paneVisible ? undefined : true}
|
||||
/* `hidden` is a base-layer rule and `flex` is a utility-layer one, so the
|
||||
attribute alone loses. Inline wins over both. */
|
||||
style={paneVisible ? undefined : { display: "none" }}
|
||||
>
|
||||
<div
|
||||
ref={panelRef}
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
import { createContext, useContext, type ReactNode } from "react";
|
||||
|
||||
/**
|
||||
* "Is the pane I belong to the one on screen?"
|
||||
*
|
||||
* The main area keeps every tab *mounted* and hides the inactive ones with a
|
||||
* `hidden` class, so their state survives a tab switch. A `ui/Modal` opened
|
||||
* inside one of those panes does not go quiet when its pane does: it portals
|
||||
* to `document.body`, where an ancestor's `display:none` cannot reach it. So a
|
||||
* dialog opened in project A stayed painted over project B after a tab switch,
|
||||
* kept its focus trap and its Escape binding, and — because it is a blocking
|
||||
* overlay — refused every native file drop in the window.
|
||||
*
|
||||
* `App` publishes the answer around each pane it mounts — it is what decides
|
||||
* which one is on screen — and `Modal` reads it. Nothing else needs to:
|
||||
* dialogs are the only thing in the app that escapes its pane's subtree.
|
||||
*
|
||||
* Default `true`, so a dialog with no pane above it — host settings, the
|
||||
* Docker install prompt — behaves exactly as it always has.
|
||||
*/
|
||||
const PaneVisibilityContext = createContext(true);
|
||||
|
||||
export function PaneVisibilityProvider({
|
||||
visible,
|
||||
children,
|
||||
}: {
|
||||
visible: boolean;
|
||||
children: ReactNode;
|
||||
}) {
|
||||
return (
|
||||
<PaneVisibilityContext.Provider value={visible}>
|
||||
{children}
|
||||
</PaneVisibilityContext.Provider>
|
||||
);
|
||||
}
|
||||
|
||||
/** True unless an ancestor pane says it is currently hidden. */
|
||||
export function usePaneVisible(): boolean {
|
||||
return useContext(PaneVisibilityContext);
|
||||
}
|
||||
Reference in New Issue
Block a user