feat: add layout improvements behind feature flag (#8552)

* add style fixes behind feature flag * add tests * use data attribute for targeting * remove usage of unsafeCSS * simplify accessing children in tests * Apply suggestions from code review Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com> * update data attribute names * Update packages/horizontal-layout/src/vaadin-horizontal-layout-styles.js * Update packages/vertical-layout/src/vaadin-vertical-layout-styles.js * Update vaadin-horizontal-layout-styles.js * update feature flag name * use auto-generated lit tests --------- Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
vaadin · Jan 31, 2025 · fa0c544 · fa0c544
1 parent f047f75
commit fa0c544
Show file tree

Hide file tree

Showing 10 changed files with 276 additions and 4 deletions.
diff --git a/packages/horizontal-layout/src/vaadin-horizontal-layout-styles.d.ts b/packages/horizontal-layout/src/vaadin-horizontal-layout-styles.d.ts
@@ -5,4 +5,4 @@
  */
 import type { CSSResult } from 'lit';
 
-export const horizontalLayoutStyles: CSSResult;
+export const horizontalLayoutStyles: CSSResult[];
diff --git a/packages/horizontal-layout/src/vaadin-horizontal-layout-styles.js b/packages/horizontal-layout/src/vaadin-horizontal-layout-styles.js
@@ -5,7 +5,7 @@
  */
 import { css } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
 
-export const horizontalLayoutStyles = css`
+export const baseStyles = css`
   :host {
     display: flex;
     box-sizing: border-box;
@@ -28,3 +28,21 @@ export const horizontalLayoutStyles = css`
     gap: 1em;
   }
 `;
+
+// Layout improvements are part of a feature for Flow users where children that have been configured to use full size
+// using `HasSize.setSizeFull()` and others, get additional styles so that they effectively take the remaining space in
+// the layout, rather than explicitly use 100% width/height. The respective data attributes are set by Flow's `HasSize`
+// class.
+const enableLayoutImprovements = window.Vaadin.featureFlags.layoutComponentImprovements;
+const layoutImprovementStyles = css`
+  ::slotted([data-width-full]) {
+    flex: 1;
+  }
+
+  ::slotted(vaadin-horizontal-layout[data-width-full]),
+  ::slotted(vaadin-vertical-layout[data-width-full]) {
+    min-width: 0;
+  }
+`;
+
+export const horizontalLayoutStyles = enableLayoutImprovements ? [baseStyles, layoutImprovementStyles] : [baseStyles];
diff --git a/packages/horizontal-layout/test/enable-layout-improvements.js b/packages/horizontal-layout/test/enable-layout-improvements.js
@@ -0,0 +1,3 @@
+window.Vaadin ??= {};
+window.Vaadin.featureFlags ??= {};
+window.Vaadin.featureFlags.layoutComponentImprovements = true;
diff --git a/packages/horizontal-layout/test/horizontal-layout.common.js b/packages/horizontal-layout/test/horizontal-layout.common.js
@@ -110,4 +110,52 @@ describe('vaadin-horizontal-layout', () => {
       expect(wrapper.scrollWidth).to.equal(200);
     });
   });
+
+  describe('layout improvements disabled', () => {
+    let layout, children;
+
+    describe('flex', () => {
+      beforeEach(async () => {
+        layout = fixtureSync(`
+          <vaadin-horizontal-layout>
+            <div></div>
+            <div data-width-full></div>
+          </vaadin-horizontal-layout>
+        `);
+        children = Array.from(layout.children);
+        await nextFrame();
+      });
+
+      it('should not set flex on any children', () => {
+        children.forEach((child) => {
+          expect(getComputedStyle(child).flex).to.equal('0 1 auto');
+        });
+      });
+    });
+
+    describe('min-width', () => {
+      beforeEach(async () => {
+        layout = fixtureSync(`
+          <vaadin-horizontal-layout>
+            <div></div>
+            <div data-width-full></div>
+            <vaadin-button></vaadin-button>
+            <vaadin-button data-width-full></vaadin-button>
+            <vaadin-horizontal-layout></vaadin-horizontal-layout>
+            <vaadin-horizontal-layout data-width-full></vaadin-horizontal-layout>
+            <vaadin-vertical-layout></vaadin-vertical-layout>
+            <vaadin-vertical-layout data-width-full></vaadin-vertical-layout>
+          </vaadin-horizontal-layout>
+        `);
+        children = Array.from(layout.children);
+        await nextFrame();
+      });
+
+      it('should not set min-width on any children', () => {
+        children.forEach((child) => {
+          expect(getComputedStyle(child).minWidth).to.equal('auto');
+        });
+      });
+    });
+  });
 });
