fixed progress bar overflow in onboarding steps#726
fixed progress bar overflow in onboarding steps#726rahulharpal1603 merged 2 commits intoAOSSIE-Org:mainfrom
Conversation
|
|
📝 WalkthroughWalkthroughThis PR decouples user-facing progress from internal step indexing by computing a visible step index in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
|
|
This solves issue #638 |
|
Hello! I am participating in GSoC and would like to work on this issue. Could you please assign it to me? |
|
@Garry400 Please fix the Lint issues ASAP. |
|
Never mind, there was only a small fix needed, which I did myself. |
rahulharpal1603
left a comment
There was a problem hiding this comment.
@Garry400 Thanks!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsx (2)
22-26: Consider adding JSDoc to clarify the dual index pattern.The interface now contains both
stepIndex(for internal state management) andcurrentStepDisplayIndex(for UI display). Adding brief JSDoc comments would help future maintainers understand the distinction between these two indices.📝 Suggested documentation enhancement
interface AvatarNameSelectionStepProps { + /** Internal step index including hidden/internal steps, used for state management */ stepIndex: number; totalSteps: number; + /** Display-only index for visible onboarding steps, used for progress UI */ currentStepDisplayIndex: number; }
76-83: Consider extracting the progress calculation to reduce duplication.The progress bar width calculation is correct and consistent with the percentage display. However, the same calculation
((currentStepDisplayIndex + 1) / totalSteps) * 100appears in both line 73 and line 80.🔎 Optional DRY refactor
const handleNextClick = () => { dispatch(setName(name)); dispatch(setAvatar(selectedAvatar)); localStorage.setItem('name', name); localStorage.setItem('avatar', selectedAvatar); dispatch(markCompleted(stepIndex)); }; + + const progressPercentage = ((currentStepDisplayIndex + 1) / totalSteps) * 100; if (localStorage.getItem('name') && localStorage.getItem('avatar')) { return null; } return ( <> <Card className="flex max-h-full w-1/2 flex-col gap-3 border p-4"> <CardHeader className="p-3"> <div className="text-muted-foreground mb-1 flex justify-between text-xs"> <span> Step {currentStepDisplayIndex + 1} of {totalSteps} </span> <span> - {Math.round(((currentStepDisplayIndex + 1) / totalSteps) * 100)}% + {Math.round(progressPercentage)}% </span> </div> <div className="bg-muted mb-2 h-1.5 w-full rounded-full"> <div className="bg-primary h-full rounded-full transition-all duration-300" style={{ - width: `${((currentStepDisplayIndex + 1) / totalSteps) * 100}%`, + width: `${progressPercentage}%`, }} /> </div>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
- GitHub Check: Tauri Build Check (windows-latest)
- GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
- GitHub Check: Backend Tests
🔇 Additional comments (2)
frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsx (2)
28-32: LGTM!The prop destructuring correctly includes the new
currentStepDisplayIndexparameter, matching the interface definition.
68-75: The progress calculation is correct and properly handled; no additional validation is needed.The
currentStepDisplayIndexis derived fromVISIBLE_STEPS.indexOf(stepName)in the parent component, which returns valid indices (0, 1, 2) forAvatarSelectionStep,FolderSetupStep, andThemeSelectionStep—the only components that actually use this prop.UpdateStepandServerCheckreceive the prop but do not use it, so the -1 edge case fromindexOf()never impacts rendering. The step label and progress percentage calculations are mathematically correct and will display properly across all steps.
Fixes #638 issue where the onboarding progress bar exceeded 100% and showed incorrect step numbers due to using a global step index that included hidden steps.
Changes:
Result:
Progress and step count now accurately represent user-facing onboarding steps.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.