Support reusable application theme #9650
Conversation
The location should be as same as app theme in frontend/themes/. I.e. `target\classes\META-INF\VAADIN\static\themes\reusable-theme`.
tanbt
force-pushed
the
reusable-theme
branch
from
8eac0d4
to
958536e
Compare
tanbt
changed the title
flow-tests/test-application-theme/test-theme-reusable/README.md
Outdated
Show resolved
Hide resolved
...cation-theme/test-theme-reusable/src/main/java/com/vaadin/flow/uitest/ui/theme/AppShell.java
Outdated
Show resolved
Hide resolved
flow-tests/test-application-theme/test-theme-reusable/frontend/typescript/hello-world-view.ts
Outdated
Show resolved
Hide resolved
flow-tests/test-application-theme/test-theme-reusable/frontend/index.ts
Outdated
Show resolved
Hide resolved
flow-tests/test-application-theme/test-theme-reusable/frontend/index.html
Outdated
Show resolved
Hide resolved
...theme/test-theme-reusable/src/test/java/com/vaadin/flow/uitest/ui/theme/ReusableThemeIT.java
Outdated
Show resolved
Hide resolved
flow-tests/test-resources/src/main/java/com.vaadin.apptheme.reusabletheme/Dependency.java
Outdated
Show resolved
Hide resolved
flow-tests/test-resources/src/main/resources/META-INF/resources/themes/reusable-theme/README.md
Outdated
Show resolved
Hide resolved
Co-authored-by: caalador <mikael.grankvist@gmail.com>
...-theme/reusable-theme/src/main/resources/META-INF/resources/themes/reusable-theme/global.css
Show resolved
Hide resolved
|
SonarQube analysis reported 31 issues
Top 10 extra issuesNote: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:
|
| /** | ||
| * The name of the application theme root folder. | ||
| */ | ||
| public static final String APPLICATION_THEME_ROOT = "themes"; |
There was a problem hiding this comment.
@taefi is running for this to be in FrontendUtils.
Of course, agree to define it once, but I have a feeling that it is better to define it here since many other
default/hardcoded path and folder constants defined in this file, e.g. NODE_MODULES, FRONTEND,
DEFAULT_NODE_DIR, DEFAULT_FRONTEND_DIR, etc. I propose to keep it here but rename it to
APPLICATION_THEME_ROOT as Tan defined.
There was a problem hiding this comment.
So should I move this constant to FrontendUtils? Or @taefi will do that in his PR?
There was a problem hiding this comment.
Please resolve this kind of trivial issues as fast as possible by e.g. just agreeing over Slack
| @@ -134,7 +134,7 @@ protected void writeImportLines(List<String> lines) { | |||
|
|
|||
| if (themeDef != null && !"".equals(themeDef.getName())) { | |||
| // If we define a theme name we need to import theme/theme-generated.js | |||
| lines.add("import {applyTheme} from 'theme/theme-generated.js';"); | |||
| lines.add("import {applyTheme} from 'themes/theme-generated.js';"); | |||
There was a problem hiding this comment.
non-blocking: I'm not sure if this related to this PR (?) but the comment should have been changed too
| /** | ||
| * The name of the application theme root folder. | ||
| */ | ||
| public static final String APPLICATION_THEME_ROOT = "themes"; |
There was a problem hiding this comment.
Please resolve this kind of trivial issues as fast as possible by e.g. just agreeing over Slack
|
I guess @taefi can continue from |
Resolves #9585 # Conflicts: # flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskCopyFrontendFiles.java # flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdateThemeImport.java # flow-tests/pom.xml
Resolves #9585 # Conflicts: # flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskCopyFrontendFiles.java # flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdateThemeImport.java # flow-tests/pom.xml
Resolves #9585