Reader: Escape in a dialog no longer closes the book

NEWS.md

@@ -11,6 +11,9 @@ border-radius: 128px;
 - Features
     - Online tagging matches to review show a toolbar Review button and a
       tab-title count instead of popping up a dialog.
+- Fixes
+    - Escape in a reader dialog no longer closes the book, and keys typed in a
+      dialog no longer turn pages.
 
 ## v2.5.3
 

frontend/src/components/reader/drawer/reader-settings-scope.vue

@@ -58,9 +58,9 @@
 <script>
 import { mapActions, mapState } from "pinia";
 
+import { useReaderKeyUp } from "@/components/reader/use-reader-keyup";
 import { useAuthStore } from "@/stores/auth";
 import { useReaderStore } from "@/stores/reader";
-import { useEventListener } from "@vueuse/core";
 
 import GLOBAL_DEFAULTS from "@/choices/reader-defaults.json";
 
@@ -118,7 +118,6 @@ export default {
     };
   },
   computed: {
-    ...mapState(useAuthStore, ["isAuthDialogOpen"]),
     ...mapState(useAuthStore, {
       globalDefaults: (state) => state.defaults?.reader ?? GLOBAL_DEFAULTS,
     }),
@@ -190,7 +189,7 @@ export default {
     },
   },
   created() {
-    useEventListener(document, "keyup", this._keyUpListener);
+    useReaderKeyUp(this._keyUpListener);
   },
   methods: {
     ...mapActions(useReaderStore, [
@@ -216,9 +215,6 @@ export default {
     },
     _keyUpListener(event) {
       event.stopPropagation();
-      if (this.isAuthDialogOpen) {
-        return;
-      }
       let updates;
       switch (event.key) {
         case "w":

frontend/src/components/reader/toolbars/nav/reader-toolbar-nav.vue

@@ -30,9 +30,8 @@ import PaginationSlider from "@/components/pagination-slider.vue";
 import PaginationToolbar from "@/components/pagination-toolbar.vue";
 import ReaderBookChangeNavButton from "@/components/reader/toolbars/nav/reader-book-change-nav-button.vue";
 import ReaderNavButton from "@/components/reader/toolbars/nav/reader-nav-button.vue";
-import { useAuthStore } from "@/stores/auth";
+import { useReaderKeyUp } from "@/components/reader/use-reader-keyup";
 import { useReaderStore } from "@/stores/reader";
-import { useEventListener } from "@vueuse/core";
 
 const PREV = "prev";
 const NEXT = "next";
@@ -46,7 +45,6 @@ export default {
     ReaderBookChangeNavButton,
   },
   computed: {
-    ...mapState(useAuthStore, ["isAuthDialogOpen"]),
     ...mapState(useReaderStore, [
       "activeSettings",
       "isReadInReverse",
@@ -90,7 +88,7 @@ export default {
     },
   },
   created() {
-    useEventListener(document, "keyup", this._keyUpListener);
+    useReaderKeyUp(this._keyUpListener);
   },
   methods: {
     ...mapActions(useReaderStore, [
@@ -109,9 +107,6 @@ export default {
     },
     _keyUpListener(event) {
       event.stopPropagation();
-      if (this.isAuthDialogOpen) {
-        return;
-      }
       switch (event.key) {
         case " ":
           if (

frontend/src/components/reader/toolbars/top/reader-toolbar-top.vue

@@ -64,11 +64,10 @@ import FavoriteToggle from "@/components/favorite-toggle.vue";
 import MetadataDialog from "@/components/metadata/metadata-dialog.vue";
 import OnlineTagReviewButton from "@/components/online-tag/review-button.vue";
 import ReaderArcSelect from "@/components/reader/toolbars/top/reader-arc-select.vue";
+import { useReaderKeyUp } from "@/components/reader/use-reader-keyup";
 import SettingsDrawerButton from "@/components/settings/button.vue";
-import { useAuthStore } from "@/stores/auth";
 import { useCommonStore } from "@/stores/common";
 import { useReaderStore } from "@/stores/reader";
-import { useEventListener } from "@vueuse/core";
 
 export default {
   name: "ReaderTitleToolbar",
@@ -100,7 +99,6 @@ export default {
     };
   },
   computed: {
-    ...mapState(useAuthStore, ["isAuthDialogOpen"]),
     ...mapState(useReaderStore, ["activeTitle", "closeBookRoute"]),
     ...mapState(useReaderStore, {
       showToolbars: (state) => state.showToolbars,
@@ -152,7 +150,7 @@ export default {
     },
   },
   created() {
-    useEventListener(document, "keyup", this._keyUpListener);
+    useReaderKeyUp(this._keyUpListener);
   },
   methods: {
     ...mapActions(useCommonStore, ["setTimestamp"]),
@@ -166,9 +164,6 @@ export default {
     },
     _keyUpListener(event) {
       event.stopPropagation();
-      if (this.isAuthDialogOpen) {
-        return;
-      }
       switch (event.key) {
         case "Escape":
           this.$refs.closeBook.$el.click();

frontend/src/components/reader/use-reader-keyup.js

@@ -0,0 +1,49 @@
+/**
+Document keyup listener for the reader's keyboard shortcuts that ignores keys
+meant for a dialog or menu open over the reader.
+
+Used by: reader-toolbar-top.vue, reader-toolbar-nav.vue,
+reader-settings-scope.vue
+*/
+import { useEventListener } from "@vueuse/core";
+
+const OVERLAY_SELECTOR = ".v-overlay";
+
+export function isInOverlay(target) {
+  return Boolean(target?.closest?.(OVERLAY_SELECTOR));
+}
+
+/**
+Calls `listener` on document keyup unless the key went to a Vuetify overlay.
+
+Vuetify closes an overlay on Escape keydown and focuses its activator, so by
+keyup neither the target nor the overlay's state shows the key was the
+overlay's. Decide on keydown instead; the capture phase still sees keydowns
+the overlay stops.
+*/
+export function useReaderKeyUp(listener) {
+  const overlayKeys = new Set();
+  useEventListener(
+    document,
+    "keydown",
+    (event) => {
+      // Holding a key repeats keydown wherever the first one moved focus.
+      if (event.repeat) {
+        return;
+      }
+      if (isInOverlay(event.target)) {
+        overlayKeys.add(event.code);
+      } else {
+        overlayKeys.delete(event.code);
+      }
+    },
+    { capture: true },
+  );
+  useEventListener(document, "keyup", (event) => {
+    const pressedInOverlay = overlayKeys.delete(event.code);
+    if (pressedInOverlay || isInOverlay(event.target)) {
+      return;
+    }
+    listener(event);
+  });
+}

frontend/src/stores/auth.js

@@ -59,14 +59,6 @@ export const useAuthStore = defineStore("auth", {
     isUserAdmin() {
       return this.user && (this.user.isStaff || this.user.isSuperuser);
     },
-    isAuthDialogOpen() {
-      return (
-        this.showLoginDialog ||
-        this.showChangePasswordDialog ||
-        this.showProfileDialog ||
-        this.showResetPasswordRequestDialog
-      );
-    },
     isBanner(state) {
       return Boolean(state.adminFlags.bannerText);
     },

frontend/tests/unit/use-reader-keyup.test.js

@@ -0,0 +1,149 @@
+/*
+ * The reader's keyboard shortcuts must not act on keys meant for a dialog or
+ * menu over the reader. Vuetify closes an overlay on Escape keydown and
+ * focuses its activator, so the Escape keyup lands outside any overlay.
+ */
+import { createTestingPinia } from "@pinia/testing";
+import { shallowMount } from "@vue/test-utils";
+import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
+import { effectScope } from "vue";
+
+import ReaderSettingsScope from "@/components/reader/drawer/reader-settings-scope.vue";
+import ReaderToolbarNav from "@/components/reader/toolbars/nav/reader-toolbar-nav.vue";
+import ReaderToolbarTop from "@/components/reader/toolbars/top/reader-toolbar-top.vue";
+import {
+  isInOverlay,
+  useReaderKeyUp,
+} from "@/components/reader/use-reader-keyup";
+import vuetify from "@/plugins/vuetify";
+
+const KEYS = Object.freeze({ ArrowRight: "ArrowRight", Escape: "Escape" });
+
+function press(type, target, code, init = {}) {
+  target.dispatchEvent(
+    new KeyboardEvent(type, {
+      bubbles: true,
+      code,
+      key: KEYS[code],
+      ...init,
+    }),
+  );
+}
+
+function setupDom() {
+  document.body.innerHTML = `
+    <button id="activator"></button>
+    <div class="v-overlay">
+      <div class="v-overlay__content"><button id="inOverlay"></button></div>
+    </div>`;
+  return {
+    activator: document.querySelector("#activator"),
+    inOverlay: document.querySelector("#inOverlay"),
+    overlay: document.querySelector(".v-overlay"),
+  };
+}
+
+function tap(target, code) {
+  press("keydown", target, code);
+  press("keyup", target, code);
+}
+
+describe("useReaderKeyUp", () => {
+  let dom, listener, scope;
+
+  beforeEach(() => {
+    dom = setupDom();
+    listener = vi.fn();
+    scope = effectScope();
+    scope.run(() => useReaderKeyUp(listener));
+  });
+
+  afterEach(() => {
+    scope.stop();
+    document.body.replaceChildren();
+  });
+
+  test("keys outside an overlay reach the listener", () => {
+    tap(document.body, "ArrowRight");
+    expect(listener).toHaveBeenCalledOnce();
+    expect(listener.mock.calls[0][0].key).toBe("ArrowRight");
+  });
+
+  test("keys inside an overlay are ignored", () => {
+    tap(dom.inOverlay, "ArrowRight");
+    press("keyup", dom.inOverlay, "Escape");
+    expect(listener).not.toHaveBeenCalled();
+  });
+
+  test("Escape that closes an overlay is ignored after focus leaves it", () => {
+    press("keydown", dom.inOverlay, "Escape");
+    dom.overlay.remove();
+    press("keyup", dom.activator, "Escape");
+    expect(listener).not.toHaveBeenCalled();
+
+    // The next Escape is the reader's again.
+    tap(dom.activator, "Escape");
+    expect(listener).toHaveBeenCalledOnce();
+  });
+
+  test("a held key stays with the overlay it was pressed in", () => {
+    press("keydown", dom.inOverlay, "Escape");
+    press("keydown", dom.activator, "Escape", { repeat: true });
+    press("keyup", dom.activator, "Escape");
+    expect(listener).not.toHaveBeenCalled();
+  });
+
+  test("an overlay that stops keydown still keeps its key", () => {
+    dom.inOverlay.addEventListener("keydown", (event) =>
+      event.stopPropagation(),
+    );
+    press("keydown", dom.inOverlay, "Escape");
+    press("keyup", dom.activator, "Escape");
+    expect(listener).not.toHaveBeenCalled();
+  });
+
+  test("stopping the scope removes the listeners", () => {
+    scope.stop();
+    tap(document.body, "ArrowRight");
+    expect(listener).not.toHaveBeenCalled();
+  });
+
+  test("targets without closest are outside any overlay", () => {
+    expect(isInOverlay(document)).toBe(false);
+    expect(isInOverlay(globalThis)).toBe(false);
+    expect(isInOverlay(null)).toBe(false);
+  });
+});
+
+describe.each([
+  ["ReaderToolbarTop", ReaderToolbarTop],
+  ["ReaderToolbarNav", ReaderToolbarNav],
+  ["ReaderSettingsScope", ReaderSettingsScope],
+])("%s keyup shortcuts", (_name, component) => {
+  let dom, keyUpListener, wrapper;
+
+  beforeEach(() => {
+    dom = setupDom();
+    keyUpListener = vi.spyOn(component.methods, "_keyUpListener");
+    keyUpListener.mockImplementation(() => {});
+    wrapper = shallowMount(component, {
+      global: { plugins: [createTestingPinia(), vuetify] },
+    });
+  });
+
+  afterEach(() => {
+    wrapper.unmount();
+    keyUpListener.mockRestore();
+    document.body.replaceChildren();
+  });
+
+  test("ignore keys that went to an overlay", () => {
+    tap(dom.inOverlay, "ArrowRight");
+    press("keydown", dom.inOverlay, "Escape");
+    press("keyup", dom.activator, "Escape");
+    expect(keyUpListener).not.toHaveBeenCalled();
+
+    tap(document.body, "Escape");
+    expect(keyUpListener).toHaveBeenCalledOnce();
+  });
+});