fix(reader): give the mobile settings sheet a way to close

On a phone the settings panel had no reachable exit. `hide-close` dropped the built-in close
button and nothing replaced it, the grab handle was decorative with no swipe-to-dismiss behind
it, and the compact trigger only ever set the panel open. That left tapping the overlay as the
sole way out, and `max-h-[85vh]` closed off even that: `vh` resolves against the toolbar-retracted
viewport, so with browser chrome on screen the sheet covered nearly the whole visible area. Only
fullscreen, where the two viewports agree, left a gap big enough to hit.

Move the sheet chrome into a shared ReaderSettingsSheet instead of patching both call sites. The
epub and cbz sheets were identical copies, which is why the bug existed twice. Leave the desktop
popover path alone; it closes correctly already.

Closes #1016

client/src/features/reader/cbz/CbzReaderView.vue

@@ -16,7 +16,7 @@ import type { CbxReaderSettings } from '@bookorbit/types'
 import { DEFAULT_WIDE_PAGE_RATIO_THRESHOLD, createCbzSpreadLayout } from './lib/spread-layout'
 import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
 import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'
-import { Sheet, SheetContent } from '@/components/ui/sheet'
+import ReaderSettingsSheet from '@/features/reader/shared/components/ReaderSettingsSheet.vue'
 import CbzSettingsPanel from './components/CbzSettingsPanel.vue'
 
 const TWO_PAGE_BREAKPOINT = 900
@@ -99,8 +99,9 @@ function onSettingsOpenChange(open: boolean) {
   showSettings.value = open
 }
 
