fix(web): keep import controls inside their scroller, so the document stops scrolling (#116)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
b40262d43c
commit
f63037866e
6 files changed
+139
-27
No files matched your search
@@ -15,6 +15,10 @@
|
||||
* ~34px off-screen left, the skills menu ~92px off-screen right — with its autofocused
|
||||
* search box then horizontally scrolling the whole draft page — and the workspace menu
|
||||
* ~143px off-screen right when the ownership pills share one row);
|
||||
* - no page grows the **document**: the app shell is height-constrained and each page scrolls
|
||||
* inside its own container, so a second scrollbar means an absolutely positioned descendant
|
||||
* escaped its scroller (the Traces tree and the Agent settings page both had one, visible
|
||||
* only with a second Agent below a long list);
|
||||
* - the sidebar's "New chat" button has no background fill (same gray-scale style as nav items);
|
||||
* - the collapsed rail shows, in product-specified order, last conversation / new chat /
|
||||
* Agents / Skills / Models / Costs / Traces / Benchmark with localized (en + zh) hover
|
||||
@@ -646,6 +650,78 @@ test("layout: mobile chat dropdowns stay inside the viewport", async ({ page })
|
||||
).toBeLessThanOrEqual(40);
|
||||
});
|
||||
|
||||
/**
|
||||
* The app shell is height-constrained: every page scrolls inside its own container and the
|
||||
* document itself must never scroll. What breaks that is an absolutely positioned descendant
|
||||
* whose containing block is the initial containing block — nothing between it and the root
|
||||
* clips it, so its static position past the fold grows the **document**, producing a second
|
||||
* scrollbar that drags the whole shell up. The Traces tree and the Agent settings page both
|
||||
* had one (the `sr-only` file inputs of their import controls), visible only once a second
|
||||
* Agent sat below a long list. This sweep is the general guard: with the data that exposes it,
|
||||
* no page may grow the document.
|
||||
*/
|
||||
test("layout: no page grows the document (absolute descendants stay in their scroller)", async ({
|
||||
page,
|
||||
}) => {
|
||||
await provisionAndLogin(page.request, "layoutheight", P);
|
||||
const projects = await (await page.request.get(`${BASE}/api/projects`)).json();
|
||||
const projectId = projects.projects[0].projectId;
|
||||
const put = await page.request.put(`${BASE}/api/projects/${projectId}/models`, {
|
||||
data: {
|
||||
defaultModel: { provider: "custom", modelId: "claude-4-8" },
|
||||
models: [
|
||||
{
|
||||
provider: "custom",
|
||||
modelId: "claude-4-8",
|
||||
apiKey: "sk-mock",
|
||||
baseUrl: MOCK,
|
||||
contextWindow: 200000,
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(put.ok(), "put models").toBeTruthy();
|
||||
// A second Agent is what moves an import control below the fold; the Traces tree only lists
|
||||
// Sessions that have a Trace file, so each one has to actually run a Task.
|
||||
const second = await page.request.post(`${BASE}/api/projects/${projectId}/agents`, {
|
||||
data: { agentId: "agent_two", name: "Agent Two" },
|
||||
});
|
||||
// 409 = the Agent survives from an earlier run of this spec against the same data root
|
||||
// (provisioning is idempotent by convention here, so a spec can be rerun on its own).
|
||||
expect([201, 409], "create second agent").toContain(second.status());
|
||||
for (const agentId of ["default_agent", "agent_two"]) {
|
||||
for (let i = 0; i < (agentId === "default_agent" ? 14 : 2); i += 1) {
|
||||
const created = await (
|
||||
await page.request.post(`${BASE}/api/projects/${projectId}/agents/${agentId}/sessions`, {
|
||||
data: { provider: "custom", modelId: "claude-4-8" },
|
||||
})
|
||||
).json();
|
||||
await page.request.post(`${BASE}/api/sessions/${created.session.sessionId}/tasks`, {
|
||||
data: { input: [{ type: "text", text: "hi" }] },
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// A short viewport puts the second Agent's node below the fold with a handful of Sessions
|
||||
// instead of dozens — the same geometry a full-height window reaches with a longer list.
|
||||
// Not shorter than 420: below ~412px the sidebar's own fixed chrome (project switcher, the
|
||||
// eight nav entries, the user row) no longer fits, which grows the document for its own
|
||||
// unrelated reason and would mask what this test is after.
|
||||
await page.setViewportSize({ width: 1440, height: 420 });
|
||||
const paths = ["/traces", "/chat", "/agents", "/agents/default_agent", "/skills", "/models"];
|
||||
const overflowing = [];
|
||||
for (const path of paths) {
|
||||
await page.goto(`${BASE}${path}`);
|
||||
await page.waitForTimeout(1200);
|
||||
const grew = await page.evaluate(() => {
|
||||
const de = document.documentElement;
|
||||
return de.scrollHeight - de.clientHeight;
|
||||
});
|
||||
if (grew > 0) overflowing.push(`${path} (+${grew}px)`);
|
||||
}
|
||||
expect(overflowing, "pages whose document scrolls").toEqual([]);
|
||||
});
|
||||
|
||||
test("layout: login — blank start, non-crossing traces, lang/theme controls", async ({ page }) => {
|
||||
await page.goto(`${BASE}/login`);
|
||||
await page.getByRole("heading", { name: "PenguinHarness" }).waitFor();
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
/**
|
||||
* File input that is visually hidden but still Tab-focusable — the picker every "upload" /
|
||||
* "import" control in the app wraps in its own `<label>`.
|
||||
*
|
||||
* `display: none` would drop it out of the focus order, so the box has to be visually hidden
|
||||
* the `sr-only` way, which means `position: absolute`. That is exactly the shape that used to
|
||||
* escape its scroller and stretch the document (see the scroll-container invariant in
|
||||
* styles.css): dropped into a `static` ancestor chain, its containing block became the
|
||||
* initial containing block, so a control sitting past the fold added document-level
|
||||
* scrollable overflow and a second scrollbar.
|
||||
*
|
||||
* Hence the wrapper: it is `relative`, so the containing block travels **with the control**
|
||||
* instead of depending on whichever ancestor happens to be positioned. The wrapper is
|
||||
* `contents`-free on purpose — it is a real box, but an empty inline one with an absolutely
|
||||
* positioned child, so it contributes no width and no height to the label around it.
|
||||
*/
|
||||
import type { InputHTMLAttributes } from "react";
|
||||
|
||||
export function HiddenFileInput(props: Omit<InputHTMLAttributes<HTMLInputElement>, "type">) {
|
||||
return (
|
||||
<span className="relative">
|
||||
<input type="file" className="sr-only" {...props} />
|
||||
</span>
|
||||
);
|
||||
}
|
||||
@@ -27,6 +27,7 @@ import { useProject } from "../../state/project";
|
||||
import { Tabs } from "../../components/ui/tabs";
|
||||
import { Button } from "../../components/ui/button";
|
||||
import { toastError, toastInfo, toastSuccess } from "../../components/ui/toast";
|
||||
import { HiddenFileInput } from "../../components/ui/hidden-file-input";
|
||||
import { Input, Textarea } from "../../components/ui/input";
|
||||
import { OptionMenu, type OptionMenuChoice } from "../../components/ui/option-menu";
|
||||
import { Switch } from "../../components/ui/switch";
|
||||
@@ -156,7 +157,9 @@ export function AgentSettingsPage() {
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="no-scrollbar h-full overflow-y-auto p-4 md:p-6">
|
||||
/* relative: a scroller is its own containing block (see the invariant in styles.css) —
|
||||
this page's snapshot-import control used to grow the document from below the fold. */
|
||||
<div className="no-scrollbar relative h-full overflow-y-auto p-4 md:p-6">
|
||||
<div className="mx-auto max-w-3xl">
|
||||
<Button
|
||||
variant="ghost"
|
||||
@@ -351,14 +354,7 @@ function OverviewTab({
|
||||
<label
|
||||
className={`${TRANSFER_BUTTON_CLASS} ${importing ? "pointer-events-none opacity-60" : ""}`}
|
||||
>
|
||||
{/* sr-only rather than hidden: keeps it keyboard-Tab-focusable (same as the workspace-browser upload). */}
|
||||
<input
|
||||
type="file"
|
||||
accept=".tar.gz,.tgz"
|
||||
className="sr-only"
|
||||
disabled={importing}
|
||||
onChange={onPickFile}
|
||||
/>
|
||||
<HiddenFileInput accept=".tar.gz,.tgz" disabled={importing} onChange={onPickFile} />
|
||||
{importing ? S.agent.importing : S.agent.importSnapshot}
|
||||
</label>
|
||||
)}
|
||||
|
||||
@@ -27,6 +27,7 @@ import { Button } from "../../components/ui/button";
|
||||
import { ConfirmModal } from "../../components/ui/confirm-modal";
|
||||
import { toastError, toastSuccess } from "../../components/ui/toast";
|
||||
import { Dropdown } from "../../components/ui/dropdown";
|
||||
import { HiddenFileInput } from "../../components/ui/hidden-file-input";
|
||||
import { SkeletonList } from "../../components/ui/skeleton";
|
||||
import { CodeBlock } from "./code-block";
|
||||
|
||||
@@ -745,14 +746,7 @@ export function WorkspaceBrowser({
|
||||
</Button>
|
||||
{/* Matches the same visual style and font size (sm = text-xs) as the adjacent ghost Buttons (Details/Refresh): no border, light background on hover. */}
|
||||
<label className="inline-flex cursor-pointer items-center rounded-md border border-transparent bg-transparent px-2.5 py-1 text-xs font-medium text-gray-600 transition-colors duration-150 focus-within:ring-2 focus-within:ring-gray-400/30 hover:bg-gray-100 hover:text-gray-900 dark:text-gray-300 dark:hover:bg-gray-800 dark:hover:text-gray-100">
|
||||
{/* sr-only rather than hidden: keyboard users can still Tab-focus it (display:none would remove it from the focus order). */}
|
||||
<input
|
||||
type="file"
|
||||
multiple
|
||||
className="sr-only"
|
||||
onChange={onUpload}
|
||||
disabled={uploading}
|
||||
/>
|
||||
<HiddenFileInput multiple onChange={onUpload} disabled={uploading} />
|
||||
{uploading ? S.common.saving : S.files.upload}
|
||||
</label>
|
||||
</div>
|
||||
|
||||
@@ -17,6 +17,7 @@ import { formatBytes } from "../../lib/format";
|
||||
import { agentDisplayName, useProject } from "../../state/project";
|
||||
import { AgentAvatar } from "../../components/ui/agent-avatar";
|
||||
import { Chevron } from "../../components/ui/chevron";
|
||||
import { HiddenFileInput } from "../../components/ui/hidden-file-input";
|
||||
import { DownloadIcon, UploadIcon } from "../../components/ui/icons";
|
||||
import { toastError } from "../../components/ui/toast";
|
||||
import { Truncated } from "../../components/ui/truncated";
|
||||
@@ -197,14 +198,7 @@ function AgentNode({
|
||||
importing ? "pointer-events-none opacity-60" : ""
|
||||
}`}
|
||||
>
|
||||
{/* sr-only rather than hidden: keeps it keyboard-Tab-focusable (same as the snapshot import). */}
|
||||
<input
|
||||
type="file"
|
||||
accept=".jsonl"
|
||||
className="sr-only"
|
||||
disabled={importing}
|
||||
onChange={onPickFile}
|
||||
/>
|
||||
<HiddenFileInput accept=".jsonl" disabled={importing} onChange={onPickFile} />
|
||||
<UploadIcon size={13} />
|
||||
<span className="sr-only">{importing ? S.traces.importing : S.traces.importTrace}</span>
|
||||
</label>
|
||||
@@ -297,8 +291,12 @@ export function TracesPage() {
|
||||
|
||||
return (
|
||||
<div className="flex h-full flex-col md:flex-row">
|
||||
{/* Directory tree: Agent → Session title (≥md left column; <md top collapsible area) */}
|
||||
<aside className="max-h-52 shrink-0 overflow-y-auto border-b border-gray-200 bg-gray-50 px-1 py-2 md:max-h-none md:w-72 md:border-b-0 md:border-r dark:border-gray-800 dark:bg-gray-900">
|
||||
{/* Directory tree: Agent → Session title (≥md left column; <md top collapsible area).
|
||||
relative: a scroller is its own containing block — the invariant and the failure it
|
||||
prevents are documented in styles.css; this is where it first bit (an Agent node past
|
||||
the fold put its import control's absolutely positioned box at a document-level
|
||||
offset, growing the document). */}
|
||||
<aside className="relative max-h-52 shrink-0 overflow-y-auto border-b border-gray-200 bg-gray-50 px-1 py-2 md:max-h-none md:w-72 md:border-b-0 md:border-r dark:border-gray-800 dark:bg-gray-900">
|
||||
<p className="px-3 pb-1 text-xs font-bold uppercase tracking-wide text-gray-500">
|
||||
{S.traces.title}
|
||||
</p>
|
||||
|
||||
@@ -76,6 +76,29 @@
|
||||
#root {
|
||||
height: 100%;
|
||||
}
|
||||
/* Every scroll container is its own containing block.
|
||||
The shell is height-constrained (the three rules above) and each page scrolls inside its
|
||||
own container, so the document must never scroll. What breaks that is an absolutely
|
||||
positioned descendant of a scroller that is itself `static`: its containing block is then
|
||||
the initial containing block, which nothing clips — not the scroller's own overflow, not
|
||||
an `overflow-hidden` ancestor — so its static position past the fold becomes the
|
||||
*document's* scrollable overflow, and a second scrollbar appears that drags the whole
|
||||
shell. That bug shipped three times (sidebar session list, Traces tree, Agent settings),
|
||||
every time through a visually-hidden-but-focusable control, so the invariant is set here
|
||||
once instead of being remembered per scroller.
|
||||
Scoped to the scrollable utilities on purpose: `overflow-hidden` is also used as a plain
|
||||
clipping box (the docked panels' clipping windows), where forcing a containing block is
|
||||
more likely to surprise than to help. Declared in `base`, so an explicit
|
||||
absolute/fixed/sticky utility on the same element still wins; `position: relative` at
|
||||
`z-index: auto` creates no stacking context, so the convention in the file header holds. */
|
||||
.overflow-auto,
|
||||
.overflow-x-auto,
|
||||
.overflow-y-auto,
|
||||
.overflow-scroll,
|
||||
.overflow-x-scroll,
|
||||
.overflow-y-scroll {
|
||||
position: relative;
|
||||
}
|
||||
body {
|
||||
@apply bg-white text-gray-900 antialiased dark:bg-gray-950 dark:text-gray-100;
|
||||
font-family:
|
||||
|
||||
Reference in new issue
Block a user