From 3c09da2fb8423b236c21611fe639768e68906883 Mon Sep 17 00:00:00 2001
From: LB <mail@lb.ee>
Date: Thu, 31 Oct 2024 21:39:04 +1000
Subject: [PATCH] Enhance & rework FocusController

- Update the Controller import & identifier to be `w-focus` not `w-skip-link`
- Rework methods to be easier to maintain, pull out the focus event attachment handler to a function
- Add JSDoc examples & refine description
- Only add then remove the tabindex attribute if needed
- Rework unit tests to be the emerging common structure, add unit test coverage & test for the preservation of existing tabindex attributes
---
 .../src/controllers/FocusController.test.js   | 216 ++++++++++++++++--
 client/src/controllers/FocusController.ts     |  61 +++--
 client/src/controllers/index.ts               |   4 +-
 3 files changed, 235 insertions(+), 46 deletions(-)

diff --git a/client/src/controllers/FocusController.test.js b/client/src/controllers/FocusController.test.js
index 32644102d9..c24b4149d6 100644
--- a/client/src/controllers/FocusController.test.js
+++ b/client/src/controllers/FocusController.test.js
@@ -1,38 +1,204 @@
 import { Application } from '@hotwired/stimulus';
 
-import { SkipLinkController } from './SkipLinkController';
+import { FocusController } from './FocusController';
 
-describe('skip to the main content on clicking the skiplink', () => {
-  document.body.innerHTML = `
-  <a id="skip" class="button" data-controller="w-skip-link" data-action="click->w-skip-link#skip">Skip to main content</a>
-  <main>Main content</main>
-  <button id="other-content">other</button>`;
+jest.useFakeTimers();
 
-  const application = Application.start();
+describe('FocusController', () => {
+  let application;
 
-  application.register('w-skip-link', SkipLinkController);
+  const setup = async (
+    html = `<div>
+    <a id="skip" class="button" data-controller="w-focus" data-action="click->w-focus#focus">Skip to main content</a>
+    <main>Main content</main>
+    <button id="other-content">other</button>
+    </div>`,
+    definition = {},
+  ) => {
+    document.body.innerHTML = `<main>${html}</main>`;
 
-  const mainElement = document.querySelector('main');
+    application = Application.start();
+    application.load({
+      controllerConstructor: FocusController,
+      identifier: 'w-focus',
+      ...definition,
+    });
 
-  it('should keep tabindex, blur and focusout attribute as null when not in focus', () => {
-    expect(document.activeElement).toBe(document.body);
-    expect(mainElement.getAttribute('tabindex')).toBe(null);
+    await Promise.resolve();
+  };
+
+  afterEach(() => {
+    document.body.innerHTML = '';
+    application.stop();
   });
 
-  it('should skip to main when skip link is clicked', () => {
-    document.getElementById('skip').click();
-    expect(mainElement.getAttribute('tabindex')).toEqual('-1');
-    expect(document.activeElement).toBe(mainElement);
-    expect(mainElement.getAttribute('blur')).toBe(null);
-    expect(mainElement.getAttribute('focusout')).toBe(null);
+  describe('skip to the main content on clicking the skip link', () => {
+    beforeEach(async () => {
+      await setup();
+    });
+
+    it('should keep tabindex, blur and focusout attribute as null when not in focus', () => {
+      const mainElement = document.querySelector('main');
+
+      expect(document.activeElement).toBe(document.body);
+      expect(mainElement.getAttribute('tabindex')).toBe(null);
+    });
+
+    it('should skip to main when skip link is clicked', async () => {
+      const mainElement = document.querySelector('main');
+
+      document.getElementById('skip').click();
+
+      await jest.runAllTimersAsync();
+
+      expect(mainElement.getAttribute('tabindex')).toEqual('-1');
+      expect(document.activeElement).toBe(mainElement);
+      expect(mainElement.getAttribute('blur')).toBe(null);
+      expect(mainElement.getAttribute('focusout')).toBe(null);
+    });
+
+    it('should reset tab index when focus is moved from skip link', () => {
+      const otherContent = document.getElementById('other-content');
+      otherContent.focus();
+      expect(document.activeElement).toBe(otherContent);
+      expect(otherContent.getAttribute('tabindex')).toBe(null);
+      expect(otherContent.getAttribute('blur')).toBe(null);
+      expect(otherContent.getAttribute('focusout')).toBe(null);
+    });
   });
 
-  it('should reset tab index when focus is moved from skip link', () => {
-    const otherContent = document.getElementById('other-content');
-    otherContent.focus();
-    expect(document.activeElement).toBe(otherContent);
-    expect(otherContent.getAttribute('tabindex')).toBe(null);
-    expect(otherContent.getAttribute('blur')).toBe(null);
-    expect(otherContent.getAttribute('focusout')).toBe(null);
+  describe('using to skip to a specific element', () => {
+    beforeEach(async () => {
+      await setup(
+        `
+    <main>
+      <section>
+        <div class="section-top"><h3>Section title</h3></div>
+        <p>...lots of content...</p>
+        <button
+          type="button"
+          data-controller="w-focus"
+          data-action="w-focus#focus"
+          data-w-focus-target-value=".section-top"
+        >
+          Skip to top
+        </button>
+      </section>
+    </main>`,
+        { identifier: 'w-focus' },
+      );
+    });
+
+    it('should not have modified anything on connect', () => {
+      const sectionTop = document.querySelector('.section-top');
+      expect(sectionTop.getAttribute('tabindex')).toBe(null);
+      expect(document.activeElement).toBe(document.body);
+    });
+
+    it('should focus on the section top when the button is clicked', async () => {
+      expect(document.activeElement).toBe(document.body);
+
+      const sectionTop = document.querySelector('.section-top');
+
+      document.querySelector('button').click();
+
+      await jest.runAllTimersAsync();
+
+      expect(sectionTop.getAttribute('tabindex')).toEqual('-1');
+      expect(document.activeElement).toBe(sectionTop);
+    });
+
+    it('should reset the attributes when focus is moved from the section top', () => {
+      const sectionTop = document.querySelector('.section-top');
+
+      const button = document.querySelector('button');
+      button.focus();
+
+      expect(document.activeElement).toBe(button);
+      expect(sectionTop.getAttribute('tabindex')).toBe(null);
+    });
+  });
+
+  describe('avoid modifying tabindex if not required', () => {
+    beforeEach(async () => {
+      await setup(
+        `
+    <main>
+      <section>
+        <div class="section-top" tabindex="-1"><h3>Section title</h3></div>
+        <p>...lots of content...</p>
+        <button
+          type="button"
+          data-controller="w-focus"
+          data-action="w-focus#focus"
+          data-w-focus-target-value=".section-top"
+        >
+          Skip to top
+        </button>
+      </section>
+    </main>`,
+        { identifier: 'w-focus' },
+      );
+    });
+
+    it('should focus on the section top when the button is clicked & when focus is removed keep the tabindex', async () => {
+      expect(document.activeElement).toBe(document.body);
+
+      const sectionTop = document.querySelector('.section-top');
+
+      document.querySelector('button').click();
+
+      await jest.runAllTimersAsync();
+
+      expect(sectionTop.getAttribute('tabindex')).toEqual('-1');
+      expect(document.activeElement).toBe(sectionTop);
+
+      document.querySelector('button').focus(); // move focus away
+      sectionTop.dispatchEvent(new FocusEvent('focusout')); // simulate focusout event
+      expect(document.activeElement).toBe(document.querySelector('button'));
+      expect(sectionTop.getAttribute('tabindex')).toEqual('-1'); // tabindex should not be removed
+    });
+  });
+
+  describe('focusing on an element that is added dynamically', () => {
+    it('should support focusing on an element that is added dynamically', async () => {
+      await setup(
+        `
+    <main>
+      <button
+        type="button"
+        data-controller="w-focus"
+        data-action="w-focus#focus"
+        data-w-focus-target-value=".error-message"
+      >
+        Skip first error message
+      </button>
+      <section>
+        <p>...lots of content...</p>
+      </section>
+    </main>`,
+        { identifier: 'w-focus' },
+      );
+
+      expect(document.activeElement).toBe(document.body);
+
+      const section = document.querySelector('section');
+      const button = document.querySelector('button');
+
+      button.click();
+      await jest.runAllTimersAsync();
+
+      // No element found, defaults to not changing focus
+      expect(document.activeElement).toBe(document.body);
+
+      const errorMessage = document.createElement('div');
+      errorMessage.classList.add('error-message');
+      section.appendChild(errorMessage);
+
+      button.click();
+      await jest.runAllTimersAsync();
+
+      expect(document.activeElement).toBe(errorMessage);
+    });
   });
 });
diff --git a/client/src/controllers/FocusController.ts b/client/src/controllers/FocusController.ts
index c76d69e70b..b0ed73b193 100644
--- a/client/src/controllers/FocusController.ts
+++ b/client/src/controllers/FocusController.ts
@@ -1,33 +1,56 @@
 import { Controller } from '@hotwired/stimulus';
 
+import { forceFocus } from '../utils/forceFocus';
+
 /**
- * Appears at the top left corner of the admin page with the tab button is clicked.
+ * Allows a target element (either via `href` or the targetValue as a selector) to be focused.
+ * If the element does not have a `tabindex` attribute, it will be added and removed when the element loses focus.
+ *
+ * @description
+ * Useful for the skip link functionality, which appears at the top left corner of the admin when the tab button is clicked.
  * Used to provide an accessible skip button for keyboard control so that users can
  * easily navigate to the main content without having to navigate a long list of navigation links.
- *
  * Inspired by https://github.com/selfthinker/dokuwiki_template_writr/blob/master/js/skip-link-focus-fix.js
+ *
+ * @example As an accessible skip link
+ * ```html
+ * <a href="#main" data-controller="w-focus" data-action="w-focus#focus:prevent">Skip to main content</a>
+ * ```
+ *
+ * @example As a button to skip to the top of a section
+ * ```html
+ * <section>
+ *   <div class="section-top"><h3>Section title</h3></div>
+ *   <p>...lots of content...</p>
+ *   <button type="button" data-controller="w-focus" data-w-focus-target-value=".section-top" data-action="w-focus#focus">Skip to top</button>
+ * </section>
+ * ```
  */
-export class SkipLinkController extends Controller<HTMLAnchorElement> {
-  skipToTarget?: HTMLElement | null;
+export class FocusController extends Controller<
+  HTMLAnchorElement | HTMLButtonElement
+> {
+  static values = {
+    target: String,
+  };
 
-  connect() {
-    this.skipToTarget = document.querySelector(
-      this.element.getAttribute('href') || 'main',
+  declare targetValue: string;
+
+  get target() {
+    const selector =
+      this.targetValue || this.element.getAttribute('href') || 'main';
+
+    return (
+      this.element.closest<HTMLElement>(selector) ||
+      document.querySelector<HTMLElement>(selector)
     );
   }
 
-  handleBlur() {
-    if (!this.skipToTarget) return;
-    this.skipToTarget.removeAttribute('tabindex');
-    this.skipToTarget.removeEventListener('blur', this.handleBlur);
-    this.skipToTarget.removeEventListener('focusout', this.handleBlur);
-  }
+  focus() {
+    const target = this.target;
 
-  skip() {
-    if (!this.skipToTarget) return;
-    this.skipToTarget.setAttribute('tabindex', '-1');
-    this.skipToTarget.addEventListener('blur', this.handleBlur);
-    this.skipToTarget.addEventListener('focusout', this.handleBlur);
-    this.skipToTarget.focus();
+    if (target) {
+      forceFocus(target);
+      this.dispatch('focused', { bubbles: true, cancelable: false, target });
+    }
   }
 }
diff --git a/client/src/controllers/index.ts b/client/src/controllers/index.ts
index 66b8cbdb44..17acb01256 100644
--- a/client/src/controllers/index.ts
+++ b/client/src/controllers/index.ts
@@ -12,6 +12,7 @@ import { DialogController } from './DialogController';
 import { DismissibleController } from './DismissibleController';
 import { DrilldownController } from './DrilldownController';
 import { DropdownController } from './DropdownController';
+import { FocusController } from './FocusController';
 import { FormsetController } from './FormsetController';
 import { InitController } from './InitController';
 import { KeyboardController } from './KeyboardController';
@@ -22,7 +23,6 @@ import { ProgressController } from './ProgressController';
 import { RevealController } from './RevealController';
 import { RulesController } from './RulesController';
 import { SessionController } from './SessionController';
-import { SkipLinkController } from './SkipLinkController';
 import { SlugController } from './SlugController';
 import { SubmitController } from './SubmitController';
 import { SwapController } from './SwapController';
@@ -51,6 +51,7 @@ export const coreControllerDefinitions: Definition[] = [
   { controllerConstructor: DismissibleController, identifier: 'w-dismissible' },
   { controllerConstructor: DrilldownController, identifier: 'w-drilldown' },
   { controllerConstructor: DropdownController, identifier: 'w-dropdown' },
+  { controllerConstructor: FocusController, identifier: 'w-focus' },
   { controllerConstructor: FormsetController, identifier: 'w-formset' },
   { controllerConstructor: InitController, identifier: 'w-init' },
   { controllerConstructor: KeyboardController, identifier: 'w-kbd' },
@@ -62,7 +63,6 @@ export const coreControllerDefinitions: Definition[] = [
   { controllerConstructor: RevealController, identifier: 'w-reveal' },
   { controllerConstructor: RulesController, identifier: 'w-rules' },
   { controllerConstructor: SessionController, identifier: 'w-session' },
-  { controllerConstructor: SkipLinkController, identifier: 'w-skip-link' },
   { controllerConstructor: SlugController, identifier: 'w-slug' },
   { controllerConstructor: SubmitController, identifier: 'w-submit' },
   { controllerConstructor: SwapController, identifier: 'w-swap' },