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
5 changes: 4 additions & 1 deletion src/browser/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1251,7 +1251,10 @@ function AppInner() {

return (
<>
<div className="bg-surface-primary mobile-layout flex h-full overflow-hidden pt-[env(safe-area-inset-top)] pr-[env(safe-area-inset-right)] pb-[min(env(safe-area-inset-bottom,0px),40px)] pl-[env(safe-area-inset-left)]">
{/* mobile-bottom-inset-host: narrow layouts give this box's bottom inset up to a column that
reserves its own clearance, keyed off the column rather than the route so surfaces without
one still get it here. The sidebar is position: fixed with its own inset at these widths. */}
<div className="bg-surface-primary mobile-layout mobile-bottom-inset-host flex h-full overflow-hidden pt-[env(safe-area-inset-top)] pr-[env(safe-area-inset-right)] pb-[min(env(safe-area-inset-bottom,0px),40px)] pl-[env(safe-area-inset-left)]">
<LeftSidebar
collapsed={sidebarCollapsed}
onToggleCollapsed={handleToggleSidebar}
Expand Down
4 changes: 4 additions & 0 deletions src/browser/components/ChatPane/ChatPane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,10 @@ export const ChatPane: React.FC<ChatPaneProps> = (props) => {
<div
ref={chatAreaRef}
aria-hidden={immersiveHidden || undefined}
// Tells the app root that this column ends in a row (WorkspaceFooterBar) which reserves the
// bottom safe-area inset itself. Dropped while hidden so the inset stays with the root for
// whatever replaces the column.
data-bottom-inset-owner={immersiveHidden ? undefined : true}
className={cn(
"bg-surface-primary relative flex min-w-96 flex-1 flex-col",
// Immersive review overlays the entire workspace, so hiding the chat pane removes
Expand Down
13 changes: 9 additions & 4 deletions src/browser/components/ChatPane/WorkspaceFooterBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,12 @@ export const WorkspaceFooterBar: React.FC<WorkspaceFooterBarProps> = (props) =>
: null;

return (
// As the chat column's last row, this bar owns the mobile bottom inset: padding clears the
// home indicator, and the negative margin lets its fill reach the screen edge.
// Narrow layouts drop the app root's bottom inset, so this row owns it: a capped clearance for
// the home indicator instead of the full inset, whose reserved band is the empty space we are
// reclaiming. Wider layouts still take their clearance from the root.
<footer
data-testid="workspace-footer-bar"
className="bg-sidebar border-border-light mb-[calc(-1*min(env(safe-area-inset-bottom,0px),40px))] shrink-0 border-t pb-[min(env(safe-area-inset-bottom,0px),40px)]"
className="bg-sidebar border-border-light shrink-0 border-t [@media(max-width:768px)]:pb-[min(env(safe-area-inset-bottom,0px),8px)]"
Comment thread
ibetitsmike marked this conversation as resolved.
Comment thread
ibetitsmike marked this conversation as resolved.
Comment thread
ibetitsmike marked this conversation as resolved.
>
{/* min-h rather than a fixed height: mobile raises these buttons to 44px touch targets, and a
capped row would clip them along the same axis overflow-x-auto makes scrollable. */}
Expand All @@ -296,7 +297,11 @@ export const WorkspaceFooterBar: React.FC<WorkspaceFooterBarProps> = (props) =>
workspaceName={props.workspaceName}
tooltipSide="top"
/>
<WorkspaceLinks workspaceId={props.workspaceId} />
{/* Narrow layouts show it in the header, where it cannot scroll out of view with this row. */}
<WorkspaceLinks
workspaceId={props.workspaceId}
className="[@media(max-width:768px)]:hidden"
/>
{hasRepository && (
<>
<WorkspaceDriftIndicator
Expand Down
14 changes: 7 additions & 7 deletions src/browser/components/PRLinkBadge/PRLinkBadge.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
/**
* PR link badge component for displaying GitHub PR status in header.
*/

import {
ExternalLink,
GitPullRequest,
Expand All @@ -20,6 +16,7 @@ import { cn } from "@/common/lib/utils";
interface PRLinkBadgeProps {
prLink: GitHubPRLinkWithStatus;
onRefresh?: () => void;
className?: string;
}

/**
Expand Down Expand Up @@ -180,7 +177,7 @@ export function getTooltipContent(prLink: GitHubPRLinkWithStatus): string {
return lines.join("\n");
}

export function PRLinkBadge({ prLink }: PRLinkBadgeProps) {
export function PRLinkBadge({ prLink, className }: PRLinkBadgeProps) {
const colorClass = getStatusColorClass(prLink);
// Show pulse effect when refreshing with cached status (optimistic UI)
const isRefreshing = prLink.loading && prLink.status != null;
Expand All @@ -192,9 +189,12 @@ export function PRLinkBadge({ prLink }: PRLinkBadgeProps) {
variant="ghost"
size="sm"
className={cn(
"h-6 gap-1 px-2 text-xs font-medium",
// shrink-0 because the mobile touch-target rule puts an explicit `min-width` on links,
// replacing the flex min-content floor and letting the label spill over its neighbour.
"h-6 shrink-0 gap-1 px-2 text-xs font-medium",
colorClass,
isRefreshing && "animate-pulse"
isRefreshing && "animate-pulse",
className
)}
asChild
>
Expand Down
11 changes: 5 additions & 6 deletions src/browser/components/WorkspaceLinks/WorkspaceLinks.tsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
/**
* Component to display the PR badge in the workspace header.
* PR is detected from the workspace's current branch via `gh pr view`.
*/
/** The PR is detected from the workspace's current branch via `gh pr view`. */

import { useWorkspacePR } from "@/browser/stores/PRStatusStore";
import { PRLinkBadge } from "../PRLinkBadge/PRLinkBadge";

interface WorkspaceLinksProps {
workspaceId: string;
/** Applied to the badge itself so callers can hide it without leaving an empty flex item. */
className?: string;
}

export function WorkspaceLinks({ workspaceId }: WorkspaceLinksProps) {
export function WorkspaceLinks({ workspaceId, className }: WorkspaceLinksProps) {
const workspacePR = useWorkspacePR(workspaceId);

if (!workspacePR) {
return null;
}

return <PRLinkBadge prLink={workspacePR} />;
return <PRLinkBadge prLink={workspacePR} className={className} />;
}
6 changes: 6 additions & 0 deletions src/browser/components/WorkspaceMenuBar/WorkspaceMenuBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { WorkspaceActionsMenuContent } from "../WorkspaceActionsMenuContent/Work
import { WorkspaceTerminalIcon } from "../icons/WorkspaceTerminalIcon/WorkspaceTerminalIcon";

import { SkillIndicator } from "../SkillIndicator/SkillIndicator";
import { WorkspaceLinks } from "../WorkspaceLinks/WorkspaceLinks";
import { useAPI } from "@/browser/contexts/API";
import { useAgent } from "@/browser/contexts/AgentContext";

Expand Down Expand Up @@ -556,6 +557,11 @@ export const WorkspaceMenuBar: React.FC<WorkspaceMenuBarProps> = ({
</Popover>
</div>
<div className={cn("flex items-center gap-2", isDesktop && "titlebar-no-drag")}>
{/* The footer info bar hides its PR badge at this width, so the header carries it there. */}
<WorkspaceLinks
workspaceId={workspaceId}
className="hidden [@media(max-width:768px)]:inline-flex"
/>
<Popover open={notificationPopoverOpen} onOpenChange={setNotificationPopoverOpen}>
<Tooltip {...(notificationPopoverOpen ? { open: false } : {})}>
<TooltipTrigger asChild>
Expand Down
89 changes: 88 additions & 1 deletion src/browser/stories/App.phoneViewports.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { CUSTOM_EVENTS, createCustomEvent } from "@/common/constants/events";

import { updatePersistedState } from "@/browser/hooks/usePersistedState";
import { LEFT_SIDEBAR_COLLAPSED_KEY } from "@/common/constants/storage";
import { MOBILE_TOUCH_TARGET_PX } from "@/constants/layout";
import { MOBILE_TOUCH_TARGET_PX, NARROW_VIEWPORT_MAX_WIDTH_PX } from "@/constants/layout";

import { appMeta, AppWithMocks, PIXEL_DISABLED, type AppStory } from "./meta.js";
import { createAssistantMessage, createUserMessage } from "./mocks/messages";
Expand Down Expand Up @@ -103,6 +103,28 @@ index 1111111..2222222 100644
`;
const TOUCH_REVIEW_IMMERSIVE_NUMSTAT = "2\t0\tsrc/mobile/review.tsx";

const PR_LINK_URL = "https://github.com/coder/mux/pull/3753";
const PR_DETECTION_JSON = JSON.stringify({
number: 3753,
url: PR_LINK_URL,
state: "OPEN",
mergeable: "MERGEABLE",
mergeStateStatus: "CLEAN",
title: "Redesign the workspace chrome",
isDraft: false,
headRefName: "feature/mobile-chrome",
baseRefName: "main",
statusCheckRollup: [],
});

function countVisiblePRLinks(containerTestId: string): number {
return [
...document.querySelectorAll<HTMLElement>(
`[data-testid="${containerTestId}"] a[href="${PR_LINK_URL}"]`
),
].filter((link) => link.getBoundingClientRect().width > 0).length;
}

export default {
...appMeta,
title: "App/PhoneViewports",
Expand Down Expand Up @@ -140,6 +162,66 @@ export const IPhone16e: AppStory = {
},
};

/**
* The PR badge lives in the footer info bar on wide viewports and in the workspace header on narrow
* ones. Pixel captures the narrow placement; the play assertion covers whichever side the ambient
* viewport selects, so the test-runner exercises the wide placement.
*/
export const IPhone16ePRLinkPlacement: AppStory = {
// Mirrors the Pixel phone variant: the fixed-width decorator does not move `window.innerWidth`, so
// without this a local reviewer would see the wide placement in a story framed as a phone.
globals: {
viewport: { value: "mobile2", isRotated: false },
},
render: () => (
<AppWithMocks
setup={() =>
setupSimpleChatStory({
workspaceId: "ws-iphone-16e-pr-link",
workspaceName: "mobile-pr",
projectName: "mux",
messages: [...MESSAGES],
executeBash: (_workspaceId, script) =>
Promise.resolve({
success: true as const,
// Empty output for anything else falls through to the git status executor.
output: script.includes("gh pr view") ? PR_DETECTION_JSON : "",
exitCode: 0,
wall_duration_ms: 5,
}),
})
}
/>
),
decorators: [IPhone16eDecorator],
parameters: {
...appMeta.parameters,
pixel: {
matrix: { themes: ["dark", "light"], viewports: ["phone"] },
},
Comment thread
ibetitsmike marked this conversation as resolved.
},
play: async ({ canvasElement }) => {
await stabilizePhoneViewportStory(canvasElement);

const narrow = window.matchMedia(`(max-width: ${NARROW_VIEWPORT_MAX_WIDTH_PX}px)`).matches;
await waitFor(
() => {
const inHeader = countVisiblePRLinks("workspace-menu-bar");
const inFooter = countVisiblePRLinks("workspace-footer-bar");
const [expectedHeader, expectedFooter] = narrow ? [1, 0] : [0, 1];
if (inHeader !== expectedHeader || inFooter !== expectedFooter) {
throw new Error(
`At ${window.innerWidth}px the PR link belongs ${
narrow ? "in the header" : "in the footer"
}, but ${inHeader} were visible in the header and ${inFooter} in the footer`
);
}
},
{ timeout: 10_000 }
);
},
};

export const IPhone16eAnalyticsSidebarControl: AppStory = {
globals: {
viewport: { value: "mobile1", isRotated: false },
Expand Down Expand Up @@ -327,6 +409,11 @@ export const IPhone17ProMaxTouchReviewImmersive: AppStory = {
if (canvas.queryByRole("heading", { name: "Notes" })) {
throw new Error("Touch immersive mode should hide the desktop notes sidebar.");
}
// The chat column is hidden here, so it must not keep claiming the bottom safe-area inset
// the app root would otherwise reserve for this view's own scroll area.
if (document.querySelectorAll("[data-bottom-inset-owner]").length > 0) {
throw new Error("Immersive review left the bottom safe-area inset unowned.");
}
},
{ timeout: 10_000 }
);
Expand Down
8 changes: 8 additions & 0 deletions src/browser/styles/globals.css
Original file line number Diff line number Diff line change
Expand Up @@ -1203,6 +1203,14 @@ body,
padding-left: 0 !important;
}

/* Hand the bottom inset to a column that reserves its own clearance (see ChatPane and
WorkspaceFooterBar). This box clips overflow, so a child can never paint into its padding, and
the reserved band reads as dead space under the footer bar. Surfaces without such a column
(immersive review, settings, creation, loading shells) keep the inset here. */
.mobile-bottom-inset-host:has([data-bottom-inset-owner]) {
padding-bottom: 0 !important;
}

.mobile-overlay {
display: block !important;
}
Expand Down
5 changes: 5 additions & 0 deletions src/constants/layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ export const CREATION_COLUMN_MAX_WIDTH_CLASS = "max-w-[67rem]";
// reproduce that environment, which Storybook and Pixel cannot: neither emulates `pointer: coarse`.
export const MOBILE_TOUCH_TARGET_PX = 44;

// Width at or below which the app switches to its narrow layout (overlaying sidebar, PR badge in the
// workspace header instead of the footer info bar). Tailwind arbitrary variants cannot read a TS
// constant, so `[@media(max-width:768px)]` class strings repeat this literal.
export const NARROW_VIEWPORT_MAX_WIDTH_PX = 768;

// Keep composer controls aligned without relying on individual component defaults. This is a floor,
// not a fixed height: mobile raises touch targets to MOBILE_TOUCH_TARGET_PX, and a pill that caps
// its height would clip the controls inside it instead of growing with them.
Expand Down
Loading