fix: Stop Escape closing the composer under a modal - #401
Merged
Merged
Conversation
Composer.svelte binds a svelte:window keydown handler that closed the
composer on bare Escape with no check for what was on screen. Two
user-visible symptoms:
- Escape on the save-draft prompt did nothing. bits-ui closed the
dialog, then this handler immediately re-opened it.
- A guarded modal over the composer (settings, folder picker) was
dismissed together with the composer underneath it, popping the
save-draft prompt.
Fix is to bail when a modal layer already consumed the key:
if (e.defaultPrevented) return
if (isDialogGuardActive()) return
handleClose()
bits-ui's escape layer listens on `document` and calls preventDefault()
without stopping propagation, so the event still reaches this
window-level handler — but defaultPrevented persists across listeners
in the same dispatch. That covers every bits-ui layer (dialog,
alertdialog, dropdown, context menu) whether or not it registers the
dialog guard, including layers that deliberately ignore Escape: bits-ui
preventDefaults those too, and the composer shouldn't close under a
modal that is refusing to.
Note a DOM check for [role=dialog] does not work here, which is what I
tried first. A microtask checkpoint runs between listener callbacks, so
Svelte has already flushed the dialog out of the DOM by the time this
handler runs and the query finds nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dlapiduz
force-pushed
the
composer-escape-stacked-modal
branch
from
August 22, 2026 19:53
40d3d2d to
b8be665
Compare
Owner
|
Thank you for this and I really appreciate how thorough you are with the PR submission and code comments. Given that this is a very well scoped and minimal change with big value attached, and it's pretty much exactly how I was about to implement the fix, I will make an exception to the policy for this PR. I really hope this doesn't set the wrong precedent for future submitted PRs until I am ready though. 😅 |
hkdb
added a commit
that referenced
this pull request
Sep 18, 2026
* Bump to v0.3.4-dev * Fix post delete ghost messages on message list * New full set app icon #99 #395 (#396) * Fixed message list sender and preview for threads - #169 * Added default BCC - #341 * Added default reply-to & CC - #404 * Fixed comma display name - #398 * Fixed composer draft format detection - #420 * Added English (UK) spellcheck dictionary - #417 * Block save all attachments if flatpak - #384 * Added dot and glow dot optional accents for unread messages * Fix Italian Translations #207 (#424) * Backfilled i18n missing keys * i18n(cs): Update Czech translation (#425) * Manual merge of (#426) by @freemans32 * Added spellcheck dictionary: Dutch (#413) * fix: Stop Escape closing the composer under a modal (#401) Composer.svelte binds a svelte:window keydown handler that closed the composer on bare Escape with no check for what was on screen. Two user-visible symptoms: - Escape on the save-draft prompt did nothing. bits-ui closed the dialog, then this handler immediately re-opened it. - A guarded modal over the composer (settings, folder picker) was dismissed together with the composer underneath it, popping the save-draft prompt. Fix is to bail when a modal layer already consumed the key: if (e.defaultPrevented) return if (isDialogGuardActive()) return handleClose() bits-ui's escape layer listens on `document` and calls preventDefault() without stopping propagation, so the event still reaches this window-level handler — but defaultPrevented persists across listeners in the same dispatch. That covers every bits-ui layer (dialog, alertdialog, dropdown, context menu) whether or not it registers the dialog guard, including layers that deliberately ignore Escape: bits-ui preventDefaults those too, and the composer shouldn't close under a modal that is refusing to. Note a DOM check for [role=dialog] does not work here, which is what I tried first. A microtask checkpoint runs between listener callbacks, so Svelte has already flushed the dialog out of the DOM by the time this handler runs and the query finds nothing. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * Added window size persistence - #217 * Added mini calendar for easier navigation - #368 * Calendar color render adjustments to alleviate #406 * Fixed Microsoft calendar attachments handling - #370 * Updated what's new * Contacts and Calendar fixes for #278 - Fixed Contacts pagination - Fixed M365 contacts - sync all folders - Fixed M365 calendar - kill the silent-zero paths * Fixed draft edit logic - #392 * Empty spam + GA fix * GA fix 2 * GA fix 3 * CI and GA fix * Bumped to v0.3.4 --------- Co-authored-by: Yacine Boussoufa <yacine.boussoufa.yb@gmail.com> Co-authored-by: lorduskordus <136916485+lorduskordus@users.noreply.github.com> Co-authored-by: Diego Lapiduz <dlapiduz@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the
Escapehandling conflict reported in #400.The bug
Composer.sveltebinds asvelte:windowkeydown handler that callshandleClose()on bareEscapewith no check for what is on screen. When a modal layer sits on top of the composer, both react to the same keypress:The fix
Bail out when a modal layer has already consumed the key:
Why
defaultPreventedbits-ui's escape layer listens on
documentand callspreventDefault()without stopping propagation, so the event still reaches this window-level handler.defaultPreventedis the reliable signal because it persists across listeners within the same dispatch.This covers every bits-ui layer — dialog, alertdialog, dropdown, context menu — whether or not it registers the dialog guard. That includes layers which deliberately ignore
Escape: bits-uipreventDefaults those too, and the composer shouldn't close under a modal that is refusing to close itself.A DOM check for
[role=dialog]does not work here, which was the first approach tried. A microtask checkpoint runs between listener callbacks, so Svelte has already flushed the dialog out of the DOM by the time this handler runs and the query finds nothing.Testing
npm run check— 0 errors, 0 warningsnpm run lint— cleanEscapewith no modal open still closes the composer as before.Notes
Targeting
v0.3.4-devper the release-branch convention. 18 added lines in one file, no behavior change when no modal is present.I appreciate that CONTRIBUTING.md says the workflow isn't set up for general PRs yet — happy to leave this sitting until it is, or to close it and just keep #400 open as a report if that's easier for you.