Skip to content

Make sidebar disclosure project-specific and child-aware #3787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jshph wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Make sidebar disclosure project-specific and child-aware #3787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 85 additions & 25 deletions apps/app/src/components/sidebar/ProjectList.modes.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
useAtomValue,
} from "jotai";
import { afterEach, describe, expect, it, vi } from "vitest";
import { MemoryRouter } from "react-router-dom";
import type { ThreadListEntry } from "@bb/domain";
import { ActiveSidebarModeSections, MachineModeSections } from "./ProjectList";
import { buildMachineThreadGroups } from "@bb/client-core";
Expand All @@ -33,16 +34,34 @@ import {
} from "./sidebarCollapsedAtoms";
import { useSidebarModeSectionOrder } from "./useSidebarModeSectionOrder";
import { makeThreadListEntry } from "@bb/test-helpers/domain-fixtures";
import { TooltipProvider } from "@bb/shared-ui/tooltip";

const mockUseHosts = vi.hoisted(() => vi.fn(() => ({ data: [] })));
const mockUseSystemConfig = vi.hoisted(() =>
vi.fn(() => ({
data: { experiments: { sidebarProgressiveDisclosure: true } },
})),
);

vi.mock("@/hooks/queries/host-queries", () => ({
useHosts: mockUseHosts,
usePrimaryHost: vi.fn(() => undefined),
}));

vi.mock("@/hooks/queries/system-queries", () => ({
useSystemConfig: () => ({ data: undefined }),
useSystemConfig: mockUseSystemConfig,
}));

vi.mock("@/components/thread/ThreadActionsProvider", () => ({
useThreadActions: () => ({
renameThread: vi.fn(),
requestRename: vi.fn(),
requestDelete: vi.fn(),
archiveThreadAndChildren: vi.fn(),
unarchiveThread: vi.fn(),
togglePin: vi.fn(),
toggleRead: vi.fn(),
}),
}));

const queryClient = new QueryClient();
Expand Down Expand Up @@ -156,30 +175,34 @@ function MachineModeProbe({ threads = [] }: { threads?: ThreadListEntry[] }) {
};

return (
<QueryClientProvider client={queryClient}>
<MachineModeSections
threads={threads}
draftThreadIds={new Set()}
effectivePinnedThreadIds={new Set()}
status="ready"
showPinnedSection={false}
pinnedSection={{ label: "Pinned", content: null }}
pinnedReorderPending={false}
pinnedRootNodes={[]}
pinnedThreads={[]}
onReorderPinnedThread={vi.fn()}
threadsSection={{ label: "Threads" }}
collapsedSectionIds={collapsedSectionIdSet}
collapsedThreadIds={new Set()}
collapsedEnvironmentIds={new Set()}
compareThreads={() => 0}
renderSectionDisplayOptions={() => null}
isSectionDisplayOptionsOpen={() => false}
onToggleCollapsed={handleToggleCollapsed}
onToggleThreadCollapsed={vi.fn()}
onToggleEnvironmentCollapsed={vi.fn()}
/>
</QueryClientProvider>
<TooltipProvider>
<MemoryRouter>
<QueryClientProvider client={queryClient}>
<MachineModeSections
threads={threads}
draftThreadIds={new Set()}
effectivePinnedThreadIds={new Set()}
status="ready"
showPinnedSection={false}
pinnedSection={{ label: "Pinned", content: null }}
pinnedReorderPending={false}
pinnedRootNodes={[]}
pinnedThreads={[]}
onReorderPinnedThread={vi.fn()}
threadsSection={{ label: "Threads" }}
collapsedSectionIds={collapsedSectionIdSet}
collapsedThreadIds={new Set()}
collapsedEnvironmentIds={new Set()}
compareThreads={() => 0}
renderSectionDisplayOptions={() => null}
isSectionDisplayOptionsOpen={() => false}
onToggleCollapsed={handleToggleCollapsed}
onToggleThreadCollapsed={vi.fn()}
onToggleEnvironmentCollapsed={vi.fn()}
/>
</QueryClientProvider>
</MemoryRouter>
</TooltipProvider>
);
}

Expand Down Expand Up @@ -293,4 +316,41 @@ describe("sidebar organization mode sections", () => {
expect(screen.getByLabelText("Plan mode active")).not.toBeNull();
expect(screen.queryByLabelText("Thread working")).toBeNull();
});

it("does not progressively disclose threads in By machine", () => {
const store = createStore();
store.set(sidebarMachineSectionOrderAtom, ["machine:no-machine"]);
const threads = Array.from({ length: 7 }, (_, index) =>
makeThread({
id: `machine-thread-${index}`,
title: `Machine thread ${index}`,
titleFallback: `Machine thread ${index}`,
status: "idle",
hasPendingInteraction: false,
lastReadAt: 10,
latestAttentionAt: index,
runtime: {
displayStatus: "idle",
hostReconnectGraceExpiresAt: null,
},
activity: {
activeWorkflowCount: 0,
activeBackgroundAgentCount: 0,
activeBackgroundCommandCount: 0,
activePlanModeCount: 0,
activeGoalCount: 0,
},
}),
);

render(
<JotaiProvider store={store}>
<MachineModeProbe threads={threads} />
</JotaiProvider>,
);

expect(screen.getByText("Machine thread 6")).not.toBeNull();
expect(screen.queryByRole("button", { name: "Show more" })).toBeNull();
expect(mockUseSystemConfig).not.toHaveBeenCalled();
});
});
5 changes: 2 additions & 3 deletions apps/app/src/components/sidebar/ProjectList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,6 @@ export function MachineModeSections({
threads,
threadsSection,
}: MachineModeSectionsProps) {
const progressiveDisclosureEnabled = useSidebarProgressiveDisclosureEnabled();
const { data: hosts } = useHosts();
const [collapsedMachineKeyList, setCollapsedMachineKeyList] = useAtom(
sidebarCollapsedMachinesAtom,
Expand Down Expand Up @@ -1278,7 +1277,7 @@ export function MachineModeSections({
dndParentKey={CHRONOLOGICAL_CONTAINER_ID}
rootItems={allThreadItems}
threadListState={allThreadsListState}
progressiveDisclosureEnabled={progressiveDisclosureEnabled}
progressiveDisclosureEnabled={false}
compareThreads={compareThreads}
variant="section"
selectedThreadId={selectedThreadId}
Expand Down Expand Up @@ -1328,7 +1327,7 @@ export function MachineModeSections({
dndParentKey={sectionId}
rootItems={machineItemsBySectionId.get(sectionId)}
threadListState={section.threadListState}
progressiveDisclosureEnabled={progressiveDisclosureEnabled}
progressiveDisclosureEnabled={false}
compareThreads={compareThreads}
variant="section"
selectedThreadId={selectedThreadId}
Expand Down
87 changes: 77 additions & 10 deletions apps/app/src/components/sidebar/ProjectRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1974,6 +1974,34 @@ function isAttentionProjectThreadItem(
);
}

function getProjectThreadItemFinishedAt(item: ProjectThreadItem): number {
let finishedAt = Number.NEGATIVE_INFINITY;
for (const thread of getProjectThreadItemDescendants([item])) {
if (thread.status !== "idle" && thread.status !== "error") continue;
finishedAt = Math.max(finishedAt, thread.latestAttentionAt);
}
return finishedAt;
}

function rankDisclosureItems(
items: readonly ProjectThreadItem[],
): ProjectThreadItem[] {
const inputIndex = new Map(
items.map((item, index) => [getSidebarItemKey(item), index]),
);
return [...items].sort((left, right) => {
const leftFinishedAt = getProjectThreadItemFinishedAt(left);
const rightFinishedAt = getProjectThreadItemFinishedAt(right);
if (leftFinishedAt !== rightFinishedAt) {
return leftFinishedAt > rightFinishedAt ? -1 : 1;
}
return (
(inputIndex.get(getSidebarItemKey(left)) ?? 0) -
(inputIndex.get(getSidebarItemKey(right)) ?? 0)
);
});
}

export const ProjectThreadTree = memo(function ProjectThreadTree({
projectId,
dndParentKey,
Expand Down Expand Up @@ -2014,27 +2042,47 @@ export const ProjectThreadTree = memo(function ProjectThreadTree({
buildProjectThreadGroups(projectThreads, compareThreads, draftThreadIds),
[compareThreads, draftThreadIds, projectThreads, providedRootItems],
);
const disclosureItems = useMemo(
() =>
rankDisclosureItems(
allRootItems.filter(
(item) => !isAttentionProjectThreadItem(item, selectedThreadId),
),
),
[allRootItems, selectedThreadId],
);
const initialDisclosureItemKeys = useMemo(
() =>
new Set(
disclosureItems
.slice(0, THREAD_ITEMS_INITIAL_LIMIT)
.map(getSidebarItemKey),
),
[disclosureItems],
);
const rootItems = useMemo(() => {
if (!progressiveDisclosureEnabled) {
return allRootItems;
}
return allRootItems.filter(
(item, index) =>
index < THREAD_ITEMS_INITIAL_LIMIT ||
(item) =>
initialDisclosureItemKeys.has(getSidebarItemKey(item)) ||
revealedItemKeys.has(getSidebarItemKey(item)) ||
isAttentionProjectThreadItem(item, selectedThreadId),
);
}, [
allRootItems,
initialDisclosureItemKeys,
selectedThreadId,
revealedItemKeys,
progressiveDisclosureEnabled,
]);
const visibleItemKeys = new Set(rootItems.map(getSidebarItemKey));
const hiddenItems = allRootItems.filter(
const hiddenItems = disclosureItems.filter(
(item) => !visibleItemKeys.has(getSidebarItemKey(item)),
);
const hasMoreItems = hiddenItems.length > 0;
const hasRevealedItems = revealedItemKeys.size > 0;
const handleShowMore: MouseEventHandler<HTMLButtonElement> = (event) => {
const nextItems = hiddenItems.slice(0, THREAD_ITEMS_EXPAND_SIZE);
setRevealedItemKeys(
Expand All @@ -2050,6 +2098,10 @@ export const ProjectThreadTree = memo(function ProjectThreadTree({
: undefined,
);
};
const handleShowLess = () => {
setRevealedItemKeys(new Set());
setFocusItemKey(undefined);
};

if (threadListState.status === "loading") {
return <ThreadTreeLoadingSkeleton />;
Expand Down Expand Up @@ -2110,19 +2162,34 @@ export const ProjectThreadTree = memo(function ProjectThreadTree({
})}
/>
) : null}
{hasMoreItems ? (
<button
type="button"
onClick={handleShowMore}
className={THREAD_DISCLOSURE_CONTROL_CLASS}
{hasMoreItems || hasRevealedItems ? (
<div
className="flex gap-3"
style={{
marginLeft: getSidebarThreadRowPaddingLeft(
getProjectThreadTreeRootDepthOffset(variant),
),
}}
>
Show more
</button>
{hasMoreItems ? (
<button
type="button"
onClick={handleShowMore}
className={THREAD_DISCLOSURE_CONTROL_CLASS}
>
Show more
</button>
) : null}
{hasRevealedItems ? (
<button
type="button"
onClick={handleShowLess}
className={THREAD_DISCLOSURE_CONTROL_CLASS}
>
Show less
</button>
) : null}
</div>
) : null}
</>
);
Expand Down
53 changes: 51 additions & 2 deletions apps/app/src/components/sidebar/ProjectThreadTree.disclosure.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ describe("ProjectThreadTree progressive disclosure", () => {
expect(screen.getByText("Thread 6")).not.toBeNull();
});

it("reveals ten more items per Show more click and hides the button when exhausted", () => {
it("reveals ten more items per click and Show less restores the initial set", () => {
renderThreadTree(makePlainThreads(17));

expect(screen.getByText("Thread 4")).not.toBeNull();
Expand All @@ -159,11 +159,18 @@ describe("ProjectThreadTree progressive disclosure", () => {
expect(screen.getByText("Thread 14")).not.toBeNull();
expect(screen.queryByText("Thread 15")).toBeNull();
expect(screen.getByRole("button", { name: "Show more" })).not.toBeNull();
expect(screen.queryByRole("button", { name: "Show less" })).toBeNull();
expect(screen.getByRole("button", { name: "Show less" })).not.toBeNull();

fireEvent.click(screen.getByRole("button", { name: "Show more" }));
expect(screen.getByText("Thread 16")).not.toBeNull();
expect(screen.queryByRole("button", { name: "Show more" })).toBeNull();
expect(screen.getByRole("button", { name: "Show less" })).not.toBeNull();

fireEvent.click(screen.getByRole("button", { name: "Show less" }));
expect(screen.getByText("Thread 4")).not.toBeNull();
expect(screen.queryByText("Thread 5")).toBeNull();
expect(screen.getByRole("button", { name: "Show more" })).not.toBeNull();
expect(screen.queryByRole("button", { name: "Show less" })).toBeNull();
});

it("does not spend Show more slots on attention items", () => {
Expand Down Expand Up @@ -243,4 +250,46 @@ describe("ProjectThreadTree progressive disclosure", () => {
expect(screen.getByText("Thread 6")).not.toBeNull();
expect(screen.getByText("Thread 7")).not.toBeNull();
});

it("ranks a root tree by its most recently finished descendant", () => {
const threads = [
makeThreadListEntry({
id: "older-parent",
title: "Older parent",
titleFallback: "Older parent",
createdAt: 1,
updatedAt: 1,
latestAttentionAt: 1,
lastReadAt: 100,
}),
makeThreadListEntry({
id: "recent-child",
parentThreadId: "older-parent",
title: "Recent child",
titleFallback: "Recent child",
createdAt: 2,
updatedAt: 100,
latestAttentionAt: 100,
lastReadAt: 100,
}),
...Array.from({ length: 5 }, (_, index) =>
makeThreadListEntry({
id: `other-${index}`,
title: `Other ${index}`,
titleFallback: `Other ${index}`,
createdAt: index + 3,
updatedAt: index + 10,
latestAttentionAt: index + 10,
lastReadAt: index + 10,
}),
),
];

renderThreadTree(threads);

expect(screen.getByText("Older parent")).not.toBeNull();
expect(screen.getByText("Recent child")).not.toBeNull();
expect(screen.queryByText("Other 0")).toBeNull();
expect(screen.getByRole("button", { name: "Show more" })).not.toBeNull();
});
});
2 changes: 1 addition & 1 deletion apps/app/src/views/SettingsView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,7 @@ const EXPERIMENT_DEFINITIONS: Record<
sidebarProgressiveDisclosure: {
label: "Sidebar progressive disclosure",
description:
"In By project and By machine, show the first five groups in the current sort order, keep attention groups visible, and reveal ten more per click. Manually is unchanged.",
"In By project, show five recently finished thread trees, keep attention groups visible, and reveal or collapse more trees. Other organization modes are unchanged.",
},
timelineWindowing: {
label: "Timeline windowing",
Expand Down
Loading
Loading