fix: import parent theme lumoImports in dev bundle mode (#20325) (#20380

) * fix: import parent theme lumoImports in dev bundle mode Adds missing css imports for parent theme when given in parent theme.json with lumoImports property and running with dev bundle. Fixes: #19567 * test: fix test * test: fix test * chore: catch IllegalArgumentException in CssBundler Catch and debug log IllegalArgumentException possibly thrown from potentialFile.toPath() in Windows. Url's in css like 'https://fonts.googleapis.com/css?family=Itim' shouldn't be considered as copyable asset files. Co-authored-by: Tomi Virtanen <[email protected]>
vaadin · Oct 30, 2024 · 54e59d1 · 54e59d1
1 parent 3534d63
commit 54e59d1
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 15 deletions.
diff --git a/flow-server/src/main/java/com/vaadin/flow/server/frontend/CssBundler.java b/flow-server/src/main/java/com/vaadin/flow/server/frontend/CssBundler.java
@@ -221,20 +221,27 @@ private static boolean isPotentialThemeAsset(File themeFolder,
         boolean potentialAsset = false;
         if (!assetAliases.isEmpty()) {
             Path themeFolderPath = themeFolder.toPath().normalize();
-            Path normalized = themeFolderPath.resolve(potentialFile.toPath())
-                    .normalize();
-            if (normalized.startsWith(themeFolderPath)) {
-                // path is relative to theme folder, check if it matches an
-                // asset
-                String relativePath = themeFolderPath.relativize(normalized)
-                        .toString().replaceAll("\\\\", "/");
-                potentialAsset = assetAliases.stream()
-                        .anyMatch(relativePath::startsWith);
-                if (potentialAsset) {
-                    getLogger().debug(
-                            "Considering '{}' a potential asset of theme '{}'",
-                            relativePath, themeFolder.getName());
+            try {
+                Path normalized = themeFolderPath
+                        .resolve(potentialFile.toPath()).normalize();
+                if (normalized.startsWith(themeFolderPath)) {
+                    // path is relative to theme folder, check if it matches an
+                    // asset
+                    String relativePath = themeFolderPath.relativize(normalized)
+                            .toString().replaceAll("\\\\", "/");
+                    potentialAsset = assetAliases.stream()
+                            .anyMatch(relativePath::startsWith);
+                    if (potentialAsset) {
+                        getLogger().debug(
+                                "Considering '{}' a potential asset of theme '{}'",
+                                relativePath, themeFolder.getName());
+                    }
                 }
+            } catch (IllegalArgumentException e) {
+                getLogger().debug(
+                        "Unresolvable path '{}'. Not considered as a asset of theme '{}'",
+                        potentialFile, themeFolder.getName());
+                return false;
             }
         }
         return potentialAsset;

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
@@ -133,8 +133,8 @@ function writeThemeFiles(themeFolder, themeName, themeProperties, options) {
   }
 
   /* Theme */
+  globalFileContent.push(parentThemeGlobalImport);
   if (useDevServerOrInProductionMode) {
-    globalFileContent.push(parentThemeGlobalImport);
     globalFileContent.push(`import 'themes/${themeName}/${filename}';\n`);
 
     imports.push(`import ${variable} from 'themes/${themeName}/${filename}?inline';\n`);

diff --git a/flow-tests/test-themes/src/main/frontend/themes/app-theme/theme.json b/flow-tests/test-themes/src/main/frontend/themes/app-theme/theme.json
@@ -1,5 +1,5 @@
 {
-  "lumoImports": ["typography", "color", "spacing", "badge", "utility"],
+  "lumoImports": ["typography", "color", "spacing", "badge"],
   "parent": "parent-theme",
   "importCss": ["@fortawesome/fontawesome-free/css/all.css"],
   "assets": {

diff --git a/flow-tests/test-themes/src/main/frontend/themes/parent-theme/theme.json b/flow-tests/test-themes/src/main/frontend/themes/parent-theme/theme.json
@@ -0,0 +1,3 @@
+{
+  "lumoImports": ["utility"]
+}
diff --git a/flow-tests/test-themes/src/main/java/com/vaadin/flow/uitest/ui/theme/ThemeView.java b/flow-tests/test-themes/src/main/java/com/vaadin/flow/uitest/ui/theme/ThemeView.java
@@ -37,6 +37,7 @@ public class ThemeView extends Div {
     public static final String KEYBOARD_ID = "keyboard";
     public static final String LEMON_ID = "lemon";
     public static final String SUN_ID = "sun";
+    public static final String LUMO_BORDER_TOP_DIV = "lumo-border-top-div";
 
     public ThemeView() {
         UI.getCurrent().getPage()
@@ -93,5 +94,10 @@ public ThemeView() {
 
         add(new Div());
         add(new MyComponent().withId(MY_COMPONENT_ID));
+
+        Div lumoBorderDiv = new Div("This element has Lumo border style");
+        lumoBorderDiv.addClassName("border-t");
+        lumoBorderDiv.setId(LUMO_BORDER_TOP_DIV);
+        add(lumoBorderDiv);
     }
 }
diff --git a/flow-tests/test-themes/src/test/java/com/vaadin/flow/uitest/ui/theme/ThemeIT.java b/flow-tests/test-themes/src/test/java/com/vaadin/flow/uitest/ui/theme/ThemeIT.java
@@ -39,6 +39,7 @@
 import static com.vaadin.flow.uitest.ui.theme.ThemeView.FONTAWESOME_ID;
 import static com.vaadin.flow.uitest.ui.theme.ThemeView.KEYBOARD_ID;
 import static com.vaadin.flow.uitest.ui.theme.ThemeView.LEMON_ID;
+import static com.vaadin.flow.uitest.ui.theme.ThemeView.LUMO_BORDER_TOP_DIV;
 import static com.vaadin.flow.uitest.ui.theme.ThemeView.MY_COMPONENT_ID;
 import static com.vaadin.flow.uitest.ui.theme.ThemeView.OCTOPUSS_ID;
 import static com.vaadin.flow.uitest.ui.theme.ThemeView.SNOWFLAKE_ID;
@@ -271,6 +272,23 @@ public void cssWithAssetRelativePaths_urlPathIsNotRelative() {
                 imageUrl.matches(String.format(expectedIconsURL, "sun.svg")));
     }
 
+    /**
+     * Main theme.json does not include utility Lumo import which has border
+     * styling etc. Parent theme.json does include Lumo utility import.
+     */
+    @Test
+    public void parentTheme_lumoStyleAppliedFromParentTheme() {
+        open();
+        WebElement cssNodeLumoBorderDiv = findElement(
+                By.id(LUMO_BORDER_TOP_DIV));
+        Assert.assertEquals("solid",
+                cssNodeLumoBorderDiv.getCssValue("border-top-style"));
+        Assert.assertNotEquals("0px",
+                cssNodeLumoBorderDiv.getCssValue("border-top-width"));
+        Assert.assertEquals("rgba(26, 57, 96, 0.1)",
+                cssNodeLumoBorderDiv.getCssValue("border-top-color"));
+    }
+
     @Override
     protected String getTestPath() {
         String path = super.getTestPath();