-function openSettings() {
-  showSettings.value = true
+/** Mirrors PopoverTrigger on the wide path, so the icon means the same thing in both containers. */
+function toggleSettings() {
+  showSettings.value = !showSettings.value
 }
 
 function applySettings(s: CbxReaderSettings) {
@@ -768,29 +769,19 @@ onUnmounted(() => {
             :class="showSettings ? '!bg-muted !text-foreground' : ''"
             :title="t('reader.settings.title')"
             :aria-label="t('reader.settings.ariaLabel')"
-            @click="openSettings"
+            @click="toggleSettings"
           >
             <Settings :size="15" />
           </button>
-          <Sheet :open="showSettings" @update:open="onSettingsOpenChange">
-            <SheetContent
-              side="bottom"
-              hide-close
-              class="max-h-[85vh] gap-0 rounded-t-2xl border-border bg-card p-0"
-              :aria-label="t('reader.settings.ariaLabel')"
-            >
-              <div class="flex shrink-0 justify-center pt-2.5 pb-1">
-                <div class="h-1 w-9 rounded-full bg-border" />
-              </div>
-              <CbzSettingsPanel
-                :settings="panelSettings"
-                :can-reset="bookSettings.isCustomized.value"
-                :is-spread-active="isTwoPageEffective"
-                @update="applyPanelUpdate"
-                @reset="resetBookViewSettings"
-              />
-            </SheetContent>
-          </Sheet>
+          <ReaderSettingsSheet :open="showSettings" @update:open="onSettingsOpenChange">
+            <CbzSettingsPanel
+              :settings="panelSettings"
+              :can-reset="bookSettings.isCustomized.value"
+              :is-spread-active="isTwoPageEffective"
+              @update="applyPanelUpdate"
+              @reset="resetBookViewSettings"
+            />
+          </ReaderSettingsSheet>
         </template>
 
         <Popover v-else :open="showSettings" @update:open="onSettingsOpenChange">

client/src/features/reader/epub/components/ReaderHeader.vue

@@ -16,8 +16,8 @@ import {
 } from '@lucide/vue'
 import { useMediaQuery } from '@vueuse/core'
 import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'
-import { Sheet, SheetContent } from '@/components/ui/sheet'
 import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
+import ReaderSettingsSheet from '@/features/reader/shared/components/ReaderSettingsSheet.vue'
 import { useFullscreen } from '../../shared/composables/useFullscreen'
 
 const { t } = useI18n()
@@ -52,8 +52,9 @@ function onSettingsOpenChange(open: boolean) {
   emit('update:settingsOpen', open)
 }
 
-function openSettings() {
-  emit('update:settingsOpen', true)
+/** Mirrors PopoverTrigger on the wide path, so the icon means the same thing in both containers. */
+function toggleSettings() {
+  emit('update:settingsOpen', !props.settingsOpen)
 }
 
 function getFooterModeIcon(mode: 0 | 1 | 2) {
@@ -175,23 +176,13 @@ function getFooterModeTooltip(mode: 0 | 1 | 2): string {
           :class="props.settingsOpen ? '!bg-muted !text-foreground' : ''"
           :title="t('reader.settings.title')"
           :aria-label="t('reader.settings.ariaLabel')"
-          @click="openSettings"
+          @click="toggleSettings"
         >
           <Settings :size="18" />
         </button>
-        <Sheet :open="props.settingsOpen" @update:open="onSettingsOpenChange">
-          <SheetContent
-            side="bottom"
-            hide-close
-            class="max-h-[85vh] gap-0 rounded-t-2xl border-border bg-card p-0"
-            :aria-label="t('reader.settings.ariaLabel')"
-          >
-            <div class="flex shrink-0 justify-center pt-2.5 pb-1">
-              <div class="h-1 w-9 rounded-full bg-border" />
-            </div>
-            <slot name="settingsPanel" />
-          </SheetContent>
-        </Sheet>
+        <ReaderSettingsSheet :open="props.settingsOpen" @update:open="onSettingsOpenChange">
+          <slot name="settingsPanel" />
+        </ReaderSettingsSheet>
       </template>
 
       <Popover v-else :open="props.settingsOpen" @update:open="onSettingsOpenChange">

client/src/features/reader/epub/components/__tests__/ReaderHeader.spec.ts

@@ -19,6 +19,7 @@ const PopoverStub = defineComponent({
 
 const SheetStub = defineComponent({
   name: 'SettingsSheet',
+  props: { open: { type: Boolean, default: false } },
   emits: ['update:open'],
   template: '<div><slot /></div>',
 })
@@ -31,8 +32,7 @@ const global = {
     Popover: PopoverStub,
     PopoverTrigger: { template: '<div><slot /></div>' },
     PopoverContent: { template: '<div><slot /></div>' },
-    Sheet: SheetStub,
-    SheetContent: { template: '<div><slot /></div>' },
+    ReaderSettingsSheet: SheetStub,
   },
 }
 
@@ -45,6 +45,7 @@ function mountHeader(props: Record<string, unknown> = {}) {
       footerMode: 0,
       ...props,
     },
+    slots: { settingsPanel: '<p data-testid="settings-panel">Panel body</p>' },
     global,
   })
 }
@@ -98,5 +99,38 @@ describe('ReaderHeader', () => {
 
       viewport.isCompact = false
     })
+
+    it('renders the settings panel inside the compact sheet', () => {
+      viewport.isCompact = true
+      const wrapper = mountHeader({ settingsOpen: true })
+
+      expect(wrapper.findComponent(SheetStub).find('[data-testid="settings-panel"]').exists()).toBe(true)
+
+      viewport.isCompact = false
+    })
+
+    it('forwards the open state to the compact sheet', async () => {
+      viewport.isCompact = true
+      const wrapper = mountHeader()
+      expect(wrapper.findComponent(SheetStub).props('open')).toBe(false)
+
+      await wrapper.setProps({ settingsOpen: true })
+      expect(wrapper.findComponent(SheetStub).props('open')).toBe(true)
+
+      viewport.isCompact = false
+    })
+
+    it('toggles rather than only opening when the compact trigger is pressed', async () => {
+      // The compact trigger used to hard-set `true`, so it could never take the panel back down.
+      // The wide path gets this for free from PopoverTrigger; the compact path has to do it itself.
+      viewport.isCompact = true
+      const wrapper = mountHeader({ settingsOpen: true })
+
+      await wrapper.get('button[aria-label="Reader settings"]').trigger('click')
+
+      expect(wrapper.emitted('update:settingsOpen')?.[0]).toEqual([false])
+
+      viewport.isCompact = false
+    })
   })
 })

client/src/features/reader/shared/components/ReaderSettingsSheet.vue

@@ -0,0 +1,51 @@
+<script setup lang="ts">
+import { useI18n } from 'vue-i18n'
+import { X } from '@lucide/vue'
+import { Sheet, SheetContent } from '@/components/ui/sheet'
+
+const { t } = useI18n()
+
+defineProps<{ open: boolean }>()
+
+const emit = defineEmits<{
+  'update:open': [open: boolean]
+}>()
+
+function onOpenChange(open: boolean) {
+  emit('update:open', open)
+}
+
+function closeSheet() {
+  emit('update:open', false)
+}
+</script>
+
+<template>
+  <Sheet :open="open" @update:open="onOpenChange">
+    <!--
+      dvh, not vh: `vh` resolves against the large viewport (browser UI retracted), so with a
+      phone browser's toolbars on screen the sheet grew to cover the whole visible area and left
+      no overlay to tap. The close button below is the affordance that must not depend on that
+      gap existing at all.
+    -->
+    <SheetContent
+      side="bottom"
+      hide-close
+      class="max-h-[85dvh] gap-0 rounded-t-2xl border-border bg-card p-0"
+      :aria-label="t('reader.settings.ariaLabel')"
+    >
+      <div class="relative flex h-11 shrink-0 items-center justify-center">
+        <div aria-hidden="true" class="h-1 w-9 rounded-full bg-border" />
+        <button
+          type="button"
+          class="absolute end-0.5 top-1/2 flex size-11 -translate-y-1/2 items-center justify-center rounded-lg text-foreground transition-colors hover:bg-muted focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary/55"
+          :aria-label="t('reader.settings.close')"
+          @click="closeSheet"
+        >
+          <X :size="18" />
+        </button>
+      </div>
+      <slot />
+    </SheetContent>
+  </Sheet>
+</template>

client/src/features/reader/shared/components/__tests__/ReaderSettingsSheet.spec.ts

@@ -0,0 +1,115 @@
+import { mount } from '@vue/test-utils'
+import { defineComponent } from 'vue'
+import { describe, expect, it } from 'vitest'
+import ReaderSettingsSheet from '../ReaderSettingsSheet.vue'
+
+const SheetStub = defineComponent({
+  name: 'SheetStub',
+  props: { open: { type: Boolean, default: false } },
+  emits: ['update:open'],
+  template: '<div data-testid="sheet"><slot /></div>',
+})
+
+const SheetContentStub = defineComponent({
+  name: 'SheetContentStub',
+  template: '<div data-testid="sheet-content"><slot /></div>',
+})
+
+function mountSheet(props: Record<string, unknown> = {}) {
+  return mount(ReaderSettingsSheet, {
+    props: { open: true, ...props },
+    slots: { default: '<p data-testid="panel">Panel body</p>' },
+    global: { stubs: { Sheet: SheetStub, SheetContent: SheetContentStub } },
+  })
+}
+
+describe('ReaderSettingsSheet', () => {
+  it('renders the settings panel passed into the default slot', () => {
+    const wrapper = mountSheet()
+
+    expect(wrapper.get('[data-testid="panel"]').text()).toBe('Panel body')
+  })
+
+  it('forwards the open state to the sheet', async () => {
+    const wrapper = mountSheet({ open: false })
+    expect(wrapper.findComponent(SheetStub).props('open')).toBe(false)
+
+    await wrapper.setProps({ open: true })
+    expect(wrapper.findComponent(SheetStub).props('open')).toBe(true)
+  })
+
+  describe('closing', () => {
+    // The reported bug: `hide-close` dropped the built-in X and nothing replaced it, so on a phone
+    // there was no reachable way out of the panel at all. Every assertion here guards one of the
+    // three exits the reporter found missing.
+    it('offers a close button with an accessible label', () => {
+      const wrapper = mountSheet()
+      const close = wrapper.get('button[aria-label="Close settings"]')
+
+      expect(close.element.tagName).toBe('BUTTON')
+      expect(close.attributes('type')).toBe('button')
+    })
+
+    it('emits update:open false when the close button is activated', async () => {
+      const wrapper = mountSheet()
+
+      await wrapper.get('button[aria-label="Close settings"]').trigger('click')
+
+      expect(wrapper.emitted('update:open')).toEqual([[false]])
+    })
+
+    it('forwards the sheet dismissing itself, which covers overlay taps and Escape', () => {
+      const wrapper = mountSheet()
+
+      wrapper.findComponent(SheetStub).vm.$emit('update:open', false)
+
+      expect(wrapper.emitted('update:open')).toEqual([[false]])
+    })
+
+    it('emits nothing until something is actually activated', () => {
+      const wrapper = mountSheet()
+
+      expect(wrapper.emitted('update:open')).toBeUndefined()
+    })
+
+    it('keeps the close button as the only interactive control in the sheet chrome', () => {
+      const wrapper = mountSheet()
+      const buttons = wrapper.findAll('button')
+
+      expect(buttons).toHaveLength(1)
+      expect(buttons[0].attributes('aria-label')).toBe('Close settings')
+    })
+
+    it('leaves the grab handle decorative rather than passing it off as a control', () => {
+      // The reporter tried to drag this and it did nothing. There is no swipe-to-dismiss, so the
+      // handle must stay hidden from assistive tech instead of advertising an exit that is not there.
+      const wrapper = mountSheet()
+      const handle = wrapper.get('[data-testid="sheet-content"] .rounded-full')
+
+      expect(handle.element.tagName).toBe('DIV')
+      expect(handle.attributes('aria-hidden')).toBe('true')
+    })
+  })
+
+  describe('sheet geometry', () => {
+    it('caps its height against the dynamic viewport, not the large viewport', () => {
+      // `vh` resolves against the toolbar-retracted viewport. On a phone browser with its chrome
+      // showing, `max-h-[85vh]` covered essentially the whole visible area and left no overlay to
+      // tap, which is why the panel could only be dismissed in fullscreen. Asserting the class is
+      // crude, but jsdom has no layout, and this is the exact token that regressed.
+      const content = mountSheet().get('[data-testid="sheet-content"]')
+
+      expect(content.classes()).toContain('max-h-[85dvh]')
+      expect(content.classes()).not.toContain('max-h-[85vh]')
+    })
+
+    it('suppresses the built-in corner close so it cannot collide with the panel header', () => {
+      // Paired deliberately with the close-button tests above: `hide-close` is only defensible
+      // because this component supplies its own close.
+      const content = mountSheet().get('[data-testid="sheet-content"]')
+
+      expect(Object.keys(content.attributes())).toContain('hide-close')
+      expect(content.attributes('side')).toBe('bottom')
+    })
+  })
+})

client/src/locales/en.json

@@ -6269,6 +6269,7 @@
     "settings": {
       "title": "Settings",
       "ariaLabel": "Reader settings",
+      "close": "Close settings",
       "reset": "Reset to defaults",
       "modeLabel": "Color mode",
       "mode": {