Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: apply CssImport to exported webcomponent #19740

Merged
merged 8 commits into from
Aug 5, 2024

Conversation

tltv
Copy link
Member

@tltv tltv commented Aug 1, 2024

CssImport annotation with just a value attribute can be injected automatically to shadow root of all exported web components (embedded applications) using Constructable StyleSheets. WebComponentExporter should have a theme to make automation work properly in theme-generator.js. Theme property "autoInjectGlobalCssImports": true in theme.json enable auto injection. Disabled by default.

Fixes: #19700

Copy link

github-actions bot commented Aug 1, 2024

Test Results

1 128 files  ± 0  1 128 suites  ±0   1h 24m 7s ⏱️ + 2m 51s
7 310 tests + 3  7 260 ✅ + 3  50 💤 ±0  0 ❌ ±0 
7 667 runs  +33  7 607 ✅ +33  60 💤 ±0  0 ❌ ±0 

Results for commit 87dd3b5. ± Comparison against base commit f81d6bd.

♻️ This comment has been updated with latest results.

@@ -76,6 +78,8 @@ abstract class AbstractUpdateImports implements Runnable {
+ " tpl.innerHTML = block;\n"
+ " document.head.appendChild(tpl.content);\n" + "}";
private static final String IMPORT_INJECT = "import { injectGlobalCss } from 'Frontend/generated/jar-resources/theme-util.js';\n";
private static final String IMPORT_COMPOSE_LOAD_ON_DEMAND = "import { composeLoadOnDemand } from 'Frontend/generated/jar-resources/theme-util.js';\n";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The composeLoadOnDemand function seems not be used anymore. I suppose we can remove both the constant and the function definition in theme-utils

@@ -292,7 +314,7 @@ private Map<File, List<String>> process(Map<ChunkInfo, List<CssData>> css,
start = System.nanoTime();

chunkLoader.add("");
chunkLoader.add("const loadOnDemand = (key) => {");
chunkLoader.add("const loadOnDemand = (key, isWebcomponent) => {");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The isWebcomponent parameter seems not required anymore

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I leaved it in just in case. But as it's not used after all, I'll remove it.

Default value is false and global CssImport (with just a value) is not injected by default to exported web components.
Copy link

sonarcloud bot commented Aug 5, 2024

@tltv tltv marked this pull request as ready for review August 5, 2024 09:36
@mshabarov mshabarov merged commit b817168 into main Aug 5, 2024
41 of 42 checks passed
@mshabarov mshabarov deleted the fix/19700-cssimport-to-embedded branch August 5, 2024 11:39
vaadin-bot pushed a commit that referenced this pull request Aug 5, 2024
CssImport annotation with just a value attribute can be injected automatically to shadow root of all exported web components (embedded applications) using Constructable StyleSheets. WebComponentExporter should have a theme to make automation work properly in theme-generator.js. Theme property "autoInjectGlobalCssImports": true in theme.json enable auto injection. Disabled by default.

Fixes: #19700

Co-authored-by: Marco Collovati <marco@vaadin.com>
@vaadin-bot
Copy link
Collaborator

Hi @tltv and @mshabarov, when i performed cherry-pick to this commit to 23.5, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick b817168
error: could not apply b817168... fix: apply CssImport to exported webcomponent (#19740)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

@vaadin-bot
Copy link
Collaborator

Hi @tltv and @mshabarov, when i performed cherry-pick to this commit to 24.3, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick b817168
error: could not apply b817168... fix: apply CssImport to exported webcomponent (#19740)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

vaadin-bot added a commit that referenced this pull request Aug 5, 2024
CssImport annotation with just a value attribute can be injected automatically to shadow root of all exported web components (embedded applications) using Constructable StyleSheets. WebComponentExporter should have a theme to make automation work properly in theme-generator.js. Theme property "autoInjectGlobalCssImports": true in theme.json enable auto injection. Disabled by default.

Fixes: #19700

Co-authored-by: Tomi Virtanen <tltv@vaadin.com>
Co-authored-by: Marco Collovati <marco@vaadin.com>
tltv added a commit that referenced this pull request Aug 5, 2024
CssImport annotation with just a value attribute can be injected automatically to shadow root of all exported web components (embedded applications) using Constructable StyleSheets. WebComponentExporter should have a theme to make automation work properly in theme-generator.js. Theme property "autoInjectGlobalCssImports": true in theme.json enable auto injection. Disabled by default.

Fixes: #19700

Co-authored-by: Marco Collovati <marco@vaadin.com>
mcollovati added a commit that referenced this pull request Aug 6, 2024
…9746)

CssImport annotation with just a value attribute can be injected automatically to shadow root of all exported web components (embedded applications) using Constructable StyleSheets. WebComponentExporter should have a theme to make automation work properly in theme-generator.js. Theme property "autoInjectGlobalCssImports": true in theme.json enable auto injection. Disabled by default.

Fixes: #19700

Co-authored-by: Marco Collovati <marco@vaadin.com>
tltv added a commit that referenced this pull request Aug 6, 2024
CssImport annotation with just a value attribute can be injected automatically to shadow root of all exported web components (embedded applications) using Constructable StyleSheets. WebComponentExporter should have a theme to make automation work properly in theme-generator.js. Theme property "autoInjectGlobalCssImports": true in theme.json enable auto injection. Disabled by default.

Fixes: #19700

Co-authored-by: Marco Collovati <marco@vaadin.com>
mcollovati pushed a commit that referenced this pull request Aug 7, 2024
…9752)

CssImport annotation with just a value attribute can be injected automatically to shadow root of all exported web components (embedded applications) using Constructable StyleSheets. WebComponentExporter should have a theme to make automation work properly in theme-generator.js. Theme property "autoInjectGlobalCssImports": true in theme.json enable auto injection. Disabled by default.

Fixes: #19700
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WebComponentExporter: @CSSImport styles are registered in the head
4 participants