-
Notifications
You must be signed in to change notification settings - Fork 24
Add lit
and initial web components
#2264
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
✅ Thanks for the improvements! Browse a preview of your changes using the link below.
To edit notification comments on pull requests, go to your Netlify project configuration. |
Regular components /dist/ files are receiving uncompressed dupes of the web components code. Lemme know if you see why. |
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.
This looks directionally correct to me! I do wonder about trying to isolate scss from component code. That is, instead of having component scss with use
statements that gets loaded as a text, what if we put the translation from scss-land into lit css template tags higher up, that is, in core
, vars
, etc. Then in component code we could simply import { vars } from './where/ever.js' and use them as normal in lit.
There's kinda a delicate dance, I'll admit, between maintaining scss for child consumers (mostly cf.gov) and making the lit components more idiomatic. Not sure what the best thing is here.
As to the practicalities of this PR, looks like there's some commented stuff in the file-upload index.js file we could trim.
82faa19
to
2b7fde6
Compare
One reason for the structure you see here is that I was trying to follow the USWDS structure somewhat (note though that they have an Another approach could be to have an intermediary JS file that serves as a bridge between the component JS and the SCSS file. Kind of like what you see on https://lit.dev/docs/components/styles/#sharing-styles, but in that example |
2b7fde6
to
061ca3d
Compare
061ca3d
to
d9ad251
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'm happy to go with this and if we encounter any friction we can adapt as needed. Nice work!
I figured this out. Internally within the DS packages, we had the convenience import of, e.g., |
I'm going to merge this and open a separate PR that tweaks the optimizations. |
Additions
lit
dependency for web components.Testing
Screenshots