feat(progress): [progress] The progress example is added to adapt to the new specifications and additional features. #2315
Conversation
WalkthroughThe changes in this pull request involve modifications to several Vue component files and a LESS file related to a progress component. Key updates include changing the initial values of reactive properties and data variables, specifically altering the Changes
Possibly related PRs
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
enhancement
![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
🧹 Outside diff range and nitpick comments (4)
examples/sites/demos/pc/app/progress/progress-type-circle-composition-api.vue (2)
3-5: LGTM! Consider using a constant for consistent widths.The changes to the progress components look good. The consistent width of 124 for both circle and dashboard types improves visual coherence, and adding the "success" status to the dashboard component enhances user feedback.
Consider extracting the width value into a constant in the script section for easier maintenance:
<script setup lang="jsx"> import { ref } from 'vue' import { Progress as TinyProgress, Button as TinyButton } from '@opentiny/vue' +const PROGRESS_WIDTH = 124 // ... rest of the script </script>Then use it in the template:
-<tiny-progress type="circle" :percentage="percentage" status="exception" :width="124"></tiny-progress> -<tiny-progress type="dashboard" :percentage="percentage" status="success" :color="customColors" :width="124"> +<tiny-progress type="circle" :percentage="percentage" status="exception" :width="PROGRESS_WIDTH"></tiny-progress> +<tiny-progress type="dashboard" :percentage="percentage" status="success" :color="customColors" :width="PROGRESS_WIDTH">This approach would make future width adjustments easier and ensure consistency.
12-12: LGTM! Consider making the initial percentage configurable.The change to start the progress at 50% instead of 10% provides a more meaningful initial state for the progress components.
To enhance flexibility, consider making the initial percentage configurable:
-const percentage = ref(50) +const initialPercentage = 50 +const percentage = ref(initialPercentage)This approach allows easier adjustment of the initial percentage in the future if needed, without changing the component's internal logic.
examples/sites/demos/pc/app/progress/format-text-composition-api.vue (1)
26-26: LGTM! Consider updating the related button text for consistency.The change to initialize
testInsideasfalseis appropriate and aligns with the PR objectives. This modification ensures that the progress component initially displays the text externally, which may provide better visibility or align with updated design specifications.For consistency with this change, consider updating the text on the toggle button in the template. Currently, it shows "外置" (external) when
testInsideistrue, which might be confusing given the new default state. You could swap the order:- <tiny-button @click="testInside = !testInside">{{ testInside ? '外置' : '内置' }}文字</tiny-button> + <tiny-button @click="testInside = !testInside">{{ testInside ? '内置' : '外置' }}文字</tiny-button>This way, the button will initially show "内置" (internal), matching the new default external text state.
examples/sites/demos/pc/app/progress/progress-type-circle.vue (1)
4-6: LGTM! Consider adding comments for clarity.The changes to the
tiny-progresscomponents look good. The consistent width of 124 for both circle and dashboard types improves uniformity. The status change to "success" for the dashboard type aligns with the new specifications.Consider adding comments to explain the significance of the width value (124) and the reason for setting the dashboard type's status to "success". This would improve code maintainability and help other developers understand the design decisions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- examples/sites/demos/pc/app/progress/format-text-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/progress/format-text.vue (1 hunks)
- examples/sites/demos/pc/app/progress/progress-type-circle-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/progress/progress-type-circle.vue (2 hunks)
- packages/theme/src/progress/vars.less (1 hunks)
🧰 Additional context used
🔇 Additional comments (4)
examples/sites/demos/pc/app/progress/progress-type-circle-composition-api.vue (1)
Line range hint
1-34: Overall, the changes look good and align with the PR objectives.The modifications to both the template and script sections improve the progress component's functionality and adapt it to new specifications. The consistent width for both progress types and the more meaningful initial percentage enhance the user experience.
The suggested minor improvements (using a constant for width and making the initial percentage configurable) could further enhance maintainability and flexibility.
examples/sites/demos/pc/app/progress/progress-type-circle.vue (1)
20-20: LGTM! Consider UX implications.The change of the initial
percentagevalue from 10 to 50 aligns with the new specifications.However, it's important to consider the user experience implications:
- The progress will now start at 50%, which might not accurately represent the initial state of the process being measured.
- With the
addmethod still incrementing by 10, the progress will reach 100% much faster.To verify the impact, please run the following script:
This will help ensure consistency across all progress components and identify any potential discrepancies in initial values.
✅ Verification successful
Let's address the issue with the previous script by targeting
.vuefiles directly.
Verified! No inconsistencies found.
The change of the initial
percentagevalue to 50 inprogress-type-circle.vuealigns with the existing patterns in other progress components.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other occurrences of initial percentage values in progress components # Test: Search for initial percentage values in progress components rg --type vue 'percentage:?\s*\d+' examples/sites/demos/pc/app/progressLength of output: 101
Script:
#!/bin/bash # Description: Check for other occurrences of initial percentage values in progress components # Test: Search for initial percentage values in .vue files within progress components rg 'percentage:\s*\d+' examples/sites/demos/pc/app/progress --glob '*.vue'Length of output: 2492
examples/sites/demos/pc/app/progress/format-text.vue (1)
38-38: LGTM! Note the change in initial text position.The change from
truetofalsefortestInsideis correct and aligns with the intended behavior. This modification will cause the progress bar text to be initially positioned outside the bar instead of inside. The toggle button will now start with the text "内置" (internal), reflecting the new initial state.packages/theme/src/progress/vars.less (1)
30-30: Improved theming flexibility for exception state background colorThis change enhances the theming capabilities of the progress component:
- It now uses a CSS variable
--tv-color-error-bgfor the exception state background color, allowing for dynamic color changes across the theme.- A fallback color
#f23030is provided, ensuring compatibility even if the variable is not set.This update improves maintainability and consistency across the theme, as color changes can be managed centrally.
Note: The new fallback color (
#f23030) is slightly different from the old hardcoded color (#c7000b). Ensure this change aligns with the intended design.To confirm the usage and impact of this change, run the following script:
✅ Verification successful
Improved theming flexibility for exception state background color
This change enhances the theming capabilities of the progress component:
- It now uses a CSS variable
--tv-color-error-bgfor the exception state background color, allowing for dynamic color changes across the theme.- A fallback color
#f23030is provided, ensuring compatibility even if the variable is not set.This update improves maintainability and consistency across the theme, as color changes can be managed centrally.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other occurrences of --tv-color-error-bg and the old color value # Search for other occurrences of --tv-color-error-bg echo "Occurrences of --tv-color-error-bg:" rg --type less "--tv-color-error-bg" # Search for occurrences of the old color value echo "\nOccurrences of the old color value #c7000b:" rg --type less "#c7000b"Length of output: 1134
Script:
#!/bin/bash # Description: Correctly search for other occurrences of --tv-color-error-bg and the old color value # Search for other occurrences of --tv-color-error-bg echo "Occurrences of --tv-color-error-bg:" rg --type less '--tv-color-error-bg' # Search for occurrences of the old color value echo "\nOccurrences of the old color value #c7000b:" rg --type less '#c7000b'Length of output: 1134
Script:
#!/bin/bash # Description: Correctly search for other occurrences of --tv-color-error-bg and the old color value # Search for other occurrences of --tv-color-error-bg echo "Occurrences of --tv-color-error-bg:" rg --type less -e '--tv-color-error-bg' # Search for occurrences of the old color value echo "\nOccurrences of the old color value #c7000b:" rg --type less '#c7000b'Length of output: 2554
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (10)
examples/sites/demos/pc/app/progress/custom-color-composition-api.vue (3)
17-17: LGTM! Consider adding a comment for clarity.The change from '#409eff' to '#5cb300' for the
customColorref is consistent with the PR objectives. This updates the default color of the first progress bar from blue to green.Consider adding a comment to explain the significance of this color change, e.g.:
// Updated default color to green (#5cb300) to align with new design specifications const customColor = ref('#5cb300')
21-21: LGTM! Consider using color variables for better maintainability.The change from '#e6a23c' to '#ff8800' for the 40% percentage point in the
customColorsarray is consistent with the PR objectives. This updates the color scheme for the third progress bar, using a brighter orange.For better maintainability and consistency, consider using color variables instead of hard-coded hex values. For example:
const COLORS = { RED: '#f56c6c', ORANGE: '#ff8800', GREEN: '#5cb87a', BLUE: '#1989fa', PURPLE: '#6f7ad3' } const customColors = ref([ { color: COLORS.RED, percentage: 20 }, { color: COLORS.ORANGE, percentage: 40 }, { color: COLORS.GREEN, percentage: 60 }, { color: COLORS.BLUE, percentage: 80 }, { color: COLORS.PURPLE, percentage: 100 } ])This approach would make it easier to update colors consistently across the component if needed in the future.
37-37: LGTM! Consider using constants for better readability.The change from '#909399' to '#f23030' for percentages less than 30 in the
customColorMethodfunction is consistent with the PR objectives. This updates the color logic for the second progress bar, using a bright red for lower percentages instead of the previous grayish color.To improve code readability and maintainability, consider using constants for the color values and percentage thresholds. For example:
const COLORS = { RED: '#f23030', ORANGE: '#e6a23c', GREEN: '#67c23a' } const THRESHOLDS = { LOW: 30, MEDIUM: 70 } function customColorMethod(percentage) { if (percentage < THRESHOLDS.LOW) { return COLORS.RED } else if (percentage < THRESHOLDS.MEDIUM) { return COLORS.ORANGE } else { return COLORS.GREEN } }This approach makes the color logic more explicit and easier to update in the future.
examples/sites/demos/pc/app/progress/basic-usage-composition-api.vue (1)
20-20: LGTM! Consider adding a comment for the initial value.The change from 90 to 45 for the initial
percentagevalue is consistent with updates in other files, as mentioned in the PR summary. This modification doesn't introduce any issues and maintains the component's functionality.Consider adding a brief comment explaining the reason for choosing 45 as the initial value. This could help maintain consistency across the project and provide context for future developers. For example:
-const percentage = ref(45) +// Initial progress set to 45% for consistency across components +const percentage = ref(45)examples/sites/demos/pc/app/progress/custom-color.vue (2)
23-23: LGTM: Color change enhances visual feedback.The change from blue (#409eff) to green (#5cb300) for the default progress bar color is approved. Green is often associated with progress and success, potentially improving user experience.
Consider updating any related documentation or design guidelines to reflect this color change for consistency across the project.
44-44: LGTM: Improved visual indication for low progress.The change to bright red (#f23030) for progress less than 30% is approved. This color choice makes low progress more noticeable, potentially prompting quicker user action.
Consider the following UX implications:
- Ensure that the use of red aligns with your overall color scheme and doesn't conflict with other uses of red in the UI (e.g., for errors).
- If this component is used in long-running processes, consider whether the red color might cause unnecessary user anxiety for naturally slow processes.
examples/sites/demos/pc/app/progress/basic-usage.spec.ts (1)
23-25: LGTM: Dynamic progress update tested.The addition of the button click and subsequent progress check is a good test for dynamic updates.
Consider adding an assertion for the visible text to ensure it updates along with the
aria-valuenowattribute:await expect(page.getByText('55%')).toHaveCount(2)examples/sites/demos/pc/app/progress/custom-color.spec.ts (1)
Potential Outdated Color Values Detected
The test file
examples/sites/demos/pc/app/progress/custom-color.spec.tscontains color values that may be outdated:
rgb(230, 162, 60)rgb(25, 137, 250)Please verify if these color values are intentional or need to be updated according to the latest design specifications.
🔗 Analysis chain
Line range hint
1-33: Overall LGTM: Color expectations updated consistently.The changes in this file consistently update the color expectations for the progress bars throughout the test. The overall structure and logic of the test remain intact.
To ensure the test accurately reflects the intended behavior of the progress component:
- Verify that all color changes (or lack thereof) at each stage of the test are intentional and align with the component's design specifications.
- Consider adding comments to explain the expected color progression, especially for cases where colors remain unchanged after button clicks.
- Run the following script to check for any inconsistencies in the color values used throughout the file:
This script will help identify any inconsistencies or potentially outdated color values in the test file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistency in color values used in the test file # Test: Extract and display all unique color values echo "Unique color values used in the test:" rg -o 'rgb\([^)]+\)' examples/sites/demos/pc/app/progress/custom-color.spec.ts | sort -u # Test: Check if any old color values are still present echo "Checking for potentially outdated color values:" rg 'rgb\(64, 158, 255\)|rgb\(144, 147, 153\)|rgb\(230, 162, 60\)|rgb\(25, 137, 250\)' examples/sites/demos/pc/app/progress/custom-color.spec.tsLength of output: 798
examples/sites/demos/pc/app/progress/progress-type-circle.spec.ts (2)
13-19: LGTM! Consider updating documentation.The changes to the stroke width and path data appear to be intentional and consistent. The circular progress component has been slightly adjusted, possibly for better visibility or to align with new design specifications.
Consider updating the component's documentation to reflect these changes, especially if they affect the component's visual appearance or API.
Line range hint
1-69: Overall LGTM! Consider a holistic review of progress component changes.The updates to the progress circle component test cases appear to be consistent and intentional. They reflect changes in the component's dimensions, initial values, and increments. These modifications align with updates mentioned in other related files.
Given the extent of these changes, it would be beneficial to conduct a holistic review of all updates related to the progress component across the project. This review should ensure that:
- The changes align with the project's design specifications and UX guidelines.
- Documentation has been updated to reflect the new behavior and appearance.
- Any dependent components or features have been adjusted accordingly.
- The changes meet the objectives outlined in the PR description, particularly regarding adapting to new specifications and additional features.
Consider organizing a brief team review session to discuss these changes and their implications on the overall project architecture and user experience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (13)
- examples/sites/demos/pc/app/progress/basic-usage-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/progress/basic-usage.spec.ts (1 hunks)
- examples/sites/demos/pc/app/progress/basic-usage.vue (1 hunks)
- examples/sites/demos/pc/app/progress/custom-color-composition-api.vue (2 hunks)
- examples/sites/demos/pc/app/progress/custom-color.spec.ts (1 hunks)
- examples/sites/demos/pc/app/progress/custom-color.vue (2 hunks)
- examples/sites/demos/pc/app/progress/format-text-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/progress/format-text.spec.ts (1 hunks)
- examples/sites/demos/pc/app/progress/format-text.vue (1 hunks)
- examples/sites/demos/pc/app/progress/progress-type-circle-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/progress/progress-type-circle.spec.ts (3 hunks)
- examples/sites/demos/pc/app/progress/progress-type-circle.vue (2 hunks)
- packages/theme/src/progress/vars.less (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- examples/sites/demos/pc/app/progress/format-text-composition-api.vue
- examples/sites/demos/pc/app/progress/format-text.vue
- examples/sites/demos/pc/app/progress/progress-type-circle-composition-api.vue
- examples/sites/demos/pc/app/progress/progress-type-circle.vue
- packages/theme/src/progress/vars.less
🧰 Additional context used
🔇 Additional comments (18)
examples/sites/demos/pc/app/progress/format-text.spec.ts (2)
13-16: LGTM: Improved test coverage for text visibility toggling.The additional click on
button2followed by the visibility check forinnerTextenhances the test by verifying the component's behavior when switching between inner and outer text multiple times. This change improves the robustness of the test case.
20-20: Verify the intentionality of the percentage change.The update from '自定义文字内容 60%' to '自定义文字内容 45%' aligns with changes made in related component files. This ensures consistency between the implementation and the test.
Please confirm if this change is intentional and doesn't negatively impact other parts of the application. Run the following script to check for any other occurrences of the old percentage:
✅ Verification successful
Change Approved: Percentage Updated to 45%
The update aligns with changes in related components, ensuring consistency between implementation and the test. No impact detected on other parts of the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for occurrences of the old percentage (60%) in the codebase # Test: Search for '60%' in all files rg -i '60%' # Test: Search for the number 60 in .vue and .ts files rg '\b60\b' -g '*.{vue,ts}'Length of output: 93647
examples/sites/demos/pc/app/progress/custom-color-composition-api.vue (1)
Line range hint
1-52: Overall, the changes look good and align with the PR objectives.The updates to the color scheme in this file improve the visual representation of the progress components and adapt to the new specifications as intended. The changes are consistent across different parts of the component (template, script, and style), ensuring a cohesive update.
A few minor suggestions have been made to improve code maintainability and readability, such as using color variables and constants. These suggestions, if implemented, would make future updates easier and the code more self-documenting.
Great job on adapting the progress example to the new specifications!
examples/sites/demos/pc/app/progress/basic-usage.vue (2)
Line range hint
1-62: Component structure and functionality reviewAfter reviewing the entire component, I can confirm that the change in the initial percentage value doesn't negatively impact the overall functionality:
- The increase and decrease methods still have proper boundary checks (0-100%).
- The two progress bars and control buttons remain unchanged and should work as expected.
- The conditional rendering of the tip message is still correct.
- Styles are unchanged and appropriate for the component.
The component maintains its consistency and functionality with the updated initial percentage value.
26-26: Initial percentage value updatedThe initial value of
percentagehas been changed from 90 to 45. This change aligns with the updates mentioned in the AI summary for other similar components, ensuring consistency across the application.To ensure this change doesn't negatively impact the user experience, let's verify the initial state of the progress bars:
examples/sites/demos/pc/app/progress/custom-color.vue (2)
27-27: LGTM: Enhanced color distinction for progress milestones.The change to a more vibrant orange (#ff8800) for the 40% progress milestone is approved. This creates a more distinct color transition between progress stages.
Please verify that the new color (#ff8800) maintains sufficient contrast with the background for accessibility. You can use a color contrast checker tool to ensure it meets WCAG 2.1 Level AA standards.
Line range hint
1-62: Summary: Approved changes with minor suggestionsThe changes to the custom color implementation in the progress component have been reviewed and approved. The new color choices enhance visual feedback and create more distinct progress stages. Consider the following suggestions:
- Update related documentation to reflect the color changes.
- Verify color contrast for accessibility, especially for the new orange color (#ff8800).
- Evaluate the UX implications of using bright red for low progress states.
Overall, these changes align well with the PR objectives of adapting to new specifications and adding features to the progress example.
examples/sites/demos/pc/app/progress/basic-usage.spec.ts (4)
18-18: LGTM: Initial progress value updated.The change from '90%' to '45%' for the initial progress value is consistent with the updates mentioned in the PR summary and aligns with changes in other related files.
21-22: LGTM: ARIA attributes updated consistently.The
aria-valuenowattributes have been correctly updated to '45' for both progress bars, maintaining consistency with the visible progress percentage.
26-30: Clarify the purpose of multiple clicks without assertions.The addition of multiple button clicks without corresponding assertions is unclear. If the intention is to test the upper limit of the progress bar, consider adding explicit assertions.
Could you clarify the purpose of these additional clicks? If they're meant to test a specific behavior, consider adding assertions after each click or at least after the final click in the sequence. For example:
await demo.getByRole('button').nth(1).click() await expect(progress.nth(0)).toHaveAttribute('aria-valuenow', '65') // ... repeat for each click if necessary await demo.getByRole('button').nth(1).click() await expect(progress.nth(0)).toHaveAttribute('aria-valuenow', '100') await expect(progress.nth(1)).toHaveAttribute('aria-valuenow', '100')
31-35: Verify final progress value and loading text behavior.The changes to the loading text visibility check and final progress value raise a few questions:
- The loading text is now checked to be not visible. Is this the intended behavior after multiple progress updates?
- The final progress value is set to '90' after clicking the first button. This seems inconsistent with the earlier changes that set the initial value to '45'.
Please clarify:
- Is the loading text supposed to disappear after multiple progress updates?
- Should the final progress value be '90', or was this overlooked when updating the initial value to '45'?
Consider updating the final assertion to match the new initial value:
await expect(progress.nth(0)).toHaveAttribute('aria-valuenow', '45') await expect(progress.nth(1)).toHaveAttribute('aria-valuenow', '45')examples/sites/demos/pc/app/progress/custom-color.spec.ts (5)
13-15: LGTM: Initial color expectations updated.The changes to the expected initial colors for the progress bars are consistent with the updated design. These modifications align with the changes made in other related files.
17-17: LGTM: Consistent color expectation for progress1.The updated color expectation for progress1 after the first button click is consistent with its initial color, maintaining the intended behavior.
21-23: LGTM: Updated color expectations after second click.The color expectations for progress1 and progress3 have been appropriately updated. However, progress2's expected color remains unchanged.
Please verify if keeping progress2's color unchanged at this stage is intentional. If it's not, consider updating it to match the new color scheme.
25-27: LGTM: Updated color expectation for progress1 after third click.The color expectation for progress1 has been appropriately updated. Progress2 and progress3 colors remain unchanged from the previous state.
Please confirm if keeping progress2 and progress3 colors unchanged at this stage is intentional. If not, consider updating them to align with the new color scheme.
29-31: LGTM: Final color expectation updates.The color expectation for progress1 has been consistently updated for the final state check. Progress2 and progress3 colors remain unchanged from the previous state.
As this is the final state check in the test, please review the entire color change sequence to ensure it accurately reflects the intended behavior of the progress component. Consider adding a comment explaining the expected color progression if it's not immediately obvious.
examples/sites/demos/pc/app/progress/progress-type-circle.spec.ts (2)
53-67: LGTM! Verify new initial value and increments.The test case has been updated to reflect new initial values and increments for the progress circles. The changes appear to be consistent and align with updates mentioned in other related files.
Please confirm that the new initial value of 50 and the subsequent increments to 60 and 70 are intentional and align with the updated component behavior. You may want to run the following command to check for any related changes in the component files:
#!/bin/bash # Search for initial value and increment changes in progress component files rg -i "percentage.*=.*50|percentage.*\+.*10" --type vue src/progress
33-40: LGTM! Verify visual appearance.The changes to the C-shaped circular progress component are consistent with those made to the regular circular type. The stroke width, path data, and stroke-dasharray have been updated to reflect the new dimensions.
Please verify that these changes produce the intended visual appearance for the C-shaped progress circle. You may want to run the following command to check for any related visual regression tests:
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
Bug Fixes
tiny-progresscomponents to enhance rendering and status indication.Style