fix(milestone): [milestone] repair component sample document #2463
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes primarily involve updates to the 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 (
|
github-actions
bot
added
bug
chenxi-20
changed the title
chenxi-20
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: 5
🧹 Outside diff range and nitpick comments (12)
examples/sites/demos/pc/app/milestone/solid-style-composition-api.vue (1)
Line range hint 19-44: Enhance the educational value of the demo data
While the component implementation is correct, the demo data could be more meaningful:
- The status 'xxx' is not a descriptive name for a milestone state
- Some milestone names are repetitive (e.g., multiple "completed 完成状态")
- The timestamps could follow a more logical progression
Consider updating the demo data to better represent real-world use cases. For example:
const milestoneData = ref([
{
- name: 'completed 完成状态',
+ name: 'Requirements Gathering',
time: '2018-9-7',
status: 'completed'
},
{
- name: 'completed 完成状态',
+ name: 'Design Phase',
time: '2018-9-8',
status: 'completed'
},
{
- name: 'doing 未完成状态',
+ name: 'Development Sprint',
time: '2018-9-10',
status: 'doing'
},
{
- name: 'xxx 未完成状态',
+ name: 'Code Review',
time: '2018-9-9',
- status: 'xxx'
+ status: 'pending'
},examples/sites/demos/pc/app/milestone/show-number-composition-api.vue (2)
Line range hint 21-25: Consider documenting color meanings in statusMap.
The statusMap defines various status colors but lacks documentation about their semantic meaning or usage guidelines. This could be helpful for maintainability.
Consider adding a comment explaining the color scheme:
const statusMap = ref({
+ // Status colors following the design system:
+ // - Completed: Uses disabled control active background color
+ // - Doing: Success/progress green
+ // - Back: Error/warning red
+ // - End: Warning yellow
+ // - Cancel: Neutral grey
completed: 'var(--tv-color-bg-disabled-control-active)',
doing: '#7ED321',
back: '#f5222d',
end: '#faad14',
cancel: '#d9d9d9'
})Line range hint 21-25: Consider using design system variables for all status colors.
While 'completed' status uses a design system variable, other statuses use hardcoded hex values. For better maintainability and consistency, consider using design system variables for all status colors.
Consider updating other status colors to use design system variables if available:
const statusMap = ref({
completed: 'var(--tv-color-bg-disabled-control-active)',
- doing: '#7ED321',
- back: '#f5222d',
- end: '#faad14',
- cancel: '#d9d9d9'
+ doing: 'var(--tv-color-success)',
+ back: 'var(--tv-color-error)',
+ end: 'var(--tv-color-warning)',
+ cancel: 'var(--tv-color-text-disabled)'
})examples/sites/demos/pc/app/milestone/custom-bottom-top-composition-api.vue (1)
Line range hint 1-60: Consider implementing a status design system
The component uses various status colors and styles. Consider implementing a comprehensive status design system that includes:
- A centralized theme configuration for status colors
- Reusable status-related components or mixins
- Documentation for status styling guidelines
This would improve maintainability and consistency across the milestone components.
examples/sites/demos/pc/app/milestone/solid-style.vue (1)
20-20: Consider updating documentation for CSS variable changes.
The change from ti-common- to tv- prefix suggests a broader design system update. This example should include a comment explaining the color choice to help other developers understand the intended usage.
statusMap: {
- completed: 'var(--tv-color-bg-disabled-control-active)',
+ // Use disabled-control-active color to indicate a milestone that is no longer actionable
+ completed: 'var(--tv-color-bg-disabled-control-active)',
doing: '#7ED321',examples/sites/demos/pc/app/milestone/custom-bottom-top.vue (1)
66-68: LGTM: Consider adding a comment for the status class
The new status class implementation is clean and follows Vue best practices. Consider adding a brief comment to document its purpose.
+// Ensures consistent inline display of status indicators
.status {
display: inline-block;
}examples/sites/demos/pc/app/milestone/show-number.vue (1)
Line range hint 1-71: Consider enhancing the demo documentation.
Since this is a component sample document, it would be helpful to:
- Add comments explaining the different status colors and their use cases
- Include a description of the show-number functionality
- Document the available status types (completed, doing, back, end, cancel)
Example enhancement:
<template>
<div class="demo-milestone">
+ <!-- Demo showcases the show-number feature of the Milestone component -->
+ <!-- Toggle button to demonstrate dynamic show-number behavior -->
<tiny-button @click="handleClick(!showNum)" style="max-width: unset"
>设置 show-number 值为 {{ !showNum }}</tiny-button
>
+ <!-- Milestone component with various status types:
+ - completed: Indicates successfully completed milestones
+ - doing: Represents currently active milestones
+ - back: Shows reversed or rollback states
+ - end: Indicates terminated states
+ - cancel: Represents cancelled milestones
+ -->
<tiny-milestone
:data="milestoneData"
:milestones-status="statusMap"
:show-number="showNum"
:start="1"
></tiny-milestone>
</div>
</template>examples/sites/demos/pc/app/milestone/custom-flag-composition-api.vue (1)
Line range hint 89-91: Update deprecated deep selector syntax.
The ::v-deep combinator is deprecated. Consider updating to the new :deep() selector.
-.demo-milestone ::v-deep .tiny-milestone__description-status {
+.demo-milestone :deep(.tiny-milestone__description-status) {
margin-top: 4px;
}examples/sites/demos/pc/app/milestone/milestone-events-composition-api.vue (1)
Line range hint 1-24: Consider the following improvements to enhance maintainability.
- The status map could be defined as a constant since it doesn't need to be reactive:
-const statusMap = ref({
+const statusMap = {
completed: 'var(--tv-color-bg-disabled-control-active)',
doing: '#7ED321',
back: '#f5222d',
end: '#faad14',
cancel: '#d9d9d9'
-})
+}- Consider moving the sample milestone data to a separate file to keep the component focused on its logic.
- If possible, add TypeScript for better type safety, especially for event handlers.
Would you like me to provide a TypeScript version of this component?
examples/sites/demos/pc/app/milestone/data-field-mapping-composition-api.vue (1)
Line range hint 1-99: Enhance the demo documentation
While the demo effectively shows data field mapping, it could be more educational with inline comments explaining:
- The purpose of each status color
- The relationship between
progressandcompletedstates - The significance of the flag configurations
Consider adding JSDoc comments above the statusMap and milestoneData to explain their structure and usage:
+/**
+ * Status color mapping for milestone states
+ * @type {Record<string, string>}
+ */
const statusMap = ref({
completed: 'var(--tv-color-bg-disabled-control-active)',
doing: '#7ED321',
back: '#f5222d',
end: '#faad14',
cancel: '#d9d9d9'
})
+/**
+ * Sample milestone data demonstrating field mapping
+ * Shows various states and nested flag configurations
+ */
const milestoneData = ref([examples/sites/demos/pc/app/milestone/milestone-events.vue (1)
Line range hint 1-116: Enhance the sample documentation.
As this is a documentation example file, consider the following improvements to make it more educational:
- Add comments explaining the different status types and their use cases
- Include examples of all possible milestone states
- Document the event handlers with expected use cases
Add JSDoc comments to explain the component's purpose and usage:
<script lang="jsx">
import { TinyMilestone, TinyNotify } from '@opentiny/vue'
+ /**
+ * @description Milestone Events Demo Component
+ * This component demonstrates the usage of TinyMilestone with various states and events.
+ * - Shows different milestone statuses (completed, doing, back, end, cancel)
+ * - Demonstrates flag interactions
+ * - Includes click event handling for both nodes and flags
+ */
export default {
components: {
TinyMilestone
},examples/sites/demos/pc/app/milestone/data-field-mapping.vue (1)
Line range hint 1-116: Documentation value and completeness check.
Since this is a documentation/example component demonstrating field mapping functionality, it effectively shows:
- Custom field mapping for various milestone properties
- Different status types and their visual representation
- Flag configuration with nested data structure
The example is well-structured and provides good coverage of the component's field mapping capabilities.
Consider adding comments to explain:
- The purpose of each status color in the
statusMap - The significance of the different progress states (
completed,doing,back, etc.)
This would enhance the educational value of this example.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (14)
- examples/sites/demos/pc/app/milestone/custom-bottom-top-composition-api.vue (3 hunks)
- examples/sites/demos/pc/app/milestone/custom-bottom-top.vue (3 hunks)
- examples/sites/demos/pc/app/milestone/custom-flag-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/milestone/custom-flag.vue (1 hunks)
- examples/sites/demos/pc/app/milestone/data-field-mapping-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/milestone/data-field-mapping.vue (1 hunks)
- examples/sites/demos/pc/app/milestone/flag-before-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/milestone/flag-before.vue (1 hunks)
- examples/sites/demos/pc/app/milestone/milestone-events-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/milestone/milestone-events.vue (1 hunks)
- examples/sites/demos/pc/app/milestone/show-number-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/milestone/show-number.vue (1 hunks)
- examples/sites/demos/pc/app/milestone/solid-style-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/milestone/solid-style.vue (1 hunks)
🔇 Additional comments (14)
examples/sites/demos/pc/app/milestone/solid-style-composition-api.vue (1)
14-14: Verify the semantic meaning of the new color variable
The new color variable --tv-color-bg-disabled-control-active has a potentially confusing name as it combines both "disabled" and "active" states. This might be misleading for developers using this example as a reference.
Let's check the consistency of this color variable usage:
Consider using a more semantically clear variable name that better represents the completed state, such as --tv-color-bg-milestone-completed or similar.
✅ Verification successful
Let me try a different search strategy to verify the color variable usage.
The previous search failed due to file type restrictions. This broader search should help us understand the color variable usage and verify if it's part of the design system's theme documentation.
Based on the search results, I can now provide a definitive response about the color variable usage:
The color variable is part of the design system and used consistently
The variable --tv-color-bg-disabled-control-active is a legitimate part of the design system and is used consistently across multiple components:
- It's used in the Switch component for checked-disabled state (
--tv-Switch-checked-disabled-bg-color) - It's used consistently across all milestone examples for the completed state
- The naming follows the established pattern of state-based color variables in the theme system
The semantic meaning of "disabled-control-active" makes sense in the context:
- "disabled" refers to the visual state
- "control" indicates it's for UI controls
- "active" represents the positive/completed state
The review comment's concern about the semantic meaning can be dismissed as the variable is being used appropriately within the design system's conventions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage of the new color variable and potential naming patterns
# Check for all color variable usages in the codebase
rg --type vue "var\(--tv-color-bg-disabled-control-active\)"
# Check for similar naming patterns to understand the convention
rg --type vue "var\(--tv-color-bg-disabled-.*?\)"
# Look for any documentation about color variables
fd -t f "color|theme|style-guide" -e md
Length of output: 733
Script:
#!/bin/bash
# Search for color variable usage without file type restriction
rg "var\(--tv-color-bg-disabled-control-active\)"
# Search for similar disabled control variables
rg "var\(--tv-color.*disabled.*\)"
# Check the theme documentation content
cat examples/sites/demos/pc/webdoc/theme.md examples/sites/demos/pc/webdoc/theme-en.md
Length of output: 46817
examples/sites/demos/pc/app/milestone/show-number-composition-api.vue (1)
21-21: Verify the new design system variable usage.
The change from --ti-common-color-line-active to --tv-color-bg-disabled-control-active suggests a design system update. While the change is straightforward, it's important to ensure visual consistency.
Let's verify if this design variable is consistently used across other milestone components:
✅ Verification successful
Design system variable update is consistent across milestone components
The verification shows that the change from --ti-common-color-line-active to --tv-color-bg-disabled-control-active is part of a broader, consistent design system update:
- The old variable
--ti-common-color-line-activeis still used in some non-milestone components (user-head, grid, collapse) - The new variable
--tv-color-bg-disabled-control-activeis properly defined in the theme system (packages/theme/src/base/vars.less) and is consistently used across all milestone components:- show-number-composition-api.vue
- custom-flag.vue
- data-field-mapping.vue
- solid-style.vue
- milestone-events.vue
- and other milestone variants
The change aligns with the design system's intended use of this variable for disabled control states.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of the old color variable
rg "ti-common-color-line-active" --type vue
# Search for consistent usage of the new color variable
rg "tv-color-bg-disabled-control-active" --type vue
Length of output: 151
Script:
#!/bin/bash
# Search for any remaining instances of the old color variable without file type restriction
rg "ti-common-color-line-active"
# Search for consistent usage of the new color variable without file type restriction
rg "tv-color-bg-disabled-control-active"
# List all milestone-related files to ensure we check related components
fd milestone
Length of output: 21183
examples/sites/demos/pc/app/milestone/custom-bottom-top-composition-api.vue (2)
5-5: LGTM: Enhanced status text styling
The addition of "status" and "mb8" classes improves the visual presentation and spacing of the milestone status text.
22-22: Verify CSS variable availability
The color variable has been updated from --ti-common-color-line-active to --tv-color-bg-disabled-control-active. Please ensure this new CSS variable is properly defined in the theme system.
✅ Verification successful
Based on the search results, I can generate the final response as we have sufficient information:
CSS variable change is part of design system update
The change from --ti-common-color-line-active to --tv-color-bg-disabled-control-active is valid as part of a design system update. The new variable is properly defined in the theme system:
- Defined in base theme files (
old-theme-index.js,vars.less) - Used consistently across multiple milestone component examples
- Maps to
var(--tv-base-color-brand-3)with hex value#b3d6ff
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the definition or usage of both old and new CSS variables
echo "Searching for old variable definition/usage..."
rg "ti-common-color-line-active"
echo -e "\nSearching for new variable definition/usage..."
rg "tv-color-bg-disabled-control-active"
Length of output: 20457
examples/sites/demos/pc/app/milestone/solid-style.vue (1)
20-20: Verify the semantic meaning of the new color variable.
The change from --ti-common-color-line-active to --tv-color-bg-disabled-control-active represents a significant shift in the semantic meaning of the color. While this might be intentional, please verify that:
- Using a "disabled control" color for a "completed" state aligns with the design system guidelines
- This color provides sufficient contrast and visual hierarchy for completed milestones
✅ Verification successful
Let me gather more specific information about the color variables and theme usage.
Based on the gathered information, I can now provide a final response:
The color variable change is part of a design system update and is correct
The change from --ti-common-color-line-active to --tv-color-bg-disabled-control-active is aligned with the documented theme system update. The documentation shows that:
- There's an intentional migration from
ti-prefix totv-prefix as part of the theme system upgrade (after v3.19.0) - The milestone component is highly customizable for status and color customization, as stated in its documentation
- The old variable was also used in test files and examples, confirming this is a systematic change
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of this color variable to understand its intended context
rg --type vue "tv-color-bg-disabled-control-active"
# Search for any documentation about color usage guidelines
fd -e md -e mdx . | xargs rg -l "color|theme|design system"
Length of output: 1650
Script:
#!/bin/bash
# Check theme documentation for color variable guidelines
cat examples/sites/demos/pc/webdoc/theme.md
# Look for milestone component documentation
cat examples/sites/demos/pc/app/milestone/webdoc/milestone.en.md
# Search for other milestone-related files to understand color usage
fd milestone . --type f --exec grep -l "color"
# Search for the old color variable to understand the transition
rg "ti-common-color-line-active"
Length of output: 9226
examples/sites/demos/pc/app/milestone/custom-bottom-top.vue (1)
5-5: LGTM: Class addition enhances status styling consistency
The addition of the 'status' class aligns with the component's styling improvements while maintaining existing utility classes.
examples/sites/demos/pc/app/milestone/show-number.vue (1)
27-27: LGTM! Color variable standardization.
The change from var(--ti-common-color-line-active) to var(--tv-color-bg-disabled-control-active) aligns with the project's color variable naming convention.
examples/sites/demos/pc/app/milestone/milestone-events-composition-api.vue (1)
20-20: Review the semantic meaning of using a disabled control color for completed status.
The change from --ti-common-color-line-active to --tv-color-bg-disabled-control-active might not convey the right semantic meaning. A completed milestone represents a successful completion rather than a disabled state. Consider using a success or completion-oriented color variable instead.
Let's verify if this color variable exists in the design system:
Consider defining semantic color variables specifically for milestone states (e.g., --tv-color-milestone-completed) to maintain a clear separation between the visual representation and the component's state.
examples/sites/demos/pc/app/milestone/data-field-mapping-composition-api.vue (1)
23-23: Verify the semantic meaning of the new color variable
The new color variable --tv-color-bg-disabled-control-active has a potentially confusing name as it combines both "disabled" and "active" states. This might make the demo less clear for users.
Let's check for consistency across the design system:
✅ Verification successful
The color variable change is consistent with the design system
The new color variable --tv-color-bg-disabled-control-active is correctly used and is part of the design system's base theme. From the verification results:
- It's defined in the theme files with the value
var(--tv-base-color-brand-3)(#b3d6ff) - Used consistently across all milestone demo files
- Has a clear semantic purpose: "深色背景禁用色，开关组件'开'禁用背景色" (dark background disabled color, used for switch component's "on" disabled state)
The variable name's seeming contradiction between "disabled" and "active" is actually intentional, as it represents a specific UI state in the design system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage of the new color variable and verify naming conventions
# Expected: Find other usages to confirm this is the correct variable
# Search for the new variable name in style files
rg --type=css --type=vue "tv-color-bg-disabled-control-active"
# Search for other milestone status color variables to verify naming pattern
rg --type=css --type=vue "tv-color-bg-.*?-control"
Length of output: 171
Script:
#!/bin/bash
# Let's try different search patterns without file type restrictions
# Search for the new variable name across all files
rg "tv-color-bg-disabled-control-active"
# Search for similar color variable patterns
rg "tv-color-bg.*control"
# Search for milestone-related color variables
rg "statusMap.*color"
# Look for color variable definitions in theme or style files
fd "theme|style|color" --type f --exec rg -l "tv-color-bg"
Length of output: 42675
examples/sites/demos/pc/app/milestone/custom-flag.vue (2)
Line range hint 1-93: Verify accessibility of status colors.
The milestone component uses various colors to represent different statuses. Please ensure that all status colors meet WCAG color contrast requirements for accessibility, especially when used with text.
Let's check if there are any accessibility guidelines or tests:
#!/bin/bash
# Search for accessibility tests or documentation
rg -i "accessibility|a11y|wcag" --type md24-24: Review the semantic meaning of the color variable.
The change from --ti-common-color-line-active to --tv-color-bg-disabled-control-active raises some concerns:
- Using a "disabled control" color to represent a "completed" status might be semantically incorrect and potentially confusing for users
- The variable name suggests it's meant for disabled controls rather than status indicators
Let's verify the color variable usage across the design system:
examples/sites/demos/pc/app/milestone/flag-before.vue (1)
22-26: Verify color contrast ratios for accessibility.
Please ensure that the chosen colors maintain sufficient contrast ratios with their background colors to meet WCAG accessibility guidelines, particularly for the lighter colors like #edf7df used for the 'end' status.
You can verify the contrast ratios using the following tools:
- WebAIM Contrast Checker
- WCAG 2.1 Level AA guidelines require:
- 4.5:1 for normal text
- 3:1 for large text and UI components
examples/sites/demos/pc/app/milestone/milestone-events.vue (1)
25-25: Review the semantic meaning of the color variable.
The change from var(--ti-common-color-line-active) to var(--tv-color-bg-disabled-control-active) for the completed status seems counterintuitive. Using a "disabled" color variable for completed items might give users the wrong visual feedback, as completed items are typically shown as successfully finished rather than disabled.
Let's check if this color variable is used consistently across the design system:
examples/sites/demos/pc/app/milestone/data-field-mapping.vue (1)
28-28: Verify color variable availability and consistency.
The color variable has been changed from --ti-common-color-line-active to --tv-color-bg-disabled-control-active. While this change appears to be part of a broader standardization effort, we should ensure the new variable is properly defined and accessible.
✅ Verification successful
Color variable change is valid and consistent
The color variable --tv-color-bg-disabled-control-active is:
- Properly defined in the theme system (
packages/theme/src/base/vars.less) - Used consistently across all milestone components
- Maps to the brand color variable
--tv-base-color-brand-3(#b3d6ff) - Specifically intended for disabled control states
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the new color variable is defined in the theme/style files
# and verify consistency across other milestone components
# Search for the color variable definition
echo "Searching for color variable definition:"
rg --type css --type scss "tv-color-bg-disabled-control-active"
# Check if other milestone components use the same variable
echo -e "\nChecking consistency across milestone components:"
rg --type vue "tv-color-bg-disabled-control-active" "examples/sites/demos/pc/app/milestone/"
Length of output: 417
Script:
#!/bin/bash
# Let's try a different approach without file type restrictions
# Search for the color variable definition across all files
echo "Searching for color variable definition:"
rg "tv-color-bg-disabled-control-active"
# Check milestone components specifically
echo -e "\nChecking milestone components:"
find examples/sites/demos/pc/app/milestone -type f -exec grep -l "tv-color-bg-disabled-control-active" {} \;
# Look for any CSS/SCSS files that might contain theme variables
echo -e "\nSearching for theme files:"
fd -e css -e scss . | grep -i "theme\|style"
Length of output: 20335
| .status { | ||
| display: inline-block; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider consolidating common styles
The .status class appears to be a common styling pattern for milestone status display. Consider moving this to a shared stylesheet if it's used across multiple milestone-related components.
#!/bin/bash
# Check if similar status styles exist in other files
echo "Searching for similar status class definitions..."
rg "\.status\s*{[\s\n]*display:\s*inline-block" --type vue| @@ -24,7 +24,7 @@ export default { | |||
| normalActive: 2, | |||
| advancedActive: 2, | |||
| statusMap: { | |||
| completed: 'var(--ti-common-color-line-active)', | |||
| completed: 'var(--tv-color-bg-disabled-control-active)', | |||
There was a problem hiding this comment.
💡 Codebase verification
Based on the search results, I can generate the final response:
Design system variable change is part of a consistent update
The change from --ti-common-color-line-active to --tv-color-bg-disabled-control-active appears to be part of a systematic design token update. The new variable is:
- Properly defined in the theme system (
packages/theme/src/base/vars.less) - Used consistently across milestone component examples
- Has a clear semantic meaning (disabled control active state)
However, there are still some files using the old variable that need to be updated:
packages/vue/src/milestone/__tests__/milestone.test.tsx- Several user-head and grid operation column examples
- Some collapse component examples
🔗 Analysis chain
Verify design system variable change impact
The color variable has changed from --ti-common-color-line-active to --tv-color-bg-disabled-control-active. Let's verify this change is consistent with the design system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining usage of the old color variable
echo "Checking for old variable usage..."
rg "ti-common-color-line-active"
# Search for consistent usage of the new variable
echo "Checking new variable usage..."
rg "tv-color-bg-disabled-control-active"
Length of output: 20395
| @@ -16,7 +16,7 @@ const timeActive1 = ref(1) | |||
| const normalActive = ref(2) | |||
| const advancedActive = ref(2) | |||
| const statusMap = ref({ | |||
| completed: 'var(--ti-common-color-line-active)', | |||
| completed: 'var(--tv-color-bg-disabled-control-active)', | |||
There was a problem hiding this comment.
💡 Codebase verification
Color variable change needs to be consistent across milestone examples
The search results show inconsistency in color variable usage across milestone examples:
- Some files still use the old variable
--ti-common-color-line-active - Some files have been updated to use
--tv-color-bg-disabled-control-active
Files needing update:
packages/vue/src/milestone/__tests__/milestone.test.tsxexamples/sites/demos/pc/app/milestone/line-style.vueexamples/sites/demos/pc/app/milestone/line-style-composition-api.vue
🔗 Analysis chain
LGTM! Verify consistency of color variable naming.
The change from ti-common-color-line-active to tv-color-bg-disabled-control-active appears correct and better reflects its purpose.
Let's verify the consistency of this color variable change across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of the old color variable
# and confirm consistent usage of the new variable
# Search for old variable
echo "Checking for remaining instances of old variable:"
rg "ti-common-color-line-active"
# Search for new variable usage
echo -e "\nChecking usage of new variable:"
rg "tv-color-bg-disabled-control-active"
Length of output: 20433
| completed: 'var(--tv-color-bg-disabled-control-active)', // 组件库基础变量 | ||
| doing: '#9edb58', | ||
| back: '#c2c2c2', | ||
| end: '#edf7df', | ||
| cancel: '#f23030' |
There was a problem hiding this comment.
💡 Codebase verification
🛠️ Refactor suggestion
Based on the search results, I can see that the theme system has a comprehensive set of semantic color variables, including status colors like --tv-color-bg-disabled-control-active which is currently being used in the milestone component.
Consider using semantic status color variables consistently across all milestone states
The current implementation mixes CSS variables with direct hex values, which makes maintenance harder and breaks consistency with the design system. Here's a more appropriate mapping using semantic variables:
const statusMap = ref({
- completed: 'var(--tv-color-bg-disabled-control-active)',
- doing: '#9edb58',
- back: '#c2c2c2',
- end: '#edf7df',
- cancel: '#f23030'
+ completed: 'var(--tv-color-success-bg)', // Success state
+ doing: 'var(--tv-color-info-bg)', // In progress state
+ back: 'var(--tv-color-bg-control)', // Neutral state
+ end: 'var(--tv-color-success-bg-light)', // Success light state
+ cancel: 'var(--tv-color-error-bg)' // Error state
})This refactoring:
- Uses semantic color variables that match the status meanings
- Maintains consistency with the design system
- Makes future theme changes easier to manage
- Improves accessibility by using tested color combinations
🔗 Analysis chain
Consider using design system variables consistently across all status colors.
The current implementation has several potential issues:
- Only the 'completed' status uses a design system variable while others use hard-coded hex values
- The variable
--tv-color-bg-disabled-control-activeseems semantically incorrect for representing a 'completed' state - The mix of color formats (CSS variables and hex values) could make maintenance more challenging
Consider refactoring to use semantic design system variables consistently:
const statusMap = ref({
- completed: 'var(--tv-color-bg-disabled-control-active)',
- doing: '#9edb58',
- back: '#c2c2c2',
- end: '#edf7df',
- cancel: '#f23030'
+ completed: 'var(--tv-color-success)', // Or another appropriate semantic variable
+ doing: 'var(--tv-color-primary)',
+ back: 'var(--tv-color-secondary)',
+ end: 'var(--tv-color-info)',
+ cancel: 'var(--tv-color-danger)'
})Let's verify the available design system variables:
Consider creating a dedicated theme configuration file that maps semantic status names to design system colors. This would make it easier to maintain consistent status colors across all milestone components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for color variable definitions in the codebase
rg -g '*.{css,scss,less}' '(--tv-color-|--ti-common-color-)'
Length of output: 171373
| completed: 'var(--tv-color-bg-disabled-control-active)', // 组件库基础变量 | ||
| doing: '#9edb58', | ||
| back: '#c2c2c2', | ||
| end: '#edf7df', | ||
| cancel: '#f23030' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using CSS variables consistently across all status colors.
The current implementation mixes CSS variables with direct hex color codes, which could make theme customization and maintenance more challenging. Consider using CSS variables for all status colors to maintain consistency with the design system.
Example refactor:
statusMap: {
completed: 'var(--tv-color-bg-disabled-control-active)',
- doing: '#9edb58',
- back: '#c2c2c2',
- end: '#edf7df',
- cancel: '#f23030'
+ doing: 'var(--tv-color-success)',
+ back: 'var(--tv-color-text-disabled)',
+ end: 'var(--tv-color-bg-success-subtle)',
+ cancel: 'var(--tv-color-error)'
}This approach would:
- Ensure consistent theming across the application
- Make it easier to implement dark mode or other theme variations
- Maintain visual consistency with the rest of the component library
Committable suggestion was skipped due to low confidence.
* fix(milestones): [milestone] 修复组件示例文档 * fix(milestones): [milestone] 修复测试用例
PR
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
New Features
.statusfor improved layout behavior of status displays.Bug Fixes
completedstatus across multiple components to ensure consistency in design.Tests
Documentation