Skip to content

Reader: Escape in a dialog no longer closes the book - #971

Merged
ajslater merged 1 commit into
developfrom
fix-reader-dialog-escape
Oct 8, 2026
Merged

ajslater merged 1 commit into
developfrom
fix-reader-dialog-escape

Conversation

@ajslater

@ajslater ajslater commented Oct 8, 2026

Copy link
Copy Markdown
Owner

Summary

In the reader, pressing Escape to close the metadata dialog (m) or the online-tag Match Review dialog also closed the book. While either dialog was open, arrow keys, space, j/k, n/p and the fit/direction keys also acted on the reader underneath it.

  • Cause: the three document keyup listeners (top toolbar, nav toolbar, settings scope) bailed out only on isAuthDialogOpen. Vuetify closes an overlay on Escape keydown and moves focus back to its activator, so by keyup the dialog flag is false and the keyup target is the toolbar button, outside any overlay.
  • Fix: new useReaderKeyUp composable (frontend/src/components/reader/use-reader-keyup.js). A capture-phase keydown listener records keys pressed inside a .v-overlay; the keyup listener skips those keys and any keyup whose own target is in an overlay. All three listeners use it. It covers the auth dialogs too, so the now-unused isAuthDialogOpen getter is removed.

Why keydown, not closest() at keyup

Logged real key events in the browser. With the metadata dialog opened from the visible Tags button, Escape went:

event target in .v-overlay dialog active
keydown .v-overlay__content yes yes
keyup toolbar Tags button no no

So a keyup-time event.target.closest('.v-overlay') check misses whenever the dialog was opened from a visible activator. It only happens to work when m opened it with the toolbars hidden.

Browser verification (dev stack, fresh DB, generated CBZ)

  • m opens metadata. Inside it, ArrowRight ×2 and j don't page. Escape closes the dialog and stays in the reader.
  • Metadata opened from the Tags button (the focus-returns-to-activator case): Escape closes only the dialog.
  • Match Review dialog (admin, from the toolbar Review button): ArrowRight, space and n do nothing underneath it. Escape closes only the dialog.
  • Login dialog: typing the username and password key by key (includes m, n) doesn't open metadata or change books. Escape closes only the dialog. The old isAuthDialogOpen check missed this Escape too.
  • No dialog open: ArrowRight pages and Escape closes the book.

Tests

  • frontend/tests/unit/use-reader-keyup.test.js covers:
    • overlay vs. outside keys
    • Escape whose keyup lands on the activator after the overlay is gone
    • held-key repeats
    • an overlay that stops keydown propagation
    • scope cleanup
    • a wiring check that all three reader components route keyups through the guard
  • Mutation-checked: removing the keydown record, the capture phase, or the repeat check each fails its tests.
  • make fix, make lint and make test all pass: vitest 994 passed; pytest 1662 passed, 1 xfailed.

🤖 Generated with Claude Code

Pressing Escape to close the metadata or Match Review dialog also closed
the book, and arrow keys, space, j/k and the other shortcuts paged or
changed settings underneath an open dialog.

The reader's three document keyup listeners only bailed out on the auth
store's isAuthDialogOpen flag. That misses Escape for every dialog, auth
dialogs included: Vuetify closes an overlay on Escape keydown and moves
focus back to its activator, so by keyup the dialog flag is already
false and the keyup target is the toolbar button, outside any overlay.
Checked in a real browser: event.target.closest(".v-overlay") at keyup
misses whenever the dialog was opened from a visible activator.

useReaderKeyUp decides on keydown instead. A capture-phase keydown
listener records each key pressed inside a .v-overlay (capture so a
dialog that stops keydown still counts; repeats are ignored so a held
Escape stays with the dialog), and the keyup listener skips those keys
and any keyup whose own target is inside an overlay. All three reader
listeners use it. This covers the auth dialogs too, so the
isAuthDialogOpen getter, which had no other users, is gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ajslater
ajslater merged commit 8cd6ea6 into develop Oct 8, 2026
9 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.

1 participant