feat: document level npm css (#9559)

Now there is a possibility to use the "documentCss" key in theme.json to inject npm css into the document rool level element. This is mostly for embedded web components as they do not target document by default Fixes #9533 * Use more targeted bookkeeping for globalCss Have a full map on where the global css has been injected to so that if we add for shadow roots we don't add to document, but we are able to add to other shadow roots. # Conflicts: # flow-server/src/main/java/com/vaadin/flow/server/AppShellRegistry.java # flow-server/src/test/java/com/vaadin/flow/server/startup/VaadinAppShellInitializerTest.java
vaadin · Mar 8, 2021 · d02c0aa · d02c0aa
1 parent 57a02b2
commit d02c0aa
Show file tree

Hide file tree

Showing 11 changed files with 113 additions and 17 deletions.
diff --git a/flow-server/src/main/java/com/vaadin/flow/server/webcomponent/WebComponentGenerator.java b/flow-server/src/main/java/com/vaadin/flow/server/webcomponent/WebComponentGenerator.java
@@ -161,7 +161,7 @@ static Map<String, String> getReplacementsMap(String tag,
         if (themeName != null && !themeName.isEmpty()) {
             replacements.put("ThemeImport",
                 "import {applyTheme} from 'theme/theme-generated.js';\n\n");
-            replacements.put("ApplyTheme", "applyTheme(shadow);\n");
+            replacements.put("ApplyTheme", "applyTheme(shadow);\n    ");
         } else {
             replacements.put("ThemeImport", "");
             replacements.put("ApplyTheme", "");

diff --git a/flow-server/src/main/resources/plugins/application-theme-plugin/package.json b/flow-server/src/main/resources/plugins/application-theme-plugin/package.json
@@ -6,7 +6,7 @@
   ],
   "repository": "vaadin/flow",
   "name": "@vaadin/application-theme-plugin",
-  "version": "0.2.5",
+  "version": "0.2.7",
   "main": "application-theme-plugin.js",
   "author": "Vaadin Ltd",
   "license": "Apache-2.0",

diff --git a/flow-server/src/main/resources/plugins/application-theme-plugin/theme-generator.js b/flow-server/src/main/resources/plugins/application-theme-plugin/theme-generator.js
@@ -76,13 +76,32 @@ function generateThemeFile(themeFolder, themeName, themeProperties) {
     const variable = camelCase(filename);
     imports.push(`import ${variable} from './${filename}';\n`);
     if (filename == themeFileAlwaysAddToDocument) {
-      globalCssCode.push(`injectGlobalCss(${variable}.toString(), document);\n   `);
+      globalCssCode.push(`injectGlobalCss(${variable}.toString(), document);\n    `);
     } else {
       globalCssCode.push(`injectGlobalCss(${variable}.toString(), target);\n    `);
     }
   });
 
   let i = 0;
+  if (themeProperties.documentCss) {
+    const missingModules = checkModules(themeProperties.documentCss);
+    if(missingModules.length > 0) {
+      throw Error("Missing npm modules or files '" + missingModules.join("', '")
+        + "' for documentCss marked in 'theme.json'.\n" +
+        "Install or update package(s) by adding a @NpmPackage annotation or install it using 'npm/pnpm i'");
+
+    }
+    themeProperties.documentCss.forEach((cssImport) => {
+      const variable = 'module' + i++;
+      imports.push(`import ${variable} from '${cssImport}';\n`);
+      // Due to chrome bug https://bugs.chromium.org/p/chromium/issues/detail?id=336876 font-face will not work
+      // inside shadowRoot so we need to inject it there also.
+      globalCssCode.push(`if(target !== document) {
+      injectGlobalCss(${variable}.toString(), target);
+    }\n    `);
+      globalCssCode.push(`injectGlobalCss(${variable}.toString(), document);\n    `);
+    });
+  }
   if (themeProperties.importCss) {
     const missingModules = checkModules(themeProperties.importCss);
     if (missingModules.length > 0) {
@@ -121,13 +140,19 @@ function generateThemeFile(themeFolder, themeName, themeProperties) {
   const componentCssFlag = themeIdentifier + 'componentCss';
 
   themeFile += imports.join('');
+  themeFile += `
+window.Vaadin = window.Vaadin || {};
+window.Vaadin.Flow = window.Vaadin.Flow || {};
+window.Vaadin.Flow['${globalCssFlag}'] = window.Vaadin.Flow['${globalCssFlag}'] || [];
+`;
 
 // Don't format as the generated file formatting will get wonky!
 // If targets check that we only register the style parts once, checks exist for global css and component css
   const themeFileApply = `export const applyTheme = (target) => {
-  if (!target['${globalCssFlag}']) {
+  const injectGlobal = (window.Vaadin.Flow['${globalCssFlag}'].length === 0) || (!window.Vaadin.Flow['${globalCssFlag}'].includes(target) && target !== document);
+  if (injectGlobal) {
     ${globalCssCode.join('')}
-    target['${globalCssFlag}'] = true;
+    window.Vaadin.Flow['${globalCssFlag}'].push(target);
   }
   if (!document['${componentCssFlag}']) {
     ${componentCssCode.join('')}

diff --git a/flow-server/src/test/java/com/vaadin/flow/server/webcomponent/WebComponentGeneratorTest.java b/flow-server/src/test/java/com/vaadin/flow/server/webcomponent/WebComponentGeneratorTest.java
@@ -190,7 +190,7 @@ public void providedJSModuleContainsCorrectThemeReplacements() {
                 + "      }\n" //
                 + "    `;\n"));
 
-        MatcherAssert.assertThat(module, containsString("applyTheme(shadow);\nshadow.appendChild(style);"));
+        MatcherAssert.assertThat(module, containsString("applyTheme(shadow);\n    shadow.appendChild(style);"));
         MatcherAssert.assertThat(module,
                 containsString("customElements.define('tag', Tag);\n"));
     }

diff --git a/...st-embedding/test-embedding-application-theme/frontend/themes/embedded-theme/document.css b/...st-embedding/test-embedding-application-theme/frontend/themes/embedded-theme/document.css
@@ -2,3 +2,7 @@
     font-family: "Ostrich";
     src: url("./font/ostrich-sans-regular.ttf") format("TrueType");
 }
+
+.global {
+    color: blue;
+}
diff --git a/...test-embedding/test-embedding-application-theme/frontend/themes/embedded-theme/global.css b/...test-embedding/test-embedding-application-theme/frontend/themes/embedded-theme/global.css
@@ -5,3 +5,7 @@
     background-image: url("./img/bg.jpg");
     font-family: "Ostrich";
 }
+
+.internal {
+    color: green;
+}
diff --git a/...test-embedding/test-embedding-application-theme/frontend/themes/embedded-theme/theme.json b/...test-embedding/test-embedding-application-theme/frontend/themes/embedded-theme/theme.json
@@ -1,4 +1,5 @@
 {
+  "documentCss": ["@fortawesome/fontawesome-free/css/all.css"],
   "assets": {
     "@fortawesome/fontawesome-free": {
       "svgs/regular/**": "fortawesome/icons"

diff --git a/flow-tests/test-embedding/test-embedding-application-theme/pom.xml b/flow-tests/test-embedding/test-embedding-application-theme/pom.xml
@@ -53,6 +53,17 @@
                 <groupId>org.eclipse.jetty</groupId>
                 <artifactId>jetty-maven-plugin</artifactId>
                 <version>${jetty.version}</version>
+                <!-- do not allow faulty app shell annotations for module -->
+                <!-- This should be removed when flow-tests do not break app shell annotation-->
+                <!-- flow-tests should not allow faulty annotations -->
+                <configuration>
+                    <systemProperties>
+                        <systemProperty>
+                            <name>vaadin.allow.appshell.annotations</name>
+                            <value>false</value>
+                        </systemProperty>
+                    </systemProperties>
+                </configuration>
             </plugin>
 
             <plugin>

diff --git a/...bedding-application-theme/src/main/java/com/vaadin/flow/webcomponent/ThemedComponent.java b/...bedding-application-theme/src/main/java/com/vaadin/flow/webcomponent/ThemedComponent.java
@@ -15,9 +15,11 @@
  */
 package com.vaadin.flow.webcomponent;
 
+import com.vaadin.flow.component.dependency.NpmPackage;
 import com.vaadin.flow.component.html.Div;
 import com.vaadin.flow.component.html.Span;
 
+@NpmPackage(value = "@fortawesome/fontawesome-free", version = "5.15.1")
 public class ThemedComponent extends Div {
 
     public static final String TEST_TEXT_ID = "test-text";
@@ -26,13 +28,19 @@ public class ThemedComponent extends Div {
     public static final String MY_LIT_ID = "button";
     public static final String EMBEDDED_ID = "embedded";
 
+    public static final String HAND_ID = "sparkle-hand";
+
     public ThemedComponent() {
         setId(EMBEDDED_ID);
         final Span textSpan = new Span(
             "Welcome to the embedded application theme test");
         textSpan.setId(TEST_TEXT_ID);
 
-        add(textSpan);
+        Span hand = new Span();
+        hand.setId(HAND_ID);
+        hand.addClassNames("internal", "fas", "fa-hand-sparkles");
+
+        add(textSpan, hand);
 
         add(new Div());
         add(new MyPolymerField().withId(MY_POLYMER_ID));

diff --git a/flow-tests/test-embedding/test-embedding-application-theme/src/main/webapp/index.html b/flow-tests/test-embedding/test-embedding-application-theme/src/main/webapp/index.html
@@ -7,6 +7,9 @@
 
 <body>
 
-  <themed-component></themed-component>
+  <span class="internal global" id="overflow">Internal should not apply</span>
+
+  <themed-component id="first"></themed-component>
+  <themed-component id="second"></themed-component>
 
 </body>
diff --git a/...ication-theme/src/test/java/com/vaadin/flow/webcomponent/ApplicationThemeComponentIT.java b/...ication-theme/src/test/java/com/vaadin/flow/webcomponent/ApplicationThemeComponentIT.java
@@ -26,6 +26,7 @@
 import com.vaadin.testbench.TestBenchElement;
 
 import static com.vaadin.flow.webcomponent.ThemedComponent.EMBEDDED_ID;
+import static com.vaadin.flow.webcomponent.ThemedComponent.HAND_ID;
 import static com.vaadin.flow.webcomponent.ThemedComponent.MY_LIT_ID;
 import static com.vaadin.flow.webcomponent.ThemedComponent.MY_POLYMER_ID;
 
@@ -57,14 +58,8 @@ public void applicationTheme_GlobalCss_isUsedOnlyInEmbeddeComponent() {
         // No exception for bg-image should exist
         checkLogsForErrors();
 
-        final TestBenchElement themedComponent = $("themed-component").first();
-
-        Assert.assertEquals(
-            "url(\"" + getRootURL() + "/VAADIN/static/themes/embedded-theme/img/bg.jpg\")",
-            themedComponent.getCssValue("background-image"));
-
-        Assert.assertEquals("Ostrich",
-            themedComponent.getCssValue("font-family"));
+        validateEmbeddedComponent($("themed-component").id("first"), "first");
+        validateEmbeddedComponent($("themed-component").id("second"), "second");
 
         final WebElement body = findElement(By.tagName("body"));
         Assert.assertNotEquals(
@@ -73,11 +68,34 @@ public void applicationTheme_GlobalCss_isUsedOnlyInEmbeddeComponent() {
 
         Assert.assertNotEquals("Ostrich", body.getCssValue("font-family"));
 
-        getDriver().get(getRootURL() + "/VAADIN/static/themes/embedded-theme/img/bg.jpg");
+        Assert
+            .assertEquals("Embedded style should not match external component",
+                "rgba(0, 0, 255, 1)",
+                $(SpanElement.class).id("overflow").getCssValue("color"));
+                getDriver().get(getRootURL() + "/themes/embedded-theme/img/bg.jpg");
         Assert.assertFalse("app-theme background file should be served",
             driver.getPageSource().contains("Could not navigate"));
     }
 
+    private void validateEmbeddedComponent(TestBenchElement themedComponent, String target) {
+        Assert.assertEquals(target + " didn't contain the background image",
+            "url(\"" + getRootURL() + "/VAADIN/static/themes/embedded-theme/img/bg.jpg\")",
+            themedComponent.getCssValue("background-image"));
+
+        Assert.assertEquals(target + " didn't contain font-family", "Ostrich",
+            themedComponent.getCssValue("font-family"));
+
+        final TestBenchElement embeddedComponent = themedComponent
+            .$(DivElement.class).id(EMBEDDED_ID);
+
+        final SpanElement handElement = embeddedComponent.$(SpanElement.class)
+            .id(HAND_ID);
+
+        Assert
+            .assertEquals("Color should have been applied", "rgba(0, 128, 0, 1)",
+                handElement.getCssValue("color"));
+    }
+
     @Test
     public void componentThemeIsApplied_forPolymerAndLit() {
         open();
@@ -105,6 +123,28 @@ public void componentThemeIsApplied_forPolymerAndLit() {
     }
 
     @Test
+    public void documentCssFonts_fontsAreAppliedAndAvailable() {
+        open();
+        checkLogsForErrors();
+        final TestBenchElement themedComponent = $("themed-component").first();
+        final TestBenchElement embeddedComponent = themedComponent
+            .$(DivElement.class).id(EMBEDDED_ID);
+
+        final SpanElement handElement = embeddedComponent.$(SpanElement.class)
+            .id(HAND_ID);
+        Assert.assertEquals("Font family faulty", "\"Font Awesome 5 Free\"",
+            handElement.getCssValue("font-family"));
+        Assert.assertEquals("Font weight faulty", "900",
+            handElement.getCssValue("font-weight"));
+        Assert.assertEquals("display value faulty", "inline-block",
+            handElement.getCssValue("display"));
+
+        getDriver().get(getRootURL()
+            + "/path/VAADIN/static/@fortawesome/fontawesome-free/webfonts/fa-solid-900.woff2");
+        Assert.assertFalse("Font resource should be available",
+            driver.getPageSource().contains("HTTP ERROR 404 Not Found"));
+    }
+
     public void documentCssFonts_fromLocalCssFile_fontAppliedToDocumentRoot() {
         open();