diff --git a/web/src/lib/actions/__test__/focus-trap.spec.ts b/web/src/lib/actions/__test__/focus-trap.spec.ts index d92d8e037d..b03064a91d 100644 --- a/web/src/lib/actions/__test__/focus-trap.spec.ts +++ b/web/src/lib/actions/__test__/focus-trap.spec.ts @@ -1,8 +1,11 @@ import FocusTrapTest from '$lib/actions/__test__/focus-trap-test.svelte'; +import { setDefaultTabbleOptions } from '$lib/utils/focus-util'; import { render, screen } from '@testing-library/svelte'; import userEvent from '@testing-library/user-event'; import { tick } from 'svelte'; +setDefaultTabbleOptions({ displayCheck: 'none' }); + describe('focusTrap action', () => { const user = userEvent.setup(); @@ -38,6 +41,7 @@ describe('focusTrap action', () => { const openButton = screen.getByText('Open'); await user.click(openButton); + await tick(); expect(document.activeElement).toEqual(screen.getByTestId('one')); screen.getByText('Close').click(); diff --git a/web/src/lib/actions/focus-trap.ts b/web/src/lib/actions/focus-trap.ts index 1564dd90d0..2f3868e97b 100644 --- a/web/src/lib/actions/focus-trap.ts +++ b/web/src/lib/actions/focus-trap.ts @@ -1,5 +1,5 @@ import { shortcuts } from '$lib/actions/shortcut'; -import { getFocusable } from '$lib/utils/focus-util'; +import { getTabbable } from '$lib/utils/focus-util'; import { tick } from 'svelte'; interface Options { @@ -18,18 +18,21 @@ export function focusTrap(container: HTMLElement, options?: Options) { }; }; - const setInitialFocus = () => { - const focusableElement = getFocusable(container)[0]; - // Use tick() to ensure focus trap works correctly inside - void tick().then(() => focusableElement?.focus()); + const setInitialFocus = async () => { + const focusableElement = getTabbable(container, false)[0]; + if (focusableElement) { + // Use tick() to ensure focus trap works correctly inside + await tick(); + focusableElement?.focus(); + } }; if (withDefaults(options).active) { - setInitialFocus(); + void setInitialFocus(); } const getFocusableElements = () => { - const focusableElements = getFocusable(container); + const focusableElements = getTabbable(container); return [ focusableElements.at(0), // focusableElements.at(-1), @@ -73,6 +76,7 @@ export function focusTrap(container: HTMLElement, options?: Options) { destroy() { destroyShortcuts?.(); if (triggerElement instanceof HTMLElement) { + console.log('destroy triggerElement', triggerElement.textContent); triggerElement.focus(); } }, diff --git a/web/src/lib/components/assets/thumbnail/__test__/thumbnail.spec.ts b/web/src/lib/components/assets/thumbnail/__test__/thumbnail.spec.ts index f7447551f0..b6e053da12 100644 --- a/web/src/lib/components/assets/thumbnail/__test__/thumbnail.spec.ts +++ b/web/src/lib/components/assets/thumbnail/__test__/thumbnail.spec.ts @@ -1,5 +1,6 @@ import { getIntersectionObserverMock } from '$lib/__mocks__/intersection-observer.mock'; import Thumbnail from '$lib/components/assets/thumbnail/thumbnail.svelte'; +import { getTabbable } from '$lib/utils/focus-util'; import { assetFactory } from '@test-data/factories/asset-factory'; import { fireEvent, render, screen } from '@testing-library/svelte'; @@ -30,51 +31,47 @@ describe('Thumbnail component', () => { it('should only contain a single tabbable element (the container)', () => { const asset = assetFactory.build({ originalPath: 'image.jpg', originalMimeType: 'image/jpeg' }); - render(Thumbnail, { + const { baseElement } = render(Thumbnail, { asset, - focussed: false, selected: true, }); - const container = screen.getByTestId('container-with-tabindex'); - expect(container.getAttribute('tabindex')).toBe('0'); + const container = baseElement.querySelector('[data-thumbnail-focus-container]'); + expect(container).not.toBeNull(); + expect(container!.getAttribute('tabindex')).toBe('0'); - // This isn't capturing all tabbable elements, but should be the most likely ones. Mainly guarding against - // inserting extra tabbable elments in future in - let allTabbableElements = screen.queryAllByRole('link'); - allTabbableElements = allTabbableElements.concat(screen.queryAllByRole('checkbox')); - expect(allTabbableElements.length).toBeGreaterThan(0); - for (const tabbableElement of allTabbableElements) { - const testIdValue = tabbableElement.dataset.testid; - if (testIdValue === null || testIdValue !== 'container-with-tabindex') { - expect(tabbableElement.getAttribute('tabindex')).toBe('-1'); - } - } + // Guarding against inserting extra tabbable elments in future in + const tabbables = getTabbable(container!); + expect(tabbables.length).toBe(0); }); it('handleFocus should be called on focus of container', async () => { const asset = assetFactory.build({ originalPath: 'image.jpg', originalMimeType: 'image/jpeg' }); const handleFocusSpy = vi.fn(); - render(Thumbnail, { + const { baseElement } = render(Thumbnail, { asset, handleFocus: handleFocusSpy, }); - const container = screen.getByTestId('container-with-tabindex'); - await fireEvent(container, new FocusEvent('focus')); + const container = baseElement.querySelector('[data-thumbnail-focus-container]'); + expect(container).not.toBeNull(); + await fireEvent(container as HTMLElement, new FocusEvent('focus')); expect(handleFocusSpy).toBeCalled(); }); - it('element will be focussed if not already', () => { + it('element will be focussed if not already', async () => { const asset = assetFactory.build({ originalPath: 'image.jpg', originalMimeType: 'image/jpeg' }); const handleFocusSpy = vi.fn(); - render(Thumbnail, { + const { baseElement } = render(Thumbnail, { asset, - focussed: true, handleFocus: handleFocusSpy, }); + const container = baseElement.querySelector('[data-thumbnail-focus-container]'); + expect(container).not.toBeNull(); + await fireEvent(container as HTMLElement, new FocusEvent('focus')); + expect(handleFocusSpy).toBeCalled(); }); }); diff --git a/web/src/lib/components/assets/thumbnail/thumbnail.svelte b/web/src/lib/components/assets/thumbnail/thumbnail.svelte index 93a4e3c6cc..c01c1c9bea 100644 --- a/web/src/lib/components/assets/thumbnail/thumbnail.svelte +++ b/web/src/lib/components/assets/thumbnail/thumbnail.svelte @@ -25,7 +25,7 @@ import ImageThumbnail from './image-thumbnail.svelte'; import VideoThumbnail from './video-thumbnail.svelte'; import { onMount } from 'svelte'; - import { getFocusable } from '$lib/utils/focus-util'; + import { focusNext } from '$lib/utils/focus-util'; interface Props { asset: AssetResponseDto; @@ -34,7 +34,6 @@ thumbnailWidth?: number | undefined; thumbnailHeight?: number | undefined; selected?: boolean; - focussed?: boolean; selectionCandidate?: boolean; disabled?: boolean; disableLinkMouseOver?: boolean; @@ -57,7 +56,6 @@ thumbnailWidth = undefined, thumbnailHeight = undefined, selected = false, - focussed = false, selectionCandidate = false, disabled = false, disableLinkMouseOver = false, @@ -78,17 +76,11 @@ } = TUNABLES; let usingMobileDevice = $derived(mobileDevice.pointerCoarse); - let focussableElement: HTMLElement | undefined = $state(); + let element: HTMLElement | undefined = $state(); let mouseOver = $state(false); let loaded = $state(false); let thumbError = $state(false); - $effect(() => { - if (focussed && document.activeElement !== focussableElement) { - focussableElement?.focus(); - } - }); - let width = $derived(thumbnailSize || thumbnailWidth || 235); let height = $derived(thumbnailSize || thumbnailHeight || 235); @@ -223,31 +215,14 @@ if (evt.key === 'x') { onSelect?.(asset); } - if (document.activeElement === focussableElement && evt.key === 'Escape') { - const focusable = getFocusable(document); - const index = focusable.indexOf(focussableElement); - - let i = index + 1; - while (i !== index) { - const next = focusable[i]; - if (next.dataset.thumbnailFocusContainer !== undefined) { - if (i === focusable.length - 1) { - i = 0; - } else { - i++; - } - continue; - } - next.focus(); - break; - } + if (document.activeElement === element && evt.key === 'Escape') { + focusNext((element) => element.dataset.thumbnailFocusContainer === undefined, true); } }} onclick={handleClick} - bind:this={focussableElement} + bind:this={element} onfocus={handleFocus} data-thumbnail-focus-container - data-testid="container-with-tabindex" tabindex={0} role="link" > diff --git a/web/src/lib/components/elements/buttons/skip-link.svelte b/web/src/lib/components/elements/buttons/skip-link.svelte index a1a24634c4..331814813c 100644 --- a/web/src/lib/components/elements/buttons/skip-link.svelte +++ b/web/src/lib/components/elements/buttons/skip-link.svelte @@ -1,6 +1,7 @@