From db030025271fc9e43f8642f01c771603da191027 Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Wed, 29 Jul 2026 11:14:26 +0200 Subject: [PATCH] fix(canvas): don't show "No matches" while the space sidebar is loading The list's render branches were independent conditions rather than a mutually exclusive chain, so a cold load with a filter active drew the skeleton and the Recent section's "No matches" empty state at the same time. The list now has one loading state covering pinned and recent together: until items arrive, a single skeleton speaks for the whole list, and a zero-length result is only reported as empty once loading settles. Also fixes "No matches" flashing while items are still arriving from the other queries, and the two empty states stacking when a filter or search is set on an empty space. Generated-By: PostHog Code Task-Id: 3bebbb5d-6e22-46c9-bacc-1c6d60bc9539 --- .../canvas/components/ChannelSidebar.test.tsx | 164 ++++++++++++++++++ .../canvas/components/ChannelSidebar.tsx | 52 +++++- 2 files changed, 211 insertions(+), 5 deletions(-) create mode 100644 packages/ui/src/features/canvas/components/ChannelSidebar.test.tsx diff --git a/packages/ui/src/features/canvas/components/ChannelSidebar.test.tsx b/packages/ui/src/features/canvas/components/ChannelSidebar.test.tsx new file mode 100644 index 0000000000..53fb6a5c3d --- /dev/null +++ b/packages/ui/src/features/canvas/components/ChannelSidebar.test.tsx @@ -0,0 +1,164 @@ +import type { ChannelItemModel } from "@posthog/core/canvas/channelItems"; +import { Theme } from "@radix-ui/themes"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + items: [] as ChannelItemModel[], + isLoading: false, + channelMissing: false, +})); + +vi.mock("@posthog/ui/features/canvas/hooks/useChannelItems", () => ({ + useChannelItems: () => ({ + items: mocks.items, + actions: { open: vi.fn(), togglePin: vi.fn(), archive: vi.fn() }, + me: { uuid: "me-uuid" }, + isLoading: mocks.isLoading, + channelMissing: mocks.channelMissing, + }), +})); +vi.mock("@posthog/ui/features/feature-flags/useFeatureFlag", () => ({ + useFeatureFlag: () => false, +})); +vi.mock("@tanstack/react-router", () => ({ + useNavigate: () => vi.fn(), + useRouterState: () => "/website/channel-1", +})); + +// Both mount their own query stacks; this suite is about the list's own +// loading-vs-empty decisions. +vi.mock("@posthog/ui/features/canvas/components/ChannelBackRow", () => ({ + ChannelBackRow: () => null, +})); +vi.mock("@posthog/ui/features/canvas/components/ChannelsFab", () => ({ + ChannelsFab: () => null, +})); + +// The row context menu's hooks reach for a QueryClient and the DI container, +// neither of which a unit test has. Stubbed at the module boundary, as +// WebsiteLayout.test.tsx does for the same reason. +vi.mock("@posthog/ui/features/tasks/useTaskContextMenu", () => ({ + useTaskContextMenu: () => ({ + showContextMenu: vi.fn(), + editingTaskId: null, + setEditingTaskId: vi.fn(), + }), +})); +vi.mock("@posthog/ui/features/tasks/useTaskMutations", () => ({ + useRenameTask: () => ({ renameTask: vi.fn() }), +})); +vi.mock("@posthog/ui/features/tasks/useTasks", () => ({ + useTasks: () => ({ data: [] }), +})); + +import { ChannelSidebar } from "./ChannelSidebar"; + +function item(overrides: Partial = {}): ChannelItemModel { + return { + key: "task:task-1", + kind: "task", + id: "task-1", + title: "Investigate signup drop-off", + ts: Date.parse("2026-07-17T12:00:00.000Z"), + pinned: false, + rawStatus: null, + authorUser: null, + authorName: "Someone else", + // Not the viewer, so filtering to "Me" leaves nothing. + authorUuid: "someone-else-uuid", + templateId: null, + ...overrides, + }; +} + +// A fresh element per render: React bails out of re-rendering an element it has +// already seen by reference, and these tests change the hook's answer between +// renders rather than the props. +const sidebar = () => ( + + + +); + +function renderSidebar() { + return render(sidebar()); +} + +describe("ChannelSidebar", () => { + beforeEach(() => { + mocks.items = []; + mocks.isLoading = false; + mocks.channelMissing = false; + }); + + it.each([ + { + what: "nothing has arrived yet", + state: { items: [], isLoading: true }, + shown: [] as string[], + hidden: ["Recent", "No matches", "Nothing here yet"], + }, + { + what: "the space is settled and genuinely empty", + state: { items: [], isLoading: false }, + shown: ["Nothing here yet"], + hidden: ["Recent", "No matches"], + }, + { + what: "the space is settled with items", + state: { items: [item()], isLoading: false }, + shown: ["Recent", "Investigate signup drop-off"], + hidden: ["No matches", "Nothing here yet"], + }, + ])("shows one state when $what", ({ state, shown, hidden }) => { + mocks.items = state.items; + mocks.isLoading = state.isLoading; + + const { container } = renderSidebar(); + + for (const text of shown) { + expect(screen.getByText(text)).toBeInTheDocument(); + } + for (const text of hidden) { + expect(screen.queryByText(text)).not.toBeInTheDocument(); + } + expect( + container.querySelector("[aria-busy]")?.getAttribute("aria-busy"), + ).toBe(String(state.isLoading)); + }); + + it("doesn't call a cold load 'no matches' while a filter is active", async () => { + const user = userEvent.setup(); + mocks.items = [item()]; + const { rerender } = renderSidebar(); + + // Filter down to the viewer's own items; this one is someone else's, so the + // settled list really has no matches. + await user.click(screen.getByRole("button", { name: "Filter" })); + await user.click(await screen.findByRole("menuitemradio", { name: "Me" })); + expect(screen.getByText("No matches")).toBeInTheDocument(); + + // Reloading the space empties the list again — that isn't a verdict. + mocks.items = []; + mocks.isLoading = true; + rerender(sidebar()); + + expect(screen.queryByText("No matches")).not.toBeInTheDocument(); + expect(screen.queryByText("Nothing here yet")).not.toBeInTheDocument(); + }); + + it("shows a single empty state when the last item goes away under a search", async () => { + const user = userEvent.setup(); + mocks.items = [item()]; + const { rerender } = renderSidebar(); + + await user.click(screen.getByRole("button", { name: "Search" })); + mocks.items = []; + rerender(sidebar()); + + expect(screen.getByText("No matches")).toBeInTheDocument(); + expect(screen.queryByText("Nothing here yet")).not.toBeInTheDocument(); + }); +}); diff --git a/packages/ui/src/features/canvas/components/ChannelSidebar.tsx b/packages/ui/src/features/canvas/components/ChannelSidebar.tsx index ac5bd8ebd8..e1589e2a1f 100644 --- a/packages/ui/src/features/canvas/components/ChannelSidebar.tsx +++ b/packages/ui/src/features/canvas/components/ChannelSidebar.tsx @@ -195,6 +195,34 @@ function ChannelItemsSkeleton() { ); } +/** `ready` is the list itself, whether or not the filters leave any rows in it. */ +type ListState = "unavailable" | "loading" | "empty" | "ready"; + +/** + * What the list shows, decided in one place. These were four conditions spread + * across the render tree, which let a cold load draw the skeleton and the "No + * matches" empty state at the same time. + * + * `narrowed` — a filter, or an open search box — is what makes no items mean + * "nothing matches" rather than "nothing here". + */ +function listStateOf({ + channelMissing, + isLoading, + itemCount, + narrowed, +}: { + channelMissing: boolean; + isLoading: boolean; + itemCount: number; + narrowed: boolean; +}): ListState { + if (channelMissing) return "unavailable"; + if (isLoading && itemCount === 0) return "loading"; + if (itemCount === 0 && !narrowed) return "empty"; + return "ready"; +} + /** * The channel pane of the sidebar slider: the way back to the channel list, * the channel's sections, then its pinned and recent tasks & canvases. @@ -246,6 +274,20 @@ export function ChannelSidebar({ channelId }: { channelId: string }) { [items, query, createdByFilter, statusFilter, me], ); + const narrowed = filtersActive || searchOpen; + const listState = listStateOf({ + channelMissing, + isLoading, + itemCount: items.length, + narrowed, + }); + // The list's two sections, which only exist once there are items. With + // everything pinned there's nothing left to list — but keep the header while + // it's narrowed, so you can undo whatever emptied it. + const showPinned = listState === "ready" && pinnedItems.length > 0; + const showRecent = + listState === "ready" && (items.some((i) => !i.pinned) || narrowed); + const taskRow = (item: (typeof items)[number]) => ( - {isLoading && items.length === 0 && } + {listState === "loading" && } - {channelMissing && ( + {listState === "unavailable" && ( @@ -378,7 +420,7 @@ export function ChannelSidebar({ channelId }: { channelId: string }) { )} - {pinnedItems.length > 0 && ( + {showPinned && ( <> Pinned
@@ -387,7 +429,7 @@ export function ChannelSidebar({ channelId }: { channelId: string }) { )} - {(items.some((i) => !i.pinned) || filtersActive || searchOpen) && ( + {showRecent && ( <> )} - {!isLoading && !channelMissing && items.length === 0 && ( + {listState === "empty" && (