Skip to content
This repository was archived by the owner on Aug 6, 2026. It is now read-only.
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
2 changes: 1 addition & 1 deletion packages/ui/src/features/command/CommandMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ export function CommandMenu({ open, onOpenChange }: CommandMenuProps) {
? [
{
id: "open-review-panel",
label: "Open review panel",
label: "Open diff view",
icon: (
<ViewVerticalIcon className="h-3 w-3 rotate-180 text-gray-11" />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Non-blocking: I wonder if we should change the icon to reflect that it's just a diff wheel view and not a split

),
Expand Down
2 changes: 1 addition & 1 deletion packages/ui/src/features/command/keyboard-shortcuts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export const KEYBOARD_SHORTCUTS: KeyboardShortcut[] = [
{
id: "toggle-review-panel",
keys: SHORTCUTS.TOGGLE_REVIEW_PANEL,
description: "Toggle review panel",
description: "Toggle diff view",
category: "navigation",
},
{
Expand Down
54 changes: 39 additions & 15 deletions packages/ui/src/features/panels/components/LeafNodeRenderer.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { Cloud as CloudIcon } from "@phosphor-icons/react";
import {
Empty,
EmptyDescription,
EmptyHeader,
EmptyMedia,
EmptyTitle,
} from "@posthog/quill";
import type { Task } from "@posthog/shared/domain-types";
import { Flex, Text } from "@radix-ui/themes";
import type React from "react";
import { useMemo } from "react";
import { useHostCapabilities } from "../../../shell/useHostCapabilities";
Expand Down Expand Up @@ -56,23 +62,29 @@ export const LeafNodeRenderer: React.FC<LeafNodeRendererProps> = ({
const activeTabId = tabs.some((t) => t.id === node.content.activeTabId)
? node.content.activeTabId
: (tabs[0]?.id ?? node.content.activeTabId);
const hiddenTabIds = useMemo(() => {
const visibleTabIds = new Set(tabs.map((tab) => tab.id));
const hiddenIds: string[] = [];
for (const tab of node.content.tabs) {
if (!visibleTabIds.has(tab.id)) hiddenIds.push(tab.id);
}
return hiddenIds;
}, [node.content.tabs, tabs]);

const cloudEmptyState = useMemo(
() =>
isCloud ? (
<Flex
align="center"
justify="center"
height="100%"
className="bg-(--gray-2)"
>
<Flex direction="column" align="center" gap="2">
<CloudIcon size={24} className="text-gray-10" />
<Text color="gray" className="text-sm">
Cloud runs are read-only
</Text>
</Flex>
</Flex>
<Empty className="h-full border-0 bg-(--gray-2)">
<EmptyHeader>
<EmptyMedia variant="icon">
<CloudIcon size={24} className="text-gray-10" />
</EmptyMedia>
<EmptyTitle>Cloud runs are read-only</EmptyTitle>
<EmptyDescription>
Local workspace tools are unavailable for this run.
</EmptyDescription>
</EmptyHeader>
</Empty>
) : undefined,
[isCloud],
);
Expand All @@ -95,8 +107,20 @@ export const LeafNodeRenderer: React.FC<LeafNodeRendererProps> = ({
onPanelFocus={onPanelFocus}
draggingTabId={draggingTabId}
draggingTabPanelId={draggingTabPanelId}
allowPanelSplit={!isCloud}
onAddTerminal={hideTerminal ? undefined : () => onAddTerminal(node.id)}
onSplitPanel={(direction) => onSplitPanel(node.id, direction)}
onSplitPanel={
isCloud ? undefined : (direction) => onSplitPanel(node.id, direction)
}
Comment on lines +112 to +114

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Cloud drag splitting remains enabled

When a tab is dragged onto an edge drop zone in a cloud layout, PanelDropZones invokes the store's split operation independently of onSplitPanel, causing an unsupported split pane—including an empty pane when its terminal tab is filtered—to be created and persisted despite this cloud guard.

Rule Used: When implementing new features, ensure that the UI... (source)

Learned From
PostHog/posthog#32595
PostHog/posthog#32677

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/panels/components/LeafNodeRenderer.tsx
Line: 110-112

Comment:
**Cloud drag splitting remains enabled**

When a tab is dragged onto an edge drop zone in a cloud layout, `PanelDropZones` invokes the store's split operation independently of `onSplitPanel`, causing an unsupported split pane—including an empty pane when its terminal tab is filtered—to be created and persisted despite this cloud guard.

**Rule Used:** When implementing new features, ensure that the UI... ([source](https://app.greptile.com/posthog-org-19734/-/custom-context?memory=5d57f0af-0be1-44de-8885-055f27e2885f))

**Learned From**
[PostHog/posthog#32595](https://github.com/PostHog/posthog/pull/32595)
[PostHog/posthog#32677](https://github.com/PostHog/posthog/pull/32677)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

onClosePanel={
tabs.length === 0 && hiddenTabIds.length > 0
? () => {
for (const tabId of hiddenTabIds) {
closeTab(taskId, node.id, tabId);
}
}
: undefined
}
emptyState={cloudEmptyState}
/>
);
Expand Down
26 changes: 25 additions & 1 deletion packages/ui/src/features/panels/components/TabbedPanel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import { render, screen } from "@testing-library/react";
import { describe, expect, it, vi } from "vitest";
import type { PanelContent } from "../panelTypes";

const panelDropZonesSpy = vi.hoisted(() =>
vi.fn((_props: { allowSplit?: boolean }) => null),
);

vi.mock("@dnd-kit/react", () => ({
useDroppable: () => ({ ref: vi.fn() }),
}));
Expand All @@ -16,7 +20,7 @@ vi.mock("@posthog/host-router/react", () => ({
}));

vi.mock("./PanelDropZones", () => ({
PanelDropZones: () => null,
PanelDropZones: panelDropZonesSpy,
}));

vi.mock("./PanelTab", () => ({
Expand Down Expand Up @@ -52,6 +56,26 @@ function content(activeTabId: string): PanelContent {
}

describe("TabbedPanel", () => {
it("disables split drop zones when panel splitting is unavailable", () => {
const droppableContent = { ...content("logs"), droppable: true };

render(
<Theme>
<TabbedPanel
panelId="main"
mountScopeKey="task-a"
content={droppableContent}
draggingTabId="logs"
allowPanelSplit={false}
/>
</Theme>,
);

expect(panelDropZonesSpy.mock.lastCall?.[0]).toEqual(
expect.objectContaining({ allowSplit: false }),
);
});

it("retains visited tabs within a task and resets them for another task", () => {
const { rerender } = render(
<Theme>
Expand Down
24 changes: 19 additions & 5 deletions packages/ui/src/features/panels/components/TabbedPanel.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useDroppable } from "@dnd-kit/react";
import { Plus, SquareSplitHorizontalIcon } from "@phosphor-icons/react";
import { Plus, SquareSplitHorizontalIcon, X } from "@phosphor-icons/react";
import { useHostTRPCClient } from "@posthog/host-router/react";
import { PanelDropZones } from "@posthog/ui/features/panels/components/PanelDropZones";
import type { SplitDirection } from "@posthog/ui/features/panels/panelLayoutStore";
Expand Down Expand Up @@ -65,8 +65,10 @@ interface TabbedPanelProps {
onPanelFocus?: (panelId: string) => void;
draggingTabId?: string | null;
draggingTabPanelId?: string | null;
allowPanelSplit?: boolean;
onAddTerminal?: () => void;
onSplitPanel?: (direction: SplitDirection) => void;
onClosePanel?: () => void;
rightContent?: React.ReactNode;
emptyState?: React.ReactNode;
}
Expand All @@ -82,8 +84,10 @@ export const TabbedPanel: React.FC<TabbedPanelProps> = ({
onPanelFocus,
draggingTabId = null,
draggingTabPanelId = null,
allowPanelSplit = true,
onAddTerminal,
onSplitPanel,
onClosePanel,
rightContent,
emptyState,
}) => {
Expand Down Expand Up @@ -230,12 +234,21 @@ export const TabbedPanel: React.FC<TabbedPanelProps> = ({
<Box flexShrink="0" className="h-[32px] min-w-[90px]" />
)}
</Flex>
{(rightContent || (content.droppable && onSplitPanel)) && (
{(rightContent ||
onClosePanel ||
(content.droppable && onSplitPanel)) && (
<Flex
align="center"
className="absolute top-0 right-0 h-[32px] border-b border-b-(--gray-6) border-l border-l-(--gray-6) bg-(--color-background)"
>
{rightContent}
{onClosePanel && (
<Tooltip content="Close panel" side="bottom">
<TabBarButton ariaLabel="Close panel" onClick={onClosePanel}>
<X size={14} />
</TabBarButton>
</Tooltip>
)}
{content.droppable && onSplitPanel && (
<Tooltip content="Split panel" side="bottom">
<TabBarButton
Expand Down Expand Up @@ -296,11 +309,12 @@ export const TabbedPanel: React.FC<TabbedPanelProps> = ({
panelId={panelId}
isDragging={!!draggingTabId}
allowSplit={
// Allow split if:
allowPanelSplit &&
// Within a splittable layout, allow the edge drop zones if:
// 1. Current panel has > 1 tab (same-panel split), OR
// 2. Dragging from a different panel (cross-panel split)
content.tabs.length > 1 ||
(draggingTabPanelId !== null && draggingTabPanelId !== panelId)
(content.tabs.length > 1 ||
(draggingTabPanelId !== null && draggingTabPanelId !== panelId))
}
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ function TaskDiffStatsBadge({ task }: { task: Task }) {
useDiffStatsToggle(task, "split");
return (
<Tooltip
content={isOpen ? "Close review panel" : "Open review panel"}
content={isOpen ? "Close diff view" : "Open diff view"}
shortcut={formatHotkey(SHORTCUTS.TOGGLE_REVIEW_PANEL)}
side="bottom"
>
Expand Down
Loading