fix: keyboard focus and space handling from accessibility audit

packages/design-system/src/components/OcDrop/dropRegistry.spec.ts

@@ -1,6 +1,8 @@
 import {
+  getOutermostDropAnchor,
   hideAncestorDrops,
   isInDropChain,
+  isInOpenDrop,
   registerOpenDrop,
   unregisterOpenDrop
 } from './dropRegistry'
@@ -218,4 +220,40 @@ describe('dropRegistry', () => {
       expect(hide).not.toHaveBeenCalled()
     })
   })
+
+  describe('getOutermostDropAnchor', () => {
+    it('returns null for an element outside of any drop', () => {
+      expect(getOutermostDropAnchor(createAnchor(document.body))).toBeNull()
+    })
+
+    it('returns the anchor of the outermost drop, even after the drops closed', () => {
+      const [parent, nested] = track(createDrop('parent'), createDrop('nested'))
+      const toggle = createAnchor(document.body)
+      register(parent, toggle)
+      register(nested, createAnchor(parent))
+      unregisterOpenDrop(nested)
+      unregisterOpenDrop(parent)
+
+      expect(getOutermostDropAnchor(createAnchor(nested))).toBe(toggle)
+    })
+
+    it('returns null for a drop without anchor element', () => {
+      const [drop] = track(createDrop('drop'))
+      register(drop)
+
+      expect(getOutermostDropAnchor(createAnchor(drop))).toBeNull()
+    })
+  })
+
+  describe('isInOpenDrop', () => {
+    it('returns whether the drop of an element is open', () => {
+      const [drop] = track(createDrop('drop'))
+      const item = createAnchor(drop)
+      register(drop)
+      expect(isInOpenDrop(item)).toBe(true)
+
+      unregisterOpenDrop(drop)
+      expect(isInOpenDrop(item)).toBe(false)
+    })
+  })
 })

packages/design-system/src/components/OcDrop/dropRegistry.ts

