feat: improve task sidebar overflow cue (#1908)
This commit is contained in:
@@ -102,8 +102,9 @@ explicitly requests task tracking.
|
||||
commit=$(git commit-tree "$tree" -m "media: screenshots for PR #<N>")
|
||||
git push origin "$commit:refs/heads/media/pr-<N>-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/<owner>/<repo>/<media-commit-sha>/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):
|
||||
``
|
||||
Bare image URLs render as clickable links instead of previews, so never add a screenshot URL without the surrounding `` 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/<PR_NUMBER> --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 <PR_NUMBER> --json body --jq .body > /tmp/pr-body-latest.md`.
|
||||
2. Change only the section owned by this operation. For screenshots, wrap
|
||||
the section in `<!-- kandev-screenshots-start -->` and
|
||||
`<!-- kandev-screenshots-end -->` markers (or replace the existing
|
||||
`## Screenshots` block if those markers are not present). Preserve every
|
||||
byte outside that range, including
|
||||
`<!-- kandev-preview-start --> ... <!-- kandev-preview-end -->`.
|
||||
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 (``) rather than a bare URL. Use `gh pr view <PR_NUMBER> --json body --jq .body` for this check.
|
||||
|
||||
Never commit the screenshot binaries to the PR branch itself — only to the throwaway `media/pr-<N>-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
|
||||
|
||||
@@ -5,11 +5,17 @@ import { ScrollArea as ScrollAreaPrimitive } from "radix-ui";
|
||||
|
||||
import { cn } from "./lib/utils";
|
||||
|
||||
function ScrollArea({
|
||||
className,
|
||||
children,
|
||||
...props
|
||||
}: React.ComponentProps<typeof ScrollAreaPrimitive.Root>) {
|
||||
type DataAttributes = {
|
||||
[key: `data-${string}`]: string | number | boolean | undefined;
|
||||
};
|
||||
|
||||
type ScrollAreaProps = React.ComponentProps<typeof ScrollAreaPrimitive.Root> & {
|
||||
viewportProps?: React.ComponentProps<typeof ScrollAreaPrimitive.Viewport> & DataAttributes;
|
||||
};
|
||||
|
||||
function ScrollArea({ className, children, viewportProps, ...props }: ScrollAreaProps) {
|
||||
const { className: viewportClassName, ...restViewportProps } = viewportProps ?? {};
|
||||
|
||||
return (
|
||||
<ScrollAreaPrimitive.Root
|
||||
data-slot="scroll-area"
|
||||
@@ -17,8 +23,12 @@ function ScrollArea({
|
||||
{...props}
|
||||
>
|
||||
<ScrollAreaPrimitive.Viewport
|
||||
{...restViewportProps}
|
||||
data-slot="scroll-area-viewport"
|
||||
className="focus-visible:ring-ring/50 size-full rounded-[inherit] transition-[color,box-shadow] outline-none focus-visible:ring-[3px] focus-visible:outline-1"
|
||||
className={cn(
|
||||
"focus-visible:ring-ring/50 size-full rounded-[inherit] transition-[color,box-shadow] outline-none focus-visible:ring-[3px] focus-visible:outline-1",
|
||||
viewportClassName,
|
||||
)}
|
||||
>
|
||||
{children}
|
||||
</ScrollAreaPrimitive.Viewport>
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -24,7 +24,7 @@ export function TasksSection({ collapsed }: TasksSectionProps) {
|
||||
headerAction={<TasksViewPicker />}
|
||||
grow
|
||||
>
|
||||
<div className="flex-1 min-h-0 [&_[data-testid=task-sidebar]]:bg-transparent [&_[data-testid=task-sidebar-scroll]]:bg-transparent">
|
||||
<div className="-mx-2 flex-1 min-h-0 [&_[data-testid=task-sidebar]]:bg-transparent [&_[data-testid=task-sidebar-scroll]]:bg-transparent">
|
||||
<TaskSessionSidebar workspaceId={workspaceId} workflowId={workflowId} hideFilterBar />
|
||||
</div>
|
||||
</AppSidebarSection>
|
||||
|
||||
@@ -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 (
|
||||
<PanelRoot data-testid="task-sidebar">
|
||||
{!hideFilterBar && <SidebarFilterBar />}
|
||||
<PanelBody className="space-y-4 p-0" data-testid="task-sidebar-scroll">
|
||||
<TaskSidebarScrollArea>
|
||||
<TaskSwitcher {...switcherProps} />
|
||||
<PluginSlot name="task-sidebar" />
|
||||
</PanelBody>
|
||||
</TaskSidebarScrollArea>
|
||||
<SidebarDialogs
|
||||
actions={sidebarActions}
|
||||
repositories={repositories}
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useLayoutEffect, useRef, useState, type ReactNode } from "react";
|
||||
import { ScrollArea } from "@kandev/ui/scroll-area";
|
||||
|
||||
const SCROLL_END_TOLERANCE_PX = 1;
|
||||
|
||||
export function TaskSidebarScrollArea({ children }: { children: ReactNode }) {
|
||||
const scrollRef = useRef<HTMLDivElement>(null);
|
||||
const contentRef = useRef<HTMLDivElement>(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 (
|
||||
<ScrollArea
|
||||
type="auto"
|
||||
className="task-sidebar-scroll-root min-h-0 flex-1"
|
||||
viewportProps={{
|
||||
ref: scrollRef,
|
||||
className: "task-sidebar-scroll",
|
||||
"data-can-scroll-down": canScrollDown,
|
||||
"data-testid": "task-sidebar-scroll",
|
||||
}}
|
||||
>
|
||||
<div ref={contentRef} className="space-y-4">
|
||||
{children}
|
||||
</div>
|
||||
</ScrollArea>
|
||||
);
|
||||
}
|
||||
@@ -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<HTMLElement>("[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,
|
||||
|
||||
Reference in New Issue
Block a user