-
Notifications
You must be signed in to change notification settings - Fork 16
Add add-chessboard-background and is-resizable mixins, refactor SCSS #3671
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
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughIntroduces two Sass mixins (add-chessboard-background, is-resizable) in src/style/mixins.scss and refactors multiple component/example SCSS files to use them, replacing inline SVG backgrounds and manual resizable CSS. Removes various explicit sizing/border/pseudo-element hint rules and imports the mixins module where used. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesPossibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3671/ |
e22d188 to
e0b5080
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.
Actionable comments posted: 8
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
src/components/action-bar/examples/_application.scss(1 hunks)src/components/ai-avatar/examples/ai-avatar-basic.scss(1 hunks)src/components/card/examples/card-resizable-container.scss(1 hunks)src/components/chart/examples/chart-resizable-container.scss(1 hunks)src/components/chip/chip.scss(1 hunks)src/components/color-picker/color-picker.scss(1 hunks)src/components/info-tile/examples/info-tile.scss(1 hunks)src/components/menu/examples/menu-surface-width.scss(2 hunks)src/components/notched-outline/examples/notched-outline-basic.scss(1 hunks)src/components/text-editor/examples/text-editor-size.scss(1 hunks)src/components/text-editor/prosemirror-adapter/plugins/image/view.scss(1 hunks)src/style/mixins.scss(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/components/**/*.scss
📄 CodeRabbit inference engine (.cursor/rules/css-class-names-in-components-using-shadow-dom.mdc)
Do not use BEM-style class names in CSS for components, as components use shadow-DOM.
Files:
src/components/notched-outline/examples/notched-outline-basic.scsssrc/components/menu/examples/menu-surface-width.scsssrc/components/color-picker/color-picker.scsssrc/components/chip/chip.scsssrc/components/info-tile/examples/info-tile.scsssrc/components/text-editor/prosemirror-adapter/plugins/image/view.scsssrc/components/action-bar/examples/_application.scsssrc/components/text-editor/examples/text-editor-size.scsssrc/components/ai-avatar/examples/ai-avatar-basic.scsssrc/components/card/examples/card-resizable-container.scsssrc/components/chart/examples/chart-resizable-container.scss
**/*.{tsx,scss}
⚙️ CodeRabbit configuration file
**/*.{tsx,scss}: Almost all our components use shadow-DOM. Therefore, we have no need of BEM-style class names in our CSS.
Files:
src/components/notched-outline/examples/notched-outline-basic.scsssrc/components/menu/examples/menu-surface-width.scsssrc/components/color-picker/color-picker.scsssrc/components/chip/chip.scsssrc/components/info-tile/examples/info-tile.scsssrc/components/text-editor/prosemirror-adapter/plugins/image/view.scsssrc/components/action-bar/examples/_application.scsssrc/components/text-editor/examples/text-editor-size.scsssrc/style/mixins.scsssrc/components/ai-avatar/examples/ai-avatar-basic.scsssrc/components/card/examples/card-resizable-container.scsssrc/components/chart/examples/chart-resizable-container.scss
🧠 Learnings (1)
📓 Common learnings
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3658
File: src/components/profile-picture/profile-picture.scss:120-124
Timestamp: 2025-09-03T07:17:23.267Z
Learning: Issue #3666 in the lime-elements repository tracks the removal of a CSS hack in profile-picture component that overrides icon color via --limel-theme-on-surface-color, to be replaced with proper icon color support in limel-icon-button.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Docs / Publish Docs
- GitHub Check: Build
- GitHub Check: Test
🔇 Additional comments (16)
src/components/text-editor/prosemirror-adapter/plugins/image/view.scss (1)
21-21: LGTM: mixin replaces inline SVG cleanlyBackground is now standardized and easier to maintain.
src/components/notched-outline/examples/notched-outline-basic.scss (2)
1-1: LGTM: imports mixins correctlyPath looks correct relative to this file’s location.
9-9: LGTM: uses chessboard mixinKeeps example concise and consistent.
src/components/chip/chip.scss (1)
152-152: LGTM: standardized invalid-state backgroundMixin usage matches PR direction; background-color layering is preserved.
src/components/menu/examples/menu-surface-width.scss (1)
11-16: LGTM: resizable via mixin; clamp usage is sensibleProduces a 30rem ideal with 10rem–100% bounds for the demo.
src/components/info-tile/examples/info-tile.scss (2)
1-1: LGTM: mixins import
3-12: LGTM: resizable container standardizedParameters are explicit and readable.
src/components/color-picker/color-picker.scss (1)
44-44: LGTM: replaces variable-based chessboard with mixinKeeps z-index layering intact.
src/components/ai-avatar/examples/ai-avatar-basic.scss (2)
1-1: LGTM: mixins import
8-15: LGTM: concise resizable setupHorizontal-only with small mins suits the demo.
src/components/action-bar/examples/_application.scss (2)
1-1: Mixins import path looks correct.
No issues.
12-16: Good migration to is-resizable; verify interaction with existing container rules.
Current block still has overflow: hidden; fixed height; and border set on Lines 5–7. If the intent was to drop these per the PR, confirm and remove to avoid clipping/hard sizing conflicting with the mixin.src/components/text-editor/examples/text-editor-size.scss (2)
1-1: Mixins import added—LGTM.
4-8: Resizing moved to mixin—confirm scroll/overflow behavior.
Ensure the mixin sets overflow and box-sizing so long content remains usable within the container.src/components/card/examples/card-resizable-container.scss (1)
1-1: Mixins import path is correct.src/components/chart/examples/chart-resizable-container.scss (1)
1-1: Mixins import—LGTM.
6dd10ca to
4e6f271
Compare
|
🎉 This PR is included in version 38.23.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fix #3669
Summary by CodeRabbit
New Features
Style
Refactor
Review:
Browsers tested:
(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)
Windows:
Linux:
macOS:
Mobile: