fix: Fixed display of 0% inside label in progress bar #326
Conversation
|
Warning Rate limit exceeded@andreyjamer has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 49 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes involve the addition of a new Changes
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 (
|
✅ Deploy Preview for sensational-seahorse-8635f8 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/components/FwbProgress/FwbProgress.vue (1)
56-59: Effective implementation ofsafeProgresscomputed property.The
safeProgresscomputed property is a great solution to ensure minimum visibility for small progress values. It cleverly uses theprogressSafeSizesobject to determine a minimum size based on the component's size prop.This implementation effectively addresses the PR's main objective of fixing the display of small percentages in the progress bar.
Consider adding a brief comment explaining the purpose of
progressSafeSizesand what values it contains. This would enhance code maintainability. For example:// progressSafeSizes contains minimum percentage values for each size variant // to ensure visibility of small progress values const safeProgress = computed(() => { const size = progressSafeSizes[props.size] return props.progress <= size ? size : props.progress })src/components/FwbProgress/composables/useProgressClasses.ts (1)
Line range hint
1-67: Consider additional changes to address small value visibilityWhile the new
progressSafeSizesconstant is a step in the right direction, the current implementation doesn't seem to directly address the issue of displaying small values (like 0%) in progress bars. Consider the following suggestions:
- Implement a minimum width for the colored portion of the progress bar when the value is greater than 0% but very small.
- Add a class to ensure the label remains visible even when the progress bar is small.
Here's a potential implementation to consider:
// Add this function to handle minimum width function getProgressWidth(value: number): string { return value > 0 && value < 1 ? '1%' : `${value}%`; } // Modify the useProgressClasses function to include this logic export function useProgressClasses(props: UseProgressClassesProps) { // ... existing code ... const progressWidth = computed(() => getProgressWidth(props.value.value)); const bindClasses = computed(() => { return classNames( barColorClasses[props.color.value], progressSizeClasses[props.size.value], 'min-w-[1.5em]', // Ensures a minimum width for label visibility ) }) // ... rest of the function ... return { innerClasses: bindClasses, outerClasses, outsideLabelClasses, progressWidth, } }This suggestion ensures that:
- The progress bar always has a minimum width to display the label.
- For very small non-zero values, the progress bar will still be visible.
Please adjust the implementation as needed to fit your specific requirements and component structure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- docs/components/progress/examples/FwbProgressExampleLabelInside.vue (1 hunks)
- src/components/FwbProgress/FwbProgress.vue (3 hunks)
- src/components/FwbProgress/composables/useProgressClasses.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/components/progress/examples/FwbProgressExampleLabelInside.vue
🔇 Additional comments (2)
src/components/FwbProgress/FwbProgress.vue (2)
22-23: Excellent changes to improve small progress value visibility!The modifications in these lines effectively address the issue of displaying small progress values:
- Using
safeProgressinstead ofprogressfor the width ensures a minimum visible width for the progress bar.- Adding the
min-w-maxclass guarantees that the inner div will always be wide enough to display its content, solving the problem of the "0%" label being nearly invisible.These changes align perfectly with the PR objective of fixing the display of small percentages in the progress bar.
34-34: Correct import for computed property.Good job importing the
computedfunction from Vue. This is necessary for defining thesafeProgresscomputed property and follows the best practice of importing only the required functions.
| export const progressSafeSizes: Record<ProgressSize, number> = { | ||
| sm: 1, | ||
| md: 2, | ||
| lg: 3, | ||
| xl: 4, | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Action Required: progressSafeSizes is currently not utilized in the codebase.
The progressSafeSizes constant is exported in useProgressClasses.ts but is not referenced in any other files. Please verify if this constant is intended for future implementations. If it is not needed, consider removing it to keep the codebase clean and maintainable.
🔗 Analysis chain
LGTM! New constant added for progress bar size mapping.
The new progressSafeSizes constant appears to be related to the PR objective of fixing the display of small values in progress bars. It provides a numeric mapping for each progress bar size, which could be useful for determining minimum width or visibility thresholds for labels.
To ensure this constant is being utilized as intended, please run the following script to check its usage across the codebase:
If the constant is not used anywhere, consider adding a TODO comment to implement its usage or remove it if it's no longer needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of progressSafeSizes in other files
# Test: Search for progressSafeSizes usage
rg --type typescript -A 5 'progressSafeSizes'
Length of output: 80
Script:
#!/bin/bash
# Description: Verify the usage of progressSafeSizes in TypeScript files by searching .ts and .tsx extensions
# Test: Search for progressSafeSizes usage in .ts and .tsx files
rg -A 5 'progressSafeSizes' --type-add 'custom_ts:(**/*.ts|**/*.tsx)' --type custom_ts
Length of output: 146
Script:
#!/bin/bash
# Description: Verify the usage of progressSafeSizes in TypeScript files by searching .ts and .tsx extensions
# Test: Search for progressSafeSizes usage in .ts and .tsx files
rg 'progressSafeSizes' -g '*.ts' -g '*.tsx' -A 5
Length of output: 517
There is a bug with progress bar for small values. When the bar is too small, then the inside label is almost invisible.
Summary by CodeRabbit
New Features
FwbProgressExampleLabelInsidecomponent.safeProgressto ensure progress values do not exceed defined maximums.progressSafeSizesfor mapping progress sizes to numeric values.Bug Fixes