Skip to content

Commit bcb5932

Browse files
committed
address review comments
1 parent 7d3fe4d commit bcb5932

17 files changed

+76
-41
lines changed

packages/component-base/src/dom-utils.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export function getClosestElement(selector, node) {
7474
return null;
7575
}
7676

77-
return node.closest(selector) || getClosestElement(selector, node.getRootNode().host);
77+
return node.closest?.(selector) || getClosestElement(selector, node.getRootNode().host);
7878
}
7979

8080
/**

packages/master-detail-layout/src/vaadin-master-detail-layout.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import { html, LitElement, nothing } from 'lit';
77
import { getFocusableElements } from '@vaadin/a11y-base/src/focus-utils.js';
88
import { defineCustomElement } from '@vaadin/component-base/src/define.js';
9+
import { getClosestElement } from '@vaadin/component-base/src/dom-utils.js';
910
import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js';
1011
import { PolylitMixin } from '@vaadin/component-base/src/polylit-mixin.js';
1112
import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
@@ -465,7 +466,7 @@ class MasterDetailLayout extends ElementMixin(ThemableMixin(PolylitMixin(LitElem
465466

466467
/** @private */
467468
get __ancestorLayouts() {
468-
const parent = this.parentElement && this.parentElement.closest(this.constructor.is);
469+
const parent = getClosestElement(this.constructor.is, this.parentNode);
469470
return parent ? [...parent.__ancestorLayouts, parent] : [];
470471
}
471472

packages/master-detail-layout/test/detail-auto-size.test.js

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from '@vaadin/chai-plugins';
2-
import { fixtureSync } from '@vaadin/testing-helpers';
2+
import { defineCE, fixtureSync } from '@vaadin/testing-helpers';
33
import sinon from 'sinon';
44
import '../src/vaadin-master-detail-layout.js';
55
import { onceResized } from './helpers.js';
@@ -61,21 +61,39 @@ describe('detail auto size', () => {
6161
return layout.style.getPropertyValue('--_detail-cached-size');
6262
}
6363

64+
const shadowElement = defineCE(
65+
class extends HTMLElement {
66+
constructor() {
67+
super();
68+
this.attachShadow({ mode: 'open' });
69+
this.shadowRoot.appendChild(this.querySelector('template').content);
70+
}
71+
},
72+
);
73+
6474
beforeEach(async () => {
6575
outer = fixtureSync(`
6676
<vaadin-master-detail-layout style="width: 1200px;" master-size="100px" expand="both">
6777
<div>Outer Master</div>
78+
6879
<vaadin-master-detail-layout slot="detail" master-size="100px">
6980
<div>Middle Master</div>
70-
<vaadin-master-detail-layout slot="detail" master-size="100px">
71-
<div>Inner Master</div>
72-
<div slot="detail" style="width: 100px;">Inner Detail</div>
73-
</vaadin-master-detail-layout>
81+
82+
<${shadowElement} slot="detail">
83+
<template>
84+
<vaadin-master-detail-layout master-size="100px">
85+
<div>Inner Master</div>
86+
87+
<div slot="detail" style="width: 100px;">Inner Detail</div>
88+
</vaadin-master-detail-layout>
89+
</template>
90+
</${shadowElement}>
7491
</vaadin-master-detail-layout>
7592
</vaadin-master-detail-layout>
7693
`);
7794
middle = outer.querySelector('vaadin-master-detail-layout');
78-
inner = middle.querySelector('vaadin-master-detail-layout');
95+
inner = middle.querySelector('[slot="detail"]').shadowRoot.querySelector('vaadin-master-detail-layout');
96+
7997
await onceResized(outer);
8098
await onceResized(middle);
8199
await onceResized(inner);
@@ -100,24 +118,22 @@ describe('detail auto size', () => {
100118
it('should update cached sizes on ancestors after detail content changes', () => {
101119
inner.querySelector('[slot="detail"]').style.width = '200px';
102120
inner.recalculateLayout();
103-
104121
expect(getCachedSize(inner)).to.equal('201px');
105122
expect(getCachedSize(middle)).to.equal('302px');
106123
expect(getCachedSize(outer)).to.equal('403px');
107124
});
108125

109126
it('should toggle overlay on ancestors when detail content outgrows or fits available space', () => {
110-
// Outer: 100px master + 303px detail = 403px, host is 1200px → no overlay
111-
expect(outer.hasAttribute('overlay')).to.be.false;
112-
113127
// Grow inner detail so outer needs 100px + 1103px = 1203px > 1200px
114128
inner.querySelector('[slot="detail"]').style.width = '1000px';
115129
inner.recalculateLayout();
130+
expect(middle.hasAttribute('overlay')).to.be.true;
116131
expect(outer.hasAttribute('overlay')).to.be.true;
117132

118133
// Shrink back so outer needs 100px + 303px = 403px < 1200px
119134
inner.querySelector('[slot="detail"]').style.width = '100px';
120135
inner.recalculateLayout();
136+
expect(middle.hasAttribute('overlay')).to.be.false;
121137
expect(outer.hasAttribute('overlay')).to.be.false;
122138
});
123139
});

packages/master-detail-layout/test/visual/base/master-detail-layout.test.js

Lines changed: 47 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -148,59 +148,77 @@ describe('master-detail-layout', () => {
148148
});
149149
});
150150

151-
describe('detail auto size', () => {
152-
let layouts;
151+
describe('nested layouts', () => {
152+
let outer, inner;
153153

154154
function openDetail(layout) {
155-
return layout._setDetail(layout.querySelector(':scope > [slot="detail-hidden"]'));
155+
// Set detail content and skip animation
156+
return layout._setDetail(layout.querySelector(':scope > [slot="detail-hidden"]'), true);
156157
}
157158

158159
beforeEach(async () => {
159-
mdl = fixtureSync(
160+
outer = fixtureSync(
160161
`
161-
<vaadin-master-detail-layout expand="detail" master-size="300px" no-animation style="max-width: calc(300px + 250px + 200px + 100px + 1px * 3); height: 400px; border: 1px solid lightgray;">
162-
<div>Master 0</div>
163-
<vaadin-master-detail-layout expand="detail" master-size="250px" no-animation slot="detail-hidden">
164-
<div>Master 1</div>
165-
<vaadin-master-detail-layout expand="detail" master-size="200px" no-animation slot="detail-hidden">
166-
<div>Master 2</div>
167-
<div slot="detail-hidden" style="min-width: 100px">Detail 2</div>
168-
</vaadin-master-detail-layout>
162+
<vaadin-master-detail-layout master-size="300px" style="height: 400px; border: 1px solid lightgray;">
163+
<div>
164+
Outer Master
165+
</div>
166+
<vaadin-master-detail-layout master-size="200px" slot="detail-hidden">
167+
<div>
168+
Inner Master
169+
</div>
170+
<div slot="detail-hidden" style="min-width: 100px">
171+
Inner Detail
172+
</div>
169173
</vaadin-master-detail-layout>
170174
</vaadin-master-detail-layout>
171175
`,
172176
div,
173177
);
174-
175-
layouts = [...div.querySelectorAll('vaadin-master-detail-layout')];
178+
inner = outer.querySelector('vaadin-master-detail-layout');
176179
await onceResized(div);
177180
});
178181

179182
it('default', async () => {
180-
await visualDiff(div, `detail-auto-size`);
183+
await visualDiff(div, `nested-layouts`);
181184
});
182185

183-
[0, 1, 2].forEach((depth) => {
184-
it(`opened (depth ${depth})`, async () => {
185-
for (const layout of layouts.slice(0, depth + 1)) {
186-
await openDetail(layout);
187-
}
186+
describe('outer opened', () => {
187+
beforeEach(async () => {
188+
await openDetail(outer);
189+
});
190+
191+
it('default', async () => {
192+
await visualDiff(div, `nested-layouts-outer-opened`);
193+
});
194+
195+
it('overflow', async () => {
196+
div.style.width = '400px';
188197
await onceResized(div);
189-
await visualDiff(div, `detail-auto-size-opened-${depth}`);
198+
await visualDiff(div, `nested-layouts-outer-opened-outer-overflow`);
190199
});
191200
});
192201

193-
['600px', '400px', '200px'].forEach((width, depth) => {
194-
it(`overflow (depth ${depth})`, async () => {
195-
div.style.width = width;
196-
await onceResized(div);
202+
describe('inner opened', () => {
203+
beforeEach(async () => {
204+
await openDetail(outer);
205+
await openDetail(inner);
206+
});
207+
208+
it('default', async () => {
209+
await visualDiff(div, `nested-layouts-inner-opened`);
210+
});
197211

198-
for (const layout of layouts) {
199-
await openDetail(layout);
200-
}
212+
it('outer overflow', async () => {
213+
div.style.width = '400px';
214+
await onceResized(div);
215+
await visualDiff(div, `nested-layouts-inner-opened-outer-overflow`);
216+
});
201217

218+
it('inner overflow', async () => {
219+
div.style.width = '200px';
202220
await onceResized(div);
203-
await visualDiff(div, `detail-auto-size-overflow-${depth}`);
221+
await visualDiff(div, `nested-layouts-inner-opened-inner-overflow`);
204222
});
205223
});
206224
});

0 commit comments

Comments
 (0)