-
Notifications
You must be signed in to change notification settings - Fork 87
refactor!: remove Lumo global .js style modules #10138
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
Conversation
c8a3de6 to
f3c92c8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not understand why we keep the individual utility modules. Wasn't the idea to modify react-components to add a static version of Utility.module.css to the repo instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, we could then drop utilities too. My only concern with copying is that file can be out of sync if we make dome changes, unless we generate it from utility.css from Lumo package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generating it from the CSS file also shouldn't be too much of an issue. I'll try to take a look at that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created vaadin/react-components#358 to generate the utilities CSS modules from the CSS sources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, merged and removed utilities in this PR as well.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused what the purpose of this is now, as it just imports random bits and pieces that are remaining. I don't think the package needs to have an entrypoint. Then again I'm lacking context on icon set and such.
Maybe something to discuss in the Daily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The iconset is the only part still loaded by Lumo class. I agree that we could drop an entrypoint, I just wasn't sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll merge this PR for now, let's discuss removal of all-imports.js on Monday. One of its benefits is to ensure user apps load vaadin-iconset with the following JS import (even though it's not a common scenario):
import '@vaadin/vaadin-lumo-styles';|
This ticket/PR has been released with Vaadin 25.0.0-alpha10. |



Description
Removed Lumo
.jsstyle modules, except for mixins (used by some add-ons) which are deprecated.Type of change