refactor(onboarding): implement dynamic step count calculation#52
Merged
DavidsonGomes merged 3 commits intoMay 18, 2026
Conversation
- Replace hardcoded value '7' in progress calculation with dynamic 'totalSteps' - Fix incorrect display showing '6' instead of '7' in progress text - Centralize step fields in 'onboardingSteps' array for maintainability - Calculate totalSteps from array length to automatically reflect changes This allows the onboarding progress to automatically adapt when the number of steps changes.
- Verify total steps is calculated dynamically (not hardcoded) - Test progress updates correctly when fields are filled - Test 'Other' channel requires additional input to count as filled - Test progress percentage calculation is accurate - Document redirect behavior for unauthenticated users These tests ensure the refactor works correctly and prevent regressions.
Reviewer's GuideRefactors onboarding progress tracking to derive both the filled step count and total step count from a centralized steps array, and adds a new test suite that asserts dynamic step calculation, progress behavior, and edge cases like the "Other" channel and redirect logic. Sequence diagram for dynamic onboarding progress updatesequenceDiagram
actor User
participant OnboardingPage
participant onboardingSteps
participant ProgressBar
User->>OnboardingPage: Change form field value
OnboardingPage->>OnboardingPage: Update form state
OnboardingPage->>OnboardingPage: Recompute isChannelValid
OnboardingPage->>onboardingSteps: Build array from form fields
OnboardingPage->>OnboardingPage: filledCount = filter(onboardingSteps).length
OnboardingPage->>OnboardingPage: totalSteps = onboardingSteps.length
OnboardingPage->>OnboardingPage: progressPct = round(filledCount / totalSteps * 100)
OnboardingPage->>ProgressBar: Render with progressPct and text filledCount of totalSteps
ProgressBar-->>User: Display updated progress (e.g., 3 de 7)
Class diagram for dynamic onboarding progress calculationclassDiagram
class OnboardingPage {
+form_teamSize: string
+form_dailyVolume: string
+form_mainChannel: string
+form_mainChannelOther: string
+form_biggestPain: string
+form_crmExperience: string
+form_mainGoal: string
+isChannelValid: boolean
+onboardingSteps: readonly any[]
+filledCount: number
+totalSteps: number
+progressPct: number
+handleSubmit(): Promise~void~
}
class useLanguageHook {
+t(key: string): string
}
class useAuthHook {
+isAuthenticated: boolean
+refreshUser(): Promise~void~
}
class setupService {
+saveSurvey(data: any): Promise~any~
}
class surveyService {
+saveSurvey(data: any): Promise~any~
}
class AppLogo {
}
OnboardingPage --> useLanguageHook : uses
OnboardingPage --> useAuthHook : uses
OnboardingPage --> setupService : calls
OnboardingPage --> surveyService : calls
OnboardingPage --> AppLogo : renders
%% Internal relationships
OnboardingPage "1" o-- "*" onboardingSteps : derives
OnboardingPage ..> filledCount : calculates
OnboardingPage ..> totalSteps : length of onboardingSteps
OnboardingPage ..> progressPct : from filledCount and totalSteps
OnboardingPage ..> isChannelValid : derived from mainChannel and mainChannelOther
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new tests are tightly coupled to the exact Portuguese copy and string format (e.g.
/0 de 7$/i,/de 7$/i), which can become brittle if translations or wording change; consider exposing a progress-specific hook/utility or a data attribute so tests can assert the numeric logic without depending on localized text. - In the progress percentage test you assert 100% completion by matching an inline
stylestring (div[style*="width: 100%"]), which is sensitive to implementation details; prefer asserting against a role/ARIA attribute, a data-testid, or a numeric prop/value that represents the progress percentage.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new tests are tightly coupled to the exact Portuguese copy and string format (e.g. `/0 de 7$/i`, `/de 7$/i`), which can become brittle if translations or wording change; consider exposing a progress-specific hook/utility or a data attribute so tests can assert the numeric logic without depending on localized text.
- In the progress percentage test you assert 100% completion by matching an inline `style` string (`div[style*="width: 100%"]`), which is sensitive to implementation details; prefer asserting against a role/ARIA attribute, a data-testid, or a numeric prop/value that represents the progress percentage.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…testability - Add role="progressbar" with aria-valuenow, aria-valuemin, aria-valuemax - Add data-testid="progress-bar-fill" for progress bar - Add data-testid="progress-text" with data-filled and data-total attributes - Update tests to use data-testid and ARIA instead of text matching - Remove dependency on translated text in tests - Remove dependency on inline style strings in tests This makes tests more robust and less coupled to implementation details.
Contributor
Author
Atualizações recentesMelhorei os testes seguindo o feedback do code review:
Isso torna os testes mais robustos e menos frágeis a mudanças na interface. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactored the onboarding page to calculate the total number of steps dynamically instead of using hardcoded values. This makes the progress tracking more maintainable and automatically adapts when steps are added or removed.
Changes
OnboardingPage.tsx:
7in progress calculation with dynamictotalSteps6instead of7in progress textonboardingStepsarraytotalStepsfrom array lengthOnboardingPage.spec.tsx (new):
Motivation
Previously, the progress count had two hardcoded values:
7(correct number of fields)6(incorrect)This inconsistency and hardcoding made it error-prone when modifying the onboarding steps.
Testing
All 5 new tests pass:
Checklist
Summary by Sourcery
Tests: