fix: inherit parent styles to extensions without is getter (#3710) (#…

…3713) Co-authored-by: Tomi Virkki <tomivirkki@users.noreply.github.com>
vaadin · Apr 21, 2022 · 838d100 · 838d100
1 parent 8dcaccd
commit 838d100
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 23 deletions.
diff --git a/packages/vaadin-themable-mixin/test/lit-setup.js b/packages/vaadin-themable-mixin/test/lit-setup.js
@@ -6,25 +6,29 @@ import { ThemableMixin } from '../vaadin-themable-mixin.js';
  * By default, the suite creates PolymerElement based custom elements, but in the
  * -lit.test.js tests, we specifically want to create LitElement based custom elements instead.
  */
-window.defineCustomElementFunction = (name, parentName, content, styles) => {
+// eslint-disable-next-line max-params
+window.defineCustomElementFunction = (name, parentName, content, styles, noIs) => {
   const parentElement = parentName ? customElements.get(parentName) : LitElement;
   class CustomElement extends ThemableMixin(parentElement) {
-    static get is() {
-      return name;
-    }
-
     static get styles() {
       return styles ? unsafeCSS(styles) : undefined;
     }
+  }
 
-    render() {
-      if (content) {
-        const template = document.createElement('template');
-        template.innerHTML = content;
-        return template.content;
+  if (content) {
+    CustomElement.prototype.render = function () {
+      const template = document.createElement('template');
+      template.innerHTML = content;
+      return template.content;
+    };
+  }
+
+  if (!noIs) {
+    Object.defineProperty(CustomElement, 'is', {
+      get() {
+        return name;
       }
-      return super.render();
-    }
+    });
   }
 
   customElements.define(name, CustomElement);

diff --git a/packages/vaadin-themable-mixin/test/themable-mixin.test.js b/packages/vaadin-themable-mixin/test/themable-mixin.test.js
@@ -11,15 +11,14 @@ const createStyles =
 
 const defineCustomElement =
   window.defineCustomElementFunction ||
-  ((name, parentName, content, styles) => {
+  // eslint-disable-next-line max-params
+  ((name, parentName, content, styles, noIs) => {
     const parentElement = parentName ? customElements.get(parentName) : PolymerElement;
-    class CustomElement extends ThemableMixin(parentElement) {
-      static get is() {
-        return name;
-      }
+    class CustomElement extends ThemableMixin(parentElement) {}
 
-      static get template() {
-        if (content) {
+    if (content) {
+      Object.defineProperty(CustomElement, 'template', {
+        get() {
           if (styles) {
             content = `<style>${styles}</style>${content}`;
           }
@@ -28,8 +27,15 @@ const defineCustomElement =
           template.innerHTML = content;
           return template;
         }
-        return super.template;
-      }
+      });
+    }
+
+    if (!noIs) {
+      Object.defineProperty(CustomElement, 'is', {
+        get() {
+          return name;
+        }
+      });
     }
 
     customElements.define(name, CustomElement);
@@ -196,8 +202,12 @@ defineCustomElement('test-qux', '', '<div part="text" id="text">text</div>');
 
 defineCustomElement('test-own-template', 'test-foo', '<div part="text" id="text">text</div>');
 
+defineCustomElement('test-own-template-no-is', 'test-foo', '<div part="text" id="text">text</div>', undefined, true);
+
 defineCustomElement('test-no-template', '', '');
 
+defineCustomElement('test-inherited-no-content-no-is', 'test-foo', '', undefined, true);
+
 defineCustomElement('test-style-override', '', '<div part="text" id="text">text</div>');
 
 defineCustomElement(
@@ -226,7 +236,9 @@ describe('ThemableMixin', () => {
         <test-baz></test-baz>
         <test-qux></test-qux>
         <test-own-template></test-own-template>
+        <test-own-template-no-is></test-own-template-no-is>
         <test-no-template></test-no-template>
+        <test-inherited-no-content-no-is></test-inherited-no-content-no-is>
         <test-style-override></test-style-override>
         <test-own-styles></test-own-styles>
       </div>
@@ -280,6 +292,16 @@ describe('ThemableMixin', () => {
     expect(getComputedStyle(getText(components['test-own-template'])).backgroundColor).to.equal('rgb(255, 0, 0)');
   });
 
+  it('should inherit parent themes to own custom template with no is defined', async () => {
+    expect(getComputedStyle(getText(components['test-own-template-no-is'])).backgroundColor).to.equal('rgb(255, 0, 0)');
+  });
+
+  it('should inherit parent themes with no is nor content defined', async () => {
+    expect(getComputedStyle(getText(components['test-inherited-no-content-no-is'])).backgroundColor).to.equal(
+      'rgb(255, 0, 0)'
+    );
+  });
+
   it('should override vaadin module styles', () => {
     expect(getComputedStyle(getText(components['test-style-override'])).color).to.equal('rgb(0, 0, 0)');
   });

diff --git a/packages/vaadin-themable-mixin/vaadin-themable-mixin.js b/packages/vaadin-themable-mixin/vaadin-themable-mixin.js
@@ -179,7 +179,15 @@ function getThemes(tagName) {
  * @returns {boolean}
  */
 function hasThemes(tagName) {
-  const elementClass = customElements.get(tagName);
+  return classHasThemes(customElements.get(tagName));
+}
+
+/**
+ * Check if the custom element type has themes applied.
+ * @param {Function} elementClass
+ * @returns {boolean}
+ */
+function classHasThemes(elementClass) {
   return elementClass && Object.prototype.hasOwnProperty.call(elementClass, '__themes');
 }
 
@@ -197,7 +205,7 @@ export const ThemableMixin = (superClass) =>
       super.finalize();
 
       const template = this.prototype._template;
-      if (!template || hasThemes(this.is)) {
+      if (!template || classHasThemes(this)) {
         return;
       }