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: support import without url keyword in theme generator #14234

Merged
merged 3 commits into from Aug 1, 2022

Conversation

mcollovati
Copy link
Collaborator

@mcollovati mcollovati commented Jul 29, 2022

With Vite it may happend that '@import url(...)' in CSS are rewritten
as '@import ...', that is a valid CSS statement.
Currently the theme generator does not handle the case and this prevents
the resource to be added to the head section. A warning is also
logged on browser console, stating import rules not allowed when
creating stylesheets.
This change updates the importMatcher regex to handle @import without url
In addition it also handles media query declared in import statements.

mshabarov
mshabarov previously approved these changes Jul 29, 2022
@mshabarov mshabarov enabled auto-merge (squash) July 29, 2022 09:27
@mcollovati mcollovati disabled auto-merge July 29, 2022 09:37
@mcollovati mcollovati marked this pull request as draft July 29, 2022 09:38
@mcollovati
Copy link
Collaborator Author

Need to do an additional check for Vite

@github-actions
Copy link

github-actions bot commented Jul 29, 2022

Unit Test Results

   905 files  ±0     905 suites  ±0   56m 50s ⏱️ +7s
5 896 tests ±0  5 853 ✔️ ±0  43 💤 ±0  0 ±0 
6 117 runs   - 1  6 067 ✔️  - 1  50 💤 ±0  0 ±0 

Results for commit 6896c89. ± Comparison against base commit e44a7eb.

♻️ This comment has been updated with latest results.

With Vite it may happend that '@import url(...)' in CSS are rewritten
as '@import ...', that is a valid CSS statement.
Currently the theme generator does not handle the case and this prevents
the resource to be added to the head section. A warning is also
logged on browser console, stating import rules not allowed when
creating stylesheets.
This change updates the importMatcher regex to handle @import without url
@mcollovati mcollovati force-pushed the fix/theme_generator_support_import_without_url branch from 377b8f4 to 6896c89 Compare July 29, 2022 13:16
@vaadin-bot vaadin-bot added +0.0.1 and removed +1.0.0 labels Jul 29, 2022
@sonarcloud
Copy link

sonarcloud bot commented Jul 29, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mcollovati mcollovati marked this pull request as ready for review July 29, 2022 13:34
@mshabarov mshabarov merged commit e4f7e10 into master Aug 1, 2022
@mshabarov mshabarov deleted the fix/theme_generator_support_import_without_url branch August 1, 2022 07:43
vaadin-bot pushed a commit that referenced this pull request Aug 1, 2022
With Vite it may happend that '@import url(...)' in CSS are rewritten
as '@import ...', that is a valid CSS statement.
Currently the theme generator does not handle the case and this prevents
the resource to be added to the head section. A warning is also
logged on browser console, stating import rules not allowed when
creating stylesheets.
This change updates the importMatcher regex to handle @import without url
mshabarov pushed a commit that referenced this pull request Aug 1, 2022
…14243)

With Vite it may happend that '@import url(...)' in CSS are rewritten
as '@import ...', that is a valid CSS statement.
Currently the theme generator does not handle the case and this prevents
the resource to be added to the head section. A warning is also
logged on browser console, stating import rules not allowed when
creating stylesheets.
This change updates the importMatcher regex to handle @import without url

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

This ticket/PR has been released with Vaadin 23.2.0.beta1 and is also targeting the upcoming stable 23.2.0 version.

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.

None yet

3 participants