@@ -11,16 +11,24 @@ type OpenDrop = {
  * drops apart from a real outside click, and lets a nested drop close its ancestors.
  */
 const openDrops = new Map<HTMLElement, OpenDrop>()
+// the anchors drops were opened from, still known after they closed, e.g. to return the focus there
+const dropAnchors = new WeakMap<HTMLElement, HTMLElement | null>()
 const dropContainerSelector = '.oc-drop, [data-oc-drop-container]'
 
 export function registerOpenDrop(dropEl: HTMLElement, entry: OpenDrop): void {
   openDrops.set(dropEl, entry)
+  dropAnchors.set(dropEl, entry.getAnchor())
 }
 
 export function unregisterOpenDrop(dropEl: HTMLElement): void {
   openDrops.delete(dropEl)
 }
 
+/** Checks if `el` sits inside a drop that is currently open. */
+export function isInOpenDrop(el: Element): boolean {
+  return openDrops.has(el.closest<HTMLElement>(dropContainerSelector))
+}
+
 /** Collects the open drops that `dropEl` was opened from, closest ancestor first. */
 function getAncestorDrops(dropEl: HTMLElement): OpenDrop[] {
   const ancestors: OpenDrop[] = []
@@ -59,6 +67,21 @@ export function isInDropChain(dropEl: HTMLElement, target: Node): boolean {
   return getAncestorDrops(nestedDrop).some((entry) => entry === openDrops.get(dropEl))
 }
 
+/** Returns the element that opened the outermost drop `el` sits in, e.g. the toggle of a nested menu. */
+export function getOutermostDropAnchor(el: Element): HTMLElement | null {
+  const visited = new Set<HTMLElement>()
+  let anchor: HTMLElement | null = null
+  let dropEl = el.closest<HTMLElement>(dropContainerSelector)
+
+  while (dropEl && dropAnchors.get(dropEl) && !visited.has(dropEl)) {
+    visited.add(dropEl)
+    anchor = dropAnchors.get(dropEl)
+    dropEl = anchor.closest<HTMLElement>(dropContainerSelector)
+  }
+
+  return anchor
+}
+
 /** Closes all open drops that `dropEl` was opened from. */
 export function hideAncestorDrops(dropEl: HTMLElement): void {
   getAncestorDrops(dropEl).forEach((entry) => entry.hide())

packages/design-system/src/components/OcModal/OcModal.spec.ts

@@ -1,6 +1,7 @@
 import { defaultPlugins, mount, shallowMount } from '@opencloud-eu/web-test-helpers'
 import Modal from './OcModal.vue'
 import OcButton from './../OcButton/OcButton.vue'
+import { registerOpenDrop, unregisterOpenDrop } from '../OcDrop/dropRegistry'
 
 const defaultProps = {
   title: 'Example title',
@@ -143,6 +144,149 @@ describe('OcModal', () => {
     })
   })
 
+  describe('focus return after closing', () => {
+    let openDrops: HTMLElement[]
+
+    beforeEach(() => {
+      vi.useFakeTimers()
+      openDrops = []
+    })
+
+    afterEach(() => {
+      openDrops.forEach((dropEl) => unregisterOpenDrop(dropEl))
+      document.body.innerHTML = ''
+      vi.useRealTimers()
+    })
+
+    function createButton(parent: HTMLElement) {
+      const button = document.createElement('button')
+      parent.appendChild(button)
+      return button
+    }
+
+    // drops are teleported to the body, they only know the element they were opened from
+    function openDrop(anchor: HTMLElement) {
+      const dropEl = document.createElement('div')
+      dropEl.classList.add('oc-drop')
+      document.body.appendChild(dropEl)
+      registerOpenDrop(dropEl, { getAnchor: () => anchor, hide: vi.fn() })
+      openDrops.push(dropEl)
+      return dropEl
+    }
+
+    function closeDrop(dropEl: HTMLElement) {
+      unregisterOpenDrop(dropEl)
+      dropEl.remove()
+    }
+
+    // the modal remembers the focused element while being set up, so the opener needs the focus first
+    function openModalFrom(opener: HTMLElement) {
+      opener.focus()
+      return mount(Modal, {
+        global: {
+          renderStubDefaultSlot: true,
+          plugins: [...defaultPlugins()],
+          stubs: { 'focus-trap': true }
+        },
+        props: defaultProps
+      })
+    }
+
+    it('focuses the anchor of the drop the modal was opened from', () => {
+      const anchor = createButton(document.body)
+      const dropEl = openDrop(anchor)
+      const wrapper = openModalFrom(createButton(dropEl))
+      closeDrop(dropEl)
+      expect(document.activeElement).toBe(document.body)
+
+      wrapper.unmount()
+      // focus-trap returns the focus with a delay as well, the modal has to refocus after it
+      expect(document.activeElement).toBe(document.body)
+      vi.runAllTimers()
+
+      expect(document.activeElement).toBe(anchor)
+    })
+
+    it('focuses the toggle of the outermost drop if opened from a nested drop', () => {
+      const toggle = createButton(document.body)
+      const parentDrop = openDrop(toggle)
+      const nestedDrop = openDrop(createButton(parentDrop))
+      const wrapper = openModalFrom(createButton(nestedDrop))
+      closeDrop(nestedDrop)
+      closeDrop(parentDrop)
+
+      wrapper.unmount()
+      vi.runAllTimers()
+
+      expect(document.activeElement).toBe(toggle)
+    })
+
+    it('focuses the drop anchor if the focus stayed on the item of the closed drop', () => {
+      const anchor = createButton(document.body)
+      const dropEl = openDrop(anchor)
+      const item = createButton(dropEl)
+      const wrapper = openModalFrom(item)
+      // the drop is only hidden, so its item keeps the focus
+      unregisterOpenDrop(dropEl)
+      expect(document.activeElement).toBe(item)
+
+      wrapper.unmount()
+      vi.runAllTimers()
+
+      expect(document.activeElement).toBe(anchor)
+    })
+
+    it('keeps the focus on the item if its drop is still open', () => {
+      const dropEl = openDrop(createButton(document.body))
+      const item = createButton(dropEl)
+      const wrapper = openModalFrom(item)
+
+      wrapper.unmount()
+      vi.runAllTimers()
+
+      expect(document.activeElement).toBe(item)
+    })
+
+    it('leaves the focus alone if it already moved to another element', () => {
+      const dropEl = openDrop(createButton(document.body))
+      const wrapper = openModalFrom(createButton(dropEl))
+      closeDrop(dropEl)
+      const other = createButton(document.body)
+
+      wrapper.unmount()
+      other.focus()
+      vi.runAllTimers()
+
+      expect(document.activeElement).toBe(other)
+    })
+
+    it('does nothing if the drop anchor was removed', () => {
+      const anchor = createButton(document.body)
+      const focusSpy = vi.spyOn(anchor, 'focus')
+      const dropEl = openDrop(anchor)
+      const wrapper = openModalFrom(createButton(dropEl))
+      closeDrop(dropEl)
+      anchor.remove()
+
+      wrapper.unmount()
+      expect(() => vi.runAllTimers()).not.toThrow()
+
+      expect(focusSpy).not.toHaveBeenCalled()
+      expect(document.activeElement).toBe(document.body)
+    })
+
+    it('does not move the focus if the modal was not opened from a drop', () => {
+      const opener = createButton(document.body)
+      const wrapper = openModalFrom(opener)
+      opener.blur()
+
+      wrapper.unmount()
+      expect(() => vi.runAllTimers()).not.toThrow()
+
+      expect(document.activeElement).toBe(document.body)
+    })
+  })
+
   it('displays loading state', async () => {
     const waitForSpinnerToShow = async () => {
       await wrapper.vm.$nextTick()

packages/design-system/src/components/OcModal/OcModal.vue

@@ -110,14 +110,15 @@
 </template>
 
 <script setup lang="ts">
-import { computed, ref, unref, useId, useTemplateRef, watch } from 'vue'
+import { computed, onUnmounted, ref, unref, useId, useTemplateRef, watch } from 'vue'
 import { FocusTrap } from 'focus-trap-vue'
 import OcButton, { Props as ButtonProps } from '../OcButton/OcButton.vue'
 import OcTextInput from '../OcTextInput/OcTextInput.vue'
 import { FocusTargetOrFalse, FocusTrapTabbableOptions } from 'focus-trap'
 import { ContextualHelperData, isComposingEvent } from '../../helpers'
 import { useGettext } from 'vue3-gettext'
 import { useEventListener } from '@vueuse/core'
+import { getOutermostDropAnchor, isInOpenDrop } from '../OcDrop/dropRegistry'
 
 export interface Props {
   /**
@@ -335,6 +336,32 @@ watch(
   { immediate: true }
 )
 
+// focus-trap returns the focus to the element that opened the modal. A menu item loses it once its
+// drop closed, so return the focus to the element that opened the menu instead
+const opener = document.activeElement
+const dropAnchor = opener && getOutermostDropAnchor(opener)
+
+function isFocusLost() {
+  const active = document.activeElement
+  if (!active || active === document.body) {
+    return true
+  }
+  return active === opener && !isInOpenDrop(opener)
+}
+
+onUnmounted(() => {
+  if (!dropAnchor) {
+    return
+  }
+  // runs after focus-trap's own (delayed) attempt to return the focus
+  setTimeout(() => {
+    if (!dropAnchor.isConnected || !isFocusLost()) {
+      return
+    }
+    dropAnchor.focus()
+  })
+})
+
 const initialFocusRef = computed<FocusTargetOrFalse>(() => {
   if (focusTrapInitial || focusTrapInitial === false) {
     return focusTrapInitial as FocusTargetOrFalse

packages/web-app-admin-settings/src/composables/keyboardActions/useKeyboardTableNavigation.ts

@@ -83,15 +83,6 @@ export function useKeyboardTableNavigation(
     selectedRows.value = [...unref(paginatedResources)]
   }
 
-  function handleSpaceAction(event: KeyboardEvent) {
-    // a focused checkbox toggles its own row natively
-    if (event.target instanceof HTMLInputElement && event.target.type === 'checkbox') {
-      return
-    }
-    event.preventDefault()
-    toggleLastSelectedRow()
-  }
-
   function handleEscAction() {
     keyActions.resetSelectionCursor()
     selectedRows.value = []
@@ -106,6 +97,6 @@ export function useKeyboardTableNavigation(
     handleShiftAction(false)
   )
   keyActions.bindKeyAction({ modifier: Modifier.Ctrl, primary: Key.A }, handleSelectAllAction)
-  keyActions.bindKeyAction({ primary: Key.Space }, handleSpaceAction, { preventDefault: false })
+  keyActions.bindKeyAction({ primary: Key.Space }, toggleLastSelectedRow)
   keyActions.bindKeyAction({ primary: Key.Esc }, handleEscAction)
 }