Skip to content

fix: portal keyboard shortcuts overlay out of blurred header - #837

Merged
Kingsman-99 merged 1 commit into
Stellar-split:mainfrom
therealbibson:fix/658-header-shortcuts-modal-overlay
Sep 28, 2026
Merged

Kingsman-99 merged 1 commit into
Stellar-split:mainfrom
therealbibson:fix/658-header-shortcuts-modal-overlay

Conversation

@therealbibson

Copy link
Copy Markdown
Contributor

Overview

Makes the header ? button actually surface the shortcut reference overlay.

The button, the modal and the ShortcutRegistry wiring already existed on main (landed with the keyboard-shortcuts overlay work), which is why this looked done — but clicking the button still failed to produce a usable overlay. The overlay is fixed inset-0, and it is mounted from HeaderShortcutsButton, which lives inside Navbar's <header>:

<header class="sticky top-0 z-40 … backdrop-blur-md">

A backdrop-filter other than none makes an element a containing block for position: fixed descendants. So the overlay's inset-0 resolved against the header box, not the viewport, and the dialog was clipped into the header strip. Its z-50 also only competed inside the header's own (z-40) stacking context, so the backdrop could never cover the page. From a user's point of view the button did nothing.

This PR portals the overlay to document.body so it escapes that containing block and stacking context, and hardens the category grouping while it is there.

Related Issue

Closes #658

Changes

  • [MODIFY] src/components/KeyboardShortcutsModal.tsx

    • Portal the overlay: renders through createPortal(..., document.body) behind a mount guard, so the overlay is no longer a descendant of the blurred header and fixed inset-0 resolves against the viewport. Closing behaviour is unchanged — the same onClose is still wired to the backdrop, the close button and FocusTrap's Escape handler.
    • Stable category order: groupShortcuts now derives a rank from the data instead of leaking registration order. Categories lead in the order the issue documents (Navigation, Invoices, Payments), any other category follows alphabetically, and General is always last so it never crowds out real categories.
    • DOM-safe section ids: section headings previously used id={kbd-group-${group}}. A category containing a space made aria-labelledby a token list that resolved to nothing, silently dropping the section's accessible name. Ids are now slugified (Zebra Extras → kbd-group-zebra-extras) and the heading/section pair is always resolvable.
    • Empty groups are no longer created, so the overlay cannot render a category header with no rows.
  • [ADD] src/__tests__/KeyboardShortcutsModal.portal.test.tsx

    • Asserts the dialog renders outside the host that mounted the trigger and is attached to document.body, while the trigger button stays in place.
    • Asserts the documented category order and that General sorts last.
    • Asserts every aria-labelledby resolves to a real, whitespace-free id and that multi-word categories slugify correctly.
    • Asserts no registered entry is silently dropped, and that the close button still dismisses the portalled overlay.

Verification Results

Implemented via GitHub Contents/Git API (no local clone).
Acceptance criteria mapping:
✅ Clicking the shortcuts button opens the modal with a grouped reference list
   (the overlay is now portalled out of the header's `backdrop-blur` containing block)
✅ The shortcut list is populated from ShortcutRegistry
   (unchanged: still useShortcutRegistry(); ordering is now derived, not incidental)
✅ The modal is closeable with Escape and the close button
   (unchanged paths; covered by the existing suite plus the new close-button test)
✅ Existing tests continue to pass
   (existing KeyboardShortcutsModal.test.tsx queries via `screen`, so portal
    rendering does not affect it; changes are confined to this one component)
Acceptance Criteria Status
Clicking the shortcuts button opens a modal with a grouped shortcut reference list ✅ Overlay portalled to document.body; no longer clipped by the header's containing block
The shortcut list is populated from ShortcutRegistry ✅ Still useShortcutRegistry(); category order now stable (Navigation, Invoices, Payments, …, General)
The modal is closeable with Escape and the close button ✅ Same onClose wiring for backdrop, close button and FocusTrap Escape
Existing tests continue to pass ✅ Existing screen-based tests unaffected; new tests added in src/__tests__/KeyboardShortcutsModal.portal.test.tsx

@vercel

vercel Bot commented Sep 27, 2026

Copy link
Copy Markdown

@therealbibson is attempting to deploy a commit to the kingsman-99's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Sep 27, 2026

Copy link
Copy Markdown

@therealbibson Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Kingsman-99
Kingsman-99 merged commit ed91002 into Stellar-split:main Sep 28, 2026
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HeaderShortcutsButton: Wire button to open keyboard shortcut reference modal

2 participants