Skip to content

Commit

Permalink
refactor: rename the app-theme root folder from theme to themes (#9626)
Browse files Browse the repository at this point in the history
Fixes #9611
  • Loading branch information
taefi authored and caalador committed Mar 4, 2021
1 parent 3ce7e16 commit 4933dd3
Show file tree
Hide file tree
Showing 28 changed files with 35 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void execute() throws ExecutionFailedException {

try {
FileUtils.write(themeImportFile, String.format(
"import {applyTheme as _applyTheme} from 'theme/%s/%s.generated.js';%n"
"import {applyTheme as _applyTheme} from 'themes/%s/%s.generated.js';%n"
+ "export const applyTheme = _applyTheme;%n",
theme.getName(), theme.getName()), StandardCharsets.UTF_8);
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ const { copyStaticAssets } = require('./theme-copy');

let logger;

// matches theme folder name in 'theme/my-theme/my-theme.js'
const nameRegex = /theme\/(.*)\/\1.generated.js/g;
// matches theme folder name in 'themes/my-theme/my-theme.js'
const nameRegex = /themes\/(.*)\/\1.generated.js/g;

/**
* The application theme plugin is for generating, collecting and copying of theme files for the application theme.
Expand Down Expand Up @@ -65,11 +65,11 @@ class ApplicationThemePlugin {
for (let i = 0; i<this.options.themeProjectFolders.length; i++) {
const themeProjectFolder = this.options.themeProjectFolders[i];
if (fs.existsSync(themeProjectFolder)) {
logger.info("Searching theme folder ", themeProjectFolder, " for theme ", themeName);
logger.info("Searching themes folder ", themeProjectFolder, " for theme ", themeName);
const handled = handleThemes(themeName, themeProjectFolder, this.options.projectStaticAssetsOutputFolder);
if (handled) {
if(themeFound) {
throw new Error("Found theme filed in '" + themeProjectFolder + "' and '"
throw new Error("Found theme files in '" + themeProjectFolder + "' and '"
+ themeFound + "'. Theme should only be available in one folder");
}
logger.info("Found theme files from '", themeProjectFolder, "'");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
],
"repository": "vaadin/flow",
"name": "@vaadin/application-theme-plugin",
"version": "0.2.4",
"version": "0.2.5",
"main": "application-theme-plugin.js",
"author": "Vaadin Ltd",
"license": "Apache-2.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ function copyStaticAssets(themeName, themeProperties, projectStaticAssetsOutputF
Object.keys(copyRules).forEach((copyRule) => {
const nodeSources = path.resolve('node_modules/', module, copyRule);
const files = glob.sync(nodeSources, {nodir: true});
const targetFolder = path.resolve(projectStaticAssetsOutputFolder, "theme", themeName, copyRules[copyRule]);
const targetFolder = path.resolve(projectStaticAssetsOutputFolder, "themes", themeName, copyRules[copyRule]);

fs.mkdirSync(targetFolder, {
recursive: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

/**
* This file handles the generation of the '[theme-name].js' to
* the theme/[theme-name] folder according to properties from 'theme.json'.
* the themes/[theme-name] folder according to properties from 'theme.json'.
*/
const glob = require('glob');
const path = require('path');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
],
"repository": "vaadin/flow",
"name": "@vaadin/theme-loader",
"version": "0.0.3",
"version": "0.0.4",
"main": "theme-loader.js",
"author": "Vaadin Ltd",
"license": "Apache-2.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ module.exports = function (source, map) {
const logger = this.getLogger("theme-loader");

let themeFolder = handledResourceFolder;
// Recurse up until we find the theme folder or don't have 'theme' on the path.
while (themeFolder.indexOf("theme") > 1
&& path.basename(path.resolve(themeFolder, "..")) !== "theme") {
// Recurse up until we find the themes folder or don't have 'themes' on the path.
while (themeFolder.indexOf("themes") > 1
&& path.basename(path.resolve(themeFolder, "..")) !== "themes") {
themeFolder = path.resolve(themeFolder, "..");
}
// If we have found no theme folder return without doing anything.
if (path.basename(path.resolve(themeFolder, "..")) !== "theme") {
// If we have found no themes folder return without doing anything.
if (path.basename(path.resolve(themeFolder, "..")) !== "themes") {
this.callback(null, source, map);
return;
}
Expand All @@ -35,11 +35,11 @@ module.exports = function (source, map) {
source = source.replace(urlMatcher, function (match, url, quoteMark, replace, fileUrl, endString) {
let absolutePath = path.resolve(handledResourceFolder, replace, fileUrl);
if (fs.existsSync(absolutePath) && absolutePath.startsWith(themeFolder)) {
const frontendThemeFolder = "theme/" + path.basename(themeFolder);
const frontendThemeFolder = "themes/" + path.basename(themeFolder);
logger.debug("Updating url for file", "'" + replace + fileUrl + "'", "to use", "'" + frontendThemeFolder + "/" + fileUrl + "'");
const pathResolved = absolutePath.substring(themeFolder.length).replace(/\\/g, '/');

// keep the url the same except replace the ./ or ../ to theme/[themeFolder]
// keep the url the same except replace the ./ or ../ to themes/[themeFolder]
if (quoteMark) {
return url + quoteMark + frontendThemeFolder + pathResolved + endString;
}
Expand Down
9 changes: 5 additions & 4 deletions flow-server/src/main/resources/webpack.generated.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ const path = require('path');
const baseDir = path.resolve(__dirname);
// the folder of app resources (main.js and flow templates)

// this matches /theme/my-theme/ and is used to check css url handling and file path build.
const themePartRegex = /(\\|\/)theme\1[\s\S]*?\1/;
// this matches /themes/my-theme/ and is used to check css url handling and file path build.
const themePartRegex = /(\\|\/)themes\1[\s\S]*?\1/;

const frontendFolder = '[to-be-generated-by-flow]';

Expand Down Expand Up @@ -49,7 +49,7 @@ const projectStaticAssetsOutputFolder = [to-be-generated-by-flow];

// Folders in the project which can contain application themes
const themeProjectFolders = projectStaticAssetsFolders.map((folder) =>
path.resolve(folder, 'theme')
path.resolve(folder, 'themes')
);


Expand Down Expand Up @@ -165,7 +165,7 @@ module.exports = {
url: (url, resourcePath) => {
// Only translate files from node_modules
const resolve = resourcePath.match(/(\\|\/)node_modules\1/);
const themeResource = resourcePath.match(themePartRegex) && url.match(/^theme\/[\s\S]*?\//);
const themeResource = resourcePath.match(themePartRegex) && url.match(/^themes\/[\s\S]*?\//);
return resolve || themeResource;
},
// use theme-loader to also handle any imports in css files
Expand Down Expand Up @@ -242,6 +242,7 @@ module.exports = {
})] : []),

new ApplicationThemePlugin({
// The following matches target/flow-frontend/theme/theme-generated.js and not frontend/themes
themeResourceFolder: path.resolve(flowFrontendFolder, 'theme'),
themeProjectFolders: themeProjectFolders,
projectStaticAssetsOutputFolder: projectStaticAssetsOutputFolder,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,20 +45,20 @@ public void applicationTheme_GlobalCss_isUsedOnlyInEmbeddeComponent() {
final TestBenchElement themedComponent = $("themed-component").first();

Assert.assertEquals(
"url(\"" + getRootURL() + "/VAADIN/static/theme/embedded-theme/img/bg.jpg\")",
"url(\"" + getRootURL() + "/VAADIN/static/themes/embedded-theme/img/bg.jpg\")",
themedComponent.getCssValue("background-image"));

Assert.assertEquals("Ostrich",
themedComponent.getCssValue("font-family"));

final WebElement body = findElement(By.tagName("body"));
Assert.assertNotEquals(
"url(\"" + getRootURL() + "/path/VAADIN/static/theme/embedded-theme/img/bg.jpg\")",
"url(\"" + getRootURL() + "/path/VAADIN/static/themes/embedded-theme/img/bg.jpg\")",
body.getCssValue("background-image"));

Assert.assertNotEquals("Ostrich", body.getCssValue("font-family"));

getDriver().get(getRootURL() + "/VAADIN/static/theme/embedded-theme/img/bg.jpg");
getDriver().get(getRootURL() + "/VAADIN/static/themes/embedded-theme/img/bg.jpg");
Assert.assertFalse("app-theme background file should be served",
driver.getPageSource().contains("Could not navigate"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public ThemeView() {
faText.setId(FONTAWESOME_ID);

Image snowFlake = new Image(
"theme/app-theme/fortawesome/icons/snowflake.svg", "snowflake");
"themes/app-theme/fortawesome/icons/snowflake.svg", "snowflake");
snowFlake.setHeight("1em");
snowFlake.setId(SNOWFLAKE_ID);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ public class ThemeIT extends ChromeBrowserTest {

@Test
public void secondTheme_staticFilesNotCopied() {
getDriver().get(getRootURL() + "/path/theme/app-theme/img/bg.jpg");
getDriver().get(getRootURL() + "/path/VAADIN/static/themes/app-theme/img/bg.jpg");
Assert.assertFalse("app-theme static files should be copied",
driver.getPageSource().contains("HTTP ERROR 404"));

getDriver().get(getRootURL() + "/path/theme/app-theme/no-copy.txt");
getDriver().get(getRootURL() + "/path/VAADIN/static/themes/no-copy/no-copy.txt");
Assert.assertTrue("no-copy theme should not be handled",
driver.getPageSource().contains("HTTP ERROR 404"));
}
Expand All @@ -54,15 +54,15 @@ public void applicationTheme_GlobalCss_isUsed() {
checkLogsForErrors();

final WebElement body = findElement(By.tagName("body"));
// Note theme/app-theme gets VAADIN/static from the file-loader
// Note themes/app-theme gets VAADIN/static from the file-loader
Assert.assertEquals(
"url(\"" + getRootURL() + "/path/VAADIN/static/theme/app-theme/img/bg.jpg\")",
"url(\"" + getRootURL() + "/path/VAADIN/static/themes/app-theme/img/bg.jpg\")",
body.getCssValue("background-image"));

Assert.assertEquals("Ostrich", body.getCssValue("font-family"));

// Note theme/app-theme gets VAADIN/static from the file-loader
getDriver().get(getRootURL() + "/path/VAADIN/static/theme/app-theme/img/bg.jpg");
// Note themes/app-theme gets VAADIN/static from the file-loader
getDriver().get(getRootURL() + "/path/VAADIN/static/themes/app-theme/img/bg.jpg");
Assert.assertFalse("app-theme background file should be served",
driver.getPageSource().contains("Could not navigate"));
}
Expand Down Expand Up @@ -114,10 +114,10 @@ public void subCssWithRelativePath_urlPathIsNotRelative() {
open();
checkLogsForErrors();

// Note theme/app-theme gets VAADIN/static from the file-loader
// Note themes/app-theme gets VAADIN/static from the file-loader
Assert.assertEquals("Imported css file URLs should have been handled.",
"url(\"" + getRootURL()
+ "/path/VAADIN/static/theme/app-theme/icons/archive.png\")",
+ "/path/VAADIN/static/themes/app-theme/icons/archive.png\")",
$(SpanElement.class).id(SUB_COMPONENT_ID)
.getCssValue("background-image"));
}
Expand All @@ -128,9 +128,9 @@ public void staticModuleAsset_servedFromAppTheme() {
checkLogsForErrors();

Assert.assertEquals(
"Node assets should have been copied to 'theme/app-theme'",
"Node assets should have been copied to 'themes/app-theme'",
getRootURL()
+ "/path/theme/app-theme/fortawesome/icons/snowflake.svg",
+ "/path/themes/app-theme/fortawesome/icons/snowflake.svg",
$(ImageElement.class).id(SNOWFLAKE_ID).getAttribute("src"));

open(getRootURL() + "/path/" + $(ImageElement.class).id(SNOWFLAKE_ID)
Expand Down

0 comments on commit 4933dd3

Please sign in to comment.