Skip to content

Commit 44973d1

Browse files
committed
fix(webapp): address review on Esc-to-close changes
- MFA setup: only Escape and the close button cancel the pending TOTP secret; outside clicks are prevented and onOpenChange guards on `open`. - Make Esc-from-input a per-panel decision: drop it on the panels that host an editor or a form (waitpoint token, span, bulk-action inspector) and blur instead of closing when the agent composer holds a draft. - Scope the overlay guard to Escape shortcuts so it no longer changes behaviour for every Button/LinkButton shortcut app-wide. - Reserve room for the sheet's floating close button in the alerts and schedule inspector headers, and drop a vestigial `justify-between`. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent f1ba0d2 commit 44973d1

11 files changed

Lines changed: 35 additions & 15 deletions

File tree

.server-changes/esc-to-close-fixes.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ area: webapp
33
type: fix
44
---
55

6-
Esc now closes the chat panel, the authenticator app setup dialog, and detail panels while you're typing in one of their fields, and every side sheet shows its Esc shortcut. Closing a dialog with Esc no longer also closes the panel behind it, and the Vercel setup modal no longer shows a close button it ignores.
6+
Esc now closes the authenticator app setup window, the chat panel, and detail panels while you're typing in one of their fields. Panels that hold an editor or a form you're filling in keep what you've typed instead of closing, and clicking outside the authenticator app setup window no longer cancels the setup. Every side sheet now shows its Esc shortcut, closing a dialog with Esc no longer also closes the panel behind it, and the Vercel setup modal no longer shows a close button it ignores.

