From 4a0ef3c66c2644610e95a613fe643754bba06dbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Thu, 23 Jul 2026 22:29:49 +0100 Subject: [PATCH] feat: improve task sidebar overflow cue (#1908) --- .agents/skills/pr/SKILL.md | 31 ++++- apps/packages/ui/src/scroll-area.tsx | 22 ++- apps/web/app/globals.css | 41 ++++++ .../app-sidebar/sections/tasks-section.tsx | 2 +- .../components/task/task-session-sidebar.tsx | 7 +- .../task/task-sidebar-scroll-area.tsx | 56 ++++++++ .../task/sidebar-scroll-preservation.spec.ts | 130 +++++++++++++++++- 7 files changed, 276 insertions(+), 13 deletions(-) create mode 100644 apps/web/components/task/task-sidebar-scroll-area.tsx diff --git a/.agents/skills/pr/SKILL.md b/.agents/skills/pr/SKILL.md index 03b4d637f4..0217538124 100644 --- a/.agents/skills/pr/SKILL.md +++ b/.agents/skills/pr/SKILL.md @@ -102,8 +102,9 @@ explicitly requests task tracking. commit=$(git commit-tree "$tree" -m "media: screenshots for PR #") git push origin "$commit:refs/heads/media/pr--screenshots" ``` - (A quoting glitch can make the first push report failure — retry with the literal commit SHA.) Reference each image in the PR body under a `## Screenshots` section using dash-named files (no spaces): - `https://raw.githubusercontent.com////shot.png` + (A quoting glitch can make the first push report failure — retry with the literal commit SHA.) Reference each image in the PR body under a `## Screenshots` section using GitHub-Flavored Markdown image syntax and dash-named files (no spaces): + `![Short descriptive caption](https://raw.githubusercontent.com////shot.png)` + Bare image URLs render as clickable links instead of previews, so never add a screenshot URL without the surrounding `![alt text](...)` wrapper. Append the section to the PR body: ```bash @@ -115,6 +116,32 @@ explicitly requests task tracking. gh api --method PATCH repos/:owner/:repo/pulls/ --input /tmp/pr-body-payload.json ``` + **Preserve the existing PR description:** The PR body is a shared, mutable + document. Preview automation and other bots may add sections after the PR + is created, so never PATCH a body reconstructed from the creation-time + template or a stale `/tmp/pr-body.md`. Before every post-creation update: + + 1. Fetch the current body from GitHub and use that exact response as the + merge base: + `gh pr view --json body --jq .body > /tmp/pr-body-latest.md`. + 2. Change only the section owned by this operation. For screenshots, wrap + the section in `` and + `` markers (or replace the existing + `## Screenshots` block if those markers are not present). Preserve every + byte outside that range, including + ` ... `. + 3. Re-fetch the body immediately before PATCH and compare it with the + snapshot used in step 1. If it changed, discard the pending payload, + re-fetch, and merge again; do not overwrite the newer body. + 4. After PATCH, read the body back and verify both the intended change and + all previously present sentinel sections are still present. + + The REST PATCH endpoint replaces the complete body and does not provide a + convenient description-level compare-and-swap, so this fetch/merge/check + sequence is required even when the edit appears small. + + Before treating screenshot publication as complete, inspect the submitted body and verify every screenshot entry is a Markdown image embed (`![...](https://raw.githubusercontent.com/.../*.png)`) rather than a bare URL. Use `gh pr view --json body --jq .body` for this check. + Never commit the screenshot binaries to the PR branch itself — only to the throwaway `media/pr--screenshots` ref (`git rm` them from the PR branch tip if they were committed there earlier; with squash-merge, deleting at tip is enough). The `docs/screenshots/` directory is for product/docs imagery that is meant to merge — don't confuse the two. The media branch must survive branch-cleanup sweeps; deleting it 404s the images in the PR body, so don't treat "unmerged branch" as automatically safe to delete. If the capture worker reported that screenshots are impossible in the diff --git a/apps/packages/ui/src/scroll-area.tsx b/apps/packages/ui/src/scroll-area.tsx index e13a927efb..5dd3650746 100644 --- a/apps/packages/ui/src/scroll-area.tsx +++ b/apps/packages/ui/src/scroll-area.tsx @@ -5,11 +5,17 @@ import { ScrollArea as ScrollAreaPrimitive } from "radix-ui"; import { cn } from "./lib/utils"; -function ScrollArea({ - className, - children, - ...props -}: React.ComponentProps) { +type DataAttributes = { + [key: `data-${string}`]: string | number | boolean | undefined; +}; + +type ScrollAreaProps = React.ComponentProps & { + viewportProps?: React.ComponentProps & DataAttributes; +}; + +function ScrollArea({ className, children, viewportProps, ...props }: ScrollAreaProps) { + const { className: viewportClassName, ...restViewportProps } = viewportProps ?? {}; + return ( {children} diff --git a/apps/web/app/globals.css b/apps/web/app/globals.css index 50c80961b9..e5bb7ad47e 100644 --- a/apps/web/app/globals.css +++ b/apps/web/app/globals.css @@ -358,6 +358,47 @@ background-clip: padding-box; } +.task-sidebar-scroll[data-can-scroll-down="true"] { + mask-image: linear-gradient(to bottom, black 0, black calc(100% - 3.5rem), transparent 100%); + -webkit-mask-image: linear-gradient( + to bottom, + black 0, + black calc(100% - 3.5rem), + transparent 100% + ); +} + +.task-sidebar-scroll-root [data-slot="scroll-area-scrollbar"] { + z-index: 1; + background: transparent; +} + +.task-sidebar-scroll-root [data-slot="scroll-area-thumb"] { + background: var(--scrollbar-thumb); +} + +.task-sidebar-scroll-root [data-slot="scroll-area-thumb"]:hover { + background: var(--scrollbar-thumb-hover); +} + +@media (hover: hover) and (pointer: fine) { + .task-sidebar-scroll-root [data-slot="scroll-area-scrollbar"] { + opacity: 0; + pointer-events: none; + transition: opacity 120ms ease; + } + + .task-sidebar-scroll-root:is(:hover, :focus-within) .task-sidebar-scroll { + mask-image: none; + -webkit-mask-image: none; + } + + .task-sidebar-scroll-root:is(:hover, :focus-within) [data-slot="scroll-area-scrollbar"] { + opacity: 1; + pointer-events: auto; + } +} + /* xterm.js scrollbar styling */ .xterm .xterm-scrollable-element > .scrollbar { width: 10px !important; diff --git a/apps/web/components/app-sidebar/sections/tasks-section.tsx b/apps/web/components/app-sidebar/sections/tasks-section.tsx index a501dc348c..6547563eb4 100644 --- a/apps/web/components/app-sidebar/sections/tasks-section.tsx +++ b/apps/web/components/app-sidebar/sections/tasks-section.tsx @@ -24,7 +24,7 @@ export function TasksSection({ collapsed }: TasksSectionProps) { headerAction={} grow > -
+
diff --git a/apps/web/components/task/task-session-sidebar.tsx b/apps/web/components/task/task-session-sidebar.tsx index 0b8b3ec2de..7049402b18 100644 --- a/apps/web/components/task/task-session-sidebar.tsx +++ b/apps/web/components/task/task-session-sidebar.tsx @@ -13,7 +13,8 @@ import { buildTaskSwitcherProps } from "./task-session-sidebar-switcher-props"; import { SidebarFilterBar } from "./sidebar-filter/sidebar-filter-bar"; import { MOCK_ITEMS, MOCK_SIDEBAR } from "./sidebar-mock-data"; import { SidebarDialogs } from "./task-session-sidebar-dialogs"; -import { PanelRoot, PanelBody } from "./panel-primitives"; +import { PanelRoot } from "./panel-primitives"; +import { TaskSidebarScrollArea } from "./task-sidebar-scroll-area"; import { useAppStore, useAppStoreApi } from "@/components/state-provider"; import { useWorkspaceSidebarTasks } from "@/hooks/domains/kanban/use-workspace-sidebar-tasks"; import { useTaskActions, useArchiveAndSwitchTask } from "@/hooks/use-task-actions"; @@ -626,10 +627,10 @@ export const TaskSessionSidebar = memo(function TaskSessionSidebar({ return ( {!hideFilterBar && } - + - + (null); + const contentRef = useRef(null); + const [canScrollDown, setCanScrollDown] = useState(false); + + const updateScrollCue = useCallback(() => { + const element = scrollRef.current; + if (!element) return; + const remaining = element.scrollHeight - element.clientHeight - element.scrollTop; + setCanScrollDown(remaining > SCROLL_END_TOLERANCE_PX); + }, []); + + useLayoutEffect(() => { + const scrollElement = scrollRef.current; + if (!scrollElement) return; + + updateScrollCue(); + scrollElement.addEventListener("scroll", updateScrollCue, { passive: true }); + window.addEventListener("resize", updateScrollCue); + + const observer = + typeof ResizeObserver === "undefined" ? null : new ResizeObserver(updateScrollCue); + observer?.observe(scrollElement); + if (contentRef.current) observer?.observe(contentRef.current); + + return () => { + scrollElement.removeEventListener("scroll", updateScrollCue); + window.removeEventListener("resize", updateScrollCue); + observer?.disconnect(); + }; + }, [updateScrollCue]); + + return ( + +
+ {children} +
+ + ); +} diff --git a/apps/web/e2e/tests/task/sidebar-scroll-preservation.spec.ts b/apps/web/e2e/tests/task/sidebar-scroll-preservation.spec.ts index 818f428db1..f236b1bde5 100644 --- a/apps/web/e2e/tests/task/sidebar-scroll-preservation.spec.ts +++ b/apps/web/e2e/tests/task/sidebar-scroll-preservation.spec.ts @@ -1,6 +1,9 @@ import { test, expect } from "../../fixtures/test-base"; import { SessionPage } from "../../pages/session-page"; +const OVERLAY_SCROLLBAR_SELECTOR = + "[data-slot='scroll-area-scrollbar'][data-orientation='vertical']"; + /** * Regression: when clicking another task in the sidebar, the sidebar's * scroll position would jump back to the top. Dockview rebuilds panel slots @@ -9,7 +12,132 @@ import { SessionPage } from "../../pages/session-page"; * position. usePortalSlot now snapshots scroll positions inside each portal * via a capturing scroll listener and restores them after every reattach. */ -test.describe("sidebar scroll preservation across task switch", () => { +test.describe("sidebar scrolling", () => { + test("keeps the empty list transparent and uses the full sidebar width", async ({ testPage }) => { + await testPage.goto("/"); + + const tasksToggle = testPage.getByRole("button", { name: "Tasks", exact: true }); + if ((await tasksToggle.getAttribute("aria-expanded")) !== "true") { + await tasksToggle.click(); + } + + const appSidebar = testPage.getByTestId("app-sidebar"); + const taskSidebar = testPage.getByTestId("task-sidebar"); + const scrollRoot = taskSidebar.locator("[data-slot='scroll-area']"); + await expect(taskSidebar.getByText("No tasks yet.")).toBeVisible(); + + await expect + .poll(() => scrollRoot.evaluate((element) => getComputedStyle(element).backgroundColor)) + .toBe("rgba(0, 0, 0, 0)"); + + const [navigationBox, taskSidebarBox] = await Promise.all([ + appSidebar.locator("nav").boundingBox(), + taskSidebar.boundingBox(), + ]); + expect(navigationBox).not.toBeNull(); + expect(taskSidebarBox).not.toBeNull(); + expect(taskSidebarBox!.x).toBeCloseTo(navigationBox!.x, 0); + expect(taskSidebarBox!.x + taskSidebarBox!.width).toBeCloseTo( + navigationBox!.x + navigationBox!.width, + 0, + ); + }); + + test("fades overflowing tasks and reveals the scrollbar on hover", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + + const taskCount = 25; + const taskIds: string[] = []; + for (let index = 0; index < taskCount; index++) { + const task = await apiClient.createTask( + seedData.workspaceId, + `Overflow Task ${String(index).padStart(2, "0")}`, + { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + taskIds.push(task.id); + } + + await testPage.goto(`/t/${taskIds.at(-1)!}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + + const scrollContainer = testPage.getByTestId("task-sidebar-scroll"); + await expect(scrollContainer).toBeVisible(); + await expect(session.sidebar.getByTestId("sidebar-task-item")).toHaveCount(taskCount, { + timeout: 10_000, + }); + + const overlayGeometry = await scrollContainer.evaluate((element) => { + // Headless Chromium uses platform overlay scrollbars, while desktop + // WebViews can reserve a native gutter. Force that platform mode so the + // regression is deterministic in CI. + element.style.scrollbarGutter = "stable"; + const row = element.querySelector("[data-testid='sidebar-task-item']"); + if (!row) throw new Error("Expected a rendered sidebar task row"); + const containerRect = element.getBoundingClientRect(); + const rowRect = row.getBoundingClientRect(); + return { + rowRightGap: containerRect.right - rowRect.right, + }; + }); + expect(overlayGeometry.rowRightGap).toBeLessThanOrEqual(1); + + await expect(scrollContainer).toHaveAttribute("data-can-scroll-down", "true"); + const overlayScrollbar = session.sidebar.locator(OVERLAY_SCROLLBAR_SELECTOR); + await expect(overlayScrollbar).toBeAttached(); + const restingStyles = await scrollContainer.evaluate((element) => { + const styles = getComputedStyle(element); + return { + maskImage: styles.maskImage, + }; + }); + expect(restingStyles.maskImage).toContain("linear-gradient"); + await expect(overlayScrollbar).toHaveCSS("opacity", "0"); + + await scrollContainer.hover(); + const hoverStyles = await scrollContainer.evaluate((element) => { + const styles = getComputedStyle(element); + return { + maskImage: styles.maskImage, + }; + }); + expect(hoverStyles.maskImage).toBe("none"); + await expect(overlayScrollbar).toHaveCSS("opacity", "1"); + + await scrollContainer.evaluate((element) => { + element.scrollTop = element.scrollHeight; + }); + await expect(scrollContainer).toHaveAttribute("data-can-scroll-down", "false"); + }); + + test("does not mount an overlay scrollbar when the task list fits", async ({ + testPage, + apiClient, + seedData, + }) => { + const task = await apiClient.createTask(seedData.workspaceId, "Short Task List", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + + const scrollContainer = testPage.getByTestId("task-sidebar-scroll"); + await expect(scrollContainer).toHaveAttribute("data-can-scroll-down", "false"); + await expect(session.sidebar.locator(OVERLAY_SCROLLBAR_SELECTOR)).toHaveCount(0); + }); + test("clicking another task does not reset the sidebar scroll position", async ({ testPage, apiClient,