Skip to content

Commit b2c5aa0

Browse files
authored
fix(golden-layout): DH-22431: preserve focus indicator on nested dashboard tab (#2664)
- When a dashboard is nested inside another dashboard, focusin events bubble from the inner content up to the outer container's contentElement. The outer Tab's focusin handler would clear the inner tab's lm_focusin class and claim focus for itself, leaving the active tab indicator in the wrong place. - _onTabContentFocusIn now ignores events whose target lives inside a nested LayoutManager. _onTabClick treats focus inside a nested layout as 'not in this container' so clicking the outer tab properly transfers the indicator back out. - Tested with the following deeply nested layout: ``` from deephaven import ui @ui.component def deeply_nested_dashboard_component(): """A dashboard nested inside a panel, which is inside another dashboard.""" return ui.panel( ui.dashboard( ui.row( ui.panel(ui.text("Content Level 1"), title="Level 1"), ui.panel( ui.dashboard( ui.row( ui.panel(ui.text("Content Level 2"), title="Level 2"), ui.panel(ui.text("Deepest Content"), title="Deepest Panel"), ) ), title="Nested Dashboard Container", ), ) ), title="Outer Dashboard", ) ui_deeply_nested_dashboard = deeply_nested_dashboard_component() ```
1 parent 49781a7 commit b2c5aa0

2 files changed

Lines changed: 153 additions & 12 deletions

File tree

packages/golden-layout/src/__tests__/tab.test.ts

Lines changed: 104 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,53 @@
1+
import $ from 'jquery';
12
import LayoutManager from '../LayoutManager';
2-
import { Stack } from '../items';
3-
import { createLayout, cleanupLayout } from '../test-utils/testUtils';
3+
import type { ItemContainer } from '../container';
4+
import { Component, Stack } from '../items';
5+
import {
6+
createLayout,
7+
cleanupLayout,
8+
waitForLayoutInit,
9+
} from '../test-utils/testUtils';
10+
11+
class FocusableComponent {
12+
constructor(container: ItemContainer) {
13+
container.getElement().html('<button type="button">focus target</button>');
14+
}
15+
}
16+
17+
class NestedDashboardComponent {
18+
innerLayout: LayoutManager;
19+
20+
constructor(container: ItemContainer) {
21+
const nestedHost = document.createElement('div');
22+
nestedHost.style.width = '100%';
23+
nestedHost.style.height = '100%';
24+
container.getElement().append(nestedHost);
25+
26+
this.innerLayout = new LayoutManager(
27+
{
28+
content: [
29+
{
30+
type: 'stack',
31+
content: [
32+
{
33+
type: 'component',
34+
componentName: 'innerComponent',
35+
},
36+
],
37+
},
38+
],
39+
},
40+
nestedHost
41+
);
42+
43+
this.innerLayout.registerComponent('innerComponent', FocusableComponent);
44+
this.innerLayout.init();
45+
46+
container.on('destroy', () => {
47+
this.innerLayout.destroy();
48+
});
49+
}
50+
}
451

552
describe('tabs apply their configuration', () => {
653
let layout: LayoutManager | null = null;
@@ -73,4 +120,59 @@ describe('tabs apply their configuration', () => {
73120
(header.tabs[1] as unknown as { _dragListener: unknown })._dragListener
74121
).not.toBeDefined();
75122
});
123+
124+
it('keeps focus styling on the nested dashboard tab', async () => {
125+
const container = document.createElement('div');
126+
container.style.width = '800px';
127+
container.style.height = '600px';
128+
document.body.appendChild(container);
129+
130+
layout = new LayoutManager(
131+
{
132+
content: [
133+
{
134+
type: 'stack',
135+
content: [
136+
{
137+
type: 'component',
138+
componentName: 'nestedDashboard',
139+
},
140+
],
141+
},
142+
],
143+
},
144+
container
145+
);
146+
147+
layout.registerComponent('nestedDashboard', NestedDashboardComponent);
148+
layout.init();
149+
await waitForLayoutInit(layout);
150+
151+
const outerStack = layout.root.contentItems[0] as Stack;
152+
const outerTab = outerStack.header.tabs[0];
153+
const nestedLayoutHost = outerStack.contentItems[0] as Component;
154+
const nestedComponent =
155+
nestedLayoutHost.instance as NestedDashboardComponent;
156+
await waitForLayoutInit(nestedComponent.innerLayout);
157+
158+
const innerStack = nestedComponent.innerLayout.root
159+
.contentItems[0] as Stack;
160+
const innerTab = innerStack.header.tabs[0];
161+
const innerComponent = innerStack.contentItems[0] as Component;
162+
const focusTarget = innerComponent.container
163+
.getElement()
164+
.find('button')[0] as HTMLButtonElement;
165+
166+
focusTarget.focus();
167+
168+
expect(innerTab.element.hasClass('lm_focusin')).toBe(true);
169+
expect(outerTab.element.hasClass('lm_focusin')).toBe(false);
170+
171+
// jsdom doesn't implement scrollIntoView, which Tab._onTabClick calls.
172+
Element.prototype.scrollIntoView = jest.fn();
173+
outerTab.element.trigger($.Event('click', { button: 0 }));
174+
175+
expect(outerTab.element.hasClass('lm_focusin')).toBe(true);
176+
expect(innerTab.element.hasClass('lm_focusin')).toBe(false);
177+
});
76178
});

