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

refactor: drop use of include option with registerStyles #2454

Merged
merged 4 commits into from
Sep 6, 2021

Conversation

tomivirkki
Copy link
Member

@tomivirkki tomivirkki commented Sep 5, 2021

Adds a deprecation warning for the { include } option in registerStyles API.

Also drops internal use of { include } in favor of using ES modules for style sharing.

It's really recommended to use the "Hide whitespace changes" option in the "Files changed" view of this PR.

@tomivirkki tomivirkki marked this pull request as draft September 5, 2021 16:23
@tomivirkki tomivirkki force-pushed the include-deprecation-warning branch 2 times, most recently from 96749a4 to 828b80d Compare September 6, 2021 06:39
@tomivirkki tomivirkki marked this pull request as ready for review September 6, 2021 07:43
packages/select/theme/lumo/vaadin-select-styles.js Outdated Show resolved Hide resolved
packages/text-area/theme/lumo/vaadin-text-area-styles.js Outdated Show resolved Hide resolved
packages/text-area/theme/lumo/vaadin-text-area-styles.js Outdated Show resolved Hide resolved
}
`;

registerStyles('vaadin-combo-box-overlay', [overlay, menuOverlayCore, comboBoxOverlay], {
Copy link
Member

Choose a reason for hiding this comment

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

I was wondering if this PR addresses #1697 too? Or is that still an issue with new includes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems that this PR alone doesn't solve the issue. Maybe if we added some extra logic to registerStyles to eliminate duplicate CSSResults etc

@sonarcloud
Copy link

sonarcloud bot commented Sep 6, 2021

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 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@tomivirkki tomivirkki merged commit 1ed5818 into master Sep 6, 2021
@tomivirkki tomivirkki deleted the include-deprecation-warning branch September 6, 2021 13:03
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with platform 22.0.0.alpha3 and is also targeting the upcoming stable 22.0.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.

3 participants