fix(UProgress): return correct max value on 0#5662
Conversation
Shooteger
changed the title
commit: |
|
@benjamincanac please review |
|
The |
📝 WalkthroughWalkthroughThe Progress.vue component's percent calculation logic is refactored from a switch-case structure to a guard-clause approach. The change introduces maxValue and currentModelValue variables to handle boundary conditions more explicitly. Specific checks are added for negative values, zero maximum values (which default to 100), and values exceeding the maximum. The percentage is then computed via direct division with rounding, with the final result clamped to [0, 100]. The refactoring addresses edge cases without altering the component's public API. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/components/Progress.vue`:
- Around line 98-103: The percent calculation still treats max=0 as missing
because earlier code coerces props.max with a falsy check; change the falsy
check that uses "!props.max" to an explicit undefined/null check (e.g. props.max
=== undefined or props.max == null) so realMax/ maxValue will preserve 0; keep
the percent logic in the computed (maxValue = realMax.value ?? 100 and the
currentModelValue checks in the function that returns
Math.round((currentModelValue / maxValue) * 100)) so the new guard for maxValue
=== 0 will run and produce the correct 0% behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 057a3dd3-3678-4732-a0e0-12cd0cdf8460
📒 Files selected for processing (1)
src/runtime/components/Progress.vue
| const maxValue = realMax.value ?? 100 | ||
| const currentModelValue = rootProps.value.modelValue! | ||
|
|
||
| if (currentModelValue < 0 || maxValue === 0) return 0 | ||
| if (currentModelValue > maxValue) return 100 | ||
| return Math.round((currentModelValue / maxValue) * 100) |
There was a problem hiding this comment.
max === 0 is still coerced to default 100 due to upstream falsy check
The new guard on Line 101 won’t run for max={0} because Line 82 (!props.max) turns 0 into undefined, so Line 98 falls back to 100. This leaves the original bug (data-max=100 and wrong percent) unresolved.
💡 Proposed fix
const realMax = computed(() => {
- if (isIndeterminate.value || !props.max) {
+ if (isIndeterminate.value || props.max === null || props.max === undefined) {
return undefined
}
if (Array.isArray(props.max)) {
return props.max.length - 1
}
return Number(props.max)
})
const percent = computed(() => {
if (isIndeterminate.value) {
return undefined
}
const maxValue = realMax.value ?? 100
const currentModelValue = rootProps.value.modelValue!
if (currentModelValue < 0 || maxValue === 0) return 0
if (currentModelValue > maxValue) return 100
return Math.round((currentModelValue / maxValue) * 100)
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/components/Progress.vue` around lines 98 - 103, The percent
calculation still treats max=0 as missing because earlier code coerces props.max
with a falsy check; change the falsy check that uses "!props.max" to an explicit
undefined/null check (e.g. props.max === undefined or props.max == null) so
realMax/ maxValue will preserve 0; keep the percent logic in the computed
(maxValue = realMax.value ?? 100 and the currentModelValue checks in the
function that returns Math.round((currentModelValue / maxValue) * 100)) so the
new guard for maxValue === 0 will run and produce the correct 0% behavior.
|
Closing this as Reka UI's |
2 participants
🔗 Linked issue
resolves #5647
❓ Type of change
📚 Description
This PR fixes division by zero when
maxprop is set to0by adding explicit check.The problem was, when
max={0}, the component performed(modelValue / 0) * 100, causing incorrect percentage calculation (returns 100 instead of 0).I changed the code to if/else instead of a switch for readability and added an early return when
maxValue === 0to prevent division by zero and return the correct value.📝 Checklist