frontend: dialogs keep focus, and a skip link past the chrome (task 1999)

BaseModal declared role="dialog" aria-modal="true" and then enforced none of it. Focus never moved into the panel, so Escape — handled ON the panel — did nothing at all in LabelsModal, the integration prompt and the shortcuts modal. Only the command palette escaped correctly, and only because it happens to focus its own input. Tab walked straight out of the dialog into the page that aria-modal had just told assistive tech was inert, and closing dropped focus to <body> so the next Tab restarted from the top of the document. All three are one contract, so it lives in BaseModal rather than in each of the four callers: focus in on open, Tab trapped, focus restored to the opener. The panel takes tabindex="-1" so it can hold focus itself when it wraps nothing focusable. CommandPalette's input focus still wins, because a child's mounted hook runs before its parent's. The skip link is the other half. The header and sidebar are a dozen-odd tab stops that repeat on every navigation, and a keyboard user walked all of them again to reach their notes. <main> takes tabindex="-1" as well, because several browsers scroll to a bare anchor without moving focus to it — which would have made the link look like it worked while leaving the next Tab back at the top. The rest of the audit came back clean: no click handlers on non-focusable elements, and all 30 focus:outline-none uses already pair with a focus-visible ring. M3.5's keyboard pass held up; the gaps were in focus management, not styling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-16 12:54:08 -04:00
parent d6646a64fb
commit e7ee16c6cf
2 changed files with 92 additions and 6 deletions
@@ -210,6 +210,15 @@ async function signOut() {

 <template>
  <div class="flex min-h-full flex-col">
+    <!-- First tab stop on every page. The header and sidebar are a dozen-odd tab
+         stops that repeat on every navigation; without this a keyboard user walks
+         all of them again to reach their own notes. Hidden until focused. -->
+    <a
+      href="#main"
+      class="sr-only focus:not-sr-only focus:absolute focus:left-3 focus:top-3 focus:z-50 focus:rounded-md focus:bg-white focus:px-3 focus:py-2 focus:text-sm focus:font-medium focus:shadow-lg focus:outline-none focus:ring-2 focus:ring-brand dark:focus:bg-neutral-900"
+    >
+      Skip to notes
+    </a>
    <header
      class="sticky top-0 z-20 border-b border-neutral-200 bg-neutral-50/90 backdrop-blur dark:border-neutral-800 dark:bg-neutral-950/90"
    >
@@ -416,7 +425,10 @@ async function signOut() {
        </nav>
      </aside>

-      <main class="min-w-0 flex-1"><RouterView /></main>
+      <!-- tabindex="-1" so the skip link above actually moves FOCUS here, not just
+           the viewport — several browsers scroll to a plain anchor without focusing
+           it, which leaves the next Tab back at the top of the page. -->
+      <main id="main" tabindex="-1" class="min-w-0 flex-1 focus:outline-none"><RouterView /></main>
    </div>

    <LabelsModal v-if="managing" @close="managing = false" />
@@ -1,12 +1,19 @@
 <script setup lang="ts">
 // The backdrop + centered dialog panel every modal shares: a fixed dimmed overlay and
-// a bordered, rounded panel with role="dialog". Emits `close` on Escape (while focus is
-// inside the panel — matching the app's existing modals) and on a backdrop pointer-down
-// (mousedown outside the panel). The caller sizes/shadows/pads the panel via `panelClass`
-// and fills it (its own header + content) through the default slot.
+// a bordered, rounded panel with role="dialog". Emits `close` on Escape and on a
+// backdrop pointer-down (mousedown outside the panel). The caller sizes/shadows/pads
+// the panel via `panelClass` and fills it (its own header + content) through the
+// default slot.
+//
+// Owns the dialog's focus contract on behalf of all four callers: focus moves in on
+// open, Tab is trapped inside, and focus returns to the opener on close. Doing it
+// here rather than per-modal is why CommandPalette's own input focus is the only
+// bespoke focus handling left.
 //
 // NoteEditor deliberately does NOT use this — its backdrop mousedown is drag-guarded and
 // its Escape/⌘-Enter handling is bespoke (unsaved-edit safety), so it keeps its own shell.
+import { onBeforeUnmount, onMounted, ref } from "vue";
+
 withDefaults(
  defineProps<{
    /** Panel size / shadow / padding classes (structure — rounded/border/bg — is fixed). */
@@ -19,6 +26,67 @@ withDefaults(
  { panelClass: "w-full max-w-sm shadow-xl", align: "start" },
 );
 const emit = defineEmits<{ (e: "close"): void }>();
+
+const panel = ref<HTMLElement | null>(null);
+/** Whatever had focus before we opened, so closing can hand it back. */
+let restoreTo: HTMLElement | null = null;
+
+const FOCUSABLE = [
+  "a[href]",
+  "button:not([disabled])",
+  "input:not([disabled])",
+  "select:not([disabled])",
+  "textarea:not([disabled])",
+  '[tabindex]:not([tabindex="-1"])',
+].join(",");
+
+/** Tabbable descendants, in document order, skipping anything not rendered. */
+function focusable(): HTMLElement[] {
+  if (!panel.value) return [];
+  return Array.from(panel.value.querySelectorAll<HTMLElement>(FOCUSABLE)).filter(
+    (el) => el.getClientRects().length > 0,
+  );
+}
+
+/**
+ * Keep Tab inside the panel.
+ *
+ * `aria-modal="true"` tells assistive tech the rest of the page is inert, but it
+ * doesn't enforce anything — without this, Tab walks straight out into content the
+ * user has just been told isn't there.
+ */
+function onTab(e: KeyboardEvent): void {
+  const items = focusable();
+  if (!items.length) {
+    // Nothing to land on: hold focus on the panel rather than releasing it.
+    e.preventDefault();
+    panel.value?.focus();
+    return;
+  }
+  const first = items[0];
+  const last = items[items.length - 1];
+  const active = document.activeElement;
+  if (e.shiftKey && (active === first || active === panel.value)) {
+    e.preventDefault();
+    last.focus();
+  } else if (!e.shiftKey && active === last) {
+    e.preventDefault();
+    first.focus();
+  }
+}
+
+onMounted(() => {
+  restoreTo = document.activeElement as HTMLElement | null;
+  // The fix for more than tab order: Escape is handled ON the panel, so until
+  // focus is actually inside it, Escape did nothing at all in every modal that
+  // doesn't focus its own input (LabelsModal, the integration prompt, shortcuts).
+  const target = focusable()[0] ?? panel.value;
+  target?.focus();
+});
+
+// Focus would otherwise fall back to <body>, so the next Tab restarts from the top
+// of the page rather than from whatever the user was working on.
+onBeforeUnmount(() => restoreTo?.focus?.());
 </script>

 <template>
@@ -27,13 +95,19 @@ const emit = defineEmits<{ (e: "close"): void }>();
    :class="align === 'center' ? 'items-center' : 'items-start pt-[12vh]'"
    @mousedown.self="emit('close')"
  >
+    <!-- tabindex="-1" so the panel itself can hold focus when it contains nothing
+         focusable — without it, focus would stay behind the overlay and Escape,
+         which is handled here, would never fire. -->
    <div
-      class="rounded-xl border border-neutral-200 bg-white dark:border-neutral-700 dark:bg-neutral-900"
+      ref="panel"
+      class="rounded-xl border border-neutral-200 bg-white focus:outline-none dark:border-neutral-700 dark:bg-neutral-900"
      :class="panelClass"
      role="dialog"
      aria-modal="true"
+      tabindex="-1"
      :aria-label="ariaLabel"
      @keydown.esc="emit('close')"
+      @keydown.tab="onTab"
    >
      <slot />
    </div>