Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion packages/ui/src/features/canvas/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions packages/ui/src/features/canvas/components/ChannelsList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
39 changes: 27 additions & 12 deletions packages/ui/src/features/canvas/components/ChannelsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ function ChannelSection({
{channel.name}
</span>
{hotkeySlot != null && (
<Kbd className="ml-auto shrink-0 group-hover/chan:opacity-0">
<Kbd className="ml-auto shrink-0 opacity-50 group-hover/chan:opacity-0">
{formatHotkey(`mod+${hotkeySlot}`)}
</Kbd>
)}
Expand Down Expand Up @@ -637,7 +637,7 @@ function PersonalChannelRow({ hotkeySlot }: { hotkeySlot?: number }) {
{PERSONAL_CHANNEL_NAME}
</span>
{hotkeySlot != null && (
<Kbd className="ml-auto shrink-0 group-hover/chan:opacity-0">
<Kbd className="ml-auto shrink-0 opacity-50 group-hover/chan:opacity-0">
{formatHotkey(`mod+${hotkeySlot}`)}
</Kbd>
)}
Expand Down Expand Up @@ -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);
Expand All @@ -736,10 +741,12 @@ function ChannelGroup({
render={<MenuLabel render={<button type="button" />} />}
>
<span className="relative flex size-3.5 shrink-0 items-center justify-center">
<HashIcon
size={14}
className="group-hover/group-trigger:hidden group-focus-visible/group-trigger:hidden"
/>
{!flat && (
<HashIcon
size={14}
className="group-hover/group-trigger:hidden group-focus-visible/group-trigger:hidden"
/>
)}
{isOpen ? (
<CaretDownIcon
size={14}
Expand All @@ -759,7 +766,7 @@ function ChannelGroup({
makes each expand rebuild the lot (~940ms for 46 channels, vs ~80ms
to collapse). */}
<Collapsible.Panel keepMounted>
<div className="pl-5">{children}</div>
<div className={cn(!flat && "pl-5")}>{children}</div>
</Collapsible.Panel>
</Collapsible.Root>
);
Expand Down Expand Up @@ -851,7 +858,11 @@ export function ChannelsList() {
/>

{starred.length > 0 && (
<ChannelGroup sectionId={STARRED_SECTION_ID} label="Starred">
<ChannelGroup
sectionId={STARRED_SECTION_ID}
label="Starred"
flat={channelsLayout}
>
{starred.map((channel) => (
<ChannelSection
key={channel.id}
Expand All @@ -863,7 +874,11 @@ export function ChannelsList() {
</ChannelGroup>
)}

<ChannelGroup sectionId={CHANNELS_SECTION_ID} label="Channels">
<ChannelGroup
sectionId={CHANNELS_SECTION_ID}
label="Channels"
flat={channelsLayout}
>
{!isLoading && channels.length === 0 && (
<Empty className="px-2 py-1 text-subtle-foreground text-xs">
<EmptyHeader className="text-left">
Expand Down
Original file line number Diff line number Diff line change
@@ -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<string, boolean>(),
Expand Down Expand Up @@ -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", () => {
Expand Down
17 changes: 16 additions & 1 deletion packages/ui/src/features/canvas/components/ChannelsSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
Expand All @@ -55,8 +61,17 @@ function ChannelPanes({
channelId: string | null;
showList: boolean;
}) {
const panesRef = useRef<HTMLDivElement | null>(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 (
<Box className="min-h-0 flex-1 overflow-hidden">
<Box ref={panesRef} className="min-h-0 flex-1 overflow-hidden">
<div
className={cn(
"flex h-full w-[200%] transition-transform duration-200 ease-out motion-reduce:transition-none",
Expand Down
65 changes: 65 additions & 0 deletions packages/ui/src/features/canvas/hooks/useChannelPaneSwipe.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import type { RefObject } from "react";
import { useEffect } from "react";

/** How far a horizontal gesture has to travel before it counts as a swipe. */
const SWIPE_THRESHOLD_PX = 45;
/** A pause this long ends the gesture, so momentum can't chain two swipes. */
const GESTURE_GAP_MS = 180;

/**
* Two-finger horizontal swipes over the sidebar, mapped to the pane slider.
*
* A trackpad swipe arrives as a `wheel` event carrying `deltaX`; the sidebar has
* nothing to scroll sideways, so we claim those and translate them into the
* same back/forward the back row and a channel click already do. Swipe right
* (the platform "back" direction, a negative `deltaX`) goes out to the list;
* swipe left goes back into the channel you're still scoped to.
*
* Distance is accumulated across the event stream rather than read off a single
* event — one flick is dozens of small deltas. Once a swipe fires, the gesture
* is locked until the wheel goes quiet, so the momentum tail doesn't bounce the
* panes back and forth.
*/
export function useChannelPaneSwipe(
ref: RefObject<HTMLElement | null>,
{
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]);
}
Loading