fix: incorrect info color for element plus, fixed #4532 #4566
Conversation
|
WalkthroughThe changes in this pull request introduce new CSS custom properties for the dark and default theme design tokens. Specifically, two properties, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
packages/effects/hooks/src/use-design-tokens.ts (1)
262-265: Approve addition of mask color and suggest comment enhancement.The addition of the '--el-mask-color' CSS variable effectively addresses the background color issue for ElLoading, as mentioned in the PR objectives. The conditional setting based on the dark mode state ensures appropriate contrast in both themes.
Consider enhancing the comment to provide more context:
- // 解决ElLoading背景色问题 + // Fix background color issue for v-loading directive and el-loading componentThis change would make the comment more specific and align it with the PR description, improving clarity for future developers.
packages/@core/base/design/src/design-tokens/default/index.css (1)
41-45: LGTM! Consider adding a comment for clarity.The addition of
--infoand--info-foregroundvariables aligns with the PR objectives and follows the existing pattern for color variables. This change should resolve the styling issue for the info color in ElementPlus components.For consistency with other color variable groups, consider adding a comment above these new variables:
/* Used for destructive actions such as <Button variant="destructive"> */ --destructive: 359.33 100% 65.1%; --destructive-foreground: 0 0% 98%; + /* Used for informational actions such as <message> */ + --info: 240, 5%, 96%; --info-foreground: 220, 4%, 58%; /* Used for success actions such as <message> */This comment would improve readability and maintain the documentation style of the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- packages/@core/base/design/src/design-tokens/dark/index.css (1 hunks)
- packages/@core/base/design/src/design-tokens/default/index.css (1 hunks)
- packages/effects/hooks/src/use-design-tokens.ts (2 hunks)
🔇 Additional comments (4)
packages/effects/hooks/src/use-design-tokens.ts (2)
219-219: Approve change to info color and suggest visual verification.The modification of '--el-color-info-light-9' to use the '--info' CSS variable instead of '--secondary' aligns with the PR objective to fix the incorrect info color for ElementPlus. This change is likely to improve the consistency of info-related components.
To ensure the change has the desired effect, please verify the visual appearance of info-related components, such as info alerts or info badges, in both light and dark modes. Consider adding before/after screenshots to the PR description for easier review.
Line range hint
1-274: Summary of changes and their impact.The modifications in this file effectively address the PR objectives:
- The info color for ElementPlus has been corrected by updating the '--el-color-info-light-9' CSS variable.
- The mask color for the v-loading directive and el-loading component has been fixed by introducing the '--el-mask-color' CSS variable with appropriate values for light and dark modes.
These targeted changes should resolve the reported issues without introducing significant risks. The modifications are well-commented and maintain the overall structure of the
useElementPlusDesignTokensfunction.To ensure the changes have the desired effect across the application:
- Verify the appearance of info-related components (e.g., info alerts, badges) in both light and dark modes.
- Test the v-loading directive and el-loading component in various scenarios to confirm the mask color is correct in both themes.
- Consider adding before/after screenshots to the PR description for visual confirmation of the fixes.
packages/@core/base/design/src/design-tokens/default/index.css (1)
41-42: Verify if these changes address all mentioned issues.The addition of
--infoand--info-foregroundvariables should resolve the info color issues for most ElementPlus components. However, the PR objectives also mention problems with thev-loadingdirective andel-loadingcomponent, specifically the color of the mask layer.Can you confirm if these new variables will also fix the mask layer color for the loading components? If not, additional changes might be needed to fully address all the issues mentioned in the PR description.
packages/@core/base/design/src/design-tokens/dark/index.css (1)
42-43: LGTM: Addition of info color properties aligns with PR objectivesThe new CSS custom properties
--infoand--info-foregroundhave been added correctly. These additions are consistent with the existing color scheme and use the same HSL color model. This change directly addresses the PR objective of fixing the info color for ElementPlus components, which should resolve the styling issues in multi-select dropdowns, alert components, and potentially thev-loadingdirective andel-loadingcomponent.
Description
修复ElementPlus组件info样式的问题，影响多选select组件、alert等组件
修复v-loading指令以及el-loading组件的mask层颜色
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Summary by CodeRabbit
New Features
Bug Fixes
Documentation