Skip to content
Closed
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
6 changes: 6 additions & 0 deletions .server-changes/esc-to-close-fixes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
area: webapp
type: fix
---

Esc now closes the authenticator app setup window and the chat panel, and side sheets now show a close button with their Esc shortcut. Esc in the chat panel's message box just leaves the box, so an unsent message survives and a second Esc closes the panel. Closing a dialog with Esc no longer also closes the panel behind it, and the Vercel setup modal no longer shows a close button that does nothing.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ export function DashboardAgentComposer({
e.preventDefault();
onSubmit();
}
// Esc leaves the composer, so a second Esc closes the panel and an
// unsent message survives the first one.
if (e.key === "Escape") {
e.currentTarget.blur();
}
}}
placeholder="Type a message…"
className={cn(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ClockIcon, PencilSquareIcon, XMarkIcon } from "@heroicons/react/20/solid";
import { Button } from "~/components/primitives/Buttons";
import { cn } from "~/utils/cn";

export function DashboardAgentHeader({
Expand All @@ -23,7 +24,15 @@ export function DashboardAgentHeader({
onClick={onToggleHistory}
active={view === "history"}
/>
<IconButton label="Close" icon={XMarkIcon} onClick={onClose} />
<Button
onClick={onClose}
variant="minimal/small"
TrailingIcon={XMarkIcon}
shortcut={{ key: "esc" }}
shortcutPosition="before-trailing-icon"
className="pl-1"
aria-label="Close"
/>
</div>
</div>
);
Expand Down
11 changes: 1 addition & 10 deletions apps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { useFetcher, useNavigate } from "@remix-run/react";
import { SlackIcon } from "@trigger.dev/companyicons";
import { Fragment, useEffect, useRef, useState } from "react";
import { z } from "zod";
import { ExitIcon } from "~/assets/icons/ExitIcon";
import { InlineCode } from "~/components/code/InlineCode";
import { Button, LinkButton } from "~/components/primitives/Buttons";
import { Callout, variantClasses } from "~/components/primitives/Callout";
Expand Down Expand Up @@ -116,18 +115,10 @@ export function ConfigureErrorAlerts({

return (
<div className="grid h-full grid-rows-[auto_1fr_auto] overflow-hidden">
<div className="flex items-center justify-between border-b border-grid-bright px-3 py-2">
<div className="flex items-center border-b border-grid-bright py-2 pl-3 pr-16">
<Header2 className="flex items-center gap-2">
<BellAlertIcon className="size-5 text-alerts" /> Configure alerts
</Header2>
<LinkButton
to={closeHref}
variant="minimal/small"
TrailingIcon={ExitIcon}
shortcut={{ key: "esc" }}
shortcutPosition="before-trailing-icon"
className="pl-1"
/>
</div>

<fetcher.Form method="post" action={formAction} {...getFormProps(form)} className="contents">
Expand Down
12 changes: 10 additions & 2 deletions apps/webapp/app/components/integrations/VercelOnboardingModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,11 @@ export function VercelOnboardingModal({
}
}}
>
<DialogContent className="max-w-lg" onInteractOutside={(e) => e.preventDefault()}>
<DialogContent
className="max-w-lg"
onInteractOutside={(e) => e.preventDefault()}
showCloseButton={!fromMarketplaceContext}
>
<DialogHeader>
<div className="flex items-center gap-2">
<VercelLogo className="size-5" />
Expand Down Expand Up @@ -787,7 +791,11 @@ export function VercelOnboardingModal({
}
}}
>
<DialogContent className="max-w-lg" onInteractOutside={(e) => e.preventDefault()}>
<DialogContent
className="max-w-lg"
onInteractOutside={(e) => e.preventDefault()}
showCloseButton={!fromMarketplaceContext}
>
<DialogHeader>
<div className="flex items-center gap-2">
<VercelLogo className="size-5" />
Expand Down
2 changes: 2 additions & 0 deletions apps/webapp/app/components/primitives/Buttons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ export const Button = forwardRef<HTMLButtonElement, ButtonPropsType>(
}
},
disabled: isDisabled || !props.shortcut,
elementRef: innerRef,
});

const buttonElement = (
Expand Down Expand Up @@ -467,6 +468,7 @@ export const LinkButton = ({
}
},
disabled: disabled || !props.shortcut,
elementRef: innerRef,
});

if (disabled) {
Expand Down
1 change: 0 additions & 1 deletion apps/webapp/app/components/primitives/SearchInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ export function SearchInput({
}
if (e.key === "Escape") {
if (text.length > 0) {
e.stopPropagation();
handleClear();
} else {
e.currentTarget.blur();
Comment thread
claude[bot] marked this conversation as resolved.
Expand Down
66 changes: 46 additions & 20 deletions apps/webapp/app/components/primitives/SheetV3.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,42 @@ const sheetVariants = cva(
interface SheetContentProps
extends
React.ComponentPropsWithoutRef<typeof SheetPrimitive.Content>,
VariantProps<typeof sheetVariants> {}
VariantProps<typeof sheetVariants> {
showCloseButton?: boolean;
}

const SheetCloseButtonContext = React.createContext(false);

/**
* True when an enclosing sheet renders its own floating close button, so a header
* rendered inside it has to leave room for it. False outside a sheet, which is why
* components used both in a sheet and as a full page can rely on it.
*/
export function useSheetHasCloseButton() {
return React.useContext(SheetCloseButtonContext);
}

const SheetContent = React.forwardRef<
React.ElementRef<typeof SheetPrimitive.Content>,
SheetContentProps
>(({ side = "right", className, children, ...props }, ref) => (
>(({ side = "right", className, children, showCloseButton = true, ...props }, ref) => (
<SheetPortal>
<SheetOverlay />
<SheetPrimitive.Content ref={ref} className={cn(sheetVariants({ side }), className)} {...props}>
{children}
{showCloseButton && (
// Zero-height sticky wrapper so the button stays pinned to the top right
// even when the sheet itself is the scrolling element.
<div className="sticky top-0 z-10 h-0">
<SheetPrimitive.Close className="absolute right-2 top-2 flex items-center gap-1 rounded-sm p-1 pl-0 transition hover:bg-background-hover focus-visible:focus-custom disabled:pointer-events-none">
<ShortcutKey shortcut={{ key: "esc" }} variant="small" />
<XMarkIcon className="size-4 text-text-dimmed" />
<span className="sr-only">Close</span>
</SheetPrimitive.Close>
</div>
)}
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
claude[bot] marked this conversation as resolved.
<SheetCloseButtonContext.Provider value={showCloseButton}>
{children}
</SheetCloseButtonContext.Provider>
</SheetPrimitive.Content>
</SheetPortal>
));
Expand All @@ -81,23 +107,23 @@ SheetFooter.displayName = "SheetFooter";
const SheetTitle = React.forwardRef<
React.ElementRef<typeof SheetPrimitive.Title>,
React.ComponentPropsWithoutRef<typeof SheetPrimitive.Title>
>(({ className, children, ...props }, ref) => (
<SheetPrimitive.Title
ref={ref}
className={cn(
"sticky top-0 flex items-center justify-between border-b border-grid-bright bg-background-dimmed pb-1.5 pl-3 pr-1.5 pt-2",
className
)}
{...props}
>
{children}
<SheetPrimitive.Close className="flex items-center gap-1 rounded-sm p-1 pl-0 transition hover:bg-background-hover focus-visible:focus-custom disabled:pointer-events-none">
<ShortcutKey shortcut={{ key: "esc" }} variant="small" />
<XMarkIcon className="size-4 text-text-dimmed" />
<span className="sr-only">Close</span>
</SheetPrimitive.Close>
</SheetPrimitive.Title>
));
>(({ className, children, ...props }, ref) => {
const hasCloseButton = useSheetHasCloseButton();

return (
<SheetPrimitive.Title
ref={ref}
className={cn(
"sticky top-0 flex items-center justify-between border-b border-grid-bright bg-background-dimmed pb-1.5 pl-3 pt-2",
hasCloseButton ? "pr-16" : "pr-1.5",
className
)}
{...props}
>
{children}
</SheetPrimitive.Title>
);
});
SheetTitle.displayName = SheetPrimitive.Title.displayName;

const SheetDescription = React.forwardRef<
Expand Down
11 changes: 9 additions & 2 deletions apps/webapp/app/components/schedules/ScheduleInspector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { Header2, Header3 } from "~/components/primitives/Headers";
import { InfoPanel } from "~/components/primitives/InfoPanel";
import { Paragraph } from "~/components/primitives/Paragraph";
import * as Property from "~/components/primitives/PropertyTable";
import { useSheetHasCloseButton } from "~/components/primitives/SheetV3";
import {
Table,
TableBlankRow,
Expand Down Expand Up @@ -96,6 +97,7 @@ export function ScheduleInspector({
const organization = useOrganization();
const project = useProject();
const environment = useEnvironment();
const hasSheetCloseButton = useSheetHasCloseButton();

const isUtc = schedule.timezone === "UTC";
const isImperative = schedule.type === "IMPERATIVE";
Expand All @@ -107,8 +109,13 @@ export function ScheduleInspector({
isImperative ? "grid-rows-[2.5rem_1fr_auto]" : "grid-rows-[2.5rem_1fr]"
)}
>
<div className="mx-3 flex items-center justify-between gap-2 border-b border-grid-dimmed">
<Header2 className="whitespace-nowrap">{schedule.friendlyId}</Header2>
<div
className={cn(
"mx-3 flex items-center justify-between gap-2 border-b border-grid-dimmed",
hasSheetCloseButton && "pr-14"
)}
>
<Header2 className="truncate">{schedule.friendlyId}</Header2>
{headerActions}
</div>
<div className="overflow-y-scroll scrollbar-thin scrollbar-track-transparent scrollbar-thumb-surface-control">
Expand Down
41 changes: 39 additions & 2 deletions apps/webapp/app/hooks/useShortcutKeys.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { type RefObject } from "react";
import { useHotkeys } from "react-hotkeys-hook";
import { useOperatingSystem } from "~/components/primitives/OperatingSystemProvider";
import { useShortcuts } from "~/components/primitives/ShortcutsProvider";
Expand All @@ -22,14 +23,46 @@ type useShortcutKeysProps = {
shortcut: ShortcutDefinition | undefined;
action: (event: KeyboardEvent) => void;
disabled?: boolean;
/**
* Shortcuts are registered on `document`, so this fires the shortcut from every
* input on the page, not just the ones near it. Don't set it on an Escape
* close shortcut: let the focused field handle Escape and close on the second press.
*/
enabledOnInputElements?: boolean;
/**
* The element this shortcut belongs to. When set, an Escape shortcut is ignored
* while the element sits behind an open overlay, so one Escape can't close both
* a dialog and the panel behind it. Other shortcuts are unaffected.
*/
elementRef?: RefObject<HTMLElement | null>;
};

/** Layered surfaces that own the keyboard while they're open. */
const OVERLAY_ROLES = '[role="dialog"],[role="alertdialog"],[role="listbox"],[role="menu"]';

const ESCAPE_KEYS = ["esc", "escape"];

function isEscapeShortcut(shortcut: Shortcut | undefined) {
return !!shortcut && ESCAPE_KEYS.includes(shortcut.key.toLowerCase());
}

function isBlockedByOverlay(event: KeyboardEvent, element: HTMLElement | null) {
// Radix marks everything outside an open modal `aria-hidden`, which covers
// modals that don't move focus into themselves.
if (element?.closest('[aria-hidden="true"]')) return true;

const target = event.target instanceof Element ? event.target : null;
const overlay = target?.closest(OVERLAY_ROLES);

return !!overlay && (!element || !overlay.contains(element));
}
Comment thread
claude[bot] marked this conversation as resolved.

export function useShortcutKeys({
shortcut,
action,
disabled = false,
enabledOnInputElements,
elementRef,
}: useShortcutKeysProps) {
const { platform } = useOperatingSystem();
const { areShortcutsEnabled } = useShortcuts();
Expand All @@ -40,13 +73,17 @@ export function useShortcutKeys({
const keys = createKeysFromShortcut(relevantShortcut);

const isEnabled = !disabled && areShortcutsEnabled && relevantShortcut?.enabled !== false;
const guardAgainstOverlays = isEscapeShortcut(relevantShortcut);

useHotkeys(
keys,
(event) => {
if (!event.repeat) {
action(event);
if (event.repeat) return;
if (guardAgainstOverlays && elementRef && isBlockedByOverlay(event, elementRef.current)) {
return;
}

action(event);
},
{
enabled: isEnabled,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,8 @@ export default function Page() {
<SheetContent
side="right"
className="w-[90vw] max-w-none rounded-l-lg border-l border-grid-dimmed p-0 sm:max-w-none"
// The editor's own nav bar already has an Esc-labelled Cancel button there.
showCloseButton={false}
>
{editorProps && (
<QueryEditor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ export function MfaSetupDialog({

// Show recovery codes if they exist
if (recoveryCodes && recoveryCodes.length > 0) {
// Deliberately not dismissible: MFA is already enabled at this point and the
// codes are shown once, so the only way out is confirming they're saved.
return (
<Dialog open={isOpen}>
<DialogContent showCloseButton={false}>
Expand Down Expand Up @@ -136,8 +138,8 @@ export function MfaSetupDialog({
if (!setupData) return null;

return (
<Dialog open={isOpen}>
<DialogContent showCloseButton={false}>
<Dialog open={isOpen} onOpenChange={(open) => !open && handleCancel()}>
<DialogContent onInteractOutside={(e) => e.preventDefault()}>
<DialogHeader>
<DialogTitle>Enable authenticator app</DialogTitle>
</DialogHeader>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
TableRow,
} from "~/components/primitives/Table";
import { TextLink } from "~/components/primitives/TextLink";
import { useSheetHasCloseButton } from "~/components/primitives/SheetV3";
import { TimezoneList } from "~/components/scheduled/timezones";
import { prisma } from "~/db.server";
import { useEnvironment } from "~/hooks/useEnvironment";
Expand Down Expand Up @@ -156,6 +157,7 @@ export function UpsertScheduleForm({
/** Submits via this fetcher with `_format=json` so the host can toast/close itself. */
submitFetcher?: FetcherWithComponents<unknown>;
}) {
const hasSheetCloseButton = useSheetHasCloseButton();
const actionData = useActionData();
// Only feed conform-shaped data (`status`) to `useForm` — `{ ok, message }`
// envelopes lack it and crash conform.
Expand Down Expand Up @@ -231,7 +233,12 @@ export function UpsertScheduleForm({
{...getFormProps(form)}
className="grid h-full max-h-full grid-rows-[2.5rem_1fr_auto] overflow-hidden bg-background-bright"
>
<div className="mx-3 flex min-w-0 items-center justify-between gap-2 overflow-hidden border-b border-grid-dimmed">
<div
className={cn(
"mx-3 flex min-w-0 items-center justify-between gap-2 overflow-hidden border-b border-grid-dimmed",
hasSheetCloseButton && "pr-14"
)}
>
<Header2 className="truncate">
{schedule?.friendlyId
? "Edit schedule"
Expand Down
Loading