Refactor checkbox component to remove MDC dependencies and update styles #3634
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 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 📝 WalkthroughWalkthroughThe checkbox component's SCSS and template were refactored to remove all dependencies on Material Design Components (MDC), replacing them with custom styles and markup. Related styles in list and table components were updated to accommodate the new checkbox styling. The dynamic label's gap was slightly reduced, and a helper text partial was deleted. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CheckboxComponent
participant CustomStyles
participant DOM
User->>CheckboxComponent: Interacts (click/check/uncheck)
CheckboxComponent->>DOM: Updates input state
CheckboxComponent->>CustomStyles: Applies custom classes/styles (checked, disabled, etc.)
CustomStyles->>DOM: Renders custom visuals (box, checkmark, label)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related issues
Suggested labels
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3634/ |
Kiarokh
force-pushed
the
mdc-checkbox-styles
branch
4 times, most recently
from
2a11807 to
f840d04
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the checkbox component to remove dependency on Material Design Components (MDC) styles while maintaining visual consistency. The changes implement custom CSS styling to replace MDC-specific classes and variables.
- Removes MDC-specific CSS classes and variables from checkbox templates and styles
- Implements custom CSS for checkbox appearance, states, and interactions
- Updates related components (table, list, dynamic-label) to work with the new checkbox implementation
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/components/checkbox/checkbox.scss |
Complete rewrite with custom CSS replacing MDC styles, including box styling, states, and animations |
src/components/checkbox/checkbox.template.tsx |
Simplified template structure removing MDC classes and elements |
src/components/checkbox/partial-styles/_helper-text.scss |
Removed file as helper text logic moved to main stylesheet |
src/components/table/partial-styles/_row-selection.scss |
Updated to use new checkbox CSS variables instead of MDC variables |
src/components/list/list.scss |
Removed MDC checkbox imports and added margin adjustment for checkbox elements |
src/components/dynamic-label/dynamic-label.scss |
Reduced gap size to accommodate new checkbox styling |
Comments suppressed due to low confidence (1)
src/components/checkbox/checkbox.scss:103
- The
:has()pseudo-class has limited browser support, especially in older browsers. Consider using a class-based approach or feature detection for better compatibility.
.checkbox:has(input[type='checkbox']:checked) & {
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
src/components/checkbox/checkbox.scss(1 hunks)src/components/checkbox/checkbox.template.tsx(1 hunks)src/components/checkbox/partial-styles/_helper-text.scss(0 hunks)src/components/dynamic-label/dynamic-label.scss(1 hunks)src/components/list/list.scss(1 hunks)src/components/table/partial-styles/_row-selection.scss(2 hunks)
💤 Files with no reviewable changes (1)
- src/components/checkbox/partial-styles/_helper-text.scss
🧰 Additional context used
📓 Path-based instructions (5)
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/dynamic-label/dynamic-label.scsssrc/components/list/list.scsssrc/components/table/partial-styles/_row-selection.scsssrc/components/checkbox/checkbox.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/dynamic-label/dynamic-label.scsssrc/components/list/list.scsssrc/components/checkbox/checkbox.template.tsxsrc/components/table/partial-styles/_row-selection.scsssrc/components/checkbox/checkbox.scss
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/wrap-multiple-jsx-elements-in-host-not-in-array.mdc)
When returning multiple JSX elements from the
rendermethod, never wrap them in an array literal. Instead, always wrap them in the special<Host>element. When there is already a single top-level element, it does not have to be wrapped in<Host>.
Files:
src/components/checkbox/checkbox.template.tsx
⚙️ CodeRabbit Configuration File
**/*.tsx: Our.tsxfiles are typically using StencilJS, not React. When a developer wants to return multiple top-level JSX elements from therendermethod, they will sometimes wrap them in an array literal. In these cases, rather than recommending they addkeyproperties to the elements, recommend removing the hardcoded array literal. Recommend wrapping the elements in StencilJS's special<Host>element.
Files:
src/components/checkbox/checkbox.template.tsx
**/*.{ts,tsx}
⚙️ CodeRabbit Configuration File
**/*.{ts,tsx}: Imports from other files in the same module (lime-elements) must use relative paths. Using absolute paths for imports will cause the production build to fail.
Files:
src/components/checkbox/checkbox.template.tsx
src/components/**/*.tsx
⚙️ CodeRabbit Configuration File
src/components/**/*.tsx: When contributors add new props to existing components in the lime-elements repository, they should always add documentation examples that demonstrate the new prop's usage and explain how it works. This helps with user adoption, feature discoverability, and maintains comprehensive documentation.
Files:
src/components/checkbox/checkbox.template.tsx
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
PR: Lundalogik/lime-elements#0
File: .cursor/rules/css-class-names-in-components-using-shadow-dom.mdc:0-0
Timestamp: 2025-07-21T00:27:58.489Z
Learning: Applies to src/components/**/*.scss : Do not use BEM-style class names in CSS for components, as components use shadow-DOM.
📚 Learning: in the lime-elements repository, css custom properties prefixed with `--lime-` are considered intern...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3529
File: src/components/lime-ai-avatar/lime-ai-avatar.scss:1-3
Timestamp: 2025-04-25T14:23:56.018Z
Learning: In the lime-elements repository, CSS custom properties prefixed with `--lime-` are considered internal implementation details and should not be documented in JSDoc comments, while other custom properties should be documented as they are part of the public API.
Applied to files:
src/components/dynamic-label/dynamic-label.scsssrc/components/list/list.scsssrc/components/table/partial-styles/_row-selection.scsssrc/components/checkbox/checkbox.scss
📚 Learning: in components using shadow dom (like those in lime-elements), the universal selector `*` is already ...
Learnt from: adrianschmidt
PR: Lundalogik/lime-elements#3529
File: src/components/ai-avatar/ai-avatar.scss:20-22
Timestamp: 2025-04-29T14:15:32.104Z
Learning: In components using Shadow DOM (like those in lime-elements), the universal selector `*` is already scoped to the component's shadow tree and won't cause unwanted side effects outside the component. There's no functional difference between `*` and `:host *` in this context.
Applied to files:
src/components/dynamic-label/dynamic-label.scsssrc/components/list/list.scsssrc/components/table/partial-styles/_row-selection.scsssrc/components/checkbox/checkbox.scss
📚 Learning: examples in lime elements use shadow dom (with `shadow: true` in the component decorator) for style ...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3518
File: src/components/notched-outline/examples/notched-outline-basic.scss:11-22
Timestamp: 2025-04-14T10:01:18.721Z
Learning: Examples in Lime Elements use Shadow DOM (with `shadow: true` in the component decorator) for style encapsulation, making additional class-based selectors unnecessary. This ensures styles are automatically scoped to each component without leaking to the rest of the application.
Applied to files:
src/components/dynamic-label/dynamic-label.scsssrc/components/list/list.scsssrc/components/table/partial-styles/_row-selection.scsssrc/components/checkbox/checkbox.scss
📚 Learning: in lime-elements, css custom properties follow a specific naming convention: 1. internal use variabl...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3529
File: src/components/lime-ai-avatar/lime-ai-avatar.scss:24-28
Timestamp: 2025-04-25T14:22:02.157Z
Learning: In lime-elements, CSS custom properties follow a specific naming convention:
1. Internal use variables (not for consumers) are prefixed with `--limel-component-name-`
2. External use variables (intended for consumers) are prefixed with `--component-name-`
Variables documented with JSDoc comments are typically meant for external use.
Applied to files:
src/components/dynamic-label/dynamic-label.scsssrc/components/list/list.scsssrc/components/table/partial-styles/_row-selection.scsssrc/components/checkbox/checkbox.scss
📚 Learning: in lime-elements, css custom properties like `--limel-theme-surface-background-color` are applied at...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3624
File: src/components/button-group/button-group.scss:115-118
Timestamp: 2025-07-30T14:20:41.791Z
Learning: In lime-elements, CSS custom properties like `--limel-theme-surface-background-color` are applied at :root level via `core-styles.scss` and should be expected to be available without fallbacks. The library's architecture guarantees these theme variables are present, so adding fallbacks would be redundant and go against the design intention.
Applied to files:
src/components/dynamic-label/dynamic-label.scsssrc/components/list/list.scsssrc/components/table/partial-styles/_row-selection.scsssrc/components/checkbox/checkbox.scss
📚 Learning: in lime elements components that use shadow dom (with `shadow: true` in the component decorator), ke...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3529
File: src/components/lime-ai-avatar/lime-ai-avatar.scss:96-122
Timestamp: 2025-04-25T14:18:07.755Z
Learning: In Lime Elements components that use Shadow DOM (with `shadow: true` in the component decorator), keyframe animation names don't need to be namespaced as they are automatically scoped to the component and won't collide with animations from other components.
Applied to files:
src/components/dynamic-label/dynamic-label.scsssrc/components/list/list.scsssrc/components/checkbox/checkbox.scss
📚 Learning: applies to src/components/**/*.scss : do not use bem-style class names in css for components, as com...
Learnt from: CR
PR: Lundalogik/lime-elements#0
File: .cursor/rules/css-class-names-in-components-using-shadow-dom.mdc:0-0
Timestamp: 2025-07-21T00:27:58.489Z
Learning: Applies to src/components/**/*.scss : Do not use BEM-style class names in CSS for components, as components use shadow-DOM.
Applied to files:
src/components/dynamic-label/dynamic-label.scsssrc/components/list/list.scsssrc/components/table/partial-styles/_row-selection.scsssrc/components/checkbox/checkbox.scss
📚 Learning: for stenciljs components in lime-elements, prefer using the `` element to wrap child elements ...
Learnt from: adrianschmidt
PR: Lundalogik/lime-elements#3529
File: src/components/lime-ai-avatar/lime-ai-avatar.tsx:30-38
Timestamp: 2025-04-25T13:32:56.396Z
Learning: For StencilJS components in lime-elements, prefer using the `<Host>` element to wrap child elements in the render method instead of returning an array of elements. This follows the standard pattern in the codebase and eliminates the need for key attributes.
Applied to files:
src/components/dynamic-label/dynamic-label.scsssrc/components/list/list.scss
📚 Learning: for stenciljs components in lime-elements, prefer using the `` element to wrap child elements ...
Learnt from: adrianschmidt
PR: Lundalogik/lime-elements#3529
File: src/components/lime-ai-avatar/lime-ai-avatar.tsx:30-38
Timestamp: 2025-04-25T13:32:56.396Z
Learning: For StencilJS components in lime-elements, prefer using the `<Host>` element to wrap child elements in the render method instead of returning an array of elements, which eliminates the need for key attributes and follows StencilJS best practices.
Applied to files:
src/components/dynamic-label/dynamic-label.scsssrc/components/list/list.scss
📚 Learning: in tabulator 6, the scss files are located in `dist/scss/tabulator.scss`, not in the old `src/scss/`...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3478
File: package.json:61-61
Timestamp: 2025-07-28T14:35:21.965Z
Learning: In Tabulator 6, the SCSS files are located in `dist/scss/tabulator.scss`, not in the old `src/scss/` path from version 4.x. The correct import path is `node_modules/tabulator-tables/dist/scss/tabulator.scss`.
Applied to files:
src/components/list/list.scsssrc/components/table/partial-styles/_row-selection.scsssrc/components/checkbox/checkbox.scss
📚 Learning: in limel-chip-set, the onclick handler should be placed on the content div () ra...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3518
File: src/components/chip-set/chip-set.tsx:367-385
Timestamp: 2025-04-14T12:22:22.298Z
Learning: In limel-chip-set, the onClick handler should be placed on the content div (<div slot="content">) rather than on the limel-notched-outline component, and should only be applied when type is 'input'.
Applied to files:
src/components/list/list.scsssrc/components/table/partial-styles/_row-selection.scsssrc/components/checkbox/checkbox.scss
📚 Learning: for lime-elements, example files should import types from the public api using '@limetech/lime-eleme...
Learnt from: adrianschmidt
PR: Lundalogik/lime-elements#3530
File: src/components/text-editor/examples/text-editor-composite.tsx:4-7
Timestamp: 2025-04-17T09:39:36.254Z
Learning: For lime-elements, example files should import types from the public API using 'limetech/lime-elements' rather than defining duplicate types locally. This includes types like `EditorUiType` which are part of the component's public API.
Applied to files:
src/components/table/partial-styles/_row-selection.scss
📚 Learning: lime elements generally does not support unreasonably old browsers, so modern css properties like `s...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3464
File: src/components/text-editor/prosemirror-adapter/plugins/image/view.scss:14-17
Timestamp: 2025-03-11T13:31:46.958Z
Learning: Lime Elements generally does not support unreasonably old browsers, so modern CSS properties like `scale` that have good support in current browsers are preferred over older alternatives.
Applied to files:
src/components/table/partial-styles/_row-selection.scss
📚 Learning: in scss files, css property values can be split across multiple lines when properly indented, partic...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3567
File: src/components/ai-avatar/ai-avatar.scss:0-0
Timestamp: 2025-05-28T08:37:45.610Z
Learning: In SCSS files, CSS property values can be split across multiple lines when properly indented, particularly for complex values like animation shorthands with CSS custom properties. This is valid SCSS syntax that compiles correctly to CSS and is often the result of auto-formatting tools.
Applied to files:
src/components/checkbox/checkbox.scss
🪛 Biome (2.1.2)
src/components/checkbox/checkbox.template.tsx
[error] 79-79: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
[error] 53-65: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ 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 (12)
src/components/dynamic-label/dynamic-label.scss (1)
9-9: LGTM! Clean spacing adjustment.The reduced gap aligns well with the overall spacing refinements in the checkbox component refactor.
src/components/table/partial-styles/_row-selection.scss (2)
9-9: Good alignment with checkbox refactor.The custom property
--limel-checkbox-min-heightproperly replaces the removed MDC variables and follows lime-elements naming conventions.
27-27: Appropriate vertical spacing adjustment.The padding-top adjustment helps align checkboxes properly within table rows. The
!importantis justified here to override Tabulator's default cell padding.src/components/list/list.scss (1)
143-145: Modern CSS approach for conditional styling.Using the
:has()pseudo-class to conditionally apply margin when a checkbox is present is cleaner than the previous fixed negative margin approach. This provides better layout flexibility.src/components/checkbox/checkbox.template.tsx (1)
53-64: Template structure simplification looks good.The flattened template structure with consolidated conditional classes is cleaner and aligns well with the custom styling approach.
src/components/checkbox/checkbox.scss (7)
1-13: Excellent foundation for the MDC-free implementation.The new variables and host styling provide a solid foundation. The minimum height prevents layout shifts during state changes, and the CSS custom properties follow lime-elements naming conventions correctly.
21-33: Well-structured container layout.The flex layout with isolation provides proper stacking context and alignment. The consistent height ensures good integration with forms and table rows.
35-41: Proper native checkbox hiding.Using the
visually-hiddenmixin ensures the native checkbox remains accessible to screen readers while being visually hidden.
43-70: Comprehensive label styling with proper state management.The label styling handles all necessary states (disabled, required, invalid) with appropriate visual feedback. The asterisk for required fields and color changes for invalid states improve usability.
72-166: Sophisticated checkbox box implementation.The
.boxstyling provides smooth transitions, proper state feedback, and excellent hover/focus states. The pseudo-elements for focus indication and indeterminate state are well-implemented.
168-201: Smooth SVG checkmark animation.The stroke-dasharray animation provides a pleasant checking experience. The conditional opacity based on checked and indeterminate states works correctly.
203-208: Good integration with related components.The dynamic label margin adjustments and helper line integration maintain consistency across the component ecosystem.
Kiarokh
force-pushed
the
mdc-checkbox-styles
branch
from
6e452a6 to
948122c
Compare
|
|
||
| $checkbox-distance-to-left-edge: 0.5rem; | ||
|
|
||
| .select-all, |
There was a problem hiding this comment.
hm, i can't select all by clicking the top check box anymore
Screen.Recording.2025-08-06.at.16.04.27.mov
There was a problem hiding this comment.
great catch. I'll check
There was a problem hiding this comment.
limel-checkbox is used the table component in 2 places. One is in the header row, to display a "select all" checkbox, and one in each individual row, for selecting individual rows.
In the current PR I removed MDC's styles (along with some classes) from limel-checkbox component. This change has made the "select all" checkbox unclickable. It is still possible to tab-focus the checkbox and toggle its state using keyboard. But clicking doesn't work.
When I inspect these two instances of limel-checkbox in limel-table, the only difference I see is presence of a (empty) <label> element on the other row checkboxes. But the one in the header doesn't have a label. I believe that is the reason is for the checkbox not being clickable. It seems that the label element is the only clickable part of the native checkbox element that I am using in CheckboxTemplate. The actual <input type="checkbox" element is hidden using CSS (due to styling needs). The <div class="box" that visualizes the checkbox in a custom way is actually not clickable at all, both due to pointer-events: none; and also even without that style, clicking it doesn't do anything anyways.
I'm currently investigating to see how to best solve this issue
There was a problem hiding this comment.
great! 💫 I will test once more
As a first step of getting rid of MDC, we need to make sure that the component renders correctly, without relying on any styles applied by MDC.
Kiarokh
force-pushed
the
mdc-checkbox-styles
branch
from
4c574f3 to
7f3d650
Compare
|
🎉 This PR is included in version 38.21.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
As a first step of getting rid of MDC, we need to make sure that the component renders correctly, without relying on any styles applied by MDC.
fix #3632
Summary by CodeRabbit
Refactor
Style
Chores
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: