refactor(checkbox): remove MDC dependency #3640
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 📝 WalkthroughWalkthroughRemoved MDC classes and integration from the checkbox component. The template dropped MDC-specific classes. The component now directly manages the native input element, refactoring initialization, teardown, state synchronization, and change emission without MDC. Public APIs remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Checkbox as limel-checkbox
participant Input as native input[type="checkbox"]
participant App as App Listener
User->>Input: Click/toggle
Input-->>Checkbox: change event
Checkbox->>Input: Sync checked/indeterminate states
Checkbox-->>App: Emits change with computed checked value
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Assessment against linked issues
Possibly related issues
Possibly 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. 🪧 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/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Pull Request Overview
This PR removes the Material Design Components (MDC) dependency from the checkbox component, replacing MDC-specific functionality with native browser APIs and direct DOM manipulation.
- Removes MDC checkbox and form field imports and initialization
- Replaces MDC checkbox state management with direct HTMLInputElement property manipulation
- Removes MDC-specific CSS classes and animation cleanup logic
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/components/checkbox/checkbox.tsx | Removes MDC instances and replaces with native input element management |
| src/components/checkbox/checkbox.template.tsx | Removes MDC-specific CSS classes from the template |
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3640/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/components/checkbox/checkbox.template.tsx(0 hunks)src/components/checkbox/checkbox.tsx(3 hunks)
💤 Files with no reviewable changes (1)
- src/components/checkbox/checkbox.template.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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.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.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.tsx
**/*.{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/checkbox/checkbox.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.tsx
⏰ 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
Kiarokh
force-pushed
the
checkbox-remove-mdc-dependency
branch
3 times, most recently
from
9f02cb6 to
95f1ca2
Compare
`shadowRoot` is not available in `connectedCallback` (runs before first render). So we need to guard against it in `getCheckboxElement` being `null` to avoid runtime errors.
Kiarokh
force-pushed
the
checkbox-remove-mdc-dependency
branch
from
95f1ca2 to
7c68454
Compare
|
🎉 This PR is included in version 38.21.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fix #3633
Summary by CodeRabbit
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: