diff --git a/packages/ui/src/features/canvas/components/ChannelSidebar.tsx b/packages/ui/src/features/canvas/components/ChannelSidebar.tsx index cad0552652..0e19184bba 100644 --- a/packages/ui/src/features/canvas/components/ChannelSidebar.tsx +++ b/packages/ui/src/features/canvas/components/ChannelSidebar.tsx @@ -25,6 +25,7 @@ import { Input, MenuLabel, Skeleton, + SkeletonText, } from "@posthog/quill"; import { LOOPS_FLAG } from "@posthog/shared"; import type { TaskRunStatus } from "@posthog/shared/domain-types"; @@ -157,24 +158,30 @@ function RecentSectionHeader({ ); } -// Varied widths so the loading state reads as the list it becomes. -const SKELETON_ROW_WIDTHS = [ - "w-3/5", - "w-4/5", - "w-2/5", - "w-3/4", - "w-1/2", - "w-2/3", -] as const; +// Varied widths, as percentages, so the loading state reads as the list it +// becomes rather than a stack of identical bars. +const SKELETON_ROW_WIDTHS = [60, 80, 40, 75, 50, 66] as const; function ChannelItemsSkeleton() { return (
- + {/* Stands in for the "Recent" MenuLabel, so it carries that label's scale. */} + {SKELETON_ROW_WIDTHS.map((width) => (
- + {/* SkeletonText sizes its bar off the line it stands in — text-[13px] + is a row's own type scale, so the placeholder is the height of the + title it becomes rather than a fixed pill. */} +
))}
diff --git a/packages/ui/src/features/canvas/components/ChannelsList.test.tsx b/packages/ui/src/features/canvas/components/ChannelsList.test.tsx index 0109a40b25..56e00378ee 100644 --- a/packages/ui/src/features/canvas/components/ChannelsList.test.tsx +++ b/packages/ui/src/features/canvas/components/ChannelsList.test.tsx @@ -1,5 +1,5 @@ import { Theme } from "@radix-ui/themes"; -import { render, screen } from "@testing-library/react"; +import { act, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -48,6 +48,11 @@ vi.mock("@tanstack/react-router", () => ({ useRouterState: () => "/website", })); +import { + showChannelList, + showChannelPane, +} from "@posthog/ui/features/canvas/stores/channelPaneStore"; +import { useSidebarStore } from "@posthog/ui/features/sidebar/sidebarStore"; import { ChannelsList } from "./ChannelsList"; const ME = { id: "me-id", name: "me", path: "/me" }; @@ -68,6 +73,12 @@ describe("ChannelsList", () => { mocks.channels = [ME, ENG, DESIGN]; mocks.starredPaths = []; mocks.channelsLayout = true; + // The pane store is module state: reset to its resting value so a test that + // slides the slider can't hand the next one a pre-focused search box. + showChannelPane(); + // Same for the collapse state — a test that folds a group away would + // otherwise hide its rows from every test that runs after it. + useSidebarStore.setState({ collapsedSections: new Set() }); }); it("pins #me above the channels, with its ⌘1 shortcut", () => { @@ -156,5 +167,187 @@ describe("ChannelsList", () => { expect(screen.getByText(/No spaces match/)).toBeTruthy(); }); + + // Searching is a keyboard flow start to finish: you never leave the input + // to reach the row you just named. + it("opens the highlighted result on Enter", async () => { + const user = userEvent.setup(); + renderList(); + + await user.type(screen.getByLabelText("Search spaces"), "eng"); + await user.keyboard("{Enter}"); + + expect(mocks.navigate).toHaveBeenCalledWith({ + to: "/website/$channelId", + params: { channelId: ENG.id }, + }); + }); + + it("moves the highlight with the arrow keys", async () => { + const user = userEvent.setup(); + renderList(); + + // "me", "design" and "engineering" all contain an "e"; the first is + // highlighted to begin with, so one press down lands on the second. + await user.type(screen.getByLabelText("Search spaces"), "e"); + await user.keyboard("{ArrowDown}{Enter}"); + + expect(mocks.navigate).toHaveBeenCalledWith({ + to: "/website/$channelId", + params: { channelId: ENG.id }, + }); + }); + + // Base UI's clear button is a tabIndex=-1 decoration by default, which left + // the keyboard with no way out of a query. + it("clears on Escape", async () => { + const user = userEvent.setup(); + renderList(); + const input = screen.getByLabelText("Search spaces"); + + await user.type(input, "eng"); + expect(screen.queryByText("design")).toBeNull(); + + await user.keyboard("{Escape}"); + + expect((input as HTMLInputElement).value).toBe(""); + expect(screen.getByText("design")).toBeTruthy(); + }); + + it("gives the clear button a tab stop", async () => { + const user = userEvent.setup(); + renderList(); + + await user.type(screen.getByLabelText("Search spaces"), "eng"); + + const clear = screen.getByLabelText("Clear search"); + expect((clear as HTMLElement).tabIndex).toBe(0); + + await user.tab(); + expect(document.activeElement).toBe(clear); + await user.keyboard("{Enter}"); + expect( + (screen.getByLabelText("Search spaces") as HTMLInputElement).value, + ).toBe(""); + }); + }); + + // The list is a switcher first: arrowing to a space has to work the moment + // the pane opens, not only once there's something to filter by. + describe("keyboard traversal with no query", () => { + it("walks the rows and opens the highlighted one", async () => { + const user = userEvent.setup(); + renderList(); + + await user.click(screen.getByLabelText("Search spaces")); + await user.keyboard("{ArrowDown}{Enter}"); + + expect(mocks.navigate).toHaveBeenCalledWith({ + to: "/website/$channelId", + params: { channelId: ENG.id }, + }); + }); + + // Base UI resets the highlight when the pointer leaves a row, and + // `autoHighlight="always"` then snaps it back to the top — so drifting the + // mouse across the gap between two rows threw the keyboard back to #me. + it("keeps the highlight when the pointer leaves a row", async () => { + const user = userEvent.setup(); + renderList(); + + await user.click(screen.getByLabelText("Search spaces")); + const row = screen.getByText("engineering"); + await user.hover(row); + await user.unhover(row); + await user.keyboard("{Enter}"); + + expect(mocks.navigate).toHaveBeenCalledWith({ + to: "/website/$channelId", + params: { channelId: ENG.id }, + }); + }); + + // A kept-mounted collapsed row would still be an option, so ↓ would walk + // onto spaces that were folded away. + it("drops a collapsed group's rows from the list", async () => { + const user = userEvent.setup(); + renderList(); + expect(screen.getByText("engineering")).toBeTruthy(); + + await user.click(screen.getByText("Spaces")); + + expect(screen.queryByText("engineering")).toBeNull(); + }); + }); + + // Sliding back from a space, the list is what you came here for — so it hands + // the search box the caret rather than making you click it. + describe("focus on returning to the list", () => { + it("focuses the search box when the pane slides back", async () => { + renderList(); + expect(document.activeElement).not.toBe( + screen.getByLabelText("Search spaces"), + ); + + act(() => showChannelList()); + + await waitFor(() => + expect(document.activeElement).toBe( + screen.getByLabelText("Search spaces"), + ), + ); + }); + + // Opening a row would close an ordinary combobox, and a closed one stops + // answering the arrow keys — the pane came back with its keyboard dead and + // its highlight parked on the space you had just left. + it("returns the highlight to the top with the arrows live", async () => { + const user = userEvent.setup(); + renderList(); + + await user.click(screen.getByText("engineering")); + act(() => showChannelList()); + await waitFor(() => + expect(document.activeElement).toBe( + screen.getByLabelText("Search spaces"), + ), + ); + mocks.navigate.mockClear(); + + // From the top, one press down is "engineering" again. Left where it was, + // it would have been the row after it. + await user.keyboard("{ArrowDown}{Enter}"); + + expect(mocks.navigate).toHaveBeenCalledWith({ + to: "/website/$channelId", + params: { channelId: ENG.id }, + }); + }); + + it("selects a stale query so the next keystroke replaces it", async () => { + const user = userEvent.setup(); + renderList(); + const input = screen.getByLabelText("Search spaces") as HTMLInputElement; + + await user.type(input, "eng"); + await user.click(screen.getByText("engineering")); + act(() => showChannelList()); + + await waitFor(() => expect(document.activeElement).toBe(input)); + expect(input.selectionStart).toBe(0); + expect(input.selectionEnd).toBe("eng".length); + }); + + it("leaves focus alone off the channels layout", async () => { + mocks.channelsLayout = false; + renderList(); + + act(() => showChannelList()); + + await waitFor(() => + expect(screen.queryByLabelText("Search spaces")).toBeNull(), + ); + expect(document.activeElement).toBe(document.body); + }); }); }); diff --git a/packages/ui/src/features/canvas/components/ChannelsList.tsx b/packages/ui/src/features/canvas/components/ChannelsList.tsx index 5b7eac01e1..abd850d870 100644 --- a/packages/ui/src/features/canvas/components/ChannelsList.tsx +++ b/packages/ui/src/features/canvas/components/ChannelsList.tsx @@ -19,6 +19,11 @@ import { AlertDialogFooter, AlertDialogHeader, AlertDialogTitle, + Autocomplete, + AutocompleteClear, + AutocompleteInput, + AutocompleteItem, + AutocompleteList, Button, ButtonGroup, AlertDialog as ConfirmDialog, @@ -35,7 +40,6 @@ import { DropdownMenuTrigger, Empty, EmptyHeader, - Input, Kbd, MenuLabel, Tooltip, @@ -65,7 +69,10 @@ import { useTaskChannels, } from "@posthog/ui/features/canvas/hooks/useTaskChannels"; import { useIsChannelUnread } from "@posthog/ui/features/canvas/hooks/useUnreadChannels"; -import { showChannelPane } from "@posthog/ui/features/canvas/stores/channelPaneStore"; +import { + showChannelPane, + useChannelPaneStore, +} from "@posthog/ui/features/canvas/stores/channelPaneStore"; import { resetCurrentChannel, useCurrentChannelStore, @@ -82,9 +89,76 @@ import { openTaskInput } from "@posthog/ui/router/useOpenTask"; import { track } from "@posthog/ui/shell/analytics"; import { Box, Flex } from "@radix-ui/themes"; import { useNavigate, useRouterState } from "@tanstack/react-router"; -import { Fragment, type ReactNode, useState } from "react"; +import { + type ComponentProps, + Fragment, + type ReactNode, + useEffect, + useRef, + useState, +} from "react"; import { hostClient } from "../hostClient"; +/** + * A row's clickable surface. + * + * Under the layout every row is an Autocomplete option, so ↑/↓/⏎ walk the list + * whether or not there's a query — the search box is the only thing that ever + * holds focus, and the list is what it drives. Off the layout there is no + * search box to drive anything, so the rows stay plain buttons. + * + * Both render the same quill Button underneath; the option just routes its + * clicks and highlight through Autocomplete. Rest props are forwarded because + * this is handed to `ContextMenuTrigger` as its rendered element. + */ +function SpaceRowSurface({ + asOption, + optionValue, + className, + children, + ...rest +}: ComponentProps & { + asOption: boolean; + /** Identifies the row to Autocomplete; unused off the layout. */ + optionValue: string; +}) { + if (!asOption) { + return ( + + ); + } + return ( + span]:w-full [&>span]:gap-2", + className, + )} + // The two branches take the same handlers typed against different + // elements — quill's option renders a button of its own, so what the + // callers pass is a button's props either way. + {...(rest as ComponentProps)} + > + {children} + + ); +} + // One actionable entry in a channel's menu, rendered the same whether it // surfaces in the hover "..." dropdown or the right-click context menu. type ChannelActionItem = { @@ -328,9 +402,8 @@ function ChannelSection({ }) { const spacesLayout = useChannelsLayout(); const noun = spacesLayout ? "space" : "channel"; - const navigate = useNavigate(); const pathname = useRouterState({ select: (s) => s.location.pathname }); - const setCurrentChannel = useCurrentChannelStore((s) => s.setCurrentChannel); + const openChannel = useOpenChannel(); const base = `/website/${channel.id}`; // Highlight the row whenever any of the channel's routes is open. const isActive = pathname === base || pathname.startsWith(`${base}/`); @@ -360,28 +433,12 @@ function ChannelSection({ { - track(ANALYTICS_EVENTS.CHANNEL_ACTION, { - action_type: "nav_click", - surface: "sidebar", - channel_id: channel.id, - }); - // Slide before navigating: the route effect would get there - // too, but not until the navigation resolves. - showChannelPane(); - setCurrentChannel(channel.id); - void navigate({ - to: "/website/$channelId", - params: { channelId: channel.id }, - }); - }} + onClick={() => openChannel(channel)} {...focusProps} - className="w-full min-w-0 justify-start gap-2 data-selected:bg-fill-selected data-selected:text-foreground" > {channelGlyph(channel.name, { size: 14, @@ -415,7 +472,7 @@ function ChannelSection({ {formatHotkey(`mod+${hotkeySlot}`)} )} - + } /> @@ -552,29 +609,24 @@ function ChannelSection({ // The feed and task ownership live on the per-user backend personal channel; // the "me" folder is the bridge that keeps the folder-keyed surfaces // (CONTEXT.md, artifacts) routable, created lazily on first open. -function PersonalChannelRow({ hotkeySlot }: { hotkeySlot?: number }) { +/** + * Opening the "me" row, shared by the row itself and the search results. + * + * The folder is created on first use, so every action resolves the id rather + * than closing over it — "me" is actionable before it exists. The create is + * shared (ensurePersonalChannel) so a row click racing its "+" menu can't + * provision two. + */ +function useOpenPersonalChannel(): { + ensureFolderId: () => Promise; + openPersonalChannel: () => Promise; + isCreating: boolean; +} { const navigate = useNavigate(); - const pathname = useRouterState({ select: (s) => s.location.pathname }); const setCurrentChannel = useCurrentChannelStore((s) => s.setCurrentChannel); const { channels } = useChannels(); const { createChannel, isCreating } = useChannelMutations(); - // Listing backend channels lazily provisions the personal channel server-side. - useTaskChannels(); - // The "+" dropdown (New task / New canvas), mirroring a shared channel row. - const [newMenuOpen, setNewMenuOpen] = useState(false); - const isUnread = useIsChannelUnread()(PERSONAL_CHANNEL_NAME); - const meFolder = channels.find((c) => c.name === PERSONAL_CHANNEL_NAME); - const createAndOpenCanvas = useCreateAndOpenDashboard(meFolder?.id); - const isActive = - !!meFolder && - (pathname === `/website/${meFolder.id}` || - pathname.startsWith(`/website/${meFolder.id}/`)); - - // The "me" folder is created on first use, so every action resolves the id - // rather than closing over it — the row is actionable before it exists. The - // create is shared (ensurePersonalChannel) so a row click racing its "+" menu - // can't provision two. const ensureFolderId = async (): Promise => { try { return (await ensurePersonalChannel(channels, createChannel)).id; @@ -586,7 +638,7 @@ function PersonalChannelRow({ hotkeySlot }: { hotkeySlot?: number }) { } }; - const open = async () => { + const openPersonalChannel = async () => { const channelId = await ensureFolderId(); if (!channelId) return; showChannelPane(); @@ -594,6 +646,54 @@ function PersonalChannelRow({ hotkeySlot }: { hotkeySlot?: number }) { void navigate({ to: "/website/$channelId", params: { channelId } }); }; + return { ensureFolderId, openPersonalChannel, isCreating }; +} + +/** + * Navigating into a channel, shared by the tree rows and the search results. + * Slides before navigating: the route effect would get there too, but not until + * the navigation resolves. + */ +function useOpenChannel(): (channel: Channel) => void { + const navigate = useNavigate(); + const setCurrentChannel = useCurrentChannelStore((s) => s.setCurrentChannel); + + return (channel: Channel) => { + track(ANALYTICS_EVENTS.CHANNEL_ACTION, { + action_type: "nav_click", + surface: "sidebar", + channel_id: channel.id, + }); + showChannelPane(); + setCurrentChannel(channel.id); + void navigate({ + to: "/website/$channelId", + params: { channelId: channel.id }, + }); + }; +} + +function PersonalChannelRow({ hotkeySlot }: { hotkeySlot?: number }) { + const spacesLayout = useChannelsLayout(); + const pathname = useRouterState({ select: (s) => s.location.pathname }); + const { channels } = useChannels(); + const { ensureFolderId, openPersonalChannel, isCreating } = + useOpenPersonalChannel(); + // Listing backend channels lazily provisions the personal channel server-side. + useTaskChannels(); + // The "+" dropdown (New task / New canvas), mirroring a shared channel row. + const [newMenuOpen, setNewMenuOpen] = useState(false); + const isUnread = useIsChannelUnread()(PERSONAL_CHANNEL_NAME); + + const meFolder = channels.find((c) => c.name === PERSONAL_CHANNEL_NAME); + const createAndOpenCanvas = useCreateAndOpenDashboard(meFolder?.id); + const isActive = + !!meFolder && + (pathname === `/website/${meFolder.id}` || + pathname.startsWith(`/website/${meFolder.id}/`)); + + const open = openPersonalChannel; + const newTask = async () => { const channelId = await ensureFolderId(); if (!channelId) return; @@ -618,14 +718,14 @@ function PersonalChannelRow({ hotkeySlot }: { hotkeySlot?: number }) { return ( - +
@@ -722,6 +822,7 @@ function ChannelGroup({ label, className, flat, + keepMounted = true, children, }: { sectionId: string; @@ -729,6 +830,12 @@ function ChannelGroup({ className?: string; /** Layout-only: rows sit at the label's level instead of indented under it. */ flat?: boolean; + /** + * Off under the layout: a kept-mounted collapsed row is still an Autocomplete + * option, so ↓ would walk onto spaces the user has folded away. Paying the + * rebuild on expand is better than highlighting a row nobody can see. + */ + keepMounted?: boolean; children: ReactNode; }) { const collapsedSections = useSidebarStore((s) => s.collapsedSections); @@ -777,7 +884,7 @@ function ChannelGroup({ dropdown, a tooltip and two dialogs up front, so unmounting on close makes each expand rebuild the lot (~940ms for 46 channels, vs ~80ms to collapse). */} - +
{children}
@@ -822,95 +929,214 @@ export function ChannelsList() { const noMatches = normalizedQuery !== "" && !meMatches && !searchResults.length; - return ( - // One shared provider groups every row tooltip so that once one shows, - // moving to the next row reveals its tooltip instantly (no re-delay). - - - {channelsLayout && ( - - setQuery(event.target.value)} - placeholder="Search spaces…" - aria-label="Search spaces" - className="h-7 text-[13px]" + // The option values, in the order the rows below render them. The rows are + // elements rather than a rendered collection, but Autocomplete still needs the + // list to map a highlight index onto — without it the first ArrowDown after a + // keystroke is swallowed re-establishing the highlight it already shows. + // A collapsed group renders no rows, so it contributes none. + const collapsedSections = useSidebarStore((s) => s.collapsedSections); + // "me" is provisioned on first use; before it exists it has no id to go by. + const meValue = me?.id ?? PERSONAL_CHANNEL_NAME; + const optionValues = normalizedQuery + ? [ + ...(meMatches ? [meValue] : []), + ...searchResults.map((channel) => channel.id), + ] + : [ + meValue, + ...(collapsedSections.has(STARRED_SECTION_ID) + ? [] + : starred.map((channel) => channel.id)), + ...(collapsedSections.has(CHANNELS_SECTION_ID) + ? [] + : others.map((channel) => channel.id)), + ]; + + // Coming back from a space, the list is what you came here to browse — so the + // search box takes focus and any previous query is selected, ready to be typed + // over. Only on the transition: a cold start rests on the channel pane, and + // re-focusing on every render would steal focus from the rows themselves. + const pane = useChannelPaneStore((s) => s.pane); + const previousPane = useRef(pane); + const searchRef = useRef(null); + useEffect(() => { + const cameFromChannel = previousPane.current === "channel"; + previousPane.current = pane; + if (!channelsLayout || pane !== "list" || !cameFromChannel) return; + const input = searchRef.current; + if (!input) return; + input.focus(); + // Autocomplete leaves its highlight on the row you opened and exposes no + // way to move it, so the pane would come back mid-list. Home is the key it + // listens for; sending it is how the list reopens at the top. + input.dispatchEvent( + new KeyboardEvent("keydown", { key: "Home", bubbles: true }), + ); + // After Home, so the caret it parks at the start doesn't undo the selection. + input.select(); + }, [pane, channelsLayout]); + + const rows = normalizedQuery ? ( + <> + {meMatches && } + {searchResults.map((channel) => ( + + ))} + {noMatches && ( + + + No {channelsLayout ? "spaces" : "channels"} match “{query.trim()}”. + + + )} + + ) : ( + <> + + + {starred.length > 0 && ( + + {starred.map((channel) => ( + - + ))} + + )} + + + {!isLoading && channels.length === 0 && ( + + + No {channelsLayout ? "spaces" : "channels"} yet. + + )} - {/* Bottom padding clears the floating create button (ChannelsFab), so - the last channel stays reachable at full scroll. */} - - {normalizedQuery ? ( - <> - {meMatches && } - {searchResults.map((channel) => ( - - ))} - {noMatches && ( - - - No {channelsLayout ? "spaces" : "channels"} match “ - {query.trim()}”. - - - )} - - ) : ( - <> - + {others.map((channel) => ( + + ))} + + + ); - {starred.length > 0 && ( - - {starred.map((channel) => ( - - ))} - - )} + // Bottom padding clears the floating create button (ChannelsFab), so the last + // channel stays reachable at full scroll. + const scrollClass = + "scroll-mask-4 min-h-0 flex-1 overflow-y-auto px-2 pt-2 pb-16"; + // quill sizes its list as a popup — a ~250px cap and its own 4px padding — + // and ships it unlayered, so plain utilities lose to it however they're + // ordered. Here the list *is* the pane, so the cap has to go and the pane's + // own padding has to win: `!` is what outranks an unlayered rule. + const listClass = cn( + "flex flex-col gap-px", + "!max-h-none !px-2 !pt-2 !pb-16", + scrollClass, + ); - - {!isLoading && channels.length === 0 && ( - - - No {channelsLayout ? "spaces" : "channels"} yet. - - - )} - {others.map((channel) => ( - - ))} - - - )} + const body = ( + + {channelsLayout && ( + + { + // Base UI's clear is a tabIndex=-1 decoration, so Escape is the + // keyboard way out of a query. With the box already empty there's + // nothing to clear, and Escape belongs to whoever is listening + // above (closing the sidebar, dismissing a dialog). + if (event.key !== "Escape" || query === "") return; + event.preventDefault(); + event.stopPropagation(); + setQuery(""); + }} + > + {/* Rendered here rather than via `showClear` so it can be given a + tab stop: quill passes no props to the one it renders itself. */} + setQuery("")} + /> + + + )} + {channelsLayout ? ( + // Every row is an option, filtered or not, so ↑/↓/⏎ work the moment the + // pane opens rather than only once you've typed something. + {rows} + ) : ( + + {rows} - + )} + + ); + + return ( + // One shared provider groups every row tooltip so that once one shows, + // moving to the next row reveals its tooltip instantly (no re-delay). + + {channelsLayout ? ( + // The rows render as elements — they're a tree of collapsible groups, + // not a flat collection — so `items` carries their values alone, in the + // same order. Filtering is ours (hence `filter={null}`; Base UI's matcher + // would run over an already-narrowed set). `inline` renders the list in + // the pane instead of a popup, and `defaultOpen` keeps it rendered + // without a trigger to open it. + + inline + // Pinned open, not `defaultOpen`: picking a row closes an ordinary + // combobox, and a closed one stops answering the arrow keys. This list + // is the pane itself — there is nothing to close, and coming back from + // a space has to find it live. + open + items={optionValues} + filter={null} + value={query} + autoHighlight="always" + // Without this the highlight resets on pointer-leave, and "always" + // then snaps it back to the first row — so drifting the mouse across + // the gap between two rows threw the keyboard back to the top. + keepHighlight + onValueChange={(value, eventDetails) => { + // Selecting a row would otherwise write the row's value back into + // the input; only what the user types moves the query. + if (eventDetails.reason !== "input-change") return; + if (typeof value === "string") setQuery(value); + }} + > + {body} + + ) : ( + body + )} ); }