diff --git a/packages/horizontal-layout/test/layout-improvements.test.js b/packages/horizontal-layout/test/layout-improvements.test.js
@@ -0,0 +1,67 @@
+import { expect } from '@vaadin/chai-plugins';
+import { fixtureSync, nextFrame } from '@vaadin/testing-helpers';
+import './enable-layout-improvements.js';
+import '../vaadin-horizontal-layout.js';
+
+describe('layout improvements enabled', () => {
+  let layout, children;
+
+  describe('flex', () => {
+    beforeEach(async () => {
+      layout = fixtureSync(`
+        <vaadin-horizontal-layout>
+          <div></div>
+          <div data-width-full></div>
+        </vaadin-horizontal-layout>
+      `);
+      children = Array.from(layout.children);
+      await nextFrame();
+    });
+
+    it('should set flex on full width children only', () => {
+      const fullWidthChildren = children.filter((child) => child.hasAttribute('data-width-full'));
+      const remainingChildren = children.filter((child) => !fullWidthChildren.includes(child));
+
+      fullWidthChildren.forEach((child) => {
+        expect(getComputedStyle(child).flex).to.equal('1 1 0%');
+      });
+      remainingChildren.forEach((child) => {
+        expect(getComputedStyle(child).flex).to.equal('0 1 auto');
+      });
+    });
+  });
+
+  describe('min-width', () => {
+    beforeEach(async () => {
+      layout = fixtureSync(`
+        <vaadin-horizontal-layout>
+          <div></div>
+          <div data-width-full></div>
+          <vaadin-button></vaadin-button>
+          <vaadin-button data-width-full></vaadin-button>
+          <vaadin-horizontal-layout></vaadin-horizontal-layout>
+          <vaadin-horizontal-layout data-width-full></vaadin-horizontal-layout>
+          <vaadin-vertical-layout></vaadin-vertical-layout>
+          <vaadin-vertical-layout data-width-full></vaadin-vertical-layout>
+        </vaadin-horizontal-layout>
+      `);
+      children = Array.from(layout.children);
+      await nextFrame();
+    });
+
+    it('should set min-width on layout components with full width only', () => {
+      const layoutChildren = children.filter(
+        (child) => child.localName.endsWith('layout') && child.hasAttribute('data-width-full'),
+      );
+      const remainingChildren = children.filter((child) => !layoutChildren.includes(child));
+
+      layoutChildren.forEach((child) => {
+        expect(getComputedStyle(child).minWidth).to.equal('0px');
+      });
+
+      remainingChildren.forEach((child) => {
+        expect(getComputedStyle(child).minWidth).to.equal('auto');
+      });
+    });
+  });
+});
diff --git a/packages/vertical-layout/src/vaadin-vertical-layout-styles.d.ts b/packages/vertical-layout/src/vaadin-vertical-layout-styles.d.ts
@@ -5,4 +5,4 @@
  */
 import type { CSSResult } from 'lit';
 
-export const verticalLayoutStyles: CSSResult;
+export const verticalLayoutStyles: CSSResult[];
diff --git a/packages/vertical-layout/src/vaadin-vertical-layout-styles.js b/packages/vertical-layout/src/vaadin-vertical-layout-styles.js
@@ -5,7 +5,7 @@
  */
 import { css } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
 
-export const verticalLayoutStyles = css`
+export const baseStyles = css`
   :host {
     display: flex;
     flex-direction: column;
@@ -30,3 +30,21 @@ export const verticalLayoutStyles = css`
     gap: 1em;
   }
 `;
+
+// Layout improvements are part of a feature for Flow users where children that have been configured to use full size
+// using `HasSize.setSizeFull()` and others, get additional styles so that they effectively take the remaining space in
+// the layout, rather than explicitly use 100% width/height. The respective data attributes are set by Flow's `HasSize`
+// class.
+const enableLayoutImprovements = window.Vaadin.featureFlags.layoutComponentImprovements;
+const layoutImprovementStyles = css`
+  ::slotted([data-height-full]) {
+    flex: 1;
+  }
+
+  ::slotted(vaadin-horizontal-layout[data-height-full]),
+  ::slotted(vaadin-vertical-layout[data-height-full]) {
+    min-height: 0;
+  }
+`;
+
+export const verticalLayoutStyles = enableLayoutImprovements ? [baseStyles, layoutImprovementStyles] : [baseStyles];
diff --git a/packages/vertical-layout/test/enable-layout-improvements.js b/packages/vertical-layout/test/enable-layout-improvements.js
@@ -0,0 +1,3 @@
+window.Vaadin ??= {};
+window.Vaadin.featureFlags ??= {};
+window.Vaadin.featureFlags.layoutComponentImprovements = true;
diff --git a/packages/vertical-layout/test/layout-improvements.test.js b/packages/vertical-layout/test/layout-improvements.test.js
@@ -0,0 +1,67 @@
+import { expect } from '@vaadin/chai-plugins';
+import { fixtureSync, nextFrame } from '@vaadin/testing-helpers';
+import './enable-layout-improvements.js';
+import '../vaadin-vertical-layout.js';
+
+describe('layout improvements enabled', () => {
+  let layout, children;
+
+  describe('flex', () => {
+    beforeEach(async () => {
+      layout = fixtureSync(`
+        <vaadin-vertical-layout>
+          <div></div>
+          <div data-height-full></div>
+        </vaadin-vertical-layout>
+      `);
+      children = Array.from(layout.children);
+      await nextFrame();
+    });
+
+    it('should set flex on full height children only', () => {
+      const fullHeightChildren = children.filter((child) => child.hasAttribute('data-height-full'));
+      const remainingChildren = children.filter((child) => !fullHeightChildren.includes(child));
+
+      fullHeightChildren.forEach((child) => {
+        expect(getComputedStyle(child).flex).to.equal('1 1 0%');
+      });
+      remainingChildren.forEach((child) => {
+        expect(getComputedStyle(child).flex).to.equal('0 1 auto');
+      });
+    });
+  });
+
+  describe('min-height', () => {
+    beforeEach(async () => {
+      layout = fixtureSync(`
+        <vaadin-vertical-layout>
+          <div></div>
+          <div data-height-full></div>
+          <vaadin-button></vaadin-button>
+          <vaadin-button data-height-full></vaadin-button>
+          <vaadin-horizontal-layout></vaadin-horizontal-layout>
+          <vaadin-horizontal-layout data-height-full></vaadin-horizontal-layout>
+          <vaadin-vertical-layout></vaadin-vertical-layout>
+          <vaadin-vertical-layout data-height-full></vaadin-vertical-layout>
+        </vaadin-vertical-layout>
+      `);
+      children = Array.from(layout.children);
+      await nextFrame();
+    });
+
+    it('should set min-height on layout components with full height only', () => {
+      const layoutChildren = children.filter(
+        (child) => child.localName.endsWith('layout') && child.hasAttribute('data-height-full'),
+      );
+      const remainingChildren = children.filter((child) => !layoutChildren.includes(child));
+
+      layoutChildren.forEach((child) => {
+        expect(getComputedStyle(child).minHeight).to.equal('0px');
+      });
+
+      remainingChildren.forEach((child) => {
+        expect(getComputedStyle(child).minHeight).to.equal('auto');
+      });
+    });
+  });
+});
diff --git a/packages/vertical-layout/test/vertical-layout.common.js b/packages/vertical-layout/test/vertical-layout.common.js
@@ -111,4 +111,52 @@ describe('vaadin-vertical-layout', () => {
       expect(wrapper.scrollHeight).to.equal(200);
     });
   });
+
+  describe('layout improvements disabled', () => {
+    let layout, children;
+
+    describe('flex', () => {
+      beforeEach(async () => {
+        layout = fixtureSync(`
+          <vaadin-vertical-layout>
+            <div></div>
+            <div data-height-full></div>
+          </vaadin-vertical-layout>
+        `);
+        children = Array.from(layout.children);
+        await nextFrame();
+      });
+
+      it('should not set flex on any children', () => {
+        children.forEach((child) => {
+          expect(getComputedStyle(child).flex).to.equal('0 1 auto');
+        });
+      });
+    });
+
+    describe('min-height', () => {
+      beforeEach(async () => {
+        layout = fixtureSync(`
+          <vaadin-vertical-layout>
+            <div></div>
+            <div data-height-full></div>
+            <vaadin-button></vaadin-button>
+            <vaadin-button data-height-full></vaadin-button>
+            <vaadin-horizontal-layout></vaadin-horizontal-layout>
+            <vaadin-horizontal-layout data-height-full></vaadin-horizontal-layout>
+            <vaadin-vertical-layout></vaadin-vertical-layout>
+            <vaadin-vertical-layout data-height-full></vaadin-vertical-layout>
+          </vaadin-vertical-layout>
+        `);
+        children = Array.from(layout.children);
+        await nextFrame();
+      });
+
+      it('should not set min-height on any children', () => {
+        children.forEach((child) => {
+          expect(getComputedStyle(child).minHeight).to.equal('auto');
+        });
+      });
+    });
+  });
 });