packages/golden-layout/src/controls/Tab.ts

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -170,11 +170,6 @@ export default class Tab {
170170
this.closeElement.off('click', this._onCloseClick);
171171
if (isComponent(this.contentItem)) {
172172
this.contentItem.container._contentElement.off();
173-
this.contentItem.container._contentElement[0].removeEventListener(
174-
'click',
175-
this._onTabContentFocusIn,
176-
true
177-
);
178173
}
179174
if (this._dragListener) {
180175
this.contentItem.off(
@@ -216,7 +211,20 @@ export default class Tab {
216211
/**
217212
* Callback when the contentItem is focused in
218213
*/
219-
_onTabContentFocusIn() {
214+
_onTabContentFocusIn(event?: JQuery.TriggeredEvent) {
215+
// If the focus originated from inside a nested LayoutManager (a
216+
// dashboard within a dashboard), let that nested layout's tab own
217+
// the focus indicator instead of this outer tab. The focusin event
218+
// bubbles, so without this check the outer tab would clear the
219+
// nested tab's "lm_focusin" class and claim focus for itself.
220+
if (
221+
isComponent(this.contentItem) &&
222+
event?.target instanceof Element &&
223+
this._isInNestedLayout(event.target)
224+
) {
225+
return;
226+
}
227+
220228
// Ensure only one tab is marked as having focus at a time.
221229
// In Firefox, if the focused element is removed from the DOM,
222230
// the focusout event won't trigger. This can result in two tabs
@@ -265,12 +273,13 @@ export default class Tab {
265273
this.header.parent.setActiveContentItem(this.contentItem);
266274
} else if (
267275
isComponent(this.contentItem) &&
268-
!this.contentItem.container._contentElement[0].contains(
269-
document.activeElement
270-
)
276+
!this._isFocusDirectlyInContainer()
271277
) {
272278
// if no focus inside put focus onto the container
273-
// so focusin always fires for tabclicks
279+
// so focusin always fires for tabclicks. Focus inside a nested
280+
// LayoutManager is not considered "inside" this container so
281+
// that clicking the outer tab transfers focus (and the
282+
// lm_focusin indicator) to it.
274283
this.contentItem.container._contentElement.focus();
275284
}
276285

@@ -330,4 +339,34 @@ export default class Tab {
330339
_onCloseMousedown(event: Event) {
331340
event.stopPropagation();
332341
}
342+
343+
/**
344+
* Returns true if the given element is inside this content item's
345+
* container but within a nested LayoutManager (i.e. a dashboard
346+
* embedded inside this tab's component). Such focus belongs to the
347+
* nested layout's tab, not this outer one.
348+
*/
349+
private _isInNestedLayout(element: Element): boolean {
350+
const nestedLayoutRoot = $(element).closest('.lm_goldenlayout')[0];
351+
const ownLayoutRoot = this._layoutManager.root.element[0];
352+
return nestedLayoutRoot != null && nestedLayoutRoot !== ownLayoutRoot;
353+
}
354+
355+
/**
356+
* Returns true if the document's active element is inside this tab's
357+
* container directly (not inside a nested LayoutManager).
358+
*/
359+
private _isFocusDirectlyInContainer(): boolean {
360+
if (!isComponent(this.contentItem)) {
361+
return false;
362+
}
363+
const active = document.activeElement;
364+
if (
365+
active == null ||
366+
!this.contentItem.container._contentElement[0].contains(active)
367+
) {
368+
return false;
369+
}
370+
return !this._isInNestedLayout(active);
371+
}
333372
}

0 commit comments

Comments
 (0)