fix(web): reveal command-selected sidebar tasks (#2290)
This commit is contained in:
Carlos Florêncio
@@ -1,7 +1,6 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
|
||||
import { useRouter } from "@/lib/routing/client-router";
|
||||
import { usePathname } from "@/lib/routing/client-router";
|
||||
import { useCommands, useCommandPanelOpen } from "@/lib/commands/command-registry";
|
||||
import type { CommandPanelMode, CommandItem as CommandItemType } from "@/lib/commands/types";
|
||||
@@ -16,7 +15,6 @@ import {
|
||||
} from "@/components/command-panel-scope-switcher";
|
||||
|
||||
import { listTasksByWorkspace } from "@/lib/api";
|
||||
import { linkToTask } from "@/lib/links";
|
||||
import type { Task } from "@/lib/types/http";
|
||||
import type { FileSearchResult } from "@/lib/types/backend";
|
||||
import { getWebSocketClient } from "@/lib/ws/connection";
|
||||
@@ -25,6 +23,7 @@ import { useDockviewStore } from "@/lib/state/dockview-store";
|
||||
import { getContentSearchResultValue } from "@/components/workspace-content-search";
|
||||
import { getFileName } from "@/lib/utils/file-path";
|
||||
import { isTaskWorkspaceSearchAvailable } from "@/lib/commands/task-workspace-search";
|
||||
import { useCommandPanelTaskNavigation } from "@/hooks/use-command-panel-task-navigation";
|
||||
import {
|
||||
CommandPanelView,
|
||||
MODE_COMMANDS,
|
||||
@@ -398,15 +397,24 @@ function useFirstResultSelection(
|
||||
}, [commands, contentResults, fileResults, mode, open, search, setSelectedValue, taskResults]);
|
||||
}
|
||||
|
||||
function useCommandPanelHandlers(
|
||||
state: ReturnType<typeof useCommandPanelState>,
|
||||
setOpen: (open: boolean) => void,
|
||||
commands: CommandItemType[],
|
||||
kanbanSteps: { id: string; title: string; color: string }[],
|
||||
repositories: Array<{ id: string; local_path: string }>,
|
||||
) {
|
||||
type CommandPanelHandlerOptions = {
|
||||
state: ReturnType<typeof useCommandPanelState>;
|
||||
setOpen: (open: boolean) => void;
|
||||
commands: CommandItemType[];
|
||||
kanbanSteps: { id: string; title: string; color: string }[];
|
||||
repositories: Array<{ id: string; local_path: string }>;
|
||||
onTaskSelect: (task: Task) => void;
|
||||
};
|
||||
|
||||
function useCommandPanelHandlers({
|
||||
state,
|
||||
setOpen,
|
||||
commands,
|
||||
kanbanSteps,
|
||||
repositories,
|
||||
onTaskSelect,
|
||||
}: CommandPanelHandlerOptions) {
|
||||
const { mode, search, inputCommand, setMode, setSearch, setInputCommand } = state;
|
||||
const router = useRouter();
|
||||
|
||||
const grouped = useMemo(() => {
|
||||
const map = new Map<string, CommandItemType[]>();
|
||||
@@ -452,9 +460,9 @@ function useCommandPanelHandlers(
|
||||
const handleTaskSelect = useCallback(
|
||||
(task: Task) => {
|
||||
setOpen(false);
|
||||
router.push(linkToTask(task.id));
|
||||
onTaskSelect(task);
|
||||
},
|
||||
[setOpen, router],
|
||||
[onTaskSelect, setOpen],
|
||||
);
|
||||
|
||||
const handleFileSelect = useCallback(
|
||||
@@ -513,6 +521,7 @@ export function CommandPanel() {
|
||||
const pathname = usePathname();
|
||||
const kanbanSteps = useAppStore((state) => state.kanban.steps);
|
||||
const workspaceId = useAppStore((state) => state.workspaces.activeId);
|
||||
const activeTaskId = useAppStore((state) => state.tasks.activeTaskId);
|
||||
const activeSessionId = useAppStore((s) => s.tasks.activeSessionId);
|
||||
const workspaceSearchAvailable = useAppStore((state) =>
|
||||
isTaskWorkspaceSearchAvailable(state, pathname),
|
||||
@@ -522,6 +531,7 @@ export function CommandPanel() {
|
||||
);
|
||||
const reposByWorkspace = useAppStore((s) => s.repositories.itemsByWorkspaceId);
|
||||
const repositories = workspaceId ? (reposByWorkspace[workspaceId] ?? []) : [];
|
||||
const handleTaskNavigation = useCommandPanelTaskNavigation(pathname, activeTaskId);
|
||||
|
||||
const state = useCommandPanelState(panelMode, setMode);
|
||||
const {
|
||||
@@ -566,17 +576,14 @@ export function CommandPanel() {
|
||||
setSearch,
|
||||
});
|
||||
|
||||
const {
|
||||
grouped,
|
||||
stepMap,
|
||||
repoMap,
|
||||
handleSelect,
|
||||
handleTaskSelect,
|
||||
handleFileSelect,
|
||||
handleKeyDown,
|
||||
onScopeChange,
|
||||
goBack,
|
||||
} = useCommandPanelHandlers(state, setOpen, commands, kanbanSteps, repositories);
|
||||
const handlers = useCommandPanelHandlers({
|
||||
state,
|
||||
setOpen,
|
||||
commands,
|
||||
kanbanSteps,
|
||||
repositories,
|
||||
onTaskSelect: handleTaskNavigation,
|
||||
});
|
||||
const handleContentSelect = useContentSearchResultOpener(setOpen, worktreePath, activeSessionId);
|
||||
|
||||
return (
|
||||
@@ -589,12 +596,12 @@ export function CommandPanel() {
|
||||
setSelectedValue={setSelectedValue}
|
||||
search={search}
|
||||
setSearch={setSearch}
|
||||
handleKeyDown={handleKeyDown}
|
||||
onScopeChange={onScopeChange}
|
||||
goBack={goBack}
|
||||
handleKeyDown={handlers.handleKeyDown}
|
||||
onScopeChange={handlers.onScopeChange}
|
||||
goBack={handlers.goBack}
|
||||
fileResults={fileResults}
|
||||
isSearchingFiles={isSearchingFiles}
|
||||
handleFileSelect={handleFileSelect}
|
||||
handleFileSelect={handlers.handleFileSelect}
|
||||
contentResults={contentResults}
|
||||
isSearchingContent={isSearchingContent}
|
||||
contentSearchError={contentSearchError}
|
||||
@@ -602,13 +609,13 @@ export function CommandPanel() {
|
||||
workspaceSearchAvailable={workspaceSearchAvailable}
|
||||
handleContentSelect={handleContentSelect}
|
||||
commands={commands}
|
||||
grouped={grouped}
|
||||
handleSelect={handleSelect}
|
||||
grouped={handlers.grouped}
|
||||
handleSelect={handlers.handleSelect}
|
||||
isSearching={isSearching}
|
||||
taskResults={taskResults}
|
||||
stepMap={stepMap}
|
||||
repoMap={repoMap}
|
||||
handleTaskSelect={handleTaskSelect}
|
||||
stepMap={handlers.stepMap}
|
||||
repoMap={handlers.repoMap}
|
||||
handleTaskSelect={handlers.handleTaskSelect}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -479,7 +479,6 @@ export const TaskItem = memo(function TaskItem({
|
||||
isOnLastWorkflowStep = false,
|
||||
}: TaskItemProps) {
|
||||
const effectiveMenuOpen = menuOpen || isDeleting === true;
|
||||
const isInProgress = computeIsInProgress(state, sessionState);
|
||||
const hasDiffStats = !!diffStats && (diffStats.additions > 0 || diffStats.deletions > 0);
|
||||
const showSubtaskToggle = !!subtaskCount && subtaskCount > 0 && !!onToggleSubtasks;
|
||||
const taskColor = useTaskColor(taskId);
|
||||
@@ -490,6 +489,7 @@ export const TaskItem = memo(function TaskItem({
|
||||
role="button"
|
||||
tabIndex={0}
|
||||
data-testid="sidebar-task-item"
|
||||
data-task-row-id={taskId}
|
||||
{...taskItemStateAttrs(isSelected, isMultiSelected)}
|
||||
onClick={taskItemRowClick(onSelect, onClick)}
|
||||
onKeyDown={(e) => handleTaskItemKeyDown(e, onSelect, onClick)}
|
||||
@@ -502,7 +502,7 @@ export const TaskItem = memo(function TaskItem({
|
||||
sessionState={sessionState}
|
||||
state={state}
|
||||
foregroundActivity={foregroundActivity}
|
||||
isInProgress={isInProgress}
|
||||
isInProgress={computeIsInProgress(state, sessionState)}
|
||||
hasPendingClarification={hasPendingClarification}
|
||||
hasPendingPermission={hasPendingPermission}
|
||||
isOnLastWorkflowStep={isOnLastWorkflowStep}
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions";
|
||||
import { SessionPage } from "../../pages/session-page";
|
||||
|
||||
test.describe("mobile: command panel task navigation", () => {
|
||||
test("navigates directly without targeting the hidden desktop sidebar", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
const source = await apiClient.seedTask(seedData.workspaceId, "Mobile command source task", {
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
});
|
||||
const destination = await apiClient.seedTask(
|
||||
seedData.workspaceId,
|
||||
"Mobile command destination task",
|
||||
{
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
},
|
||||
);
|
||||
|
||||
await testPage.goto(`/t/${source.task_id}`);
|
||||
const session = new SessionPage(testPage);
|
||||
await session.waitForLoad();
|
||||
await expect(testPage.getByTestId("mobile-session-menu")).toBeVisible();
|
||||
await expect(testPage.getByTestId("app-sidebar-layout")).toBeHidden();
|
||||
|
||||
const modifier = process.platform === "darwin" ? "Meta" : "Control";
|
||||
await testPage.keyboard.press(`${modifier}+k`);
|
||||
const dialog = testPage.getByRole("dialog");
|
||||
await expect(dialog).toBeVisible({ timeout: 5_000 });
|
||||
await dialog.getByRole("combobox").fill("Mobile command destination");
|
||||
const option = dialog
|
||||
.getByRole("option")
|
||||
.filter({ hasText: "Mobile command destination task" });
|
||||
await expect(option).toBeVisible({ timeout: 10_000 });
|
||||
await option.click();
|
||||
|
||||
await expect(testPage).toHaveURL(new RegExp(`/t/${destination.task_id}$`));
|
||||
const mobileTopBar = testPage
|
||||
.getByTestId("mobile-session-menu")
|
||||
.locator("xpath=ancestor::header");
|
||||
await expect(
|
||||
mobileTopBar.getByText("Mobile command destination task", { exact: true }),
|
||||
).toBeVisible();
|
||||
await expect(testPage.getByTestId("app-sidebar-layout")).toBeHidden();
|
||||
await assertNoDocumentHorizontalOverflow(testPage);
|
||||
});
|
||||
});
|
||||
@@ -258,4 +258,289 @@ test.describe("sidebar scrolling", () => {
|
||||
.poll(() => scrollContainer.evaluate((el) => el.scrollTop), { timeout: 5_000 })
|
||||
.toBeGreaterThan(scrollBefore - 50);
|
||||
});
|
||||
|
||||
test("reveals a command-selected task", async ({ testPage, apiClient, seedData }) => {
|
||||
test.setTimeout(60_000);
|
||||
|
||||
const taskCount = 25;
|
||||
const created: { id: string; title: string }[] = [];
|
||||
for (let index = 0; index < taskCount; index++) {
|
||||
const title = `Command Reveal Task ${String(index).padStart(2, "0")}`;
|
||||
const task = await apiClient.createTask(seedData.workspaceId, title, {
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
});
|
||||
created.push({ id: task.id, title });
|
||||
}
|
||||
|
||||
const initialTask = created.at(-1)!;
|
||||
await testPage.goto(`/t/${initialTask.id}`);
|
||||
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,
|
||||
});
|
||||
await scrollContainer.evaluate((element) => {
|
||||
element.scrollTop = 0;
|
||||
});
|
||||
|
||||
const dimensions = await scrollContainer.evaluate((element) => ({
|
||||
clientHeight: element.clientHeight,
|
||||
scrollHeight: element.scrollHeight,
|
||||
}));
|
||||
expect(dimensions.scrollHeight).toBeGreaterThan(dimensions.clientHeight);
|
||||
|
||||
const offscreenTitle = await scrollContainer.evaluate(
|
||||
(element, taskTitles) => {
|
||||
const containerRect = element.getBoundingClientRect();
|
||||
const rows = element.querySelectorAll<HTMLElement>("[data-testid='sidebar-task-item']");
|
||||
for (const row of rows) {
|
||||
const rowRect = row.getBoundingClientRect();
|
||||
const isOutside =
|
||||
rowRect.bottom <= containerRect.top + 1 || rowRect.top >= containerRect.bottom - 1;
|
||||
if (isOutside) {
|
||||
const title = taskTitles.find((candidate) => row.textContent?.includes(candidate));
|
||||
if (title) return title;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
},
|
||||
created.map(({ title }) => title),
|
||||
);
|
||||
if (!offscreenTitle) throw new Error("Expected a rendered task row outside the viewport");
|
||||
const targetTask = created.find(({ title }) => title === offscreenTitle)!;
|
||||
const targetRow = session.sidebarTaskItem(targetTask.title);
|
||||
await expect(targetRow).toBeVisible();
|
||||
const before = await Promise.all([scrollContainer.boundingBox(), targetRow.boundingBox()]);
|
||||
if (!before[0] || !before[1]) throw new Error("Command-selected target has no layout box");
|
||||
expect(
|
||||
before[1].y + before[1].height <= before[0].y + 1 ||
|
||||
before[1].y >= before[0].y + before[0].height - 1,
|
||||
"target task should start outside the task-list viewport",
|
||||
).toBe(true);
|
||||
|
||||
const documentScrollBefore = await testPage.evaluate(() => ({ scrollX, scrollY }));
|
||||
const modifier = process.platform === "darwin" ? "Meta" : "Control";
|
||||
await testPage.keyboard.press(`${modifier}+k`);
|
||||
const dialog = testPage.getByRole("dialog");
|
||||
await expect(dialog).toBeVisible({ timeout: 5_000 });
|
||||
await dialog.getByRole("combobox").fill(targetTask.title);
|
||||
const option = dialog.getByRole("option").filter({ hasText: targetTask.title });
|
||||
await expect(option).toBeVisible({ timeout: 10_000 });
|
||||
await option.click();
|
||||
|
||||
await expect(testPage).toHaveURL(new RegExp(`/t/${targetTask.id}$`));
|
||||
await expect(session.activeSidebarTaskItem(targetTask.title).first()).toHaveAttribute(
|
||||
"aria-current",
|
||||
"true",
|
||||
);
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const [containerBox, rowBox] = await Promise.all([
|
||||
scrollContainer.boundingBox(),
|
||||
targetRow.boundingBox(),
|
||||
]);
|
||||
if (!containerBox || !rowBox) return false;
|
||||
return (
|
||||
rowBox.y >= containerBox.y - 1 &&
|
||||
rowBox.y + rowBox.height <= containerBox.y + containerBox.height + 1
|
||||
);
|
||||
},
|
||||
{ timeout: 10_000 },
|
||||
)
|
||||
.toBe(true);
|
||||
|
||||
await expect
|
||||
.poll(() => testPage.evaluate(() => ({ scrollX, scrollY })))
|
||||
.toEqual(documentScrollBefore);
|
||||
});
|
||||
|
||||
test("reveals a command-selected task after a delayed settings navigation blocker", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(60_000);
|
||||
|
||||
const initialSettings = await apiClient.getUserSettings();
|
||||
const initialLayout =
|
||||
initialSettings.settings.changes_panel_layout === "tree" ? "tree" : "flat";
|
||||
const nextLayout = initialLayout === "tree" ? "Flat list" : "Tree";
|
||||
const taskCount = 25;
|
||||
const created: { id: string; title: string }[] = [];
|
||||
|
||||
try {
|
||||
for (let index = 0; index < taskCount; index++) {
|
||||
const title = `Blocked Command Reveal Task ${String(index).padStart(2, "0")}`;
|
||||
const task = await apiClient.createTask(seedData.workspaceId, title, {
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
});
|
||||
created.push({ id: task.id, title });
|
||||
}
|
||||
|
||||
const targetTask = created[0];
|
||||
await testPage.goto("/settings/general/appearance");
|
||||
await expect(
|
||||
testPage.getByRole("heading", { name: "Appearance", exact: true }),
|
||||
).toBeVisible();
|
||||
|
||||
await testPage.getByTestId("changes-panel-layout-select").click();
|
||||
await testPage.getByRole("option", { name: nextLayout }).click();
|
||||
await expect(testPage.getByTestId("settings-floating-save")).toBeVisible();
|
||||
|
||||
const modifier = process.platform === "darwin" ? "Meta" : "Control";
|
||||
await testPage.keyboard.press(`${modifier}+k`);
|
||||
const commandDialog = testPage.getByRole("dialog");
|
||||
await expect(commandDialog).toBeVisible({ timeout: 5_000 });
|
||||
await commandDialog.getByRole("combobox").fill(targetTask.title);
|
||||
const option = commandDialog.getByRole("option").filter({ hasText: targetTask.title });
|
||||
await expect(option).toBeVisible({ timeout: 10_000 });
|
||||
await option.click();
|
||||
|
||||
const navigationDialog = testPage.getByRole("alertdialog", {
|
||||
name: "Save changes before leaving?",
|
||||
});
|
||||
await expect(navigationDialog).toBeVisible();
|
||||
|
||||
// Let the old 60-frame sidebar reveal budget expire while the settings
|
||||
// blocker keeps the task sidebar out of the DOM.
|
||||
await testPage.waitForTimeout(1_500);
|
||||
await navigationDialog.getByRole("button", { name: "Discard and leave" }).click();
|
||||
|
||||
await expect(testPage).toHaveURL(new RegExp(`/t/${targetTask.id}$`));
|
||||
const session = new SessionPage(testPage);
|
||||
await session.waitForLoad();
|
||||
const scrollContainer = testPage.getByTestId("task-sidebar-scroll");
|
||||
const targetRow = session.sidebarTaskItem(targetTask.title);
|
||||
await expect(scrollContainer).toBeVisible();
|
||||
await expect(session.sidebar.getByTestId("sidebar-task-item")).toHaveCount(taskCount, {
|
||||
timeout: 10_000,
|
||||
});
|
||||
await expect(session.activeSidebarTaskItem(targetTask.title).first()).toHaveAttribute(
|
||||
"aria-current",
|
||||
"true",
|
||||
);
|
||||
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const [containerBox, rowBox] = await Promise.all([
|
||||
scrollContainer.boundingBox(),
|
||||
targetRow.boundingBox(),
|
||||
]);
|
||||
if (!containerBox || !rowBox) return false;
|
||||
return (
|
||||
rowBox.y >= containerBox.y - 1 &&
|
||||
rowBox.y + rowBox.height <= containerBox.y + containerBox.height + 1
|
||||
);
|
||||
},
|
||||
{ timeout: 10_000 },
|
||||
)
|
||||
.toBe(true);
|
||||
} finally {
|
||||
await apiClient.rawRequest("PATCH", "/api/v1/user/settings", {
|
||||
changes_panel_layout: initialLayout,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
test("reveals a command-selected task that starts above the sidebar viewport", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(60_000);
|
||||
|
||||
const taskCount = 25;
|
||||
const created: { id: string; title: string }[] = [];
|
||||
for (let index = 0; index < taskCount; index++) {
|
||||
const title = `Command Reveal Above Task ${String(index).padStart(2, "0")}`;
|
||||
const task = await apiClient.createTask(seedData.workspaceId, title, {
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
});
|
||||
created.push({ id: task.id, title });
|
||||
}
|
||||
|
||||
const initialTask = created.at(-1)!;
|
||||
await testPage.goto(`/t/${initialTask.id}`);
|
||||
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,
|
||||
});
|
||||
await scrollContainer.evaluate((element) => {
|
||||
element.scrollTop = element.scrollHeight;
|
||||
});
|
||||
await expect(scrollContainer).toHaveAttribute("data-can-scroll-down", "false");
|
||||
|
||||
const aboveTitle = await scrollContainer.evaluate(
|
||||
(element, taskTitles) => {
|
||||
const containerRect = element.getBoundingClientRect();
|
||||
const rows = element.querySelectorAll<HTMLElement>("[data-testid='sidebar-task-item']");
|
||||
for (const row of rows) {
|
||||
const rowRect = row.getBoundingClientRect();
|
||||
if (rowRect.bottom <= containerRect.top + 1) {
|
||||
const title = taskTitles.find((candidate) => row.textContent?.includes(candidate));
|
||||
if (title) return title;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
},
|
||||
created.map(({ title }) => title),
|
||||
);
|
||||
if (!aboveTitle) throw new Error("Expected a rendered task row above the viewport");
|
||||
const targetTask = created.find(({ title }) => title === aboveTitle)!;
|
||||
const targetRow = session.sidebarTaskItem(targetTask.title);
|
||||
const before = await Promise.all([scrollContainer.boundingBox(), targetRow.boundingBox()]);
|
||||
if (!before[0] || !before[1]) throw new Error("Command-selected target has no layout box");
|
||||
expect(before[1].y + before[1].height).toBeLessThanOrEqual(before[0].y + 1);
|
||||
|
||||
const documentScrollBefore = await testPage.evaluate(() => ({ scrollX, scrollY }));
|
||||
const modifier = process.platform === "darwin" ? "Meta" : "Control";
|
||||
await testPage.keyboard.press(`${modifier}+k`);
|
||||
const dialog = testPage.getByRole("dialog");
|
||||
await expect(dialog).toBeVisible({ timeout: 5_000 });
|
||||
await dialog.getByRole("combobox").fill(targetTask.title);
|
||||
const option = dialog.getByRole("option").filter({ hasText: targetTask.title });
|
||||
await expect(option).toBeVisible({ timeout: 10_000 });
|
||||
await option.click();
|
||||
|
||||
await expect(testPage).toHaveURL(new RegExp(`/t/${targetTask.id}$`));
|
||||
await expect(session.activeSidebarTaskItem(targetTask.title).first()).toHaveAttribute(
|
||||
"aria-current",
|
||||
"true",
|
||||
);
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const [containerBox, rowBox] = await Promise.all([
|
||||
scrollContainer.boundingBox(),
|
||||
targetRow.boundingBox(),
|
||||
]);
|
||||
if (!containerBox || !rowBox) return false;
|
||||
return (
|
||||
rowBox.y >= containerBox.y - 1 &&
|
||||
rowBox.y + rowBox.height <= containerBox.y + containerBox.height + 1
|
||||
);
|
||||
},
|
||||
{ timeout: 10_000 },
|
||||
)
|
||||
.toBe(true);
|
||||
|
||||
await expect
|
||||
.poll(() => testPage.evaluate(() => ({ scrollX, scrollY })))
|
||||
.toEqual(documentScrollBefore);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
import { act, renderHook } from "@testing-library/react";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { Task } from "@/lib/types/http";
|
||||
|
||||
const mockPush = vi.hoisted(() => vi.fn());
|
||||
const mockCancelSidebarTaskReveal = vi.hoisted(() => vi.fn());
|
||||
const mockRevealSidebarTask = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("@/lib/routing/client-router", () => ({
|
||||
useRouter: () => ({ push: mockPush }),
|
||||
}));
|
||||
vi.mock("@/lib/sidebar/task-navigation", () => ({
|
||||
cancelSidebarTaskReveal: mockCancelSidebarTaskReveal,
|
||||
revealSidebarTask: mockRevealSidebarTask,
|
||||
}));
|
||||
|
||||
import { useCommandPanelTaskNavigation } from "./use-command-panel-task-navigation";
|
||||
|
||||
const task = (id: string) => ({ id }) as Task;
|
||||
|
||||
describe("useCommandPanelTaskNavigation", () => {
|
||||
beforeEach(() => {
|
||||
mockPush.mockReset();
|
||||
mockCancelSidebarTaskReveal.mockReset();
|
||||
mockRevealSidebarTask.mockReset();
|
||||
mockRevealSidebarTask.mockReturnValue(new Promise<boolean>(() => undefined));
|
||||
});
|
||||
|
||||
it("cancels an active reveal before queuing a newer task navigation", () => {
|
||||
const { result } = renderHook(() => useCommandPanelTaskNavigation("/t/task-a", "task-a"));
|
||||
|
||||
act(() => result.current(task("task-a")));
|
||||
expect(mockRevealSidebarTask).toHaveBeenCalledWith("task-a");
|
||||
|
||||
act(() => result.current(task("task-b")));
|
||||
|
||||
expect(mockCancelSidebarTaskReveal).toHaveBeenCalledTimes(2);
|
||||
expect(mockPush).toHaveBeenNthCalledWith(2, "/t/task-b");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,37 @@
|
||||
import { useCallback, useEffect, useState } from "react";
|
||||
|
||||
import { useRouter } from "@/lib/routing/client-router";
|
||||
import { isTaskDetailPath, linkToTask } from "@/lib/links";
|
||||
import type { Task } from "@/lib/types/http";
|
||||
import { cancelSidebarTaskReveal, revealSidebarTask } from "@/lib/sidebar/task-navigation";
|
||||
|
||||
export function useCommandPanelTaskNavigation(pathname: string, activeTaskId: string | null) {
|
||||
const router = useRouter();
|
||||
const [pendingTaskRevealId, setPendingTaskRevealId] = useState<string | null>(null);
|
||||
|
||||
const handleTaskNavigation = useCallback(
|
||||
(task: Task) => {
|
||||
cancelSidebarTaskReveal();
|
||||
setPendingTaskRevealId(task.id);
|
||||
router.push(linkToTask(task.id));
|
||||
},
|
||||
[router],
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
if (
|
||||
!pendingTaskRevealId ||
|
||||
activeTaskId !== pendingTaskRevealId ||
|
||||
!isTaskDetailPath(pathname, pendingTaskRevealId)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
const taskId = pendingTaskRevealId;
|
||||
void revealSidebarTask(taskId).finally(() => {
|
||||
setPendingTaskRevealId((current) => (current === taskId ? null : current));
|
||||
});
|
||||
}, [activeTaskId, pathname, pendingTaskRevealId]);
|
||||
|
||||
return handleTaskNavigation;
|
||||
}
|
||||
@@ -0,0 +1,162 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
TASK_ROW_DOM_ATTR,
|
||||
cancelSidebarTaskReveal,
|
||||
revealSidebarTask,
|
||||
taskRowSelector,
|
||||
} from "./task-navigation";
|
||||
|
||||
type Rect = { x: number; y: number; width: number; height: number };
|
||||
const TEST_TASK_ID = "task-1";
|
||||
|
||||
function setRect(element: HTMLElement, rect: Rect) {
|
||||
vi.spyOn(element, "getBoundingClientRect").mockReturnValue({
|
||||
x: rect.x,
|
||||
y: rect.y,
|
||||
width: rect.width,
|
||||
height: rect.height,
|
||||
top: rect.y,
|
||||
right: rect.x + rect.width,
|
||||
bottom: rect.y + rect.height,
|
||||
left: rect.x,
|
||||
toJSON: () => ({}),
|
||||
} as DOMRect);
|
||||
}
|
||||
|
||||
function mountViewport(visible = true) {
|
||||
const viewport = document.createElement("div");
|
||||
viewport.dataset.testid = "task-sidebar-scroll";
|
||||
if (!visible) viewport.style.display = "none";
|
||||
setRect(viewport, { x: 0, y: 0, width: 320, height: 100 });
|
||||
document.body.appendChild(viewport);
|
||||
return viewport;
|
||||
}
|
||||
|
||||
function mountRow(viewport: HTMLElement, taskId: string, rect: Rect) {
|
||||
const row = document.createElement("div");
|
||||
row.setAttribute(TASK_ROW_DOM_ATTR, taskId);
|
||||
row.scrollIntoView = vi.fn();
|
||||
setRect(row, rect);
|
||||
viewport.appendChild(row);
|
||||
return row;
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
document.body.innerHTML = "";
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe("taskRowSelector", () => {
|
||||
it("escapes task ids for use in a CSS selector", () => {
|
||||
expect(taskRowSelector("task:a")).toBe(`[${TASK_ROW_DOM_ATTR}="task\\:a"]`);
|
||||
});
|
||||
});
|
||||
|
||||
describe("revealSidebarTask", () => {
|
||||
it("scrolls an off-screen rendered row with nearest alignment", async () => {
|
||||
const viewport = mountViewport();
|
||||
const row = mountRow(viewport, TEST_TASK_ID, { x: 0, y: 120, width: 320, height: 24 });
|
||||
|
||||
await expect(revealSidebarTask(TEST_TASK_ID, (callback) => callback())).resolves.toBe(true);
|
||||
expect(row.scrollIntoView).toHaveBeenCalledWith({ block: "nearest", inline: "nearest" });
|
||||
});
|
||||
|
||||
it("scrolls an off-screen row above the viewport with nearest alignment", async () => {
|
||||
const viewport = mountViewport();
|
||||
const row = mountRow(viewport, TEST_TASK_ID, { x: 0, y: -24, width: 320, height: 24 });
|
||||
|
||||
await expect(revealSidebarTask(TEST_TASK_ID, (callback) => callback())).resolves.toBe(true);
|
||||
expect(row.scrollIntoView).toHaveBeenCalledWith({ block: "nearest", inline: "nearest" });
|
||||
});
|
||||
|
||||
it("does not reposition a row that is already inside the viewport", async () => {
|
||||
const viewport = mountViewport();
|
||||
const row = mountRow(viewport, TEST_TASK_ID, { x: 0, y: 20, width: 320, height: 24 });
|
||||
|
||||
await expect(revealSidebarTask(TEST_TASK_ID, (callback) => callback())).resolves.toBe(true);
|
||||
expect(row.scrollIntoView).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("finds a row that renders after a later animation frame", async () => {
|
||||
const viewport = mountViewport();
|
||||
const callbacks: Array<() => void> = [];
|
||||
const navigation = revealSidebarTask("late", (callback) => callbacks.push(callback));
|
||||
|
||||
expect(callbacks).toHaveLength(1);
|
||||
const row = mountRow(viewport, "late", { x: 0, y: 120, width: 320, height: 24 });
|
||||
callbacks.shift()!();
|
||||
|
||||
await expect(navigation).resolves.toBe(true);
|
||||
expect(row.scrollIntoView).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not scroll a superseded task when its row renders late", async () => {
|
||||
const viewport = mountViewport();
|
||||
const firstCallbacks: Array<() => void> = [];
|
||||
const firstNavigation = revealSidebarTask("task-a", (callback) =>
|
||||
firstCallbacks.push(callback),
|
||||
);
|
||||
|
||||
expect(firstCallbacks).toHaveLength(1);
|
||||
|
||||
const secondRow = mountRow(viewport, "task-b", { x: 0, y: 120, width: 320, height: 24 });
|
||||
await expect(revealSidebarTask("task-b", (callback) => callback())).resolves.toBe(true);
|
||||
expect(secondRow.scrollIntoView).toHaveBeenCalledTimes(1);
|
||||
|
||||
const firstRow = mountRow(viewport, "task-a", { x: 0, y: 120, width: 320, height: 24 });
|
||||
firstCallbacks.shift()!();
|
||||
|
||||
await expect(firstNavigation).resolves.toBe(false);
|
||||
expect(firstRow.scrollIntoView).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("cancels a pending reveal before a newer route is ready", async () => {
|
||||
const viewport = mountViewport();
|
||||
const callbacks: Array<() => void> = [];
|
||||
const navigation = revealSidebarTask("pending", (callback) => callbacks.push(callback));
|
||||
|
||||
cancelSidebarTaskReveal();
|
||||
const row = mountRow(viewport, "pending", { x: 0, y: 120, width: 320, height: 24 });
|
||||
callbacks.shift()!();
|
||||
|
||||
await expect(navigation).resolves.toBe(false);
|
||||
expect(row.scrollIntoView).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("ignores a matching row inside a hidden sidebar viewport", async () => {
|
||||
const hiddenViewport = mountViewport(false);
|
||||
const hiddenRow = mountRow(hiddenViewport, TEST_TASK_ID, {
|
||||
x: 0,
|
||||
y: 120,
|
||||
width: 320,
|
||||
height: 24,
|
||||
});
|
||||
const visibleViewport = mountViewport();
|
||||
const visibleRow = mountRow(visibleViewport, TEST_TASK_ID, {
|
||||
x: 0,
|
||||
y: 120,
|
||||
width: 320,
|
||||
height: 24,
|
||||
});
|
||||
|
||||
await expect(revealSidebarTask(TEST_TASK_ID, (callback) => callback())).resolves.toBe(true);
|
||||
expect(hiddenRow.scrollIntoView).not.toHaveBeenCalled();
|
||||
expect(visibleRow.scrollIntoView).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("gives up after its bounded frame budget when no row renders", async () => {
|
||||
mountViewport();
|
||||
const callbacks: Array<() => void> = [];
|
||||
const navigation = revealSidebarTask("missing", (callback) => callbacks.push(callback));
|
||||
|
||||
let frames = 0;
|
||||
while (callbacks.length > 0 && frames < 100) {
|
||||
callbacks.shift()!();
|
||||
frames += 1;
|
||||
}
|
||||
|
||||
await expect(navigation).resolves.toBe(false);
|
||||
expect(frames).toBeLessThan(100);
|
||||
expect(callbacks).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,110 @@
|
||||
export const TASK_ROW_DOM_ATTR = "data-task-row-id";
|
||||
export const TASK_SIDEBAR_SCROLL_SELECTOR = '[data-testid="task-sidebar-scroll"]';
|
||||
|
||||
const MAX_TASK_NAVIGATION_ATTEMPTS = 60;
|
||||
let latestNavigationRequestId = 0;
|
||||
|
||||
/** Invalidates the current reveal so a pending selection cannot scroll a stale row. */
|
||||
export function cancelSidebarTaskReveal(): void {
|
||||
latestNavigationRequestId += 1;
|
||||
}
|
||||
|
||||
/** CSS selector for a rendered task row by its stable task id. */
|
||||
export function taskRowSelector(taskId: string): string {
|
||||
return `[${TASK_ROW_DOM_ATTR}="${CSS.escape(taskId)}"]`;
|
||||
}
|
||||
|
||||
function isVisible(element: HTMLElement, stopBefore?: HTMLElement): boolean {
|
||||
for (
|
||||
let current: HTMLElement | null = element;
|
||||
current && current !== stopBefore;
|
||||
current = current.parentElement
|
||||
) {
|
||||
const styles = window.getComputedStyle(current);
|
||||
if (
|
||||
styles.display === "none" ||
|
||||
styles.visibility === "hidden" ||
|
||||
styles.visibility === "collapse"
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
const rect = element.getBoundingClientRect();
|
||||
return rect.width > 0 && rect.height > 0;
|
||||
}
|
||||
|
||||
function findVisibleTaskRow(taskId: string): { row: HTMLElement; viewport: HTMLElement } | null {
|
||||
const selector = taskRowSelector(taskId);
|
||||
const viewports = document.querySelectorAll<HTMLElement>(TASK_SIDEBAR_SCROLL_SELECTOR);
|
||||
for (const viewport of viewports) {
|
||||
if (!isVisible(viewport)) continue;
|
||||
const row = viewport.querySelector<HTMLElement>(selector);
|
||||
if (row && isVisible(row, viewport)) return { row, viewport };
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function isInsideViewport(row: HTMLElement, viewport: HTMLElement): boolean {
|
||||
const rowRect = row.getBoundingClientRect();
|
||||
const viewportRect = viewport.getBoundingClientRect();
|
||||
return (
|
||||
rowRect.top >= viewportRect.top &&
|
||||
rowRect.bottom <= viewportRect.bottom &&
|
||||
rowRect.left >= viewportRect.left &&
|
||||
rowRect.right <= viewportRect.right
|
||||
);
|
||||
}
|
||||
|
||||
function defaultRequestFrame(callback: () => void): void {
|
||||
if (typeof requestAnimationFrame === "function") {
|
||||
requestAnimationFrame(callback);
|
||||
} else {
|
||||
setTimeout(callback, 16);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Reveals a rendered task row in the visible desktop sidebar.
|
||||
*
|
||||
* The row may mount after route navigation or sidebar hydration, so lookup is
|
||||
* retried for a bounded number of animation frames. A missing or hidden row is
|
||||
* intentionally a no-op so task navigation never depends on sidebar state.
|
||||
*/
|
||||
export function revealSidebarTask(
|
||||
taskId: string,
|
||||
requestFrame: (callback: () => void) => void = defaultRequestFrame,
|
||||
): Promise<boolean> {
|
||||
if (typeof document === "undefined") return Promise.resolve(false);
|
||||
|
||||
const requestId = ++latestNavigationRequestId;
|
||||
return new Promise((resolve) => {
|
||||
let attempts = 0;
|
||||
const tick = () => {
|
||||
if (requestId !== latestNavigationRequestId) {
|
||||
resolve(false);
|
||||
return;
|
||||
}
|
||||
|
||||
const match = findVisibleTaskRow(taskId);
|
||||
if (match) {
|
||||
if (requestId !== latestNavigationRequestId) {
|
||||
resolve(false);
|
||||
return;
|
||||
}
|
||||
if (!isInsideViewport(match.row, match.viewport)) {
|
||||
match.row.scrollIntoView({ block: "nearest", inline: "nearest" });
|
||||
}
|
||||
resolve(true);
|
||||
return;
|
||||
}
|
||||
|
||||
attempts += 1;
|
||||
if (attempts >= MAX_TASK_NAVIGATION_ATTEMPTS) {
|
||||
resolve(false);
|
||||
return;
|
||||
}
|
||||
requestFrame(tick);
|
||||
};
|
||||
tick();
|
||||
});
|
||||
}
|
||||
@@ -0,0 +1,121 @@
|
||||
---
|
||||
spec: docs/specs/ui/command-panel-sidebar-task-reveal.md
|
||||
created: 2026-08-05
|
||||
status: implemented
|
||||
---
|
||||
|
||||
# Implementation Plan: Command-panel Sidebar Task Reveal
|
||||
|
||||
## Overview
|
||||
|
||||
Add a bounded DOM-navigation helper for rendered task rows, invoke it when Cmd+K opens a task, and
|
||||
prove the behavior against an actually overflowing sidebar. The helper follows the retryable target
|
||||
navigation pattern already used by `apps/web/lib/review/navigation.ts`, while scoping lookup to a
|
||||
visible task-sidebar scroll viewport so the CSS-hidden desktop rail is never selected on mobile. A
|
||||
command-panel selection is queued until the canonical route and matching active task render, and
|
||||
each new selection immediately invalidates the previous latest-wins generation.
|
||||
|
||||
## Confirmed root cause
|
||||
|
||||
`useCommandPanelHandlers.handleTaskSelect` in `apps/web/components/command-panel.tsx` only closes the
|
||||
palette and pushes `linkToTask(task.id)`. `TaskSidebarScrollArea` in
|
||||
`apps/web/components/task/task-sidebar-scroll-area.tsx` only tracks its bottom-scroll cue; no code
|
||||
connects command-panel selection to the nested Radix scroll viewport. Consequently, route and
|
||||
active-task state change while an off-screen sidebar row remains outside the visible list.
|
||||
|
||||
## Frontend
|
||||
|
||||
### Sidebar target navigation
|
||||
|
||||
- Add `apps/web/lib/sidebar/task-navigation.ts` with a stable task-row DOM attribute/selector and a
|
||||
bounded `requestAnimationFrame` retry helper. It resolves the row only inside a rendered, visible
|
||||
`task-sidebar-scroll` viewport and calls `scrollIntoView({ block: "nearest", inline: "nearest" })`.
|
||||
A missing or hidden target resolves as a no-op rather than affecting navigation. A newer reveal
|
||||
request supersedes and terminates an older retry loop before it can scroll a stale row, including
|
||||
while the newer route is waiting behind a navigation guard.
|
||||
- Add a row-specific task marker to the interactive row in
|
||||
`apps/web/components/task/task-item.tsx`. Keep the existing test ID and active-task accessibility
|
||||
attributes intact.
|
||||
- Update command-panel task selection in `apps/web/hooks/use-command-panel-task-navigation.ts` to
|
||||
queue the task ID alongside the existing canonical task navigation, then start the sidebar
|
||||
reveal only after the matching task route and active-task state render. Do not move focus or
|
||||
change active sidebar view/collapse state.
|
||||
|
||||
### Mobile design contract
|
||||
|
||||
Desktop outcome: Cmd+K opens the selected task and the rendered active row becomes visible inside
|
||||
the existing task-list scroll owner. Phone entry point and presentation remain the existing direct
|
||||
task route plus the optional `SessionTaskSwitcherSheet` inset drawer; the command-panel action does
|
||||
not open that drawer. `apps/web/components/task/mobile/session-task-switcher-sheet.tsx` is the
|
||||
nearest shipped exemplar and contributes the rule that phone task navigation owns a separate,
|
||||
safe-area-aware internal scroller. Shared task search and route logic remain unchanged; the reveal
|
||||
helper selects only a layout-visible desktop scroll viewport. No new touch target, viewport-bound
|
||||
surface, or safe-area behavior is introduced.
|
||||
|
||||
## Tests
|
||||
|
||||
- **What:** selector escaping, above/below viewport behavior, immediate and delayed row discovery,
|
||||
minimum-distance scroll options, visible-sidebar scoping, latest-request supersession, and
|
||||
bounded failure.
|
||||
- **File:** `apps/web/lib/sidebar/task-navigation.test.ts`.
|
||||
- **How:** Vitest/jsdom with explicit layout visibility and queued animation-frame callbacks.
|
||||
- **Regression order:** first add the overflowing-sidebar browser scenario and run it against the
|
||||
current code to confirm the selected route changes while the target row stays outside the sidebar
|
||||
viewport; then implement the helper and rerun to green.
|
||||
|
||||
## E2E tests
|
||||
|
||||
- **Scenario:** Given enough desktop tasks to overflow the sidebar, when Cmd+K selects a task whose
|
||||
row is confirmed outside the nested viewport, then the URL, active-row state, and row/container
|
||||
bounding boxes prove the row was revealed.
|
||||
- **Scenario:** Given a dirty settings page that delays task-route activation beyond the initial
|
||||
reveal retry budget, when the user confirms the guarded navigation, then the active row is
|
||||
revealed after route activation.
|
||||
- **File:** `apps/web/e2e/tests/task/sidebar-scroll-preservation.spec.ts`.
|
||||
- **What to verify:** precondition overflow and off-screen target, command-panel selection through
|
||||
the UI, canonical `/t/:id` navigation, `aria-current`, containment within
|
||||
`task-sidebar-scroll`, and no document-scroll movement.
|
||||
- **Scenario:** Given a phone viewport, when Cmd+K selects a task, then direct task navigation still
|
||||
succeeds without revealing the hidden desktop rail.
|
||||
- **File:** `apps/web/e2e/tests/task/mobile-command-panel-task-navigation.spec.ts`.
|
||||
- **What to verify:** canonical task URL and usable mobile task surface; the desktop sidebar remains
|
||||
non-visible and document horizontal overflow is absent.
|
||||
|
||||
## Verification results
|
||||
|
||||
- Dependency setup: `cd apps && pnpm install --frozen-lockfile` passed.
|
||||
- RED regressions: the supersession unit test failed before generation cancellation, and the
|
||||
delayed-blocker desktop E2E failed before post-navigation reveal queuing; both passed after the
|
||||
implementation.
|
||||
- Unit: `cd apps/web && pnpm exec vitest run lib/sidebar/task-navigation.test.ts` — 1 file, 9
|
||||
tests passed.
|
||||
- Related unit consumers: `cd apps/web && pnpm exec vitest run lib/sidebar/task-navigation.test.ts
|
||||
hooks/use-command-panel-task-navigation.test.ts components/command-panel-content-search.test.tsx`
|
||||
— 3 files, 24 tests passed.
|
||||
- Typecheck: `cd apps/web && pnpm exec tsc --noEmit` passed.
|
||||
- Desktop focused GREEN: `cd apps/web && pnpm e2e:run --host --no-build
|
||||
tests/task/sidebar-scroll-preservation.spec.ts --grep "after a delayed settings navigation
|
||||
blocker|starts above the sidebar viewport"` — 2 tests passed.
|
||||
- Desktop full regression: `cd apps/web && pnpm e2e:run --host --no-build
|
||||
tests/task/sidebar-scroll-preservation.spec.ts` — 8 tests passed.
|
||||
- Mobile GREEN: `cd apps/web && pnpm e2e:run --host --no-build --project mobile-chrome
|
||||
tests/task/mobile-command-panel-task-navigation.spec.ts` — 1 test passed.
|
||||
- Build: `cd apps && pnpm --filter @kandev/web build:vite` passed.
|
||||
- Formatting and lint: focused Prettier check and ESLint with `--max-warnings 0` passed.
|
||||
- Repository checks: `git diff --check` passed. Managed E2E build/test artifacts were disposable
|
||||
and are not part of the change.
|
||||
|
||||
## Implementation waves
|
||||
|
||||
- [x] [Task 01 — reveal command-selected sidebar task](task-01-reveal-command-selected-task.md) — done
|
||||
|
||||
Execution is sequential in the primary conversation. No subagents are authorized.
|
||||
|
||||
## Risks and boundaries
|
||||
|
||||
- The active sidebar view may intentionally omit or collapse the task. The helper must time out
|
||||
without mutating persisted user preferences.
|
||||
- Desktop and mobile sidebar DOM can coexist because the desktop rail is hidden with responsive
|
||||
CSS. Visibility scoping is required to avoid treating hidden markup as the destination.
|
||||
- `content-visibility: auto` is used for long task lists. The E2E assertion must inspect the row's
|
||||
actual bounding box after reveal rather than assuming DOM presence proves visibility.
|
||||
@@ -0,0 +1,83 @@
|
||||
---
|
||||
id: "01-reveal-command-selected-task"
|
||||
title: "Reveal command-selected sidebar task"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/ui/command-panel-sidebar-task-reveal.md"
|
||||
---
|
||||
|
||||
# Task 01: Reveal command-selected sidebar task
|
||||
|
||||
## Acceptance
|
||||
|
||||
- The desktop regression test fails before production changes because Cmd+K navigation leaves the
|
||||
chosen rendered task row outside the overflowing sidebar viewport, then passes after the fix.
|
||||
- Cmd+K task selection navigates normally and reveals an available row with nearest-block scrolling
|
||||
without moving focus, scrolling the document, or changing sidebar view/collapse preferences.
|
||||
- Guarded navigation can outlive the initial reveal retry budget without losing the queued reveal;
|
||||
a newer selection supersedes an older pending reveal.
|
||||
- Phone Cmd+K task navigation remains direct and does not target the hidden desktop sidebar.
|
||||
|
||||
## Verification
|
||||
|
||||
1. `cd apps && pnpm install --frozen-lockfile`
|
||||
2. RED, before production changes: `cd apps/web && pnpm e2e:run --host --no-build tests/task/sidebar-scroll-preservation.spec.ts --grep "after a delayed settings navigation blocker"`
|
||||
3. Unit: `cd apps/web && pnpm exec vitest run lib/sidebar/task-navigation.test.ts`
|
||||
4. Typecheck: `cd apps/web && pnpm exec tsc --noEmit`
|
||||
5. Build: `cd apps && pnpm --filter @kandev/web build:vite`
|
||||
6. Desktop GREEN: `cd apps/web && pnpm e2e:run --host --no-build tests/task/sidebar-scroll-preservation.spec.ts`
|
||||
7. Mobile GREEN: `cd apps/web && pnpm e2e:run --host --no-build --project mobile-chrome tests/task/mobile-command-panel-task-navigation.spec.ts`
|
||||
|
||||
Confirm Playwright discovers the expected focused tests before treating either browser command as
|
||||
evidence. The managed runner performs the required production build and teardown.
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/web/lib/sidebar/task-navigation.ts`
|
||||
- `apps/web/lib/sidebar/task-navigation.test.ts`
|
||||
- `apps/web/hooks/use-command-panel-task-navigation.ts`
|
||||
- `apps/web/components/command-panel.tsx`
|
||||
- `apps/web/components/task/task-item.tsx`
|
||||
- `apps/web/e2e/tests/task/sidebar-scroll-preservation.spec.ts`
|
||||
- `apps/web/e2e/tests/task/mobile-command-panel-task-navigation.spec.ts`
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Sequential; the browser regression, navigation helper, command-panel integration, and responsive
|
||||
guard form one TDD vertical slice.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Behavioral contract: `docs/specs/ui/command-panel-sidebar-task-reveal.md`.
|
||||
- Root-cause trace and frontend design: `plan.md`.
|
||||
- Retryable navigation precedent: `apps/web/lib/review/navigation.ts` and its unit test.
|
||||
- Existing overflowing-sidebar fixture: `apps/web/e2e/tests/task/sidebar-scroll-preservation.spec.ts`.
|
||||
- Mobile composition precedent: `apps/web/components/task/mobile/session-task-switcher-sheet.tsx`.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the RED failure, implementation summary, actual files changed, exact test commands and
|
||||
counts, generated artifact paths, cleanup evidence, blockers, and remaining risks. Mark this task
|
||||
`done`, check it in `plan.md`, and replace the plan/task verification placeholders only after all
|
||||
targeted checks pass.
|
||||
|
||||
## Results
|
||||
|
||||
Implemented the bounded visible-sidebar reveal helper, latest-request cancellation, post-navigation
|
||||
command-panel queue, task-row marker, and desktop/mobile coverage. The RED supersession unit test
|
||||
and delayed-blocker desktop regression failed before their respective fixes and passed afterward.
|
||||
|
||||
- Unit: 1 file, 9 tests passed; task-navigation hook and command-panel consumers: 3 files, 24 tests passed.
|
||||
- Typecheck and Vite build: passed.
|
||||
- Desktop full sidebar-scroll E2E: 8 passed, including delayed guarded navigation and above-viewport reveal.
|
||||
- Mobile focused E2E: 1 passed under `mobile-chrome`.
|
||||
- Focused Prettier and ESLint (`--max-warnings 0`): passed.
|
||||
- `git diff --check`: passed.
|
||||
- Generated E2E output is disposable and excluded from the change; no security or trust-boundary
|
||||
changes were introduced.
|
||||
@@ -146,6 +146,7 @@ Subscription quota tracking and per-agent cheap-model profile routing.
|
||||
| [review-file-status](ui/review-file-status.md) | building |
|
||||
| [review-markdown-preview](ui/review-markdown-preview.md) | draft |
|
||||
| [sidebar-view-creation](ui/sidebar-view-creation.md) | shipped |
|
||||
| [command-panel sidebar task reveal](ui/command-panel-sidebar-task-reveal.md) | draft |
|
||||
| [sidebar-task-completion-icons](ui/sidebar-task-completion-icons.md) | shipped |
|
||||
| [slash-command-composer](ui/slash-command-composer.md) | shipped |
|
||||
| [subagent-observability](ui/subagent-observability.md) | building |
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
---
|
||||
status: draft
|
||||
created: 2026-08-05
|
||||
owner: kandev
|
||||
---
|
||||
|
||||
# Command-panel Sidebar Task Reveal
|
||||
|
||||
## Why
|
||||
|
||||
When the desktop sidebar contains more tasks than fit in its scroll viewport, opening a task from
|
||||
Cmd+K can leave the newly active row above or below the visible portion of the list. The workbench
|
||||
changes correctly, but the sidebar no longer provides visible context for where that task sits.
|
||||
|
||||
## What
|
||||
|
||||
- Selecting a task from Cmd+K continues to open the task's canonical route.
|
||||
- If that task already has a rendered row in the visible desktop task sidebar, the sidebar scrolls
|
||||
by the minimum amount needed to bring the row inside its own task-list viewport.
|
||||
- The revealed row carries the normal active-task highlight after navigation. The reveal does not
|
||||
move keyboard focus, scroll the document, or reset the task list to its top.
|
||||
- A row that is already fully visible is not unnecessarily repositioned.
|
||||
- Reveal waits through bounded asynchronous route and sidebar rendering instead of relying on a
|
||||
fixed delay. Command-panel selection queues the task ID while guarded navigation is pending and
|
||||
starts the reveal only after the canonical task route and matching active-task state render.
|
||||
- If another command-panel task is selected before the previous reveal finishes, the latest request
|
||||
immediately invalidates the earlier one; a late row from the earlier request must never be
|
||||
scrolled into view, even while the newer route is waiting behind a navigation guard.
|
||||
- If the active sidebar view filters the task out, a saved group or subtask branch hides it, or the
|
||||
desktop sidebar is not rendered, task navigation still succeeds and sidebar preferences remain
|
||||
unchanged.
|
||||
|
||||
## Failure modes
|
||||
|
||||
- Failure to find a rendered sidebar row is a no-op for the sidebar after the bounded reveal
|
||||
attempt; it never blocks or reverses task navigation.
|
||||
- A hidden desktop sidebar must not become a scroll target on phone-sized viewports.
|
||||
|
||||
## Mobile contract
|
||||
|
||||
- Phone navigation keeps its existing direct task-route outcome. Cmd+K does not open the mobile
|
||||
task-switcher sheet or interact with the CSS-hidden desktop sidebar.
|
||||
- The existing mobile task switcher remains the nearest shipped navigation exemplar: an inset
|
||||
bottom drawer with its own vertical scroll owner. This repair does not change that composition,
|
||||
its safe-area handling, or its touch targets.
|
||||
- Desktop and phone continue to share task search and route navigation; only the desktop sidebar
|
||||
receives the additional reveal behavior.
|
||||
|
||||
## Scenarios
|
||||
|
||||
- **GIVEN** an overflowing desktop task sidebar and a Cmd+K result whose rendered row is below the
|
||||
task-list viewport, **WHEN** the user selects that result, **THEN** the task route opens and the
|
||||
active row is fully inside the task-list viewport.
|
||||
- **GIVEN** an overflowing desktop task sidebar and a Cmd+K result whose rendered row is above the
|
||||
task-list viewport, **WHEN** the user selects that result, **THEN** the task route opens and the
|
||||
active row is fully inside the task-list viewport.
|
||||
- **GIVEN** a dirty settings page that blocks navigation after a Cmd+K task selection, **WHEN** the
|
||||
user confirms leaving after the initial reveal retry budget expires, **THEN** the task route opens
|
||||
and the active row is still revealed after the guarded navigation completes.
|
||||
- **GIVEN** two successive Cmd+K task selections where the first row is initially missing, **WHEN**
|
||||
the second task is revealed and the first row later renders, **THEN** only the second task is
|
||||
scrolled into view.
|
||||
- **GIVEN** a Cmd+K task result whose sidebar row is already fully visible, **WHEN** the user selects
|
||||
it, **THEN** task navigation succeeds without an unnecessary sidebar jump.
|
||||
- **GIVEN** a Cmd+K task result excluded by the active sidebar view or hidden by a persisted
|
||||
collapse, **WHEN** the user selects it, **THEN** task navigation succeeds without changing that
|
||||
view or collapse preference.
|
||||
- **GIVEN** a phone viewport, **WHEN** the user selects a task through Cmd+K, **THEN** the task route
|
||||
opens and no hidden desktop sidebar becomes the reveal target.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Changing sidebar filters, the active saved view, collapsed groups, collapsed subtask branches, or
|
||||
task ordering to force a row to render.
|
||||
- Expanding the desktop sidebar rail when the user has collapsed it.
|
||||
- Opening or redesigning the mobile task-switcher sheet.
|
||||
- Changing command-panel task search, task-route loading, or backend task APIs.
|
||||
|
||||
## Implementation plan
|
||||
|
||||
- [Command-panel sidebar task reveal](../../plans/command-panel-sidebar-task-reveal/plan.md)
|
||||
Reference in New Issue
Block a user