From 0b76f94ef251d13de56e9ca7441ea29bd02bc0da Mon Sep 17 00:00:00 2001
From: Mostafa Sadeghi <205455727+mostafasadeghidev@users.noreply.github.com>
Date: Sat, 5 Sep 2026 01:01:13 +0200
Subject: [PATCH] fix(settings): make the Esc keycap close the modal it
 advertises
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The footer reads `[Esc] close`, and the keycap looks like a button because
every `Kbd` depresses on `:active`. Clicking it played that press animation
and did nothing; the pointer over the word "close" turned into a text caret
and a click just selected the text. An operator reasonably concluded the
modal could not be dismissed with the mouse.

It could — the backdrop has always closed it, and Escape works — but a
control that animates under the cursor and then ignores the click is worse
than no control, because it answers the question wrongly.

So the hint becomes the button it already looked like. `Kbd` itself is
untouched: six other surfaces render it as a genuine keyboard hint where the
press styling is right, and only this one sits next to the word "close".

The file header claimed there was no dedicated close button. There is one
now, so it says so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/__tests__/settings/settingsModal.test.tsx | 10 +++++++++
 .../modals/Settings/SettingsModal.module.css  |  8 +++++++
 src/admin/modals/Settings/SettingsModal.tsx   | 21 ++++++++++++++-----
 3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/src/__tests__/settings/settingsModal.test.tsx b/src/__tests__/settings/settingsModal.test.tsx
index 8c4edd2d9..1f3d00f87 100644
--- a/src/__tests__/settings/settingsModal.test.tsx
+++ b/src/__tests__/settings/settingsModal.test.tsx
@@ -186,6 +186,16 @@ describe('SettingsModal — backdrop', () => {
     fireEvent.click(backdrop)
     expect(useEditorStore.getState().isSettingsOpen).toBe(false)
   })
+
+  it('clicking the Esc keycap closes the modal it advertises', () => {
+    // The keycap depresses on `:active` like every other `Kbd`, and sits
+    // beside the word "close". It used to be inert markup, so clicking it
+    // played the press animation and did nothing.
+    openModal()
+    render(<SettingsModal />)
+    fireEvent.click(screen.getByRole('button', { name: /esc\s*close/i }))
+    expect(useEditorStore.getState().isSettingsOpen).toBe(false)
+  })
 })
 
 // ---------------------------------------------------------------------------
diff --git a/src/admin/modals/Settings/SettingsModal.module.css b/src/admin/modals/Settings/SettingsModal.module.css
index e742e38e9..a75c12277 100644
--- a/src/admin/modals/Settings/SettingsModal.module.css
+++ b/src/admin/modals/Settings/SettingsModal.module.css
@@ -163,11 +163,19 @@
   border-top: 1px solid var(--overlay-10);
 }
 
+/* Sits on the footer's close Button. Keeps the row reading as a quiet hint
+   rather than a control — the keycap already carries the affordance — while
+   the element underneath is a real button. `justify-content: start` because a
+   Button centres its content by default and this row aligns with the nav
+   above it. */
 .shortcutHint {
   display: flex;
   align-items: center;
+  justify-content: start;
   gap: var(--space-xs);
   flex-wrap: wrap;
+  width: 100%;
+  padding: 0;
   color: var(--text-subtle);
   font-size: var(--text-s);
 }
diff --git a/src/admin/modals/Settings/SettingsModal.tsx b/src/admin/modals/Settings/SettingsModal.tsx
index e5cfabd13..b564d127d 100644
--- a/src/admin/modals/Settings/SettingsModal.tsx
+++ b/src/admin/modals/Settings/SettingsModal.tsx
@@ -4,8 +4,8 @@
  * Shares the visual language of the Spotlight palette and the Module
  * Inserter: a direct-token panel shell, an `--bg-surface-2` rail with
  * categorical accent icon chips, an accent-bar section header, and a
- * shared `Esc` keycap affordance (backdrop click / Esc both close — there
- * is no dedicated close button, matching the other two modals).
+ * shared `Esc` keycap affordance, which is also the close button — backdrop
+ * click and Esc close it too.
  *
  * Guideline #225 (Modal Shell Requirements, WCAG 2.1 AA):
  * - role="dialog" + aria-modal="true" + aria-labelledby
@@ -230,11 +230,22 @@ export function SettingsModal() {
 
             <div className={s.railSpring} />
 
-            <div className={s.shortcutFooter} aria-label="Settings keyboard shortcuts">
-              <div className={s.shortcutHint}>
+            {/* The keycap reads as a button — it depresses on :active like every
+                other `Kbd` — so it has to behave like one. It was a hint beside
+                the word "close", which meant clicking it played the press
+                animation and did nothing, and the pointer over the label turned
+                into a text caret. Backdrop click and Esc still work; this just
+                stops the affordance lying about itself. */}
+            <div className={s.shortcutFooter}>
+              <Button
+                variant="ghost"
+                size="sm"
+                onClick={handleClose}
+                className={s.shortcutHint}
+              >
                 <Kbd>Esc</Kbd>
                 <span>close</span>
-              </div>
+              </Button>
             </div>
           </div>