From 231ac4b67b4f3dd9c9f9cbff1fde5b5bdf7c7751 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Fri, 30 Oct 2026 23:56:20 -0700 Subject: [PATCH] fix(ui): return Escape focus to the rail item and read rail direction from the panel A desktop side panel hides once collapsed, so Escape hands focus to its visible rail item instead of the hidden disclosure. Rail selection reads the panel's collapsed class rather than the store, which follows it a microtask late. --- src/panelEscape.test.mjs | 39 +++++++++++++++++++++++++++++++-- src/ui/panelChrome.js | 16 ++++++++++++++ src/ui/panelDisclosure.js | 13 +++++++++-- src/ui/panelNavigation.js | 6 ++++- src/ui/panelNavigation.test.mjs | 12 ++++++++++ 5 files changed, 81 insertions(+), 5 deletions(-) diff --git a/src/panelEscape.test.mjs b/src/panelEscape.test.mjs index b906ecf67..0f4a1ca6c 100644 --- a/src/panelEscape.test.mjs +++ b/src/panelEscape.test.mjs @@ -42,7 +42,7 @@ function panelFixture({ id = 'data-panel', nested = false, nestedCollapsed = fal disclosure.closest = () => panel; } const document = { getElementById: (candidate) => candidate === id ? panel : null }; - const manager = new Function('document', 'collapsePanelOnEscape', `return {${method('_collapsePanelOnEscape', '_initCommandDockPins')}};`)(document, collapsePanelOnEscape); + const manager = new Function('document', 'getComputedStyle', 'collapsePanelOnEscape', `return {${method('_collapsePanelOnEscape', '_initCommandDockPins')}};`)(document, (el) => ({ visibility: el.visibility ?? 'visible' }), collapsePanelOnEscape); manager.setPanelCollapsed = (candidate, value, options) => { assert.equal(candidate, id); assert.equal(value, true); @@ -55,7 +55,7 @@ function panelFixture({ id = 'data-panel', nested = false, nestedCollapsed = fal stopPropagation() { this.propagationStopped = true; }, }; return { - manager, panel, inner, target, event, + manager, panel, inner, target, event, document, collapsed: () => collapsed, focused: () => focused, blurred: () => blurred, @@ -71,6 +71,41 @@ test('Escape collapses an expanded panel and returns focus to its disclosure', ( assert.equal(fixture.event.propagationStopped, true); }); +test('Escape hands focus to the rail item when collapsing hides the panel', () => { + const fixture = panelFixture(); + let railFocused = false; + const railItem = { + disabled: false, + getClientRects: () => [{}], + focus: () => { railFocused = true; }, + }; + fixture.panel.getClientRects = () => (fixture.collapsed() ? [] : [{}]); + fixture.document.querySelector = (selector) => { + assert.equal(selector, '.ui-rail [data-rail-item="data-panel"]'); + return railItem; + }; + assert.equal(fixture.manager._collapsePanelOnEscape(fixture.event, 'data-panel'), true); + assert.equal(fixture.collapsed(), true); + assert.equal(railFocused, true); + assert.equal(fixture.focused(), false, 'not the hidden disclosure'); +}); + +test('Escape keeps the disclosure path when the rail is hidden', () => { + const fixture = panelFixture(); + let railFocused = false; + const railItem = { + disabled: false, + visibility: 'hidden', + getClientRects: () => [{}], + focus: () => { railFocused = true; }, + }; + fixture.panel.getClientRects = () => (fixture.collapsed() ? [] : [{}]); + fixture.document.querySelector = () => railItem; + assert.equal(fixture.manager._collapsePanelOnEscape(fixture.event, 'data-panel'), true); + assert.equal(railFocused, false); + assert.equal(fixture.focused(), true); +}); + test('Escape on an expanded panel disclosure closes it without retaining focus', () => { const fixture = panelFixture({ onDisclosure: true }); assert.equal(fixture.manager._collapsePanelOnEscape(fixture.event, 'data-panel'), true); diff --git a/src/ui/panelChrome.js b/src/ui/panelChrome.js index 8f8bd219f..cb2363ebd 100644 --- a/src/ui/panelChrome.js +++ b/src/ui/panelChrome.js @@ -163,6 +163,22 @@ export class PanelChrome { this._locationSearch.value = ''; this._locationSearch.blur(); }, + // A desktop side panel hides entirely once collapsed; its rail item + // takes the focus its own disclosure can no longer hold. + returnFocusTo: () => { + const panel = document.getElementById(panelId); + if (!panel?.getClientRects || panel.getClientRects().length > 0) + return null; + const item = document.querySelector?.( + `.ui-rail [data-rail-item="${panelId}"]`, + ); + return item && + !item.disabled && + item.getClientRects().length > 0 && + getComputedStyle(item).visibility === 'visible' + ? item + : null; + }, }); } diff --git a/src/ui/panelDisclosure.js b/src/ui/panelDisclosure.js index ce6b88e17..42715bd50 100644 --- a/src/ui/panelDisclosure.js +++ b/src/ui/panelDisclosure.js @@ -39,10 +39,14 @@ export function bindPanelDisclosure({ return { destroy: () => scope.destroy() }; } -/** Close the nearest expanded panel and return focus to its disclosure. */ +/** + * Close the nearest expanded panel and return focus to its disclosure, or to + * `returnFocusTo()` when that names a control (a rail item standing in for a + * panel that collapsing hid). + */ export function collapsePanelOnEscape( event, - { panel, onChange, beforeCollapse }, + { panel, onChange, beforeCollapse, returnFocusTo }, ) { if (event.key !== 'Escape' || event.defaultPrevented) return false; if ( @@ -59,6 +63,11 @@ export function collapsePanelOnEscape( event.stopPropagation(); beforeCollapse?.(); onChange(true, { explicit: true }); + const stand = returnFocusTo?.(); + if (stand) { + stand.focus?.({ preventScroll: true }); + return true; + } const disclosure = panel.querySelector(`[data-dock-toggle-target="${panel.id}"]`) || panel.querySelector(`[data-collapse-target="${panel.id}"]`); diff --git a/src/ui/panelNavigation.js b/src/ui/panelNavigation.js index 5ef84c53a..e6fed5d64 100644 --- a/src/ui/panelNavigation.js +++ b/src/ui/panelNavigation.js @@ -95,7 +95,11 @@ export class PanelNavigation { */ select(panelId) { if (this._destroyed) return; - this._setPanelCollapsed(panelId, this._store.isOpen(panelId), { + // The panel itself, not the store: the store follows its class a + // microtask later, after a same-task setPanelCollapsed. + const panel = document.getElementById(panelId); + if (!panel) return; + this._setPanelCollapsed(panelId, !panel.classList.contains('collapsed'), { explicit: true, }); } diff --git a/src/ui/panelNavigation.test.mjs b/src/ui/panelNavigation.test.mjs index c253bb03d..2eb999d3c 100644 --- a/src/ui/panelNavigation.test.mjs +++ b/src/ui/panelNavigation.test.mjs @@ -103,6 +103,18 @@ test('selecting an item toggles its panel explicitly', async () => { ui.destroy(); }); +test('selecting right after a same-task collapse reads the panel, not the store', async () => { + const { left, calls, ui, navigation } = setup(); + await nextTick(); + // A host collapses Data Layers and selects its item in one task, before + // the store follows the class change. + left.querySelector('#data-panel').classList.add('collapsed'); + navigation.select('data-panel'); + assert.deepEqual(calls, [['data-panel', false, { explicit: true }]]); + navigation.destroy(); + ui.destroy(); +}); + test('the rails follow disclosure and availability changes', async () => { const { left, right, ui, navigation } = setup(); document.getElementById('data-panel').classList.add('collapsed');