From b8c3146ffdfdc6103cde0b4dffc3673c95bd3a06 Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Mon, 27 Jul 2026 10:25:45 +0200 Subject: [PATCH] fix(channels): flatten the list and make the panes swipeable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Starred" and "Channels" are headings over the rows, not parents of them. Drop the "#" from the heading (it read as a channel named "Starred") and the indent under it, so a channel sits at the heading's level and the glyph — "#" now, a lock once channels can be private — belongs to the row that has it. Dim the shortcut hints: at full strength they competed with the channel names they annotate. Add a two-finger horizontal swipe between the two panes, so the back row isn't the only way out of a channel and swiping the other way returns to the channel you're still scoped to. A trackpad swipe arrives as a horizontal wheel; the sidebar has nothing to scroll sideways, so the panes claim those, accumulate the deltas across the gesture, and lock until the wheel goes quiet so the momentum tail can't swipe straight back. All three stay under the channels layout flag — the alpha's tree renders as it does today. Generated-By: PostHog Code Task-Id: 1932f1b0-a072-4de2-a21d-6455e6c445ee --- packages/ui/src/features/canvas/AGENTS.md | 7 +- .../canvas/components/ChannelsList.test.tsx | 20 +++++ .../canvas/components/ChannelsList.tsx | 39 +++++++--- .../components/ChannelsSidebar.test.tsx | 75 ++++++++++++++++++- .../canvas/components/ChannelsSidebar.tsx | 17 ++++- .../canvas/hooks/useChannelPaneSwipe.ts | 65 ++++++++++++++++ 6 files changed, 208 insertions(+), 15 deletions(-) create mode 100644 packages/ui/src/features/canvas/hooks/useChannelPaneSwipe.ts diff --git a/packages/ui/src/features/canvas/AGENTS.md b/packages/ui/src/features/canvas/AGENTS.md index 0b3455fb8a..c98cf05a50 100644 --- a/packages/ui/src/features/canvas/AGENTS.md +++ b/packages/ui/src/features/canvas/AGENTS.md @@ -44,7 +44,12 @@ The root `AGENTS.md` architecture rules still apply. (`ChannelPanes` in `ChannelsSidebar.tsx`): the searchable channel list, and the channel you're in (`ChannelSidebar`, headed by `ChannelBackRow`). Both panes stay mounted — the offscreen one is `inert` — so the slide has something to - slide and returning to the list doesn't rebuild every row. + slide and returning to the list doesn't rebuild every row. A two-finger + horizontal swipe moves between them (`useChannelPaneSwipe`, wheel `deltaX` + accumulated per gesture and locked until the wheel goes quiet). +- In the list, "Starred"/"Channels" are headings, not parents: under the layout + the rows sit at the heading's level (no indent) and the "#"/lock glyph belongs + to the rows. The alpha's indented tree is unchanged. - One `ChannelsFab` serves both panes: given a `channelId` it creates inside that channel (task, canvas), and either way it can create a channel. Off the layout it keeps its original two-item menu. Archived moves out of the sidebar diff --git a/packages/ui/src/features/canvas/components/ChannelsList.test.tsx b/packages/ui/src/features/canvas/components/ChannelsList.test.tsx index ca2940fc22..cfc24bad33 100644 --- a/packages/ui/src/features/canvas/components/ChannelsList.test.tsx +++ b/packages/ui/src/features/canvas/components/ChannelsList.test.tsx @@ -82,6 +82,26 @@ describe("ChannelsList", () => { expect(me.parentElement?.textContent).toMatch(/me(⌘|Ctrl)/); }); + // "Starred" and "Channels" are headings over the rows, not parents of them — + // under the layout the rows sit at the heading's level and keep the "#" for + // themselves. The alpha's tree is unchanged. + describe("group headings", () => { + beforeEach(() => { + mocks.starredPaths = [ENG.path]; + }); + + it("does not indent rows under the layout", () => { + renderList(); + expect(screen.getByText("engineering").closest(".pl-5")).toBeNull(); + }); + + it("keeps the indented tree off the layout", () => { + mocks.channelsLayout = false; + renderList(); + expect(screen.getByText("engineering").closest(".pl-5")).toBeTruthy(); + }); + }); + describe("search", () => { // The list is the only way to switch channels now, so with a few dozen // channels it has to be filterable rather than only scrollable. diff --git a/packages/ui/src/features/canvas/components/ChannelsList.tsx b/packages/ui/src/features/canvas/components/ChannelsList.tsx index b8a9b8f1cf..df922535d1 100644 --- a/packages/ui/src/features/canvas/components/ChannelsList.tsx +++ b/packages/ui/src/features/canvas/components/ChannelsList.tsx @@ -399,7 +399,7 @@ function ChannelSection({ {channel.name} {hotkeySlot != null && ( - + {formatHotkey(`mod+${hotkeySlot}`)} )} @@ -637,7 +637,7 @@ function PersonalChannelRow({ hotkeySlot }: { hotkeySlot?: number }) { {PERSONAL_CHANNEL_NAME} {hotkeySlot != null && ( - + {formatHotkey(`mod+${hotkeySlot}`)} )} @@ -700,18 +700,23 @@ const CHANNELS_SECTION_ID = "channels:all"; // the label styling) and animates the panel height (which janked on a list this // long). Unstyled parts give a plain label row that snaps. // -// The whole header row is the trigger. It rests as a "#" and swaps to a chevron -// on hover or keyboard focus, so the row only advertises the disclosure when -// you're actually reaching for it. +// The whole header row is the trigger. Under the layout the icon well rests +// empty and fills with a chevron on hover or keyboard focus, so the row only +// advertises the disclosure when you're reaching for it — a "#" there read as a +// channel named "Starred", and the glyph belongs to the rows, not the label +// above them. function ChannelGroup({ sectionId, label, className, + flat, children, }: { sectionId: string; label: string; className?: string; + /** Layout-only: rows sit at the label's level instead of indented under it. */ + flat?: boolean; children: ReactNode; }) { const collapsedSections = useSidebarStore((s) => s.collapsedSections); @@ -736,10 +741,12 @@ function ChannelGroup({ render={} />} > - + {!flat && ( + + )} {isOpen ? ( -
{children}
+
{children}
); @@ -851,7 +858,11 @@ export function ChannelsList() { /> {starred.length > 0 && ( - + {starred.map((channel) => ( )} - + {!isLoading && channels.length === 0 && ( diff --git a/packages/ui/src/features/canvas/components/ChannelsSidebar.test.tsx b/packages/ui/src/features/canvas/components/ChannelsSidebar.test.tsx index cf1aa4e30d..c0f4daa9d8 100644 --- a/packages/ui/src/features/canvas/components/ChannelsSidebar.test.tsx +++ b/packages/ui/src/features/canvas/components/ChannelsSidebar.test.tsx @@ -1,6 +1,6 @@ import { Theme } from "@radix-ui/themes"; import { act, render, screen } from "@testing-library/react"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ featureFlags: new Map(), @@ -169,6 +169,79 @@ describe("ChannelsSidebar", () => { expect(listIsInteractive()).toBe(true); expect(screen.queryByTestId("channel-sidebar")).toBeNull(); }); + + // A trackpad swipe reaches the panes as a horizontal wheel. Right (negative + // deltaX, the platform "back" direction) leaves the channel; left returns to + // the one still scoped. + describe("swiping", () => { + // Wheel deltas within one gesture arrive back to back; a pause between + // them is what ends it. Fake timers let a test say which it's sending. + const wheel = (deltaX: number, deltaY = 0) => + act(() => { + screen.getByTestId("channels-list").dispatchEvent( + new WheelEvent("wheel", { + deltaX, + deltaY, + bubbles: true, + cancelable: true, + }), + ); + }); + const pause = () => act(() => void vi.advanceTimersByTime(500)); + + beforeEach(() => { + vi.useFakeTimers(); + mocks.routeChannelId = ENG.id; + }); + afterEach(() => vi.useRealTimers()); + + it("goes back to the list and forward into the channel", () => { + renderSidebar(); + + wheel(-80); + expect(listIsInteractive()).toBe(true); + // The channel is browsed away from, not left. + expect(useCurrentChannelStore.getState().currentChannelId).toBe(ENG.id); + + pause(); + wheel(80); + expect(listIsInteractive()).toBe(false); + }); + + // One flick is dozens of small deltas, so the distance has to add up + // across them rather than be read off any one event. + it("adds a gesture's deltas up", () => { + renderSidebar(); + wheel(-20); + expect(listIsInteractive()).toBe(false); + wheel(-20); + wheel(-20); + expect(listIsInteractive()).toBe(true); + }); + + it("ignores a mostly-vertical wheel", () => { + renderSidebar(); + wheel(-80, -200); + expect(listIsInteractive()).toBe(false); + }); + + it("forgets a nudge once the gesture ends", () => { + renderSidebar(); + wheel(-30); + pause(); + wheel(-30); + expect(listIsInteractive()).toBe(false); + }); + + // The momentum tail of one flick keeps delivering deltas; read as fresh + // travel they'd swipe straight back to where the flick started. + it("does not let one flick's momentum swipe twice", () => { + renderSidebar(); + wheel(-80); + wheel(200); + expect(listIsInteractive()).toBe(true); + }); + }); }); describe("the Archived row", () => { diff --git a/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx b/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx index fa1fab2b2d..368b193ae8 100644 --- a/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx +++ b/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx @@ -7,11 +7,13 @@ import { ChannelSidebar } from "@posthog/ui/features/canvas/components/ChannelSi import { ChannelsFab } from "@posthog/ui/features/canvas/components/ChannelsFab"; import { ChannelsList } from "@posthog/ui/features/canvas/components/ChannelsList"; import { useChannelsSidebarStore } from "@posthog/ui/features/canvas/components/channelsSidebarStore"; +import { useChannelPaneSwipe } from "@posthog/ui/features/canvas/hooks/useChannelPaneSwipe"; import { useChannelsLayout } from "@posthog/ui/features/canvas/hooks/useChannelsLayout"; import { useCurrentChannel } from "@posthog/ui/features/canvas/hooks/useCurrentChannel"; import { PERSONAL_CHANNEL_NAME } from "@posthog/ui/features/canvas/hooks/useTaskChannels"; import { useTrackChannelsSpaceViewed } from "@posthog/ui/features/canvas/hooks/useTrackChannelsSpaceViewed"; import { + showChannelList, showChannelPane, useChannelPaneStore, } from "@posthog/ui/features/canvas/stores/channelPaneStore"; @@ -47,6 +49,10 @@ import { useDeferredValue, useEffect, useRef } from "react"; * Both panes stay mounted so the slide has something to slide, and so coming * back to the list doesn't rebuild every row's menus and dialogs. The offscreen * one is `inert`, keeping it out of the tab order and off screen readers. + * + * A two-finger horizontal swipe moves between them, so the back row isn't the + * only way out of a channel — and swiping the other way returns to the channel + * that stayed scoped the whole time. */ function ChannelPanes({ channelId, @@ -55,8 +61,17 @@ function ChannelPanes({ channelId: string | null; showList: boolean; }) { + const panesRef = useRef(null); + useChannelPaneSwipe(panesRef, { + // With no channel to slide to, the list is all there is — leave the gesture + // to the platform rather than eat it for a slide that can't happen. + enabled: channelId != null, + onBack: showChannelList, + onForward: showChannelPane, + }); + return ( - +
, + { + enabled, + onBack, + onForward, + }: { enabled: boolean; onBack: () => void; onForward: () => void }, +): void { + useEffect(() => { + const element = ref.current; + if (!element || !enabled) return; + + let travelled = 0; + let lastEventAt = Number.NEGATIVE_INFINITY; + let locked = false; + + const onWheel = (event: WheelEvent) => { + // A mostly-vertical wheel is someone scrolling the list, not swiping. + if (Math.abs(event.deltaX) <= Math.abs(event.deltaY)) return; + // Claim it before anything upstream reads it as history navigation. + event.preventDefault(); + + if (event.timeStamp - lastEventAt > GESTURE_GAP_MS) { + travelled = 0; + locked = false; + } + lastEventAt = event.timeStamp; + if (locked) return; + + travelled += event.deltaX; + if (travelled <= -SWIPE_THRESHOLD_PX) { + locked = true; + onBack(); + } else if (travelled >= SWIPE_THRESHOLD_PX) { + locked = true; + onForward(); + } + }; + + element.addEventListener("wheel", onWheel, { passive: false }); + return () => element.removeEventListener("wheel", onWheel); + }, [ref, enabled, onBack, onForward]); +}