-
Notifications
You must be signed in to change notification settings - Fork 283
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(checkbox): [checkbox] Component dark theme adaptation #3007
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request restructures the checkbox component’s styling and rendering behavior. In the theme package, several SVG-related styles were removed, and new, state-specific styles (including hover and disabled states) were added in the LESS file and updated via new CSS variables in the vars file. Additionally, the Vue component’s rendering logic was adjusted to use a different icon class for the unchecked state. No public or exported entities were modified. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CB as Checkbox Component
participant SVG as SVG Icon
participant TS as Theme Styles
U->>CB: Toggle checkbox state
alt Checkbox is checked
CB->>SVG: Render icon with "icon-checked-sur" or "icon-halfselect"
else Checkbox is unchecked
CB->>SVG: Render icon with "icon-not-check"
end
SVG->>TS: Apply corresponding style variables (border, background)
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
WalkthroughThis pull request adapts the checkbox component to support a dark theme. It involves significant changes to the styling of the checkbox, including updates to the LESS files for theme variables and component styles, as well as modifications to the Vue component to reflect these style changes. Changes
|
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: 3
🧹 Nitpick comments (1)
packages/theme/src/checkbox/index.less (1)
152-180
: Good implementation of SVG styling for different checkbox states.The addition of specific SVG styling for different checkbox states (checked, half-selected, and unchecked) is well-structured and properly leverages CSS variables for theme customization.
However, there's a potential issue with the unchecked state background:
- fill: rgba(255,255,255,0.02); + fill: var(--tv-Checkbox-unchecked-bg-color);The current semi-transparent white background (
rgba(255,255,255,0.02)
) may not render correctly in all theme environments. Consider creating a new CSS variable for this purpose in the vars file to ensure consistent behavior across themes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/theme/src/checkbox/index.less
(1 hunks)packages/theme/src/checkbox/vars.less
(1 hunks)packages/vue/src/checkbox/src/pc.vue
(1 hunks)
🔇 Additional comments (1)
packages/vue/src/checkbox/src/pc.vue (1)
73-73
: Updated the unchecked state styling with an appropriate class name.This change modifies the class name for the unchecked checkbox icon from
icon-check
toicon-not-check
, which better aligns with its purpose in the UI and allows for more specific styling in the dark theme adaptation.
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: 0
♻️ Duplicate comments (1)
packages/theme/src/checkbox/index.less (1)
181-200
: 🛠️ Refactor suggestionReplace Hardcoded Color in Disabled State Icons
Within the disabled state for the checkbox input, the
path:last-child
of the.icon-checked-sur, .icon-halfselect
block (line 187) is still using a hardcoded color (#FFFFFF
). This duplicates an earlier review comment recommending the use of a CSS variable to ensure consistent theming. Consider replacing the hardcoded value with the appropriate CSS variable (for example,var(--tv-Checkbox-border-color-inverse)
) to maintain theme consistency.Proposed diff:
- fill: #FFFFFF; + fill: var(--tv-Checkbox-border-color-inverse);
🧹 Nitpick comments (1)
packages/theme/src/checkbox/index.less (1)
151-180
: Dark Theme Input Styling – Updated SVG and Icon StatesThe new styles under
&__input
effectively set the SVG dimensions and update the icon fill properties for various states. The use of CSS variables (e.g.,var(--tv-Checkbox-svg-width)
,var(--tv-Checkbox-bg-active-color)
, andvar(--tv-Checkbox-border-color-inverse)
) helps ensure consistency with the dark theme adaptation. Please double-check that the values for the subtle fill (rgba(255,255,255,0.02)
) and hover state (var(--tv-Checkbox-unchecked-border-hover-color)
) match the design specifications across all states.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/theme/src/checkbox/index.less
(1 hunks)packages/theme/src/checkbox/vars.less
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/theme/src/checkbox/vars.less
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
feat(checkbox): [checkbox] Component dark theme adaptation
Summary by CodeRabbit