fix: Fixed display of 0% inside label in progress bar #326
Conversation
WalkthroughThe pull request introduces enhancements to the Flowbite Vue 3 Progress component. The changes include adding a new computed property Changes
Sequence DiagramsequenceDiagram
participant Component as FwbProgress
participant Props as Progress Props
participant Computed as Computed Properties
Props->>Computed: Input progress value
Computed->>Computed: Calculate safeProgress
Computed->>Computed: Determine displayBar
Computed-->>Component: Render progress bar
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (2)
Finishing Touches
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 was a problem hiding this comment.
Hey,
This is clearly something that we need to fix ;)
But... If you tak a look at Flowbite's Progress Bar component... i think it shows smal percents a little better (you'll have to change style to width: 0% to see it):
Compared to what we have with your fix... it doesn't look like 0%:
@andreyjamer
Please let me know if you can make it look like the Figma component? (it's the source of truth in terms of the design for us here)
And Happy New Year! 🎉
|
Hey! Another case is the progress bar with label outside the bar. In current implementation it loses the rounded corners on the left In my fix, the bar always has a minimum width for the particular size, so the left part is always rounded. And the bar is not displayed if the value is 0 and the label is outside. |
|
Hmm... it clears a lot now ;) It looks more like Flowbite problem in general... especially the light theme. I think we should move this issue there. I've created new Issue here: themesberg/flowbite#1016 you can comment/upvote if you want ;) Let's wait with this for the response there (we don't want to go ahead of the Flowbite's main repo) |
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
Documentation