feedback7 G-UI: the import dialog traps keyboard focus and yields while the guide is open (PR #341 by @Xinglan233)

Applied as-is. The shared Modal keeps Tab inside the topmost dialog, pulls focus back if it leaves, skips disabled, hidden, inert and zero-size controls, and does not trap while a .guide overlay is open. Escape still closes the dialog. Enter is unchanged, so a focused button keeps its native click.

Tests: node --test test/ui/modal.e2e.test.js test/ui/loadout.e2e.test.js test/ui/leftovers.test.js test/ui/match-info.test.js test/ui/loadout.test.js test/ui/playtest3.test.js test/ui/playtest4.test.js test/ui/uiFixes.test.js test/ui/localArt.test.js — 114 pass, 0 fail. The modal and loadout e2e suites skipped because SP_E2E is unset. eslint on the touched files: 0 errors.

public/js/ui/components.js

@@ -386,7 +386,7 @@ export function Countdown({ deadline, seconds, total, warnAt = 10, label = 'COUN
 
 // ---- Modal & dialogs ---------------------------------------------------------------------------
 
-const modalStack = []; // open modals, topmost last: only the topmost reacts to Escape
+const modalStack = []; // open modals, topmost last: only the topmost handles keyboard focus / Escape
 
 /**
  * Modal dialog (declarative). Esc / backdrop click call onClose.
@@ -397,24 +397,46 @@ export function Modal({ open, title, micro, tone = 'mint', onClose, actions, wid
   const boxRef = useRef(null);
   const closeRef = useRef(onClose);
   closeRef.current = onClose;
-  useEffect(() => {
+  useLayoutEffect(() => {
     if (!open) return undefined;
     const token = {};
     modalStack.push(token);
     const prevFocus = typeof document !== 'undefined' ? document.activeElement : null;
+    const box = boxRef.current;
+    // The independent guide can open above SettingsModal (its z-index is higher than --z-modal).
+    const topmost = () => modalStack[modalStack.length - 1] === token && !document.querySelector('.guide');
+    const available = (el) => !el.matches(':disabled') && !el.closest('[inert]') && el.getClientRects().length > 0
+      && getComputedStyle(el).visibility !== 'hidden';
+    const tabbable = () => [...box.querySelectorAll('button, [href], input, select, textarea, [tabindex], summary')]
+      .filter((el) => el.tabIndex >= 0 && available(el));
+    const focusFirst = () => (tabbable()[0] || box).focus();
     const onKey = (e) => {
-      if (e.key !== 'Escape' || modalStack[modalStack.length - 1] !== token || !closeRef.current) return;
-      e.stopPropagation();
-      closeRef.current();
+      if (!topmost()) return;
+      if (e.key === 'Escape' && closeRef.current) {
+        e.stopPropagation();
+        closeRef.current();
+      } else if (e.key === 'Tab') {
+        const els = tabbable();
+        const first = els[0] || box, last = els[els.length - 1] || box;
+        if (!els.includes(document.activeElement) || document.activeElement === (e.shiftKey ? first : last)) {
+          e.preventDefault();
+          (e.shiftKey ? last : first).focus();
+        }
+      }
+    };
+    const onFocus = (e) => {
+      if (topmost() && !box.contains(e.target)) focusFirst();
     };
     window.addEventListener('keydown', onKey);
-    const t = setTimeout(() => {
-      const el = boxRef.current?.querySelector('[data-autofocus]') || boxRef.current?.querySelector('button, input');
-      el?.focus?.();
-    }, 30);
+    window.addEventListener('focusin', onFocus);
+    const autofocus = box.querySelector('[data-autofocus]');
+    if (topmost()) {
+      if (autofocus && available(autofocus)) autofocus.focus();
+      else focusFirst();
+    }
     return () => {
       window.removeEventListener('keydown', onKey);
-      clearTimeout(t);
+      window.removeEventListener('focusin', onFocus);
       const i = modalStack.indexOf(token);
       if (i >= 0) modalStack.splice(i, 1);
       prevFocus?.focus?.();
@@ -423,7 +445,7 @@ export function Modal({ open, title, micro, tone = 'mint', onClose, actions, wid
   if (!open) return null;
   return html`<div class="modal" role="presentation"
       onMouseDown=${(e) => { if (closeOnBackdrop && e.target === e.currentTarget && onClose) onClose(); }}>
-    <div ref=${boxRef} class=${cx('modal__box', 'brackets', `modal__box--${tone}`, cls)} role="dialog" aria-modal="true"
+    <div ref=${boxRef} class=${cx('modal__box', 'brackets', `modal__box--${tone}`, cls)} role="dialog" aria-modal="true" tabindex="-1"
          style=${width ? `width:${width}` : undefined}>
       <div class="modal__stripe" aria-hidden="true"></div>
       ${title || micro ? html`<header class="modal__head">

test/ui/loadout.e2e.test.js

@@ -134,6 +134,64 @@ describe('干员调配 overlay (real server, headless Chrome)', { skip: !ENABLED
     await ctx.close();
   });
 
+  test('import: Tab stays in the dialog and Enter cannot edit a background module', async () => {
+    const { ctx, page, problems } = await open();
+    try {
+      await clickSel(page, '.lobby-screen [data-testid="loadout-open"]');
+      await page.waitForSelector('.lo-mod.is-on');
+      const module = () => page.$eval('.lo-mod.is-on', (el) => el.dataset.module);
+      const original = await module();
+      const saved = () => page.evaluate(() => JSON.parse(localStorage.getItem('sp.pref.loadout') || '{}').entries || {});
+      const openImport = async () => {
+        await clickSel(page, '[data-testid="loadout-import"]');
+        await page.waitForSelector('.modal textarea');
+        await page.waitForFunction(() => !document.querySelector('.modal__box').getAnimations().some((a) => a.playState === 'running'));
+      };
+      const focused = () => page.evaluate(() => document.activeElement.getAttribute('data-testid'));
+      await openImport();
+      await page.click('.modal textarea');
+      await page.keyboard.down('Shift');
+      await page.keyboard.press('Tab');
+      await page.keyboard.up('Shift');
+      assert.equal(await focused(), 'loadout-io-pick', 'Shift+Tab wraps past the disabled import and hidden file input');
+      const chooser = page.waitForFileChooser();
+      await page.keyboard.press('Enter');
+      await (await chooser).cancel();
+      assert.ok(await page.$('.modal'), 'Enter acts on the file picker inside the dialog');
+      assert.equal(await module(), original, 'the background module cannot be activated');
+      assert.deepEqual(await saved(), {});
+      await page.keyboard.press('Tab');
+      assert.equal(await focused(), 'loadout-io-text', 'Tab wraps back to the textarea');
+      await page.keyboard.press('Tab');
+      assert.equal(await page.evaluate(() => document.activeElement.textContent.trim()), '取消');
+      await page.keyboard.press('Enter');
+      await page.waitForSelector('.modal', { hidden: true });
+      assert.equal(await focused(), 'loadout-import', 'cancel restores the opener');
+      assert.equal(await module(), original);
+      assert.deepEqual(await saved(), {});
+
+      await openImport();
+      await page.keyboard.press('Escape');
+      await page.waitForSelector('.modal', { hidden: true });
+      assert.ok(await page.$('.lo'), 'Esc closes only the import');
+      assert.equal(await focused(), 'loadout-import');
+
+      await openImport();
+      await page.click('.modal textarea');
+      await page.keyboard.type(JSON.stringify({ v: 1, kind: 'stronghold.loadout', entries: { [INSIDE]: { skill: 0 } } }));
+      await page.keyboard.down('Shift');
+      await page.keyboard.press('Tab');
+      await page.keyboard.up('Shift');
+      assert.equal(await focused(), 'loadout-io-apply', 'the enabled import becomes the last stop');
+      await page.keyboard.press('Enter');
+      await page.waitForSelector('.modal', { hidden: true });
+      assert.deepEqual(await saved(), { [INSIDE]: { skill: 0 } }, 'keyboard confirmation still imports');
+      assert.equal(await focused(), 'loadout-import');
+      // Optional art / fonts / audio can be absent; this keyboard regression needs no asset pack.
+      assert.deepEqual(problems.filter((p) => p.startsWith('pageerror:')), []);
+    } finally { await ctx.close(); }
+  });
+
   test('desktop: 导出 hands out a versioned payload; 导入 restores it and refuses junk', async () => {
     const { ctx, page, problems } = await open();
     await clickSel(page, '.lobby-screen [data-testid="loadout-open"]');

test/ui/modal.e2e.test.js

@@ -0,0 +1,168 @@
+// Shared Modal keyboard boundaries, real Chrome. Component fixtures cover nested / empty dialogs; the settings
+// case uses the real title screen. Opt-in: SP_E2E=1 node --test test/ui/modal.e2e.test.js
+import { test, describe, before, after } from 'node:test';
+import assert from 'node:assert/strict';
+import { existsSync } from 'node:fs';
+
+const CHROME = process.env.CHROME_PATH || '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome';
+const ENABLED = process.env.SP_E2E === '1' && existsSync(CHROME);
+
+describe('Modal keyboard focus', { skip: !ENABLED && 'set SP_E2E=1 and have Chrome to run' }, () => {
+  let srv, browser;
+  before(async () => {
+    const { startServer } = await import('../../server/index.js');
+    const puppeteer = (await import('puppeteer-core')).default;
+    srv = await startServer({ port: 0, host: '127.0.0.1', quiet: true });
+    browser = await puppeteer.launch({ executablePath: CHROME, headless: true, args: ['--no-sandbox', '--no-proxy-server'] });
+  });
+  after(async () => { await browser?.close(); await srv?.close(); });
+  const active = (page) => page.evaluate(() => document.activeElement.id);
+  const tab = async (page, reverse = false) => {
+    if (reverse) await page.keyboard.down('Shift');
+    await page.keyboard.press('Tab');
+    if (reverse) await page.keyboard.up('Shift');
+  };
+  async function fixture(page) {
+    page.setDefaultTimeout(5000);
+    await page.goto(`${srv.url}/dev/uikit.html`, { waitUntil: 'domcontentloaded' });
+    await page.evaluate(async () => {
+      const { render } = await import('/vendor/preact.module.js');
+      const { useState } = await import('/vendor/hooks.module.js');
+      const { html, Modal, DialogHost, confirmDialog } = await import('/js/ui/components.js');
+      function Fixture() {
+        const [outer, setOuter] = useState(false), [inner, setInner] = useState(false), [empty, setEmpty] = useState(false);
+        return html`<div>
+          <button id="open" onClick=${() => setOuter(true)}>Open</button>
+          <button id="empty-open" onClick=${() => setEmpty(true)}>Empty</button>
+          <button id="confirm" onClick=${async () => { const ok = await confirmDialog({ text: 'Confirm?' }); document.querySelector('#result').textContent = String(ok); }}>Confirm</button>
+          <output id="result"></output>
+          <${Modal} open=${outer} onClose=${() => setOuter(false)}>
+            <textarea id="first"></textarea>
+            <button disabled>Disabled</button><input type="hidden" /><button hidden>Hidden</button>
+            <button style="display:none">Display none</button><button style="visibility:hidden">Invisible</button>
+            <div inert><button>Inert</button></div><button tabindex="-1">Programmatic only</button>
+            <button id="inner-open" onClick=${() => setInner(true)}>Nested</button>
+            <a id="last" href="#inside">Last</a>
+          <//>
+          <${Modal} open=${inner} onClose=${() => setInner(false)}>
+            <button id="inner-first" data-autofocus>First</button><button id="inner-last">Last</button>
+          <//>
+          <${Modal} open=${empty} onClose=${() => setEmpty(false)}>No controls<//>
+          <${DialogHost} />
+        </div>`;
+      }
+      render(html`<${Fixture} />`, document.getElementById('kit'));
+    });
+  }
+
+  test('initial focus, both Tab boundaries, nested priority, recovery and cleanup', async () => {
+    const page = await browser.newPage();
+    try {
+      await fixture(page);
+      await page.click('#open');
+      await page.waitForFunction(() => document.activeElement.id === 'first');
+      await tab(page, true);
+      assert.equal(await active(page), 'last', 'hidden, disabled, inert and negative-tabindex controls are excluded');
+      await tab(page);
+      assert.equal(await active(page), 'first');
+      await tab(page);
+      assert.equal(await active(page), 'inner-open');
+      await page.keyboard.press('Enter');
+      await page.waitForFunction(() => document.activeElement.id === 'inner-first');
+      await tab(page, true);
+      assert.equal(await active(page), 'inner-last', 'only the topmost dialog traps focus');
+      await tab(page);
+      assert.equal(await active(page), 'inner-first');
+      await page.keyboard.press('Escape');
+      await page.waitForFunction(() => document.querySelectorAll('.modal').length === 1);
+      assert.equal(await active(page), 'inner-open');
+      await page.focus('#open');
+      assert.equal(await active(page), 'first', 'focus escaping to the background returns to the active dialog');
+      await page.keyboard.press('Escape');
+      await page.waitForSelector('.modal', { hidden: true });
+      assert.equal(await active(page), 'open');
+      await tab(page);
+      assert.equal(await active(page), 'empty-open', 'closed dialogs no longer trap focus');
+      await page.click('#open');
+      await page.waitForFunction(() => document.activeElement.id === 'first');
+      await tab(page, true);
+      assert.equal(await active(page), 'last', 'reopening does not leave stale listeners');
+    } finally { await page.close(); }
+  });
+
+  test('no controls: focus the dialog itself; imperative confirm retains autofocus and native Enter', async () => {
+    const page = await browser.newPage();
+    try {
+      await fixture(page);
+      await page.click('#empty-open');
+      await page.waitForFunction(() => document.activeElement.matches('.modal__box'));
+      for (const reverse of [false, true]) {
+        await tab(page, reverse);
+        assert.equal(await page.evaluate(() => document.activeElement.matches('.modal__box')), true);
+      }
+      await page.keyboard.press('Escape');
+      await page.waitForSelector('.modal', { hidden: true });
+      assert.equal(await active(page), 'empty-open');
+      await page.click('#confirm');
+      await page.waitForFunction(() => document.activeElement.hasAttribute('data-autofocus'));
+      await page.keyboard.press('Enter');
+      await page.waitForFunction(() => document.querySelector('#result').textContent === 'true');
+      assert.equal(await active(page), 'confirm');
+    } finally { await page.close(); }
+  });
+
+  test('title settings: Tab cycles and leaves hotkey capture; Esc cancels capture before closing', async () => {
+    const page = await browser.newPage();
+    try {
+      await page.setViewport({ width: 1280, height: 720 });
+      page.setDefaultTimeout(5000);
+      await page.goto(srv.url, { waitUntil: 'domcontentloaded' });
+      await page.waitForSelector('.title-settings');
+      await page.click('.title-settings');
+      await page.waitForFunction(() => document.querySelector('.modal__box')?.contains(document.activeElement));
+      const assertInside = async () => assert.equal(await page.evaluate(() => document.querySelector('.modal__box').contains(document.activeElement)), true);
+      for (const reverse of [false, true]) for (let i = 0; i < 25; i++) { await tab(page, reverse); await assertInside(); }
+      await page.focus('.set-key[data-action="retreat"]');
+      await page.keyboard.press('Enter');
+      await page.waitForSelector('.set-key.is-waiting');
+      await tab(page);
+      await page.waitForSelector('.set-key.is-waiting', { hidden: true });
+      await assertInside();
+      assert.equal(await page.$eval('.set-key[data-action="retreat"]', (el) => el.textContent.trim()), 'Q');
+      await page.focus('.set-key[data-action="retreat"]');
+      await page.keyboard.press('Enter');
+      await page.waitForSelector('.set-key.is-waiting');
+      await page.keyboard.press('Escape');
+      await page.waitForSelector('.set-key.is-waiting', { hidden: true });
+      assert.ok(await page.$('.modal'), 'capture consumes the first Esc');
+      await page.keyboard.press('Escape');
+      await page.waitForSelector('.modal', { hidden: true });
+      assert.equal(await page.evaluate(() => document.activeElement.matches('.title-settings')), true);
+      assert.ok(await page.$('.title-screen'), 'the background start button was not activated');
+    } finally { await page.close(); }
+  });
+
+  test('settings lets the higher guide keep keyboard focus and resumes after it closes', async () => {
+    const page = await browser.newPage();
+    try {
+      page.setDefaultTimeout(5000);
+      await page.goto(srv.url, { waitUntil: 'domcontentloaded' });
+      await page.waitForSelector('.title-settings');
+      await page.click('.title-settings');
+      await page.waitForSelector('.set-guide');
+      await page.waitForFunction(() => !document.querySelector('.modal__box').getAnimations().some((a) => a.playState === 'running'));
+      await page.click('.set-guide');
+      await page.waitForSelector('.guide');
+      await page.waitForFunction(() => document.querySelector('.guide').contains(document.activeElement));
+      await tab(page);
+      assert.equal(await page.evaluate(() => document.querySelector('.guide').contains(document.activeElement)), true);
+      await page.keyboard.press('Escape');
+      await page.waitForSelector('.guide', { hidden: true });
+      assert.ok(await page.$('.modal'), 'closing the guide keeps settings');
+      await tab(page);
+      assert.equal(await page.evaluate(() => document.querySelector('.modal__box').contains(document.activeElement)), true);
+      await page.click('.modal__actions button:last-child');
+      await page.waitForSelector('.modal', { hidden: true });
+    } finally { await page.close(); }
+  });
+});