Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 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
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import {
useRouter,
useRouterState,
} from "@tanstack/react-router";
import { useCallback, useEffect, useMemo } from "react";
import { memo, useCallback, useEffect, useMemo } from "react";
import { useHotkeys } from "react-hotkeys-hook";
import { shouldHandleBrowserTabSwitch } from "./browserTabShortcuts";
import {
Expand Down Expand Up @@ -126,7 +126,7 @@ type TabRef = {
appView: string | null;
};

export function BrowserTabStrip() {
function BrowserTabStripImpl() {
const spacesLayout = useChannelsLayout();
const snapshot = useTabsSnapshot();
const navigate = useNavigate();
Expand Down Expand Up @@ -381,16 +381,22 @@ export function BrowserTabStrip() {
href: locationHref,
...(railPane === "spaces" ? { listOpen, spaceId: stampedSpaceId } : {}),
};
const previousLastByPane = mirrorActive?.viewState?.lastByPane ?? {};
const viewState: TabViewState = {
// Keep the stored name when nothing has resolved yet, so a loading frame
// does not blank a background tab's label.
title: routeTitle ?? mirrorActive?.viewState?.title,
listOpen,
spaceId: stampedSpaceId,
lastByPane: {
...(mirrorActive?.viewState?.lastByPane ?? {}),
[railPane]: visit,
},
// Settings is a full-window overlay that classifies as the spaces pane, so
// recording its href here would overwrite the tab's real last spaces
// location and a later Spaces rail click would reopen Settings. Keep the
// existing map on the settings route, as the strip did before settings
// stayed mounted.
lastByPane:
routeAppView === "settings"
? previousLastByPane
: { ...previousLastByPane, [railPane]: visit },
};
const decision = decideTabNavigation({
// The SETTLED tag, not the in-flight one. Pairing the in-flight tag with
Expand Down Expand Up @@ -944,3 +950,6 @@ export function BrowserTabStrip() {
/>
);
}

// The root layout re-renders on every navigation; this keeps that from cascading here.
export const BrowserTabStrip = memo(BrowserTabStripImpl);
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import { SidebarItem } from "@posthog/ui/features/sidebar/components/SidebarItem
import { writeTaskDragData } from "@posthog/ui/features/sidebar/taskDrag";
import { SESSION_ROW_ATTRIBUTE } from "@posthog/ui/features/sidebar/useMarqueeSelection";
import { HandoffTaskDialog } from "@posthog/ui/features/task-detail/components/HandoffTaskDialog";
import { useMountedOnceOpened } from "@posthog/ui/hooks/useMountedOnceOpened";
import {
type DragEvent,
type ReactNode,
Expand Down Expand Up @@ -296,6 +297,7 @@ export function ChannelItemRow({
const subtitle = useChannelItemMetadata(item);
const [confirmDeleteOpen, setConfirmDeleteOpen] = useState(false);
const [handoffOpen, setHandoffOpen] = useState(false);
const handoffMounted = useMountedOnceOpened(handoffOpen);
const currentUser = useCurrentUser();
const canHandoff =
item.kind === "task" &&
Expand Down Expand Up @@ -454,7 +456,7 @@ export function ChannelItemRow({
</AlertDialogFooter>
</AlertDialogContent>
</AlertDialog>
{canHandoff && item.task ? (
{canHandoff && item.task && handoffMounted ? (
<HandoffTaskDialog
task={item.task}
open={handoffOpen}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,14 @@ function listStateOf({
*/
export function ChannelSidebar({ channelId }: { channelId: string }) {
const navigate = useNavigate();
const pathname = useRouterState({ select: (s) => s.location.pathname });
// Only paths inside this space matter here. Collapsing the rest to "" keeps
// every row still while the user is elsewhere (settings, another space).
const pathname = useRouterState({
select: (s) =>
s.location.pathname.startsWith(`/spaces/${channelId}`)
? s.location.pathname
: "",
});
const loopsEnabled = useFeatureFlag(LOOPS_FLAG);

const { items, actions, me, isLoading, channelMissing } =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,11 @@ vi.mock("@posthog/ui/features/canvas/components/RenameChannelModal", () => ({
}));
vi.mock("@tanstack/react-router", () => ({
useNavigate: () => mocks.navigate,
useRouterState: () => "/spaces",
useRouterState: ({
select,
}: {
select: (s: { location: { pathname: string } }) => unknown;
}) => select({ location: { pathname: "/spaces" } }),
}));

import {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ import {
} from "@posthog/ui/features/sidebar/components/items/taskStatusVocabulary";
import { useSidebarStore } from "@posthog/ui/features/sidebar/sidebarStore";
import { HandoffTaskDialog } from "@posthog/ui/features/task-detail/components/HandoffTaskDialog";
import { useMountedOnceOpened } from "@posthog/ui/hooks/useMountedOnceOpened";
import {
OverflowTickerText,
useOverflowTickerReveal,
Expand Down Expand Up @@ -436,12 +437,13 @@ const SpaceTaskRow = memo(function SpaceTaskRow({
spaceId: string;
asOption: boolean;
}) {
const pathname = useRouterState({ select: (s) => s.location.pathname });
const isActive = useRouterState({
select: (s) => s.location.pathname.endsWith(`/tasks/${item.id}`),
});
const openTask = useOpenSpaceTask();
// No PR lookup here: that is a host round trip per row, and the tree can show
// a dozen spaces' worth of rows at once.
const status = useChannelTaskStatus(item, { withPrStatus: false });
const isActive = pathname.endsWith(`/tasks/${item.id}`);
const actions = useSpaceTaskActionsContext();
// A boolean rather than the value itself, so a keypress re-renders only the
// two rows whose answer changed.
Expand All @@ -450,6 +452,8 @@ const SpaceTaskRow = memo(function SpaceTaskRow({
);

const [handoffOpen, setHandoffOpen] = useState(false);

const handoffMounted = useMountedOnceOpened(handoffOpen);
// Only the owner may hand a task off; the API 404s it for anyone else.
const currentUser = useCurrentUser();
const canHandoff =
Expand Down Expand Up @@ -529,7 +533,7 @@ const SpaceTaskRow = memo(function SpaceTaskRow({
>
{row}
</ChannelItemHoverCard>
{canHandoff && item.task ? (
{canHandoff && item.task && handoffMounted ? (
<HandoffTaskDialog
task={item.task}
open={handoffOpen}
Expand Down Expand Up @@ -711,7 +715,9 @@ function useChannelActions(channel: Channel): {
// the action is destructive and irreversible.
const [confirmDeleteOpen, setConfirmDeleteOpen] = useState(false);
const navigate = useNavigate();
const pathname = useRouterState({ select: (s) => s.location.pathname });
const insideChannel = useRouterState({
select: (s) => s.location.pathname.startsWith(`/spaces/${channel.id}`),
});
const { deleteChannel, isDeleting } = useChannelMutations();
const { isStarred, toggleStar } = useChannelStarToggle(channel);

Expand Down Expand Up @@ -751,7 +757,7 @@ function useChannelActions(channel: Channel): {
success: true,
});
// If we're inside the channel being deleted, fall back to the index.
if (pathname.startsWith(`/spaces/${channel.id}`)) {
if (insideChannel) {
void navigate({ to: "/spaces" });
}
return true;
Expand Down Expand Up @@ -948,11 +954,14 @@ const ChannelSection = memo(
}) {
const spacesLayout = useChannelsLayout();
const noun = spacesLayout ? "space" : "channel";
const pathname = useRouterState({ select: (s) => s.location.pathname });
const openChannel = useOpenChannel();
const base = `/spaces/${channel.id}`;
// Highlight the row whenever any of the channel's routes is open.
const isActive = pathname === base || pathname.startsWith(`${base}/`);
const isActive = useRouterState({
select: (s) =>
s.location.pathname === base ||
s.location.pathname.startsWith(`${base}/`),
});
const openChannel = useOpenChannel();
// Lifted so the hover button group stays visible while the menu is open.
const [menuOpen, setMenuOpen] = useState(false);
const { reveal, hoverProps, focusProps } = useOverflowTickerReveal();
Expand All @@ -973,6 +982,7 @@ const ChannelSection = memo(
confirmDelete,
isDeleting,
} = useChannelActions(channel);
const renameMounted = useMountedOnceOpened(renameOpen);

const newTask = () => {
track(ANALYTICS_EVENTS.CHANNEL_ACTION, {
Expand Down Expand Up @@ -1174,11 +1184,13 @@ const ChannelSection = memo(
</ButtonGroup>
</div>
{/* One modal for both the dropdown and context-menu "Rename" actions. */}
<RenameChannelModal
channel={channel}
open={renameOpen}
onOpenChange={setRenameOpen}
/>
{renameMounted && (
<RenameChannelModal
channel={channel}
open={renameOpen}
onOpenChange={setRenameOpen}
/>
)}
{/* Destructive confirm for "Delete channel" — spells out what's removed. */}
<ConfirmDialog
open={confirmDeleteOpen}
Expand Down Expand Up @@ -1349,7 +1361,6 @@ const PersonalChannelRow = memo(function PersonalChannelRow({
onToggleExpanded?: (spaceId: string) => void;
}) {
const spacesLayout = useChannelsLayout();
const pathname = useRouterState({ select: (s) => s.location.pathname });
const { channels } = useChannels();
const { ensureChannelId, openPersonalChannel } = useOpenPersonalChannel();

Expand All @@ -1358,10 +1369,12 @@ const PersonalChannelRow = memo(function PersonalChannelRow({
const isUnread = useIsChannelUnread()(meChannel?.id);
const unreadSessions = useUnreadSessionCount()(meChannel?.id);
const blockedSessions = useBlockedSessionCount()(meChannel?.id);
const isActive =
!!meChannel &&
(pathname === `/spaces/${meChannel.id}` ||
pathname.startsWith(`/spaces/${meChannel.id}/`));
const isActive = useRouterState({
select: (s) =>
!!meChannel &&
(s.location.pathname === `/spaces/${meChannel.id}` ||
s.location.pathname.startsWith(`/spaces/${meChannel.id}/`)),
});

const newTask = () => {
const channelId = ensureChannelId();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,28 @@ vi.mock("@posthog/ui/features/workspace/useWorkspace", () => ({
useWorkspaces: () => ({ data: {}, isFetched: true }),
}));
vi.mock("@tanstack/react-router", () => ({
useParams: () => ({ channelId: mocks.routeChannelId }),
useParams: ({
select,
}: {
select?: (params: { channelId: string | undefined }) => unknown;
} = {}) => {
const params = { channelId: mocks.routeChannelId };
return select ? select(params) : params;
},
useRouterState: ({
select,
}: {
select: (s: {
matches: { fullPath: string }[];
location: { state: { tabId?: string } };
location: { pathname: string; state: { tabId?: string } };
}) => unknown;
}) =>
select({
matches: [{ fullPath: mocks.fullPath }],
location: { state: { tabId: mocks.historyTabId } },
location: {
pathname: mocks.fullPath,
state: { tabId: mocks.historyTabId },
},
}),
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import { useSidebarEdgeHoverPeek } from "@posthog/ui/primitives/hooks/useSidebar
import { ResizableSidebar } from "@posthog/ui/primitives/ResizableSidebar";
import { navigateToArchived } from "@posthog/ui/router/navigationBridge";
import { useParams } from "@tanstack/react-router";
import { useDeferredValue, useEffect, useRef } from "react";
import { memo, useDeferredValue, useEffect, useRef } from "react";

/**
* The sidebar slider: the channel list and the channel you're in, laid out side
Expand Down Expand Up @@ -132,7 +132,7 @@ function ChannelPanes({
</div>
);
}
export function ChannelsSidebar() {
function ChannelsSidebarImpl() {
const width = useChannelsSidebarStore((state) => state.width);
const setWidth = useChannelsSidebarStore((state) => state.setWidth);
const isResizing = useChannelsSidebarStore((state) => state.isResizing);
Expand Down Expand Up @@ -204,7 +204,10 @@ export function ChannelsSidebar() {
// slide to there's only the list.
const { pane: railPane, showsActivityDetail } = useRailSurface();
const selectedActivityId = useActivitySelection()?.id;
const { feedId } = useParams({ strict: false });
const feedId = useParams({
strict: false,
select: (params) => params.feedId,
});
const pane = useChannelPaneStore((s) => s.pane);
const { isPending: pendingTabSwitch, viewState: pendingTabViewState } =
usePendingTabViewState();
Expand Down Expand Up @@ -315,3 +318,6 @@ export function ChannelsSidebar() {
</ResizableSidebar>
);
}

// The root layout re-renders on every navigation; this keeps that from cascading here.
export const ChannelsSidebar = memo(ChannelsSidebarImpl);
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { useCommandMenuStore } from "@posthog/ui/shell/commandMenuStore";
import {
type ComponentPropsWithRef,
type MouseEventHandler,
memo,
type ReactElement,
type ReactNode,
useState,
Expand Down Expand Up @@ -178,7 +179,7 @@ function ActivityNavItem({
* The app's leftmost column. Sits outside the resizable sidebar, so collapsing
* that sidebar leaves the destinations reachable.
*/
export function NavRail() {
function NavRailImpl() {
const homeEnabled = useFeatureFlag(DESKTOP_HOME_FLAG);
const loopsEnabled = useFeatureFlag(LOOPS_FLAG);
const contextEnabled = useContextLayerFlag();
Expand Down Expand Up @@ -322,3 +323,6 @@ export function NavRail() {
</TooltipProvider>
);
}

// The root layout re-renders on every navigation; this keeps that from cascading here.
export const NavRail = memo(NavRailImpl);
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ const {
vi.mock("@tanstack/react-router", () => ({
Outlet: () => null,
useNavigate: () => vi.fn(),
useParams,
useParams: (opts?: {
select?: (p: Record<string, string | undefined>) => unknown;
}) => {
const params = useParams();
return opts?.select ? opts.select(params) : params;
},
useRouterState: ({
select,
}: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,12 +347,17 @@ function CanvasBreadcrumb({

export function ShellLayout() {
const spacesLayout = useChannelsLayout();
const pathname = useRouterState({ select: (s) => s.location.pathname });
const pathname = useRouterState({
select: (s) =>
s.location.pathname.startsWith("/spaces/") ? s.location.pathname : "",
});
const selectedCanvasId = useSelectedCanvasId();
const params = useParams({ strict: false });

const channelId = params.channelId;
const dashboardId = params.dashboardId;
// Select each param on its own so an unrelated route param (a settings
// category) changing cannot re-render the shell. `useParams` without a
// selector subscribes to the whole param set, which the nearest match carries
// for the entire route chain.
const channelId = useParams({ strict: false, select: (p) => p.channelId });
const dashboardId = useParams({ strict: false, select: (p) => p.dashboardId });
const { dashboard: selectedCanvas } = useDashboard(selectedCanvasId);
const toolbarDashboardId = dashboardId ?? selectedCanvasId;
const toolbarChannelId = channelId ?? selectedCanvas?.channelId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ function useDrawnSide(
}

/** Every side belongs to a session, so nothing below this runs elsewhere. */
export function RightPanel() {
function RightPanelImpl() {
const { taskId } = useActiveSession();
// Keyed by session: carrying per-session state across a navigation draws the
// previous session's panel over the new one for a frame.
Expand Down Expand Up @@ -319,3 +319,6 @@ function SessionRightPanel({ taskId }: { taskId: string }) {
</>
);
}

// The root layout re-renders on every navigation; this keeps that from cascading here.
export const RightPanel = memo(RightPanelImpl);
Loading
Loading