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 .agents/upstream-review.md

Large diffs are not rendered by default.

57 changes: 41 additions & 16 deletions apps/web/src/components/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1879,6 +1879,8 @@ export default function ChatView(props: ChatViewProps) {
const [composerTimelineInset, setComposerTimelineInset] = useState(0);
const composerTimelineInsetRef = useRef(0);
const composerRestingRef = useRef(false);
// The last overlay height the composer published for its settled layout.
const composerOverlayHeightRef = useRef(0);
const [scrollToEndClearance, setScrollToEndClearance] = useState(0);
const isAtEndRef = useRef(true);
const isTimelineAtLogicalEnd = useCallback(
Expand Down Expand Up @@ -2295,6 +2297,7 @@ export default function ChatView(props: ChatViewProps) {
return {
id: `project-clone:${projectId}`,
variant: "info",
compact: true,
priority: "activity",
icon: <DownloadIcon />,
title: `Cloning ${name}`,
Expand All @@ -2318,6 +2321,7 @@ export default function ChatView(props: ChatViewProps) {
return {
id: `project-clone:${projectId}`,
variant: cancelled ? "warning" : "error",
compact: true,
icon: <DownloadIcon />,
title: cancelled ? `Cancelled cloning ${name}` : `Failed to clone ${name}`,
description: cancelled ? "Retry to bring in the repository." : activeProjectClone.error,
Expand Down Expand Up @@ -2781,6 +2785,7 @@ export default function ChatView(props: ChatViewProps) {
),
title: `${unavailableConnection.phase === "connecting" ? "Connecting" : "Reconnecting"} to ${activeEnvironmentUnavailableState.label}`,
description: "Finishing an update",
compact: true,
});
} else {
items.push({
Expand Down Expand Up @@ -6070,19 +6075,8 @@ export default function ChatView(props: ChatViewProps) {
[activeComposerPlan?.turnId, activeComposerTaskSteps, agentSessionLive, threadActivities],
);

const publishComposerOverlayHeight = useCallback(
(height: number) => {
const nextHeight = Math.ceil(height);
if (nextHeight <= 0) return;
const nextInset = resolveComposerTimelineInset({
currentInset: composerTimelineInsetRef.current,
overlayHeight: nextHeight,
isResting: composerRestingRef.current,
});
if (composerTimelineInsetRef.current !== nextInset) {
composerTimelineInsetRef.current = nextInset;
setComposerTimelineInset(nextInset);
}
const publishScrollToEndClearance = useCallback(
(overlayHeight: number) => {
const mainSurface = composerOverlayElement?.querySelector<HTMLElement>(
'[data-chat-composer-main-surface="true"]',
);
Expand All @@ -6092,7 +6086,7 @@ export default function ChatView(props: ChatViewProps) {
const clearance =
composerOverlayElement && mainSurface && button
? resolveScrollToEndClearance({
overlayHeight: nextHeight,
overlayHeight,
mainSurfaceTop: mainSurface.getBoundingClientRect().top,
button: button.getBoundingClientRect(),
attachments: Array.from(
Expand All @@ -6102,11 +6096,29 @@ export default function ChatView(props: ChatViewProps) {
(element) => element.getBoundingClientRect(),
),
})
: nextHeight;
: overlayHeight;
setScrollToEndClearance(clearance);
},
[composerOverlayElement],
);
const publishComposerOverlayHeight = useCallback(
(height: number) => {
const nextHeight = Math.ceil(height);
if (nextHeight <= 0) return;
composerOverlayHeightRef.current = nextHeight;
const nextInset = resolveComposerTimelineInset({
currentInset: composerTimelineInsetRef.current,
overlayHeight: nextHeight,
isResting: composerRestingRef.current,
});
if (composerTimelineInsetRef.current !== nextInset) {
composerTimelineInsetRef.current = nextInset;
setComposerTimelineInset(nextInset);
}
publishScrollToEndClearance(nextHeight);
},
[publishScrollToEndClearance],
);
// The composer reports its resting flag from a layout effect, which runs
// before this component's own layout effects and before any resize
// observation, so every measurement below sees the flag for its layout.
Expand Down Expand Up @@ -6139,7 +6151,17 @@ export default function ChatView(props: ChatViewProps) {
return () => {
resizeObserver.disconnect();
};
}, [composerOverlayElement, publishComposerOverlayHeight, showScrollToBottom]);
}, [composerOverlayElement, publishComposerOverlayHeight]);
// The pill mounts and unmounts in the same commits that expand or rest the
// composer, and a fast fling lands there while the previous resting tween
// still pins the overlay at its old height. Measuring the overlay here would
// publish that stale height against the new resting flag, drop the timeline
// reservation, and yank the scroll position. The pill only needs its
// clearance, so it reuses the height the composer last published.
useLayoutEffect(() => {
if (!composerOverlayElement) return;
publishScrollToEndClearance(composerOverlayHeightRef.current);
}, [composerOverlayElement, publishScrollToEndClearance, showScrollToBottom]);
const linkedPullRequestStatus = useLinkedThreadPullRequest(
activeThreadRef?.environmentId ?? null,
activeThreadMetadata?.linkedPullRequest,
Expand Down Expand Up @@ -6655,6 +6677,7 @@ export default function ChatView(props: ChatViewProps) {
icon: <AlarmClockIcon />,
title: "Thread woke from snooze",
description: "Send a message to continue",
compact: true,
dismissLabel: "Dismiss Woke notification",
onDismiss: acknowledgeActiveThreadWoke,
};
Expand All @@ -6670,6 +6693,7 @@ export default function ChatView(props: ChatViewProps) {
icon: isSnoozed ? <AlarmClockIcon /> : <CheckCircle2Icon />,
title: `This thread is ${isSnoozed ? "snoozed" : "settled"}`,
description: `Send a message to ${isSnoozed ? "wake" : "unsettle"}`,
compact: true,
actions: (
<Button
size="xs"
Expand Down Expand Up @@ -6776,6 +6800,7 @@ export default function ChatView(props: ChatViewProps) {
icon: <Minimize2Icon />,
title: "Resume with less context",
description: `${formatContextWindowTokens(activeContextWindow.usedTokens)} tokens from earlier`,
compact: true,
actions: compactDisabledReason ? (
<Tooltip>
<TooltipTrigger render={<span className="inline-flex">{compactAction}</span>} />
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/components/chat/ComposerBanner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ function Actions({ className, ...props }: ComponentProps<"span">) {
data-slot="composer-banner-actions"
className={cn(
"col-start-3 row-start-1 flex flex-wrap items-center justify-end gap-1",
"@max-[400px]:group-data-[composer-banner-layout=wrap-actions]/banner-row:has-[>:nth-child(2)]:col-start-2 @max-[400px]:group-data-[composer-banner-layout=wrap-actions]/banner-row:has-[>:nth-child(2)]:col-end-4 @max-[400px]:group-data-[composer-banner-layout=wrap-actions]/banner-row:has-[>:nth-child(2)]:row-start-2 @max-[400px]:group-data-[composer-banner-layout=wrap-actions]/banner-row:has-[>:nth-child(2)]:-ms-2 @max-[400px]:group-data-[composer-banner-layout=wrap-actions]/banner-row:has-[>:nth-child(2)]:justify-start",
"@max-[400px]:group-data-[composer-banner-layout=wrap-actions]/banner-row:has-[>:nth-child(2)]:col-start-2 @max-[400px]:group-data-[composer-banner-layout=wrap-actions]/banner-row:has-[>:nth-child(2)]:col-end-4 @max-[400px]:group-data-[composer-banner-layout=wrap-actions]/banner-row:has-[>:nth-child(2)]:row-start-2 @max-[400px]:group-data-[composer-banner-layout=wrap-actions]/banner-row:has-[>:nth-child(2)]:justify-end",
"@max-[320px]:group-data-[composer-banner-layout=wrap-actions-narrow]/banner-row:has-[>:nth-child(2)]:col-start-2 @max-[320px]:group-data-[composer-banner-layout=wrap-actions-narrow]/banner-row:has-[>:nth-child(2)]:col-end-4 @max-[320px]:group-data-[composer-banner-layout=wrap-actions-narrow]/banner-row:has-[>:nth-child(2)]:row-start-2 @max-[320px]:group-data-[composer-banner-layout=wrap-actions-narrow]/banner-row:has-[>:nth-child(2)]:-ms-2 @max-[320px]:group-data-[composer-banner-layout=wrap-actions-narrow]/banner-row:has-[>:nth-child(2)]:justify-start",
className,
)}
Expand Down
34 changes: 34 additions & 0 deletions apps/web/src/components/chat/ComposerBannerStack.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,38 @@ describe("ComposerBannerStack", () => {

expect(markup.indexOf("liveness")).toBeLessThan(markup.indexOf("composer-activity"));
});

it("keeps the details popover out of a compact banner until the row is narrow", () => {
// A compact banner's description is short enough to stay on screen, so the
// popover trigger only appears once the row is too narrow to show it.
const markup = renderToStaticMarkup(
<ComposerBannerStack
items={[item("clone", { description: "Finishing an update", compact: true })]}
/>,
);

expect(markup).toContain('data-composer-banner-layout="wrap-actions-narrow"');
expect(markup).toContain("@max-[400px]:inline-flex");
});

it("always offers the details popover on a banner that can truncate", () => {
const markup = renderToStaticMarkup(
<ComposerBannerStack
items={[
item("notice", {
description: "A description long enough to truncate on a narrow composer",
}),
]}
/>,
);

expect(markup).toContain('data-composer-banner-layout="wrap-actions"');
expect(markup).not.toContain("@max-[400px]:inline-flex");
});

it("renders no details popover when a banner has no description", () => {
const markup = renderToStaticMarkup(<ComposerBannerStack items={[item("bare")]} />);

expect(markup).not.toContain("Show notice details");
});
});
35 changes: 20 additions & 15 deletions apps/web/src/components/chat/ComposerBannerStack.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export interface ComposerBannerStackItem {
readonly id: string;
readonly variant: ComposerBannerVariant;
readonly priority?: "urgent" | "activity" | "notice";
readonly compact?: boolean;
readonly icon: ReactNode;
readonly title: ReactNode;
readonly description?: ReactNode;
Expand Down Expand Up @@ -270,22 +271,20 @@ function ComposerBannerStackAlert({
variant={item.variant}
density="comfortable"
>
<ComposerBanner.Row layout="wrap-actions-narrow">
<ComposerBanner.Row layout={item.compact ? "wrap-actions-narrow" : "wrap-actions"}>
<ComposerBanner.Icon className="h-(--composer-banner-icon-column) self-start">
{item.icon}
</ComposerBanner.Icon>
<ComposerBanner.Content className="whitespace-nowrap">
<span
className={cn(
"min-w-0 font-medium leading-7 sm:leading-6",
typeof item.title === "string" && "truncate",
)}
>
{item.title}
</span>
<span className="min-w-0 truncate font-medium leading-7 sm:leading-6">{item.title}</span>
{item.description ? (
<>
<span className="min-w-0 shrink-[9999] truncate text-muted-foreground @max-[400px]:sr-only">
<span className={item.compact ? "contents" : "flex min-w-8 flex-1 items-center gap-1"}>
<span
className={cn(
"min-w-0 truncate text-muted-foreground",
item.compact && "shrink-[9999] @max-[400px]:sr-only",
)}
>
{item.description}
</span>
<Popover>
Expand All @@ -296,21 +295,27 @@ function ComposerBannerStackAlert({
size="icon-xs"
variant="ghost"
aria-label="Show notice details"
className="hidden flex-none text-muted-foreground hover:text-foreground @max-[400px]:inline-flex"
className={cn(
"flex-none text-muted-foreground hover:text-foreground",
item.compact && "hidden @max-[400px]:inline-flex",
)}
/>
}
>
<InfoIcon className="size-3.5" />
</PopoverTrigger>
<PopoverPopup
aria-label="Notice details"
tooltipStyle
side="top"
className="max-w-72 whitespace-normal text-pretty"
className="max-w-80 whitespace-normal text-pretty wrap-anywhere"
>
{item.description}
<ComposerBanner.Scroll className="max-h-[min(var(--available-height),24rem,40dvh)]">
{item.description}
</ComposerBanner.Scroll>
</PopoverPopup>
</Popover>
</>
</span>
) : null}
</ComposerBanner.Content>
{item.actions || item.onDismiss ? (
Expand Down
122 changes: 63 additions & 59 deletions apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -222,66 +222,70 @@ const ComposerPendingUserInputCard = memo(function ComposerPendingUserInputCard(
) : null}
</div>
<CollapsiblePanel>
<ComposerBanner.Body className="pe-1 pb-1">
<p className="text-sm text-foreground/85">{activeQuestion.question}</p>
{activeQuestion.multiSelect ? (
<p className="mt-1 text-secondary-label text-xs">Select one or more options.</p>
) : null}
<div className="mt-2 space-y-0.5">
{activeQuestion.options.map((option, index) => {
const optionValue = option.value ?? option.label;
const isOptimisticallySelected =
optimisticSingleSelect?.questionId === activeQuestion.id &&
optimisticSingleSelect.optionValue === optionValue;
const isSelected =
isOptimisticallySelected ||
(!customAnswerActive && progress.selectedOptionValues.includes(optionValue));
const shortcutKey = index < 9 ? index + 1 : null;
const className = cn(
"group flex w-full items-center gap-2 rounded-md px-2.5 py-2 text-left outline-none transition-colors duration-150 focus-visible:ring-1 focus-visible:ring-primary/25",
isSelected
? "bg-muted/55 text-foreground"
: "bg-transparent text-foreground/85 hover:bg-muted/30",
isResponding && "opacity-50 cursor-not-allowed",
!isResponding && "cursor-pointer",
);
const content = (
<>
<div className="min-w-0 flex-1 flex flex-col gap-0.5">
<span className="text-sm font-medium">{option.label}</span>
{option.description && option.description !== option.label ? (
<span className="text-secondary-label text-[11px]">{option.description}</span>
<ComposerBanner.Scroll>
<ComposerBanner.Body className="pe-1 pb-1 wrap-anywhere">
<p className="text-sm text-foreground/85">{activeQuestion.question}</p>
{activeQuestion.multiSelect ? (
<p className="mt-1 text-secondary-label text-xs">Select one or more options.</p>
) : null}
<div className="mt-2 space-y-0.5">
{activeQuestion.options.map((option, index) => {
const optionValue = option.value ?? option.label;
const isOptimisticallySelected =
optimisticSingleSelect?.questionId === activeQuestion.id &&
optimisticSingleSelect.optionValue === optionValue;
const isSelected =
isOptimisticallySelected ||
(!customAnswerActive && progress.selectedOptionValues.includes(optionValue));
const shortcutKey = index < 9 ? index + 1 : null;
const className = cn(
"group flex w-full items-center gap-2 rounded-md px-2.5 py-2 text-left outline-none transition-colors duration-150 focus-visible:ring-1 focus-visible:ring-primary/25",
isSelected
? "bg-muted/55 text-foreground"
: "bg-transparent text-foreground/85 hover:bg-muted/30",
isResponding && "opacity-50 cursor-not-allowed",
!isResponding && "cursor-pointer",
);
const content = (
<>
<div className="min-w-0 flex-1 flex flex-col gap-0.5">
<span className="text-sm font-medium">{option.label}</span>
{option.description && option.description !== option.label ? (
<span className="text-secondary-label text-[11px]">
{option.description}
</span>
) : null}
</div>
{isSelected ? (
<CheckIcon className="size-3.5 shrink-0 text-primary" />
) : shortcutKey !== null ? (
<kbd
className={cn(
"flex size-5 shrink-0 items-center justify-center text-[10px] font-medium text-muted-foreground tabular-nums",
)}
>
{shortcutKey}
</kbd>
) : null}
</div>
{isSelected ? (
<CheckIcon className="size-3.5 shrink-0 text-primary" />
) : shortcutKey !== null ? (
<kbd
className={cn(
"flex size-5 shrink-0 items-center justify-center text-[10px] font-medium text-muted-foreground tabular-nums",
)}
>
{shortcutKey}
</kbd>
) : null}
</>
);
return (
<button
key={`${activeQuestion.id}:${optionValue}`}
type="button"
disabled={isResponding}
onClick={() => {
handleOptionSelection(activeQuestion.id, optionValue);
}}
className={className}
>
{content}
</button>
);
})}
</div>
</ComposerBanner.Body>
</>
);
return (
<button
key={`${activeQuestion.id}:${optionValue}`}
type="button"
disabled={isResponding}
onClick={() => {
handleOptionSelection(activeQuestion.id, optionValue);
}}
className={className}
>
{content}
</button>
);
})}
</div>
</ComposerBanner.Body>
</ComposerBanner.Scroll>
</CollapsiblePanel>
</Collapsible>
);
Expand Down
Loading
Loading