Fix popover focus-management gaps and add active-descendant support

.github/workflows/run-tests.yml

@@ -37,3 +37,33 @@ jobs:
 
       - name: Run tests
         run: yarn test
+
+  e2e:
+    name: UI Kit e2e tests
+    runs-on: ubuntu-22.04
+    permissions:
+      contents: read
+
+    steps:
+      - name: Checkout Repository
+        uses: actions/checkout@v4
+
+      - name: Setup node
+        uses: actions/setup-node@v3
+        with:
+          node-version: 22.1.0
+
+      - name: Install dependencies
+        run: yarn install
+
+      - name: Build utils
+        run: yarn build
+
+      - name: Typecheck e2e suite
+        run: yarn typecheck:e2e
+
+      - name: Install Playwright browsers
+        run: yarn workspace @editorjs/ui-kit playwright install --with-deps chromium webkit
+
+      - name: Run e2e tests
+        run: yarn workspace @editorjs/ui-kit test:e2e

.gitignore

@@ -9,3 +9,7 @@ node_modules/
 
 # IDE files
 .idea/
+
+# e2e artifacts
+test-results/
+playwright-report/

package.json

@@ -21,6 +21,7 @@
     "build:keyboard": "cd packages/keyboard && yarn build",
     "build:ui-kit": "cd packages/ui-kit && yarn build",
     "test": "vitest run",
+    "typecheck:e2e": "tsc -p packages/ui-kit/e2e/tsconfig.json",
     "lint": "eslint",
     "lint:fix": "yarn lint --fix",
     "generate-docs": "ts-node scripts/generateReadme.ts",

packages/dom/package.json

@@ -3,7 +3,7 @@
   "description": "Utils useful for work with dom for Editor.js tools development",
   "repository": "https://github.com/editor-js/utils/tree/main/packages/dom",
   "link": "https://github.com/editor-js/utils/tree/main/packages/dom",
-  "version": "1.1.0",
+  "version": "1.2.0",
   "main": "dist/index.js",
   "types": "dist/index.d.ts",
   "type": "module",

packages/dom/src/domIterator/domIterator.ts

@@ -39,6 +39,13 @@ export class DomIterator {
     this.focusedCssClass = focusedCssClass;
   }
 
+  /**
+   * Returns the list of items being iterated
+   */
+  public get allItems(): HTMLElement[] {
+    return this.items;
+  }
+
   /**
    * Returns Focused button Node
    */
