feat:[steps,loading,image,milstone] Support dark them #3044
Conversation
WalkthroughThis pull request updates CSS variable references in the steps component. In the Changes
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
packages/renderless/src/milestone/index.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. examples/sites/demos/pc/app/milestone/show-number.spec.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. examples/sites/demos/pc/app/milestone/solid-style.spec.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (14)
💤 Files with no reviewable changes (4)
🚧 Files skipped from review as they are similar to previous changes (10)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 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 PR introduces support for a dark theme in the Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/theme/src/steps/vars.less (2)
99-99: Dark Theme – Advanced 'Doing' Node Background Color
The variable for the advanced guide's "doing" node background is now set tovar(--tv-color-bg-3, #f0f0f0). Note that the fallback#f0f0f0is quite light compared to the dark theme elements. Please double-check that this color choice creates the proper contrast and fits the overall dark theme design.
163-163: Dark Theme – Advanced Text Color Update
The advanced text color is updated to usevar(--tv-color-text-inverse, #191919). Although the inverse variable is intended for dark themes, the fallback (#191919) remains a dark tone. It would be beneficial to confirm if this meets the design guidelines for readability on dark backgrounds.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/theme/src/steps/index.less(3 hunks)packages/theme/src/steps/vars.less(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
- GitHub Check: PR Unit Test
🔇 Additional comments (6)
packages/theme/src/steps/vars.less (2)
29-29: Dark Theme – Completed Node Background Color
The background color for completed nodes has been updated to usevar(--tv-color-text-inverse-black, #191919), which aligns well with the dark theme objective. Please verify that the fallback color (#191919) provides adequate contrast when paired with adjacent elements in dark mode.
53-53: Dark Theme – Completed Node Icon Color Update
The icon color for completed nodes now usesvar(--tv-color-icon-inverse, #191919). This change is consistent with the dark theme aesthetics. Just ensure that the chosen fallback (#191919) harmonizes with the other dark accents of the component.packages/theme/src/steps/index.less (4)
248-249: Icon Element Border Update
Within the.@{steps-prefix-cls}-iconblock, the icon border is now defined asborder: 1px solid var(--tv-Steps-node-done-border-color);. This update ensures consistency with the new color palette for completed nodes. Good work ensuring that the component styling references the updated theme variables.
877-879: Advanced Guide Label Color Adjustment
In the.labelstyle within the advanced guide current item, the text color is now updated tovar(--tv-Steps-line-doing-active-icon-color). This change appears to support the new dark theme color scheme. Please verify that the overall contrast remains optimal for readability.
1210-1213: Timeline – Process Current Icon Color Update
Within the.process-current .iconblock, the icon color is updated (line 1212) to usevar(--tv-Steps-line-doing-active-icon-color). This change is aligned with dark theme styling for active states. Confirm this visually to ensure consistency, especially in the timeline context.
1215-1217: Timeline – Fixicons Fill Color Updated
In the same timeline section, the fill property for.fixicons(line 1216) is now set tovar(--tv-Steps-line-doing-active-icon-color). This ensures that the fill color on icons in the process current state is consistent with the updated dark theme palette.
discreted66
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/theme/src/calendar-view/vars.less (2)
23-25: Review Dark Theme Variable Assignments.
The new property--tv-CalendarView-radio-icon-hover-coloris introduced with a value ofvar(--tv-color-bg), and the active inner background is updated tovar(--tv-color-bg-3, #fff). Please verify that these choices provide the desired contrast and hover effect for dark theme users—especially since dark themes often require careful color balancing.
35-35: Confirm Date Border Color Usage.
The new variable--tv-CalendarView-date-border-coloris set tovar(--tv-color-border). Ensure that this value offers sufficient contrast in both light and dark themes for clear visual separation of the date element.packages/theme/src/calendar-view/index.less (1)
52-54: Hover State Styling Validation.
Within the.tiny-radio-button__inner:hoverblock (line 52–54), the background color is now set tovar(--tv-CalendarView-radio-icon-hover-color). Verify that this provides a clear and accessible hover effect for dark mode, matching the overall theme’s design guidelines.packages/theme/src/milestone/vars.less (1)
13-78: Consider documenting theme adaptation in component comments.While the implementation of dark theme support is solid, adding documentation comments about how the milestone component adapts to different themes would be beneficial for future maintainers. Consider adding brief notes about key theming considerations in the component's documentation.
Consider adding a comment block explaining theme adaptation near the beginning of the file:
.inject-Milestone-vars() { + // Theme adaptation: + // - Text colors use inverse-black variables to ensure proper contrast in dark themes + // - Background colors adapt based on the theme context + // - Icon colors standardized to match the current theme + // 默认字号(不含旗子部分) --tv-Milestone-font-size: var(--tv-font-size-default, 14px); // doing状态文本色 --tv-Milestone-doing-text-color: var(--tv-color-text-inverse-tint, #fff);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
examples/sites/demos/pc/app/image/lazy-composition-api.vue(0 hunks)examples/sites/demos/pc/app/image/lazy.vue(0 hunks)examples/sites/demos/pc/app/loading/basic-usage-composition-api.vue(0 hunks)examples/sites/demos/pc/app/loading/basic-usage.vue(0 hunks)examples/sites/demos/pc/app/milestone/basic-usage.spec.ts(1 hunks)examples/sites/demos/pc/app/milestone/show-number.spec.ts(1 hunks)examples/sites/demos/pc/app/milestone/solid-style.spec.ts(1 hunks)packages/renderless/src/milestone/index.ts(1 hunks)packages/theme/src/calendar-view/index.less(5 hunks)packages/theme/src/calendar-view/vars.less(2 hunks)packages/theme/src/milestone/vars.less(1 hunks)packages/theme/src/steps/index.less(4 hunks)packages/theme/src/steps/vars.less(5 hunks)
💤 Files with no reviewable changes (4)
- examples/sites/demos/pc/app/loading/basic-usage.vue
- examples/sites/demos/pc/app/image/lazy.vue
- examples/sites/demos/pc/app/loading/basic-usage-composition-api.vue
- examples/sites/demos/pc/app/image/lazy-composition-api.vue
✅ Files skipped from review due to trivial changes (2)
- examples/sites/demos/pc/app/milestone/basic-usage.spec.ts
- packages/renderless/src/milestone/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/theme/src/steps/vars.less
- packages/theme/src/steps/index.less
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (12)
packages/theme/src/calendar-view/index.less (4)
24-24: Dynamic Border Color Integration.
The border on line 24 now usesvar(--tv-CalendarView-date-border-color). This change promotes theme flexibility. Confirm that this dynamic approach works seamlessly across both dark and light themes.
27-27: Border-Bottom Removal Check.
The ruleborder-bottom: none;on line 27 has been added. Please double-check that removing the bottom border aligns with the intended design, particularly in dark theme contexts where contrast and separation are critical.
128-139: Consistent Border Application in Calendar Containers.
The use ofvar(--tv-CalendarView-date-border-color)for the top border (line 128) and bottom border of the list (line 139) enhances visual consistency. Ensure that these changes create uniform boundaries in both light and dark themes.
147-148: Refining List Item Borders.
The list items now use a right border defined as1px solid var(--tv-CalendarView-date-border-color)(line 147), with the last item’s right border removed (lines 188–190). This strategy maintains a neat grid layout. Please verify that the visual separation remains effective in the dark theme implementation.Also applies to: 188-190
examples/sites/demos/pc/app/milestone/show-number.spec.ts (1)
25-46:❓ Verification inconclusive
Color standardization aligns with dark theme support.
The modification of icon colors to
rgb(25, 25, 25)is consistent with the theming updates in this PR. This ensures that the milestone component's icons will display correctly in both light and dark themes. Note that one icon color (on line 33) remainsrgb(255, 255, 255)as it appears to be for a special state (doing status).
🏁 Script executed:
#!/bin/bash # Check if these color changes are consistent across other milestone test files rg -l 'iconStyles.*color.*rgb\(25, 25, 25\)' examples/sites/demos/pc/app/milestone/Length of output: 85
I'm going to run a more targeted search in the affected file so that we can better verify the usage of these color codes. The initial search using the pattern
"iconStyles.*color.*rgb\(25, 25, 25\)"didn’t return any results—even though the file clearly shows explicit lines setting the icon colors—so I'll check directly for the color entries in the test file.Please run the following shell script to verify the occurrences of these color definitions in the file
examples/sites/demos/pc/app/milestone/show-number.spec.ts:#!/bin/bash echo "Occurrences of 'rgb(25, 25, 25)' in the file:" rg -n "'color': 'rgb(25, 25, 25)'" examples/sites/demos/pc/app/milestone/show-number.spec.ts echo "Occurrences of 'rgb(255, 255, 255)' in the file:" rg -n "'color': 'rgb(255, 255, 255)'" examples/sites/demos/pc/app/milestone/show-number.spec.tsOnce you’ve confirmed that the file contains the entries as shown in the snippet—with most icons using
'rgb(25, 25, 25)'for standardized theming and one icon retaining'rgb(255, 255, 255)'for its special state—please manually verify that this pattern is intentional and that no other test files need updating.
Review Update: Verify Color Standardization in Milestone Tests
- The milestone test file now uses
'rgb(25, 25, 25)'for icons to support the dark theme, which aligns with the theming updates in this PR.- One icon remains
'rgb(255, 255, 255)'to indicate a special (doing) state.- Action required: Since the automated search returned no output, please manually verify this behavior in all milestone test files to ensure consistency.
examples/sites/demos/pc/app/milestone/solid-style.spec.ts (2)
25-46: Color standardization consistent with dark theme implementation.The changes to icon colors (to
rgb(25, 25, 25)) match the updates in other milestone test files, ensuring consistency across different test cases. Again, the 'doing' state (line 33) correctly maintains its unique white color for proper visibility.
94-96:❓ Verification inconclusive
Verify background color compatibility with dark theme.
This test is checking for a specific background color (
rgb(179, 214, 255)) after clicking a button. Please ensure this background color is still appropriate in dark theme context, as it might need to be adjusted if it doesn't provide sufficient contrast in dark mode.
🏁 Script executed:
#!/bin/bash # Check if there are corresponding dark theme variables for this blue background color rg -A 1 "bg-color.*179, 214, 255" packages/theme/src/ || rg -A 1 "B3D6FF" packages/theme/src/Length of output: 92
Action Required: Manual Verification of Dark Theme Support
The test currently asserts that node icons use the background color
rgb(179, 214, 255). However, our shell check did not locate any corresponding dark theme variables in the repository. Please manually verify that this background color provides sufficient contrast in the dark theme. Specifically, review theming configurations within thepackages/theme/src/directory to confirm whether a different color or an adjustment might be needed for dark mode.
- Location:
examples/sites/demos/pc/app/milestone/solid-style.spec.ts(lines 94-96)- Next Step: Manually inspect dark mode configuration files in
packages/theme/src/or consult with the design team if necessary.packages/theme/src/milestone/vars.less (5)
65-65: Updated text color variable for dark theme support.Replacing
--tv-color-text-active-1with--tv-color-text-inverse-blackfor the milestone text color is appropriate for implementing dark theme support. This change ensures the text will adapt properly between light and dark themes while maintaining the same fallback color.
71-71: Updated description text color variable for theme consistency.Similar to the previous change, replacing
--tv-color-text-active-1with--tv-color-text-inverse-blackfor the flag description ensures consistent text appearance across themes. This modification helps maintain readability in both light and dark modes.
67-68:❓ Verification inconclusive
Verify flag background color in dark theme.
The flag background is set to
--tv-color-bg, #f5f5f5, which is a light gray. Ensure this color variable properly adapts in dark theme to maintain readability and visual distinction.
🏁 Script executed:
#!/bin/bash # Check how the base background color is handled in dark theme rg -A 1 "color-bg: " packages/theme/src/common/ || rg -A 1 "color-bg," packages/theme/src/common/Length of output: 230
Dark Theme Flag Background – Manual Verification Required
The original check couldn’t locate a
packages/theme/src/commondirectory, so we couldn’t verify how the dark theme overrides the--tv-color-bgvariable automatically. Please ensure that a dark theme override exists for this variable—perhaps in another file (e.g. a dedicated dark theme variables file such aspackages/theme/src/dark/vars.less)—to guarantee that the flag background adapts appropriately in dark mode.
- The script’s error indicates that the targeted directory isn’t present.
- Double-check your theme structure to confirm whether dark theme settings are managed elsewhere.
- Verify that the light gray fallback (
#f5f5f5) is suitably overridden in dark mode for readability.
37-38:❓ Verification inconclusive
Verify completed line background color compatibility with dark theme.
The completed line background color is set to
--tv-color-bg-primary, #191919, which is nearly black. In dark theme contexts, this might not provide sufficient contrast. Consider confirming whether this works well with dark backgrounds or if it needs a contrasting color variable.
🏁 Script executed:
#!/bin/bash # Check if there are corresponding dark theme variables for line background rg -A 1 "color-bg-primary.*#191919" packages/theme/src/common/Length of output: 131
Action Required: Verify Dark Theme Contrast for Completed Line Background
The completed line background is set using
--tv-color-bg-primarywith a fallback of#191919(nearly black). However, our initial search inpackages/theme/src/common/did not produce any results—likely because that directory doesn't exist or dark theme variables are defined elsewhere. Please manually verify where the dark theme variables (or equivalent contrasting definitions) are located in your codebase and confirm that the chosen color provides sufficient contrast in dark mode contexts. If the dark theme configuration is located in another directory, update the search accordingly; otherwise, consider using a more contrasting variable or color.
- File under review:
packages/theme/src/milestone/vars.less(Lines 37-38)- Next steps: Manually check other directories for dark theme variable definitions or re-run targeted searches in the repository to locate these settings.
49-50:❓ Verification inconclusive
Verify completed node background contrast in dark theme.
The completed node background is set to
--tv-color-bg-active-secondary, #ffffff(white). While this provides good contrast in light themes, ensure it adapts appropriately in dark themes to maintain sufficient contrast without being too bright.
🏁 Script executed:
#!/bin/bash # Check how the active-secondary background color is handled in dark theme rg -A 1 "color-bg-active-secondary" packages/theme/src/Length of output: 52567
ACTION: Verify Completed Node Background Contrast in Dark Theme
The milestone’s completed background color is defined as
--tv-Milestone-status-completed-bg-color: var(--tv-color-bg-active-secondary, #ffffff);
In light themes the fallback (#ffffff) ensures proper contrast, but in dark themes the variable resolves to the value set inpackages/theme/src/base/dark-theme.less—which isrgba(255, 255, 255, 0)(fully transparent). Please verify that using a transparent background against the dark theme’s underlying colors still achieves the intended contrast. If the current contrast is insufficient or visually inconsistent, consider updating the dark theme’s variable value accordingly.
discreted66
force-pushed
the
darkTheme3
branch
4 times, most recently
from
ad74fad to
2fe8783
Compare
PR
steps暗色主题适配
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit