-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat: css variables #324
feat: css variables #324
Conversation
🦋 Changeset detectedLatest commit: a1b5e92 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Reviewer's Guide by SourceryThis pull request introduces the use of CSS variables for design tokens, replacing existing --sl-* variables with --ecc-* variables across multiple stylesheets. It also updates component styles to use these new variables and imports a new primitive stylesheet. Additionally, inline CSS styles in demo HTML files are commented out, and new form fields are added to the form demo. Two new files are added: one for primitive CSS variables and another for details component styles. File-Level Changes
Tips
|
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.
Hey @anuragxxd - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
Hey @anuragxxd, great work on this. Can you just add examples of how the css variables can be used to change the theme of the components in the demo so that new users can have something to play around with in plain HTML when they're trying to get a better understanding of how to style the components. |
Sure, There is still work left on the docs part. I will try to sort that out before the weekly meeting for better momentum. |
hey @anuragxxd, you haven't provided a way to access the text in the editor. I think a simple way to handle this would be to have the component fire an event whenever the text is modified. If you want i can implement this myself incase you're too busy at the moment. I just didn't want to work on an implementation if you have already built this functionality but are yet to push it. |
hey @anuragxxd, could you please take a look at the above stated issue, it is blocker for a feature i am working on. Thank you! |
Hey @SalihuDickson, was little busy but have pushed the changes now. Also, ace will also now show the errors too based on the language! |
Signed-off-by: Anurag Gupta <[email protected]>
Hi @SalihuDickson and @JaeAeich, |
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.
LGTM!
Whole lib changed in this PR though, hopefully the design will be consistent from now on 🤞, it was a necessary change.
Please format the files though!
Signed-off-by: Anurag Gupta <[email protected]>
Description
Fixes #159
Checklist
Comments
Summary by Sourcery
This pull request introduces a comprehensive set of CSS variables to standardize and simplify the styling across the project. It refactors existing styles to use these new variables and updates various components to ensure consistency. Additionally, it enhances the form, collection, and details components with new styles and updates the demo HTML files accordingly.