@@ -55,11 +62,20 @@ export class DomIterator {
    * @param cursorPosition - new cursor position
    */
   public setCursor(cursorPosition: number): void {
-    if (cursorPosition < this.items.length && cursorPosition >= -1) {
+    if (cursorPosition >= this.items.length || cursorPosition < -1) {
+      return;
+    }
+
+    /** -1 is the 'nothing is active' position, there is no item to mark at it */
+    if (cursorPosition === -1) {
       this.dropCursor();
-      this.cursor = cursorPosition;
-      this.items[this.cursor].classList.add(this.focusedCssClass);
+
+      return;
     }
+
+    this.dropCursor();
+    this.cursor = cursorPosition;
+    this.items[this.cursor].classList.add(this.focusedCssClass);
   }
 
   /**

packages/dom/src/flipper/flipper.ts

@@ -38,6 +38,23 @@ export interface FlipperOptions {
    * Callback to set caret to the current element if possible. If not provided, caret is not set
    */
   setCaret?: (item: HTMLElement) => void;
+
+  /**
+   * If true, flipper moves real DOM focus to the current item and maintains a roving tabindex
+   * over the items: the current one becomes tabbable, the rest do not.
+   *
+   * Set it to true when the items are the thing the user is interacting with, e.g. a menu
+   * opened from a button (Block Tunes, Toolbox): screen readers only announce the element that
+   * holds the real focus, so with a highlight alone the navigation is silent for them.
+   *
+   * Leave it false when the focus has to stay somewhere else while the items are navigated,
+   * e.g. a toolbar acting on a text selection (Inline Toolbar): focusing a button drops the
+   * selection in Safari, and the formatting would have nothing left to apply to. Such a
+   * consumer can expose the highlighted item via aria-activedescendant instead.
+   *
+   * Off by default, so that the existing consumers keep their focus and caret management intact
+   */
+  focusItems?: boolean;
 }
 
 /**
@@ -81,6 +98,12 @@ export class Flipper {
    */
   private setCaret?: (item: HTMLElement) => void;
 
+  /**
+   * True if flipper should move real DOM focus to the current item
+   * @see FlipperOptions.focusItems for when either value is expected
+   */
+  private readonly focusItems: boolean;
+
   /**
    * @param options - different constructing settings
    */
@@ -89,6 +112,7 @@ export class Flipper {
     this.activateCallback = options.activateCallback;
     this.allowedKeys = options.allowedKeys || Flipper.usedKeys;
     this.setCaret = options.setCaret;
+    this.focusItems = options.focusItems === true;
   }
 
   /**
@@ -121,6 +145,7 @@ export class Flipper {
 
     if (cursorPosition !== undefined) {
       this.iterator.setCursor(cursorPosition);
+      this.updateFocus();
     }
 
     /**
@@ -141,8 +166,13 @@ export class Flipper {
   public deactivate(): void {
     this.activated = false;
     this.dropCursor();
+    this.resetFocus();
 
-    document.removeEventListener('keydown', this.onKeyDown);
+    /**
+     * Capturing flag has to match the one the listener was added with, otherwise nothing is
+     * removed and the handler stays on the document for the lifetime of the page
+     */
+    document.removeEventListener('keydown', this.onKeyDown, true);
   }
 
   /**
@@ -169,6 +199,22 @@ export class Flipper {
     this.flipCallback();
   }
 
+  /**
+   * Items the flipper currently navigates between.
+   * Not necessarily the full list it was constructed with: consumers narrow it down by
+   * re-activating the flipper with a subset, for example while a search filter is applied
+   */
+  public get currentItems(): HTMLElement[] {
+    return this.iterator.allItems;
+  }
+
+  /**
+   * Item the cursor currently points at, null when no item is focused
+   */
+  public get currentItem(): HTMLElement | null {
+    return this.iterator.currentItem;
+  }
+
   /**
    * Return true if some button is focused
    */
@@ -298,10 +344,44 @@ export class Flipper {
     }
   }
 
+  /**
+   * Moves real DOM focus to the current item and makes it the only tabbable one.
+   * Does nothing unless the flipper is constructed with the 'focusItems' option
+   */
+  private updateFocus(): void {
+    if (!this.focusItems) {
+      return;
+    }
+
+    const currentItem = this.iterator.currentItem;
+
+    this.iterator.allItems.forEach((item) => {
+      item.tabIndex = item === currentItem ? 0 : -1;
+    });
+
+    /** Scrolling is handled separately, right after the flip */
+    currentItem?.focus({ preventScroll: true });
+  }
+
+  /**
+   * Makes all the items untabbable. Called once the flipper is deactivated,
+   * so that items of a closed popover do not catch the Tab key
+   */
+  private resetFocus(): void {
+    if (!this.focusItems) {
+      return;
+    }
+
+    this.iterator.allItems.forEach((item) => {
+      item.tabIndex = -1;
+    });
+  }
+
   /**
    * Fired after flipping in any direction
    */
   private flipCallback(): void {
+    this.updateFocus();
     this.setCaretToCurrentItem();
 
     if (this.iterator.currentItem) {

packages/ui-kit/e2e/fixtures/confirmation-toggle.html

@@ -0,0 +1,55 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <title>UI Kit e2e — confirmation toggle</title>
+</head>
+<body>
+<div style="position: relative; width: 500px; padding: 12px;">
+  <button id="before">Before</button>
+  <div style="position: relative;">
+    <button id="trigger">Open menu</button>
+  </div>
+</div>
+
+<script type="module">
+  import { PopoverDesktop } from '/src/index.ts';
+
+  const icon = '<svg width="16" height="16" viewBox="0 0 16 16"><circle cx="8" cy="8" r="6"/></svg>';
+
+  window.__activated = [];
+
+  /**
+   * Records item activation so tests can assert it without relying on visuals
+   * @param {string} name - name of the activated item
+   */
+  const activate = (name) => () => window.__activated.push(name);
+
+  const popover = new PopoverDesktop({
+    scopeElement: document.body,
+    items: [
+      {
+        icon,
+        title: 'Pin',
+        name: 'pin',
+        /** A toggle that asks for a confirmation too: clicking both toggles it and swaps it into confirmation mode */
+        toggle: true,
+        onActivate: activate('pin'),
+        confirmation: {
+          icon,
+          title: 'Sure?',
+          name: 'pin-confirm',
+          onActivate: activate('pin-confirm'),
+        },
+      },
+    ],
+  });
+
+  document.getElementById('trigger').insertAdjacentElement('afterend', popover.getElement());
+  document.getElementById('trigger').addEventListener('click', () => popover.show());
+
+  window.popover = popover;
+  document.body.dataset.ready = 'true';
+</script>
+</body>
+</html>

packages/ui-kit/e2e/fixtures/html-items.html

@@ -0,0 +1,88 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <title>UI Kit e2e — html items</title>
+</head>
+<body>
+<div style="position: relative; width: 500px; padding: 12px;">
+  <button id="before">Before</button>
+  <div style="position: relative;">
+    <button id="trigger">Open menu</button>
+  </div>
+</div>
+
+<script type="module">
+  import { PopoverDesktop, PopoverEvent, PopoverItemType } from '/src/index.ts';
+
+  const icon = '<svg width="16" height="16" viewBox="0 0 16 16"><circle cx="8" cy="8" r="6"/></svg>';
+
+  window.__activated = [];
+
+  /**
+   * Records item activation so tests can assert it without relying on visuals
+   * @param {string} name - name of the activated item
+   */
+  const activate = (name) => () => window.__activated.push(name);
+
+  /**
+   * Html item that opens a submenu, as a legacy inline tool with actions does in Editor.js
+   */
+  const moreElement = document.createElement('div');
+
+  moreElement.innerHTML = '<button>More</button>';
+
+  /**
+   * Html item holding the native controls a custom tune may render: a checkbox, a select, a link
+   */
+  const controlsElement = document.createElement('div');
+
+  controlsElement.innerHTML = '<label><input type="checkbox"> Check</label>'
+    + '<select aria-label="Pick"><option>One</option></select>'
+    + '<a href="#link">Link</a>';
+
+  window.__activeDescendants = [];
+
+  const popover = new PopoverDesktop({
+    scopeElement: document.body,
+    items: [
+      {
+        icon,
+        title: 'Plain',
+        name: 'plain',
+        onActivate: activate('plain'),
+      },
+      {
+        type: PopoverItemType.Html,
+        element: moreElement,
+        name: 'html-parent',
+        children: {
+          items: [
+            {
+              icon,
+              title: 'Child A',
+              name: 'child-a',
+              onActivate: activate('child-a'),
+            },
+          ],
+        },
+      },
+      {
+        type: PopoverItemType.Html,
+        element: controlsElement,
+        name: 'html-controls',
+      },
+    ],
+  });
+
+  /** Every id the popover reports the highlighted item with, in order */
+  popover.on(PopoverEvent.ActiveDescendantChanged, id => window.__activeDescendants.push(id));
+
+  document.getElementById('trigger').insertAdjacentElement('afterend', popover.getElement());
+  document.getElementById('trigger').addEventListener('click', () => popover.show());
+
+  window.popover = popover;
+  document.body.dataset.ready = 'true';
+</script>
+</body>
+</html>