address review comments

Author: vursen

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

@@ -74,7 +74,7 @@ export function getClosestElement(selector, node) {
     return null;
   }
 
-  return node.closest(selector) || getClosestElement(selector, node.getRootNode().host);
+  return node.closest?.(selector) || getClosestElement(selector, node.getRootNode().host);
 }
 
 /**

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

@@ -6,6 +6,7 @@
 import { html, LitElement, nothing } from 'lit';
 import { getFocusableElements } from '@vaadin/a11y-base/src/focus-utils.js';
 import { defineCustomElement } from '@vaadin/component-base/src/define.js';
+import { getClosestElement } from '@vaadin/component-base/src/dom-utils.js';
 import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js';
 import { PolylitMixin } from '@vaadin/component-base/src/polylit-mixin.js';
 import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
@@ -465,7 +466,7 @@ class MasterDetailLayout extends ElementMixin(ThemableMixin(PolylitMixin(LitElem
 
   /** @private */
   get __ancestorLayouts() {
-    const parent = this.parentElement && this.parentElement.closest(this.constructor.is);
+    const parent = getClosestElement(this.constructor.is, this.parentNode);
     return parent ? [...parent.__ancestorLayouts, parent] : [];
   }
 

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

@@ -1,5 +1,5 @@
 import { expect } from '@vaadin/chai-plugins';
-import { fixtureSync } from '@vaadin/testing-helpers';
+import { defineCE, fixtureSync } from '@vaadin/testing-helpers';
 import sinon from 'sinon';
 import '../src/vaadin-master-detail-layout.js';
 import { onceResized } from './helpers.js';
@@ -61,21 +61,39 @@ describe('detail auto size', () => {
       return layout.style.getPropertyValue('--_detail-cached-size');
     }
 
+    const shadowElement = defineCE(
+      class extends HTMLElement {
+        constructor() {
+          super();
+          this.attachShadow({ mode: 'open' });
+          this.shadowRoot.appendChild(this.querySelector('template').content);
+        }
+      },
+    );
+
     beforeEach(async () => {
       outer = fixtureSync(`
         <vaadin-master-detail-layout style="width: 1200px;" master-size="100px" expand="both">
           <div>Outer Master</div>
+
           <vaadin-master-detail-layout slot="detail" master-size="100px">
             <div>Middle Master</div>
-            <vaadin-master-detail-layout slot="detail" master-size="100px">
-              <div>Inner Master</div>
-              <div slot="detail" style="width: 100px;">Inner Detail</div>
-            </vaadin-master-detail-layout>
+
+            <${shadowElement} slot="detail">
+              <template>
+                <vaadin-master-detail-layout master-size="100px">
+                  <div>Inner Master</div>
+
+                  <div slot="detail" style="width: 100px;">Inner Detail</div>
+                </vaadin-master-detail-layout>
+              </template>
+            </${shadowElement}>
           </vaadin-master-detail-layout>
         </vaadin-master-detail-layout>
       `);
       middle = outer.querySelector('vaadin-master-detail-layout');
-      inner = middle.querySelector('vaadin-master-detail-layout');
+      inner = middle.querySelector('[slot="detail"]').shadowRoot.querySelector('vaadin-master-detail-layout');
+
       await onceResized(outer);
       await onceResized(middle);
       await onceResized(inner);
@@ -100,24 +118,22 @@ describe('detail auto size', () => {
       it('should update cached sizes on ancestors after detail content changes', () => {
         inner.querySelector('[slot="detail"]').style.width = '200px';
         inner.recalculateLayout();
-
         expect(getCachedSize(inner)).to.equal('201px');
         expect(getCachedSize(middle)).to.equal('302px');
         expect(getCachedSize(outer)).to.equal('403px');
       });
 
       it('should toggle overlay on ancestors when detail content outgrows or fits available space', () => {
-        // Outer: 100px master + 303px detail = 403px, host is 1200px → no overlay
-        expect(outer.hasAttribute('overlay')).to.be.false;
-
         // Grow inner detail so outer needs 100px + 1103px = 1203px > 1200px
         inner.querySelector('[slot="detail"]').style.width = '1000px';
         inner.recalculateLayout();
+        expect(middle.hasAttribute('overlay')).to.be.true;
         expect(outer.hasAttribute('overlay')).to.be.true;
 
         // Shrink back so outer needs 100px + 303px = 403px < 1200px
         inner.querySelector('[slot="detail"]').style.width = '100px';
         inner.recalculateLayout();
+        expect(middle.hasAttribute('overlay')).to.be.false;
         expect(outer.hasAttribute('overlay')).to.be.false;
       });
     });

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

@@ -148,59 +148,77 @@ describe('master-detail-layout', () => {
     });
   });
 
-  describe('detail auto size', () => {
-    let layouts;
+  describe('nested layouts', () => {
+    let outer, inner;
 
     function openDetail(layout) {
-      return layout._setDetail(layout.querySelector(':scope > [slot="detail-hidden"]'));
+      // Set detail content and skip animation
+      return layout._setDetail(layout.querySelector(':scope > [slot="detail-hidden"]'), true);
     }
 
     beforeEach(async () => {
-      mdl = fixtureSync(
+      outer = fixtureSync(
         `
-          <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;">
-            <div>Master 0</div>
-            <vaadin-master-detail-layout expand="detail" master-size="250px" no-animation slot="detail-hidden">
-              <div>Master 1</div>
-              <vaadin-master-detail-layout expand="detail" master-size="200px" no-animation slot="detail-hidden">
-                <div>Master 2</div>
-                <div slot="detail-hidden" style="min-width: 100px">Detail 2</div>
-              </vaadin-master-detail-layout>
+          <vaadin-master-detail-layout master-size="300px" style="height: 400px; border: 1px solid lightgray;">
+            <div>
+              Outer Master
+            </div>
+            <vaadin-master-detail-layout master-size="200px" slot="detail-hidden">
+              <div>
+                Inner Master
+              </div>
+              <div slot="detail-hidden" style="min-width: 100px">
+                Inner Detail
+              </div>
             </vaadin-master-detail-layout>
           </vaadin-master-detail-layout>
         `,
         div,
       );
-
-      layouts = [...div.querySelectorAll('vaadin-master-detail-layout')];
+      inner = outer.querySelector('vaadin-master-detail-layout');
       await onceResized(div);
     });
 
     it('default', async () => {
-      await visualDiff(div, `detail-auto-size`);
+      await visualDiff(div, `nested-layouts`);
     });
 
-    [0, 1, 2].forEach((depth) => {
-      it(`opened (depth ${depth})`, async () => {
-        for (const layout of layouts.slice(0, depth + 1)) {
-          await openDetail(layout);
-        }
+    describe('outer opened', () => {
+      beforeEach(async () => {
+        await openDetail(outer);
+      });
+
+      it('default', async () => {
+        await visualDiff(div, `nested-layouts-outer-opened`);
+      });
+
+      it('overflow', async () => {
+        div.style.width = '400px';
         await onceResized(div);
-        await visualDiff(div, `detail-auto-size-opened-${depth}`);
+        await visualDiff(div, `nested-layouts-outer-opened-overflow`);
       });
     });
 
-    ['600px', '400px', '200px'].forEach((width, depth) => {
-      it(`overflow (depth ${depth})`, async () => {
-        div.style.width = width;
-        await onceResized(div);
+    describe('inner opened', () => {
+      beforeEach(async () => {
+        await openDetail(outer);
+        await openDetail(inner);
+      });
+
+      it('default', async () => {
+        await visualDiff(div, `nested-layouts-inner-opened`);
+      });
 
-        for (const layout of layouts) {
-          await openDetail(layout);
-        }
+      it('outer overflow', async () => {
+        div.style.width = '400px';
+        await onceResized(div);
+        await visualDiff(div, `nested-layouts-inner-opened-outer-overflow`);
+      });
 
+      it('inner overflow', async () => {
+        div.style.width = '200px';
         await onceResized(div);
-        await visualDiff(div, `detail-auto-size-overflow-${depth}`);
+        await visualDiff(div, `nested-layouts-inner-opened-inner-overflow`);
       });
     });
   });