apps/webapp/app/components/dashboard-agent/DashboardAgentComposer.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ export function DashboardAgentComposer({
3131
e.preventDefault();
3232
onSubmit();
3333
}
34+
// Keep an unsent message: Esc blurs first, so only a second Esc closes the panel.
35+
if (e.key === "Escape" && value.trim().length > 0) {
36+
e.stopPropagation();
37+
e.currentTarget.blur();
38+
}
3439
}}
3540
placeholder="Type a message…"
3641
className={cn(

apps/webapp/app/components/errors/ConfigureErrorAlerts.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ export function ConfigureErrorAlerts({
115115

116116
return (
117117
<div className="grid h-full grid-rows-[auto_1fr_auto] overflow-hidden">
118-
<div className="flex items-center justify-between border-b border-grid-bright px-3 py-2">
118+
<div className="flex items-center border-b border-grid-bright py-2 pl-3 pr-16">
119119
<Header2 className="flex items-center gap-2">
120120
<BellAlertIcon className="size-5 text-alerts" /> Configure alerts
121121
</Header2>

apps/webapp/app/components/primitives/SheetV3.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ const SheetTitle = React.forwardRef<
9898
<SheetPrimitive.Title
9999
ref={ref}
100100
className={cn(
101-
"sticky top-0 flex items-center justify-between border-b border-grid-bright bg-background-dimmed pb-1.5 pl-3 pr-12 pt-2",
101+
"sticky top-0 flex items-center justify-between border-b border-grid-bright bg-background-dimmed pb-1.5 pl-3 pr-16 pt-2",
102102
className
103103
)}
104104
{...props}

apps/webapp/app/components/schedules/ScheduleInspector.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,14 @@ export function ScheduleInspector({
107107
isImperative ? "grid-rows-[2.5rem_1fr_auto]" : "grid-rows-[2.5rem_1fr]"
108108
)}
109109
>
110-
<div className="mx-3 flex items-center justify-between gap-2 border-b border-grid-dimmed">
111-
<Header2 className="whitespace-nowrap">{schedule.friendlyId}</Header2>
110+
<div
111+
className={cn(
112+
"mx-3 flex items-center justify-between gap-2 border-b border-grid-dimmed",
113+
// Without header actions the sheet's own floating close button sits here.
114+
!headerActions && "pr-14"
115+
)}
116+
>
117+
<Header2 className="truncate">{schedule.friendlyId}</Header2>
112118
{headerActions}
113119
</div>
114120
<div className="overflow-y-scroll scrollbar-thin scrollbar-track-transparent scrollbar-thumb-surface-control">

apps/webapp/app/hooks/useShortcutKeys.tsx

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,22 @@ type useShortcutKeysProps = {
2525
disabled?: boolean;
2626
enabledOnInputElements?: boolean;
2727
/**
28-
* The element this shortcut belongs to. When set, the shortcut is ignored while
29-
* the element sits behind an open overlay, so one Escape can't close both a
30-
* dialog and the panel behind it.
28+
* The element this shortcut belongs to. When set, an Escape shortcut is ignored
29+
* while the element sits behind an open overlay, so one Escape can't close both
30+
* a dialog and the panel behind it. Other shortcuts are unaffected.
3131
*/
3232
elementRef?: RefObject<HTMLElement | null>;
3333
};
3434

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

38+
const ESCAPE_KEYS = ["esc", "escape"];
39+
40+
function isEscapeShortcut(shortcut: Shortcut | undefined) {
41+
return !!shortcut && ESCAPE_KEYS.includes(shortcut.key.toLowerCase());
42+
}
43+
3844
function isBlockedByOverlay(event: KeyboardEvent, element: HTMLElement | null) {
3945
// Radix marks everything outside an open modal `aria-hidden`, which covers
4046
// modals that don't move focus into themselves.
@@ -62,12 +68,15 @@ export function useShortcutKeys({
6268
const keys = createKeysFromShortcut(relevantShortcut);
6369

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

6673
useHotkeys(
6774
keys,
6875
(event) => {
6976
if (event.repeat) return;
70-
if (elementRef && isBlockedByOverlay(event, elementRef.current)) return;
77+
if (guardAgainstOverlays && elementRef && isBlockedByOverlay(event, elementRef.current)) {
78+
return;
79+
}
7180

7281
action(event);
7382
},

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens.$waitpointParam/route.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ export default function Page() {
115115
to={`${v3WaitpointTokensPath(organization, project, environment)}${location.search}`}
116116
variant="minimal/small"
117117
TrailingIcon={ExitIcon}
118-
shortcut={{ key: "esc", enabledOnInputElements: true }}
118+
shortcut={{ key: "esc" }}
119119
shortcutPosition="before-trailing-icon"
120120
className="pl-1"
121121
/>

apps/webapp/app/routes/resources.account.mfa.setup/MfaSetupDialog.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,8 @@ export function MfaSetupDialog({
138138
if (!setupData) return null;
139139

140140
return (
141-
<Dialog open={isOpen} onOpenChange={handleCancel}>
142-
<DialogContent>
141+
<Dialog open={isOpen} onOpenChange={(open) => !open && handleCancel()}>
142+
<DialogContent onInteractOutside={(e) => e.preventDefault()}>
143143
<DialogHeader>
144144
<DialogTitle>Enable authenticator app</DialogTitle>
145145
</DialogHeader>

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ function SpanBody({
333333
onClick={closePanel}
334334
variant="minimal/small"
335335
TrailingIcon={ExitIcon}
336-
shortcut={{ key: "esc", enabledOnInputElements: true }}
336+
shortcut={{ key: "esc" }}
337337
shortcutPosition="before-trailing-icon"
338338
className="pl-1"
339339
/>

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.bulkaction.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ export function CreateBulkActionInspector({
315315
variant="minimal/small"
316316
onClick={() => del([...RUNS_BULK_INSPECTOR_UI_SEARCH_PARAMS])}
317317
TrailingIcon={ExitIcon}
318-
shortcut={{ key: "esc", enabledOnInputElements: true }}
318+
shortcut={{ key: "esc" }}
319319
shortcutPosition="before-trailing-icon"
320320
className="pl-1"
321321
/>

0 commit comments

Comments
 (0)