mirror of
https://github.com/coder/coder.git
synced 2026-10-31 02:43:21 +08:00
fix(site/src): show chat instead of the right panel on small viewports (#28387)
On small viewports, the agent chat page showed the wrong panel. The right panel filled the screen and hid the chat. before: https://github.com/user-attachments/assets/fa3986b8-949a-4aad-8e98-d64316ade3f5 after: https://github.com/user-attachments/assets/bc56359d-2ffe-4f95-b47b-638261379da8 ## Changes - Below the `lg` breakpoint, the page shows one panel at a time. A right panel left open on a wide window used to hide the chat when the window narrowed. The page now shows the chat. - The panel preference is not changed by a resize. When the window widens, the panel comes back. When the user taps a tab while narrow, the panel opens as before. ## Tests - Unit tests for the new `useRightPanelNarrowSuppression` hook cover mount, both breakpoint crossings, and the explicit override. - A new mobile-viewport story asserts the chat is visible when a panel was left open. <details> <summary>Decision log</summary> - The right panel's open state persists in localStorage across all viewport sizes. `max-lg:hidden` on the chat made that stale state hide the chat on narrow windows. - The narrow suppression is transient state, not a write to the persisted preference. A temporary resize keeps the user's desktop layout intact. - The hook adjusts state during render (the React "adjusting state when a prop changes" pattern) instead of an effect, per FE8. - Verified all resize sequences live against a dev deployment: narrow with a panel open, widen to restore, drill in while narrow, and explicit close while narrow. </details> 🤖 This PR was created by [Coder Agents](https://coder.com/docs/ai-coder/agents) on behalf of @bpmct. --------- Co-authored-by: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com>
This commit is contained in:
co-authored by
Michael Suchacz
parent
6348c83142
commit
bb7f2f2332
@@ -0,0 +1,14 @@
|
||||
import { useSyncExternalStore } from "react";
|
||||
import {
|
||||
belowLgViewportMediaQuery,
|
||||
createMediaQuerySubscribe,
|
||||
isBelowLgViewport,
|
||||
} from "#/utils/mobile";
|
||||
|
||||
const subscribeBelowLgViewport = createMediaQuerySubscribe(
|
||||
belowLgViewportMediaQuery,
|
||||
);
|
||||
|
||||
export const useIsBelowLgViewport = (): boolean => {
|
||||
return useSyncExternalStore(subscribeBelowLgViewport, isBelowLgViewport);
|
||||
};
|
||||
@@ -1,11 +1,13 @@
|
||||
import { useSyncExternalStore } from "react";
|
||||
import { belowMdViewportMediaQuery, isBelowMdViewport } from "#/utils/mobile";
|
||||
import {
|
||||
belowMdViewportMediaQuery,
|
||||
createMediaQuerySubscribe,
|
||||
isBelowMdViewport,
|
||||
} from "#/utils/mobile";
|
||||
|
||||
const subscribeBelowMdViewport = (onStoreChange: () => void) => {
|
||||
const mediaQuery = window.matchMedia(belowMdViewportMediaQuery);
|
||||
mediaQuery.addEventListener("change", onStoreChange);
|
||||
return () => mediaQuery.removeEventListener("change", onStoreChange);
|
||||
};
|
||||
const subscribeBelowMdViewport = createMediaQuerySubscribe(
|
||||
belowMdViewportMediaQuery,
|
||||
);
|
||||
|
||||
export const useIsBelowMdViewport = (): boolean => {
|
||||
return useSyncExternalStore(subscribeBelowMdViewport, isBelowMdViewport);
|
||||
|
||||
@@ -38,12 +38,14 @@ import {
|
||||
MockWorkspaceAgent,
|
||||
mockApiError,
|
||||
} from "#/testHelpers/entities";
|
||||
import { setupMatchMedia } from "#/testHelpers/matchMedia";
|
||||
import {
|
||||
withAuthProvider,
|
||||
withDashboardProvider,
|
||||
withProxyProvider,
|
||||
withWebSocket,
|
||||
} from "#/testHelpers/storybook";
|
||||
import { belowLgViewportMediaQuery } from "#/utils/mobile";
|
||||
import AgentChatPage, { RIGHT_PANEL_OPEN_KEY } from "./AgentChatPage";
|
||||
import type { AgentsPageOutletContext } from "./AgentsPageLayout";
|
||||
import { buildLongConversation } from "./components/ChatConversation/storyFixtures";
|
||||
@@ -1612,6 +1614,102 @@ export const PlanModeFromChatState: Story = {
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* A right panel left open from a wide viewport must not hide chat on
|
||||
* narrow viewports; the panel is suppressed until explicitly opened.
|
||||
*/
|
||||
export const NarrowViewportShowsChatOverOpenPanel: Story = {
|
||||
beforeEach: () => {
|
||||
localStorage.setItem(RIGHT_PANEL_OPEN_KEY, "true");
|
||||
return () => localStorage.removeItem(RIGHT_PANEL_OPEN_KEY);
|
||||
},
|
||||
parameters: {
|
||||
viewport: { defaultViewport: "mobile1" },
|
||||
pixel: { matrix: { viewports: ["phone"] } },
|
||||
queries: buildQueries(
|
||||
{
|
||||
id: CHAT_ID,
|
||||
...baseChatFields,
|
||||
title: "Build a feature",
|
||||
status: "waiting",
|
||||
},
|
||||
{ messages: [], queued_messages: [], has_more: false },
|
||||
),
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
await waitFor(() => {
|
||||
expect(canvas.getByRole("region", { name: "Messages" })).toBeVisible();
|
||||
});
|
||||
expect(
|
||||
canvas.queryByRole("tab", { name: "Summary" }),
|
||||
).not.toBeInTheDocument();
|
||||
|
||||
// Explicitly toggling the panel while narrow clears the suppression.
|
||||
const messagesRegion = canvas.getByRole("region", { name: "Messages" });
|
||||
const user = userEvent.setup();
|
||||
await user.click(canvas.getByRole("button", { name: "Toggle panel" }));
|
||||
await waitFor(() => {
|
||||
expect(canvas.getByRole("tab", { name: "Summary" })).toBeVisible();
|
||||
});
|
||||
expect(messagesRegion.checkVisibility()).toBe(false);
|
||||
},
|
||||
};
|
||||
|
||||
let narrowingMedia: ReturnType<typeof setupMatchMedia> | undefined;
|
||||
|
||||
/**
|
||||
* A panel expanded on a wide viewport must not stay fullscreen when the
|
||||
* window narrows: suppression hides it, expansion included, and chat
|
||||
* takes over. The breakpoint crossing is simulated by stubbing
|
||||
* matchMedia because the story viewport cannot change mid-play.
|
||||
*/
|
||||
export const NarrowingSuppressesExpandedPanel: Story = {
|
||||
beforeEach: () => {
|
||||
localStorage.setItem(RIGHT_PANEL_OPEN_KEY, "true");
|
||||
narrowingMedia = setupMatchMedia({ [belowLgViewportMediaQuery]: false });
|
||||
return () => {
|
||||
narrowingMedia?.restore();
|
||||
narrowingMedia = undefined;
|
||||
localStorage.removeItem(RIGHT_PANEL_OPEN_KEY);
|
||||
};
|
||||
},
|
||||
parameters: {
|
||||
queries: buildQueries(
|
||||
{
|
||||
id: CHAT_ID,
|
||||
...baseChatFields,
|
||||
title: "Build a feature",
|
||||
status: "waiting",
|
||||
},
|
||||
{ messages: [], queued_messages: [], has_more: false },
|
||||
),
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
const user = userEvent.setup();
|
||||
await waitFor(() => {
|
||||
expect(canvas.getByRole("tab", { name: "Summary" })).toBeVisible();
|
||||
});
|
||||
|
||||
const messagesRegion = canvas.getByRole("region", { name: "Messages" });
|
||||
await user.click(canvas.getByRole("button", { name: "Expand panel" }));
|
||||
await waitFor(() => {
|
||||
expect(messagesRegion.checkVisibility()).toBe(false);
|
||||
});
|
||||
|
||||
narrowingMedia?.setMatches(belowLgViewportMediaQuery, true);
|
||||
await waitFor(() => {
|
||||
expect(messagesRegion.checkVisibility()).toBe(true);
|
||||
});
|
||||
// The suppressed panel is display:none, so its tab is no longer
|
||||
// accessible to role queries.
|
||||
expect(
|
||||
canvas.queryByRole("tab", { name: "Summary" }),
|
||||
).not.toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
/** Full layout with actions menu and diff panel portaled to the right slot. */
|
||||
export const CompletedWithDiffPanel: Story = {
|
||||
beforeEach: () => {
|
||||
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
MockWorkspaceAgent,
|
||||
MockWorkspaceApp,
|
||||
} from "#/testHelpers/entities";
|
||||
import { setupMatchMedia } from "#/testHelpers/matchMedia";
|
||||
import {
|
||||
buildInactiveChatQueueReconciliation,
|
||||
draftInputStorageKeyPrefix,
|
||||
@@ -31,6 +32,7 @@ import {
|
||||
settlePromotedQueueHead,
|
||||
submitEdit,
|
||||
useConversationEditingState,
|
||||
useRightPanelNarrowSuppression,
|
||||
waitForPendingChatSettingsSyncs,
|
||||
} from "./AgentChatPage";
|
||||
import type { ChatMessageInputRef } from "./components/AgentChatInput";
|
||||
@@ -1405,3 +1407,52 @@ describe("isChatAgentBindingUnresolved", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useRightPanelNarrowSuppression", () => {
|
||||
const belowLgQuery = "(max-width: 1023px)";
|
||||
|
||||
const setupBelowLg = (initialBelowLg: boolean) => {
|
||||
const media = setupMatchMedia({ [belowLgQuery]: initialBelowLg });
|
||||
return {
|
||||
setBelowLg: (value: boolean) => media.setMatches(belowLgQuery, value),
|
||||
};
|
||||
};
|
||||
|
||||
it("suppresses the panel when mounted below the lg breakpoint", () => {
|
||||
setupBelowLg(true);
|
||||
const { result } = renderHook(() => useRightPanelNarrowSuppression());
|
||||
expect(result.current.suppressed).toBe(true);
|
||||
});
|
||||
|
||||
it("does not suppress the panel when mounted at or above lg", () => {
|
||||
setupBelowLg(false);
|
||||
const { result } = renderHook(() => useRightPanelNarrowSuppression());
|
||||
expect(result.current.suppressed).toBe(false);
|
||||
});
|
||||
|
||||
it("suppresses on narrowing and clears on widening", () => {
|
||||
const media = setupBelowLg(false);
|
||||
const { result } = renderHook(() => useRightPanelNarrowSuppression());
|
||||
|
||||
act(() => media.setBelowLg(true));
|
||||
expect(result.current.suppressed).toBe(true);
|
||||
|
||||
act(() => media.setBelowLg(false));
|
||||
expect(result.current.suppressed).toBe(false);
|
||||
});
|
||||
|
||||
it("stays cleared after an explicit clearSuppression until the next narrowing", () => {
|
||||
const media = setupBelowLg(false);
|
||||
const { result } = renderHook(() => useRightPanelNarrowSuppression());
|
||||
|
||||
act(() => media.setBelowLg(true));
|
||||
expect(result.current.suppressed).toBe(true);
|
||||
|
||||
act(() => result.current.clearSuppression());
|
||||
expect(result.current.suppressed).toBe(false);
|
||||
|
||||
act(() => media.setBelowLg(false));
|
||||
act(() => media.setBelowLg(true));
|
||||
expect(result.current.suppressed).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -67,6 +67,7 @@ import type { ChatMessagePart } from "#/api/typesGenerated";
|
||||
import { useProxy } from "#/contexts/ProxyContext";
|
||||
import { useAuthenticated } from "#/hooks/useAuthenticated";
|
||||
import { useAIGatewayEnabled } from "#/hooks/useEmbeddedMetadata";
|
||||
import { useIsBelowLgViewport } from "#/hooks/useIsBelowLgViewport";
|
||||
import {
|
||||
getDefaultOrganizationName,
|
||||
useDashboard,
|
||||
@@ -132,6 +133,29 @@ import {
|
||||
/** localStorage key controlling whether the right panel is visible. */
|
||||
export const RIGHT_PANEL_OPEN_KEY = "agents.right-panel-open";
|
||||
|
||||
/**
|
||||
* Below the `lg` breakpoint, chat and the right panel are mutually
|
||||
* exclusive, so a panel left open on a wide window would hide chat as
|
||||
* soon as the window narrows. This suppresses the panel while narrow
|
||||
* without touching the persisted preference: widening restores the
|
||||
* panel, and an explicit user action (clearSuppression) overrides it.
|
||||
*/
|
||||
export function useRightPanelNarrowSuppression(): {
|
||||
suppressed: boolean;
|
||||
clearSuppression: () => void;
|
||||
} {
|
||||
const isBelowLg = useIsBelowLgViewport();
|
||||
const [suppressed, setSuppressed] = useState(isBelowLg);
|
||||
const [prevIsBelowLg, setPrevIsBelowLg] = useState(isBelowLg);
|
||||
// Render-time state adjustment on breakpoint crossings; see
|
||||
// https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes
|
||||
if (isBelowLg !== prevIsBelowLg) {
|
||||
setPrevIsBelowLg(isBelowLg);
|
||||
setSuppressed(isBelowLg);
|
||||
}
|
||||
return { suppressed, clearSuppression: () => setSuppressed(false) };
|
||||
}
|
||||
|
||||
const lastModelConfigIDStorageKey = "agents.last-model-config-id";
|
||||
|
||||
const AGENT_BINDING_REPAIR_POLL_MS = 30_000;
|
||||
@@ -857,17 +881,20 @@ const AgentChatPage: FC = () => {
|
||||
// Right panel open/closed state is owned here so the loading
|
||||
// skeleton and the loaded view share the same layout, preventing
|
||||
// a horizontal shift when data arrives.
|
||||
const [showSidebarPanel, setShowSidebarPanel] = useState(() => {
|
||||
const [sidebarPanelPreference, setSidebarPanelPreference] = useState(() => {
|
||||
return localStorage.getItem(RIGHT_PANEL_OPEN_KEY) === "true";
|
||||
});
|
||||
const handleSetShowSidebarPanel = (
|
||||
next: boolean | ((prev: boolean) => boolean),
|
||||
) => {
|
||||
setShowSidebarPanel((prev) => {
|
||||
const value = typeof next === "function" ? next(prev) : next;
|
||||
localStorage.setItem(RIGHT_PANEL_OPEN_KEY, String(value));
|
||||
return value;
|
||||
});
|
||||
const { suppressed: panelSuppressedOnNarrow, clearSuppression } =
|
||||
useRightPanelNarrowSuppression();
|
||||
// Canonical panel visibility: the persisted preference gated by the
|
||||
// narrow-viewport suppression. Only this derived value may be
|
||||
// rendered or handed to children; the raw preference stays local.
|
||||
const showSidebarPanel = sidebarPanelPreference && !panelSuppressedOnNarrow;
|
||||
|
||||
const handleSetShowSidebarPanel = (next: boolean) => {
|
||||
clearSuppression();
|
||||
setSidebarPanelPreference(next);
|
||||
localStorage.setItem(RIGHT_PANEL_OPEN_KEY, String(next));
|
||||
};
|
||||
|
||||
const chatQuery = useQuery({
|
||||
|
||||
@@ -167,7 +167,7 @@ interface AgentChatPageViewProps {
|
||||
// Right panel state (owned by the parent so loading and
|
||||
// loaded views share the same layout).
|
||||
showSidebarPanel: boolean;
|
||||
onSetShowSidebarPanel: (next: boolean | ((prev: boolean) => boolean)) => void;
|
||||
onSetShowSidebarPanel: (next: boolean) => void;
|
||||
|
||||
// Sidebar content data.
|
||||
prNumber: number | undefined;
|
||||
@@ -429,7 +429,11 @@ export const AgentChatPageView: FC<AgentChatPageViewProps> = ({
|
||||
const [dragVisualExpanded, setDragVisualExpanded] = useState<boolean | null>(
|
||||
null,
|
||||
);
|
||||
const visualExpanded = dragVisualExpanded ?? isRightPanelExpanded;
|
||||
// Expansion must never outlive the panel: when narrow-viewport
|
||||
// suppression or an explicit close hides the panel, gate expansion
|
||||
// off (rather than resetting it) so it is restored with the panel.
|
||||
const visualExpanded =
|
||||
showSidebarPanel && (dragVisualExpanded ?? isRightPanelExpanded);
|
||||
|
||||
const [sidebarTabId, setSidebarTabIdState] = useState<string | null>(() =>
|
||||
getPersistedSidebarTabId(agentId),
|
||||
@@ -872,7 +876,8 @@ export const AgentChatPageView: FC<AgentChatPageViewProps> = ({
|
||||
parentChat={parentChat}
|
||||
panel={{
|
||||
showSidebarPanel,
|
||||
onToggleSidebar: () => onSetShowSidebarPanel((prev) => !prev),
|
||||
onToggleSidebar: () =>
|
||||
onSetShowSidebarPanel(!showSidebarPanel),
|
||||
}}
|
||||
onArchiveAgent={handleArchiveAgentAction}
|
||||
onUnarchiveAgent={handleUnarchiveAgentAction}
|
||||
@@ -1021,7 +1026,7 @@ export const AgentChatPageView: FC<AgentChatPageViewProps> = ({
|
||||
</div>
|
||||
<RightPanel
|
||||
isOpen={shouldShowSidebar}
|
||||
isExpanded={isRightPanelExpanded}
|
||||
isExpanded={showSidebarPanel && isRightPanelExpanded}
|
||||
onToggleExpanded={() => setIsRightPanelExpanded((prev) => !prev)}
|
||||
onClose={() => onSetShowSidebarPanel(false)}
|
||||
onVisualExpandedChange={setDragVisualExpanded}
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
/**
|
||||
* Replaces `window.matchMedia` with a controllable stub for tests and
|
||||
* stories. Queries listed in `initialMatches` report their configured
|
||||
* value; every other query delegates to the real `matchMedia` (or
|
||||
* reports `false` where none exists, e.g. jsdom) so unrelated
|
||||
* responsive components keep behaving truthfully. `setMatches` updates
|
||||
* a query and notifies its registered change listeners; `restore` puts
|
||||
* the original `window.matchMedia` back.
|
||||
*/
|
||||
export const setupMatchMedia = (initialMatches: Record<string, boolean>) => {
|
||||
const matches = { ...initialMatches };
|
||||
const listeners = new Map<string, Set<EventListenerOrEventListenerObject>>();
|
||||
const listenersFor = (query: string) => {
|
||||
let set = listeners.get(query);
|
||||
if (!set) {
|
||||
set = new Set();
|
||||
listeners.set(query, set);
|
||||
}
|
||||
return set;
|
||||
};
|
||||
const original = window.matchMedia;
|
||||
const originalFn =
|
||||
typeof original === "function" ? original.bind(window) : undefined;
|
||||
Object.defineProperty(window, "matchMedia", {
|
||||
configurable: true,
|
||||
writable: true,
|
||||
value: (query: string): MediaQueryList => {
|
||||
if (!(query in matches) && originalFn) {
|
||||
return originalFn(query);
|
||||
}
|
||||
return {
|
||||
get matches() {
|
||||
return matches[query] ?? false;
|
||||
},
|
||||
media: query,
|
||||
onchange: null,
|
||||
addEventListener: (
|
||||
_type: string,
|
||||
listener: EventListenerOrEventListenerObject,
|
||||
) => {
|
||||
listenersFor(query).add(listener);
|
||||
},
|
||||
removeEventListener: (
|
||||
_type: string,
|
||||
listener: EventListenerOrEventListenerObject,
|
||||
) => {
|
||||
listenersFor(query).delete(listener);
|
||||
},
|
||||
dispatchEvent: () => true,
|
||||
addListener: () => {},
|
||||
removeListener: () => {},
|
||||
} satisfies MediaQueryList;
|
||||
},
|
||||
});
|
||||
return {
|
||||
setMatches: (query: string, value: boolean) => {
|
||||
matches[query] = value;
|
||||
const event = new Event("change");
|
||||
for (const listener of listenersFor(query)) {
|
||||
if (typeof listener === "function") {
|
||||
listener(event);
|
||||
} else {
|
||||
listener.handleEvent(event);
|
||||
}
|
||||
}
|
||||
},
|
||||
restore: () => {
|
||||
Object.defineProperty(window, "matchMedia", {
|
||||
configurable: true,
|
||||
writable: true,
|
||||
value: original,
|
||||
});
|
||||
},
|
||||
};
|
||||
};
|
||||
@@ -8,6 +8,19 @@ export const isMobileViewport = (): boolean => {
|
||||
return window.matchMedia("(max-width: 639px)").matches;
|
||||
};
|
||||
|
||||
/**
|
||||
* Builds a `useSyncExternalStore` subscribe function that notifies on
|
||||
* changes to the given media query, so every viewport hook shares one
|
||||
* listener lifecycle implementation.
|
||||
*/
|
||||
export const createMediaQuerySubscribe =
|
||||
(query: string) =>
|
||||
(onStoreChange: () => void): (() => void) => {
|
||||
const mediaQuery = window.matchMedia(query);
|
||||
mediaQuery.addEventListener("change", onStoreChange);
|
||||
return () => mediaQuery.removeEventListener("change", onStoreChange);
|
||||
};
|
||||
|
||||
export const belowMdViewportMediaQuery = "(max-width: 767px)";
|
||||
|
||||
/**
|
||||
@@ -21,3 +34,16 @@ export const belowMdViewportMediaQuery = "(max-width: 767px)";
|
||||
export const isBelowMdViewport = (): boolean => {
|
||||
return window.matchMedia(belowMdViewportMediaQuery).matches;
|
||||
};
|
||||
|
||||
export const belowLgViewportMediaQuery = "(max-width: 1023px)";
|
||||
|
||||
/**
|
||||
* Returns `true` when the viewport width is below the `lg` Tailwind
|
||||
* breakpoint (< 1024 px). Use this to align with `lg:` Tailwind
|
||||
* utilities that switch between a side-by-side layout and a
|
||||
* single-panel-at-a-time layout (e.g. the Agents chat page's chat vs.
|
||||
* right panel split).
|
||||
*/
|
||||
export const isBelowLgViewport = (): boolean => {
|
||||
return window.matchMedia(belowLgViewportMediaQuery).matches;
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user