diff --git a/apps/code/snapshots.yml b/apps/code/snapshots.yml index e2fc980da7..fc8cd61bb6 100644 --- a/apps/code/snapshots.yml +++ b/apps/code/snapshots.yml @@ -680,6 +680,14 @@ snapshots: hash: v1.k4693efd2.e01a8a548c694761b9f2a575268f420f653ad7eb9d26c2e33dc245ab03415fcf.Z5Ut_gF62xXnxkDgBXxE3yk6UDIAEVX5Sw_rKBPsqps settings-personalizationsettings--synced-truncated--light: hash: v1.k4693efd2.3c9f0dfa759b8c174abd8754593f0db9fd1518ff5225cccdcf0c3e83d4cdbe77.BqdmyuIiYJuGmHCQiRVAaBMk-xgycYOgVfwuMqma4LY + sidebar-taskitem--in-repository-group--dark: + hash: v1.k4693efd2.601f072b55c719b74db2abf8e4aa7f268f7c2cf80a279590fe841198392b49ee.sL5RJdxs9Ur7fkB9J_SO2L9JXFEpW1l1fmmQpd_-hEA + sidebar-taskitem--in-repository-group--light: + hash: v1.k4693efd2.0ae532ca59c02e645355bb9f08af295fb0c2afcd2c8c3cc6acd178f276acb416.6sNqaOArOyd2Wns3UDl8uQyd4aiwAAncKTbYidsZ9yw + sidebar-taskitem--pinned--dark: + hash: v1.k4693efd2.fef3cdc0c8a5485cabbb7ac37830d119d8939e864e972ee7a9cbd5e8c1a99fa9.6r2emBachF9VHn2cz9a2ktly_0P6sGl1iKfoQt9Sxu4 + sidebar-taskitem--pinned--light: + hash: v1.k4693efd2.8e36c4705d8650d806cf7c933684299aca5d751d1dbffd58e836e20015115bd5.gkDV3puY-rvStAc8WBaLWOOakDzZDYv8B3wTukt4QbQ skill-buttons-skillbuttonactionmessage--add-analytics--dark: hash: v1.k4693efd2.6d4ce4fac8e23a50efbbfaf24c2e11f8981f6778af2bfb2ca559749d7bda0eca.OnypKY5jyJSbQUZtTVcUy-eXC2euU-xQ2wFBYJOsKwI skill-buttons-skillbuttonactionmessage--add-analytics--light: diff --git a/packages/core/src/sidebar/groupTasks.ts b/packages/core/src/sidebar/groupTasks.ts index af674467d1..b1a4f33b87 100644 --- a/packages/core/src/sidebar/groupTasks.ts +++ b/packages/core/src/sidebar/groupTasks.ts @@ -16,7 +16,19 @@ export interface GroupableTask { originProduct?: string; } +/** The registered-folder fields that decide which group a folder belongs to. */ +export interface GroupableFolder { + path: string; + remoteUrl: string | null; + /** + * Root of the main checkout when this folder is a linked git worktree, null + * or absent for a main clone. + */ + mainRepoPath?: string | null; +} + export const CUSTOM_IMAGES_GROUP_ID = "custom-images"; +export const CUSTOM_IMAGES_GROUP_NAME = "Custom images"; export interface TaskGroup { id: string; @@ -53,10 +65,9 @@ export function getRepositoryInfo( return null; } -export function folderGroupId(folder: { - path: string; - remoteUrl: string | null; -}): string { +export function folderGroupId( + folder: Pick, +): string { if (folder.remoteUrl) { return normalizeRepoKey(folder.remoteUrl).toLowerCase(); } @@ -70,13 +81,10 @@ export function folderGroupId(folder: { * so prefer a folder that is not a linked worktree (`mainRepoPath` is set only * on linked worktrees). */ -export function findGroupFolder< - F extends { - path: string; - remoteUrl: string | null; - mainRepoPath?: string | null; - }, ->(folders: F[], groupId: string): F | undefined { +export function findGroupFolder( + folders: F[], + groupId: string, +): F | undefined { const matches = folders.filter((f) => folderGroupId(f) === groupId); return matches.find((f) => !f.mainRepoPath) ?? matches[0]; } @@ -95,7 +103,7 @@ export function groupByRepository( ? CUSTOM_IMAGES_GROUP_ID : (repository?.fullPath ?? "other"); const groupName = isImageBuilder - ? "Custom images" + ? CUSTOM_IMAGES_GROUP_NAME : (repository?.name ?? "Other"); let group = groupMap.get(groupId); diff --git a/packages/core/src/sidebar/taskContext.test.ts b/packages/core/src/sidebar/taskContext.test.ts new file mode 100644 index 0000000000..6ad6f27bea --- /dev/null +++ b/packages/core/src/sidebar/taskContext.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, it } from "vitest"; +import { + formatTaskContext, + type TaskContextFolder, + type TaskContextTask, +} from "./taskContext"; + +const CODE_REPO = { + fullPath: "posthog/code", + name: "code", + organization: "PostHog", +}; + +function task(overrides: Partial = {}): TaskContextTask { + return { + repository: CODE_REPO, + workspaceMode: "local", + branchName: null, + linkedBranch: null, + ...overrides, + }; +} + +const mainClone: TaskContextFolder = { + name: "PostHog Desktop", + path: "/repos/code", + remoteUrl: "posthog/code", + mainRepoPath: null, +}; + +describe("formatTaskContext", () => { + it.each<{ name: string; task: TaskContextTask; expected: string | null }>([ + { + name: "repository only when the task has no branch of its own", + task: task(), + expected: "code", + }, + { + name: "repository and linked branch", + task: task({ linkedBranch: "posthog-code/fix-login" }), + expected: "code · posthog-code/fix-login", + }, + { + name: "the worktree's checked-out branch when no branch is linked yet", + task: task({ workspaceMode: "worktree", branchName: "wt/parser" }), + expected: "code · wt/parser", + }, + { + name: "the linked branch in preference to the checked-out branch", + task: task({ + workspaceMode: "worktree", + branchName: "wt/parser", + linkedBranch: "posthog-code/parser", + }), + expected: "code · posthog-code/parser", + }, + { + name: "no branch for a local task sitting on the default branch", + // A local workspace reports whatever branch the checkout is on, which is + // shared by every local task in the repo. Only `linkedBranch` is the + // task's own, so `main` must not leak into the line. + task: task({ workspaceMode: "local", branchName: "main" }), + expected: "code", + }, + { + name: "repository only for a cloud task with no linked branch", + task: task({ workspaceMode: "cloud" }), + expected: "code", + }, + { + name: "the custom-images group name for image-builder tasks", + task: task({ repository: null, originProduct: "image_builder" }), + expected: "Custom images", + }, + { + name: "the branch alone when the task has no repository", + task: task({ repository: null, linkedBranch: "posthog-code/orphan" }), + expected: "posthog-code/orphan", + }, + { + name: "null when there is neither a repository nor a branch", + task: task({ repository: null }), + expected: null, + }, + ])("renders $name", ({ task: subject, expected }) => { + expect(formatTaskContext(subject)).toBe(expected); + }); + + it("labels the repository with the registered folder's name", () => { + expect(formatTaskContext(task(), [mainClone])).toBe("PostHog Desktop"); + }); + + it("labels a worktree task with its main checkout's folder name", () => { + const worktree: TaskContextFolder = { + name: "code-wt", + path: "/repos/code-wt", + remoteUrl: "posthog/code", + mainRepoPath: "/repos/code", + }; + + expect( + formatTaskContext( + task({ workspaceMode: "worktree", branchName: "wt/parser" }), + [worktree, mainClone], + ), + ).toBe("PostHog Desktop · wt/parser"); + }); + + it("falls back to the repository name when no folder is registered", () => { + const unrelated: TaskContextFolder = { + name: "posthog", + path: "/repos/posthog", + remoteUrl: "posthog/posthog", + mainRepoPath: null, + }; + + expect(formatTaskContext(task(), [unrelated])).toBe("code"); + }); +}); diff --git a/packages/core/src/sidebar/taskContext.ts b/packages/core/src/sidebar/taskContext.ts new file mode 100644 index 0000000000..6ac82f47c8 --- /dev/null +++ b/packages/core/src/sidebar/taskContext.ts @@ -0,0 +1,64 @@ +import { + CUSTOM_IMAGES_GROUP_NAME, + findGroupFolder, + type GroupableFolder, +} from "./groupTasks"; +import type { TaskData } from "./sidebarData.types"; + +/** The `TaskData` fields a context line reads. */ +export type TaskContextTask = Pick< + TaskData, + | "repository" + | "originProduct" + | "workspaceMode" + | "branchName" + | "linkedBranch" +>; + +/** A registered folder, plus the display name the group header would use. */ +export interface TaskContextFolder extends GroupableFolder { + name: string; +} + +function repositoryLabel( + task: TaskContextTask, + folders: TaskContextFolder[], +): string | null { + if (task.originProduct === "image_builder") return CUSTOM_IMAGES_GROUP_NAME; + const repository = task.repository; + if (!repository) return null; + // Prefer the registered folder's name, which is what the group header for + // this repo reads when a folder is registered. Without one we take the bare + // repo name and skip the organization prefix `groupByRepository` adds when + // two groups collide on a name: that is a decision across the whole group + // set, and pinned tasks are partitioned out before the groups are built, so + // a pinned task's repo may have no group to borrow a name from at all. + return findGroupFolder(folders, repository.fullPath)?.name ?? repository.name; +} + +function branchLabel(task: TaskContextTask): string | null { + // `linkedBranch` is the branch the task produced, and is deliberately left + // unset while a task sits on the repo's default branch — so this never + // degrades into a "· main" that repeats on every row. A worktree falls back + // to its checked-out branch, which is the whole reason the worktree exists. + if (task.linkedBranch) return task.linkedBranch; + return task.workspaceMode === "worktree" ? task.branchName : null; +} + +/** + * "Where does this task live" line for a task row rendered outside its + * repository group — today the pinned section, which floats above the per-repo + * groups and so loses the context its group header would have given it. + * + * Reads as ` · `, dropping either half when unknown, and + * returns null when the task has no repository or branch to report at all. + */ +export function formatTaskContext( + task: TaskContextTask, + folders: TaskContextFolder[] = [], +): string | null { + const repository = repositoryLabel(task, folders); + const branch = branchLabel(task); + if (repository && branch) return `${repository} · ${branch}`; + return repository ?? branch; +} diff --git a/packages/ui/src/features/sidebar/components/SidebarItem.tsx b/packages/ui/src/features/sidebar/components/SidebarItem.tsx index 6a7506cb48..f167f5cde9 100644 --- a/packages/ui/src/features/sidebar/components/SidebarItem.tsx +++ b/packages/ui/src/features/sidebar/components/SidebarItem.tsx @@ -38,9 +38,11 @@ interface SidebarItemProps { function SidebarItemLabel({ label, grow, + className, }: { label: React.ReactNode; grow: boolean; + className?: string; }) { const canTooltip = typeof label === "string" || typeof label === "number"; @@ -56,7 +58,10 @@ function SidebarItemLabel({ }, []); const span = ( - + {label} ); @@ -99,6 +104,9 @@ export function SidebarItem({ "group flex w-full cursor-default text-left text-[13px] leading-snug transition-colors", "focus-visible:-outline-offset-2 focus-visible:outline-2 focus-visible:outline-accent-8", "disabled:opacity-100 data-active:bg-fill-selected data-selected:bg-(--gray-3)", + // Quill's Button pins a fixed single-line height, which would clip the + // second line — opt out and grow with the content instead. + subtitle && "h-auto! min-h-7 py-1", isDimmed && "opacity-50", )} data-active={isActive || undefined} @@ -130,9 +138,13 @@ export function SidebarItem({ {endContent} {subtitle ? ( - - {subtitle} - + // Shares the label's truncate-plus-tooltip treatment, so a long + // `repository · branch` stays readable when it doesn't fit. + ) : null} diff --git a/packages/ui/src/features/sidebar/components/TaskListView.tsx b/packages/ui/src/features/sidebar/components/TaskListView.tsx index f21d0b11fc..0a507149d4 100644 --- a/packages/ui/src/features/sidebar/components/TaskListView.tsx +++ b/packages/ui/src/features/sidebar/components/TaskListView.tsx @@ -11,6 +11,7 @@ import type { TaskData, TaskGroup, } from "@posthog/core/sidebar/sidebarData.types"; +import { formatTaskContext } from "@posthog/core/sidebar/taskContext"; import { MenuLabel } from "@posthog/quill"; import { builderHog } from "@posthog/ui/assets/hedgehogs"; import { useFolders } from "@posthog/ui/features/folders/useFolders"; @@ -61,6 +62,7 @@ function SectionLabel({ label }: { label: string }) { function TaskRow({ task, + subtitle, isActive, isSelected, hideHoverActions, @@ -76,6 +78,7 @@ function TaskRow({ depth = 0, }: { task: TaskData; + subtitle?: string; isActive: boolean; isSelected: boolean; hideHoverActions: boolean; @@ -105,6 +108,7 @@ function TaskRow({ depth={depth} taskId={task.id} label={task.title} + subtitle={subtitle} isActive={isActive} isSelected={isSelected} isArchiving={isArchiving} @@ -208,6 +212,10 @@ export function TaskListView({ ): string | undefined { + return ( + formatTaskContext({ + repository: null, + branchName: null, + linkedBranch: null, + ...task, + }) ?? undefined + ); +} + +interface Row { + taskId: string; + label: string; + subtitle?: string; + /** Age of the task's last activity, resolved to a timestamp at render. */ + ageHours: number; + isPinned?: boolean; + workspaceMode?: "local" | "worktree" | "cloud"; + taskRunStatus?: "completed"; + prState?: "open"; +} + +const noop = () => {}; + +function TaskRows({ + rows, + sectionLabel, +}: { + rows: Row[]; + sectionLabel?: string; +}) { + // Ages become timestamps here rather than in `args`, so the relative labels + // are read off the same clock that renders them. The visual-regression runner + // freezes the clock in a decorator (apps/code/.storybook/preview.tsx) while + // story args are evaluated at import time against the real one — anchoring + // args to `Date.now()` collapses every label to "now" in CI. + const now = Date.now(); + return ( +
+ {sectionLabel ? ( + {sectionLabel} + ) : null} + {rows.map(({ ageHours, ...row }) => ( + + ))} +
+ ); +} + +const meta: Meta = { + title: "Sidebar/TaskItem", + component: TaskRows, + decorators: [ + (Story) => ( +
+ +
+ ), + ], +}; + +export default meta; +type Story = StoryObj; + +/** + * A row inside a repository group: one line, because the group header above it + * already says which repository the task belongs to. + */ +export const InRepositoryGroup: Story = { + args: { + sectionLabel: "code", + rows: [ + { + taskId: "t1", + label: "Add keyboard shortcuts to the command palette", + ageHours: 1, + workspaceMode: "cloud", + taskRunStatus: "completed", + }, + { + taskId: "t2", + label: "Support drag and drop between projects", + ageHours: 26, + workspaceMode: "worktree", + prState: "open", + }, + ], + }, +}; + +/** + * The pinned section floats above every repository group, so each row carries + * its own `repository · branch` context line. + */ +export const Pinned: Story = { + args: { + sectionLabel: "Pinned", + rows: [ + { + taskId: "p1", + label: "Fix flaky timeout in the worktree cleanup test", + subtitle: context({ + repository: POSTHOG_REPO, + workspaceMode: "worktree", + branchName: "fix/worktree-cleanup-flake", + }), + ageHours: 0, + isPinned: true, + workspaceMode: "worktree", + }, + { + taskId: "p2", + label: "Add a retry banner to the diff viewer", + subtitle: context({ + repository: CODE_REPO, + workspaceMode: "local", + branchName: "main", + linkedBranch: "posthog-code/diff-retry-banner", + }), + ageHours: 3, + isPinned: true, + prState: "open", + }, + { + taskId: "p3", + label: "Document the release checklist for the desktop app", + subtitle: context({ repository: CODE_REPO, workspaceMode: "cloud" }), + ageHours: 27, + isPinned: true, + workspaceMode: "cloud", + taskRunStatus: "completed", + }, + { + taskId: "p4", + label: "Try out the new onboarding copy", + subtitle: context({}), + ageHours: 30, + isPinned: true, + }, + ], + }, +}; diff --git a/packages/ui/src/features/sidebar/components/items/TaskItem.test.tsx b/packages/ui/src/features/sidebar/components/items/TaskItem.test.tsx new file mode 100644 index 0000000000..0044d10fde --- /dev/null +++ b/packages/ui/src/features/sidebar/components/items/TaskItem.test.tsx @@ -0,0 +1,43 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import { TaskItem } from "./TaskItem"; + +vi.mock("@posthog/ui/router/navigationBridge", () => ({ + navigateToPullRequestView: vi.fn(), +})); + +function renderTaskItem(props: { subtitle?: string } = {}) { + return render( + {}} + onContextMenu={() => {}} + {...props} + />, + ); +} + +describe("TaskItem", () => { + it("renders the context line under the title when given one", () => { + renderTaskItem({ subtitle: "code · posthog-code/fix-login" }); + + expect( + screen.getByText("code · posthog-code/fix-login"), + ).toBeInTheDocument(); + }); + + it("stays a single line when there is no context to show", () => { + renderTaskItem(); + + // Only the title renders — rows inside a repository group already get their + // repository from the group header, so they must not grow a second line. + expect( + screen.getByText("Write runbook for 5xx errors"), + ).toBeInTheDocument(); + expect(screen.getByRole("button").textContent).toBe( + "Write runbook for 5xx errors", + ); + }); +}); diff --git a/packages/ui/src/features/sidebar/components/items/TaskItem.tsx b/packages/ui/src/features/sidebar/components/items/TaskItem.tsx index 3d68b86474..3f3a146a49 100644 --- a/packages/ui/src/features/sidebar/components/items/TaskItem.tsx +++ b/packages/ui/src/features/sidebar/components/items/TaskItem.tsx @@ -33,6 +33,8 @@ interface TaskItemProps { depth?: number; taskId: string; label: string; + /** Second line under the title, e.g. the task's `repository · branch`. */ + subtitle?: string; isActive: boolean; isSelected?: boolean; /** Archive request in flight: show a spinner and suppress hover actions. */ @@ -106,6 +108,7 @@ export function TaskItem({ depth = 0, taskId, label, + subtitle, isActive, isSelected = false, isArchiving = false, @@ -208,6 +211,7 @@ export function TaskItem({ depth={depth} icon={icon} label={label} + subtitle={subtitle} isActive={isActive} isSelected={isSelected} isDimmed={isArchiving}