feat: add new step one for personal details [Onboarding-Form] #1083
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbit
WalkthroughIntroduces a new multi-step onboarding form component architecture with a reusable base component for state and validation management, a first-step form component for profile setup, a progress stepper header, and updates to the main stepper to handle step-based navigation. Adds configuration constants and validation utilities for the form flow. Changes
Sequence DiagramsequenceDiagram
participant User
participant Stepper as new-stepper.hbs
participant Header as stepper-header
participant Step as BaseStepComponent
participant Storage as localStorage
User->>Stepper: interact with form
Stepper->>Header: render with currentStep
Header->>Header: compute progressPercentage
User->>Step: enter/change field value
activate Step
Step->>Step: debounced inputHandler
Step->>Step: validateWordCount
Step->>Step: revalidate all fields
Step->>Step: update errorMessage
Step->>Storage: persist form state
deactivate Step
Step->>Stepper: update isValid prop
Stepper->>Stepper: enable/disable Next button
User->>Stepper: click Next
Stepper->>Stepper: incrementStep
Stepper->>Header: update currentStep
Header->>Header: recalculate progressPercentage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Inconsistent text formatting ▹ view | |
 |
Non-idiomatic Text Content ▹ view | |
 |
Separate Input Validation from Business Logic ▹ view | |
|
Missing Component Interface Definition ▹ view | |
|
Unexplained Magic Numbers ▹ view | |
 |
Missing memoization for computed getters ▹ view | |
 |
Progress percentage can exceed 100% ▹ view | |
|
No validation for non-numeric step values ▹ view | |
|
Unclear Number Type Coercion ▹ view | |
|
Unclear HTML Safety Implementation ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| app/constants/new-join-form.js | ✅ |
| app/components/new-join-steps/stepper-header.js | ✅ |
| app/components/new-stepper.js | ✅ |
| app/components/new-join-steps/new-step-one.js | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
Deploying www-rds with
|
| Latest commit: |
8b271fa
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d3cd1697.www-rds.pages.dev |
| Branch Preview URL: | https://feat-new-step-1.www-rds.pages.dev |
MayankBansal12
force-pushed
the
feat/new-step-1
branch
2 times, most recently
from
fa00939 to
317b529
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
app/components/new-join-steps/base-step.js(1 hunks)app/components/new-join-steps/new-step-one.hbs(1 hunks)app/components/new-join-steps/new-step-one.js(1 hunks)app/components/new-join-steps/stepper-header.hbs(1 hunks)app/components/new-join-steps/stepper-header.js(1 hunks)app/components/new-stepper.hbs(2 hunks)app/components/new-stepper.js(1 hunks)app/components/reusables/input-box.hbs(1 hunks)app/constants/new-join-form.js(1 hunks)app/utils/validator.js(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: AnujChhikara
Repo: Real-Dev-Squad/website-www PR: 1090
File: app/templates/admin/applications.hbs:0-0
Timestamp: 2025-11-06T20:21:16.012Z
Learning: In the Real-Dev-Squad/website-www repository, user AnujChhikara prefers to submit incremental PRs where UI templates/infrastructure are added first, followed by backing implementation (routes, controllers, data fetching logic) in subsequent PRs.
Learnt from: MayankBansal12
Repo: Real-Dev-Squad/website-www PR: 1038
File: tests/integration/components/new-signup/info-test.js:12-14
Timestamp: 2025-05-06T21:51:26.893Z
Learning: In the website-www repository's new-signup implementation, step constants (FIRST_STEP, SECOND_STEP, etc.) are defined in controllers based on the NEW_SIGNUP_STEPS array from constants. When testing components that use these controller properties, these constants need to be defined in the test context before being referenced in test functions.
Learnt from: MayankBansal12
Repo: Real-Dev-Squad/website-www PR: 1038
File: tests/integration/components/new-signup/info-test.js:12-14
Timestamp: 2025-05-06T21:51:26.893Z
Learning: In the website-www repository, step constants for the new signup flow (FIRST_STEP, SECOND_STEP, etc.) are defined in controllers based on the NEW_SIGNUP_STEPS array. Tests that simulate controller behavior need to define these constants in the test context.
📚 Learning: 2025-05-06T21:51:26.893Z
Learnt from: MayankBansal12
Repo: Real-Dev-Squad/website-www PR: 1038
File: tests/integration/components/new-signup/info-test.js:12-14
Timestamp: 2025-05-06T21:51:26.893Z
Learning: In the website-www repository's new-signup implementation, step constants (FIRST_STEP, SECOND_STEP, etc.) are defined in controllers based on the NEW_SIGNUP_STEPS array from constants. When testing components that use these controller properties, these constants need to be defined in the test context before being referenced in test functions.
Applied to files:
app/constants/new-join-form.jsapp/components/new-stepper.js
📚 Learning: 2025-05-06T21:51:26.893Z
Learnt from: MayankBansal12
Repo: Real-Dev-Squad/website-www PR: 1038
File: tests/integration/components/new-signup/info-test.js:12-14
Timestamp: 2025-05-06T21:51:26.893Z
Learning: In the website-www repository, step constants for the new signup flow (FIRST_STEP, SECOND_STEP, etc.) are defined in controllers based on the NEW_SIGNUP_STEPS array. Tests that simulate controller behavior need to define these constants in the test context.
Applied to files:
app/constants/new-join-form.jsapp/components/new-stepper.js
🧬 Code graph analysis (2)
app/components/new-join-steps/base-step.js (1)
app/utils/validator.js (3)
validateWordCount(17-32)validateWordCount(17-32)wordCount(19-19)
app/components/new-join-steps/new-step-one.js (2)
app/components/new-join-steps/base-step.js (1)
BaseStepComponent(9-111)app/constants/new-join-form.js (4)
ROLE_OPTIONS(8-14)ROLE_OPTIONS(8-14)NEW_STEP_LIMITS(16-23)NEW_STEP_LIMITS(16-23)
🪛 ESLint
app/components/new-stepper.js
[error] 5-5: 'NEW_FORM_STEPS' is defined but never used.
(no-unused-vars)
⏰ 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). (2)
- GitHub Check: build (18.x)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (7)
app/components/new-join-steps/stepper-header.hbs (1)
1-14: LGTM! Well-structured accessible template.The stepper header template is well-implemented with proper accessibility attributes (aria-live, role="progressbar", aria-valuemin/max/now) and clean integration with the component's computed values.
app/components/reusables/input-box.hbs (1)
16-16: LGTM! Clean addition of disabled state support.The disabled binding follows the existing pattern and integrates cleanly with the other input attributes.
app/components/new-join-steps/new-step-one.js (1)
7-36: Component structure looks good.The component properly extends
BaseStepComponent, uses service injection, and implements the required hooks (storageKey,postLoadInitialize). The auto-fill logic forfullNameappropriately checks for existing data before populating.Note: The validation approach using word counts was flagged in the constants file review.
app/components/new-stepper.hbs (1)
3-59: LGTM! Well-structured conditional rendering.The template properly implements step-based navigation with:
- Conditional StepperHeader display (hidden on welcome screen)
- Appropriate step component rendering (WelcomeScreen at MIN_STEP, NewStepOne at step 1)
- Navigation controls with proper disabled state logic (Next button enabled when form is valid)
The disabled logic
@disabled={{not (or this.preValid this.isValid)}}correctly enables the Next button when either the initial state or current state is valid, supporting step revisitation.app/components/new-join-steps/base-step.js (1)
45-46: Clarify the distinction betweensetIsValidandsetIsPreValid.The component calls both
this.args.setIsPreValid(lines 45, 50) andthis.args.setIsValid(line 108). The semantic difference between these two callbacks is unclear. IfsetIsPreValidrepresents "validity before user interaction" andsetIsValidrepresents "current validity," consider adding documentation or renaming for clarity.Additionally,
setIsPreValid(false)is called on line 50 at the start of every input handler, which means the form is marked invalid before validation runs. Verify this is the intended behavior.Consider adding JSDoc comments to clarify the expected behavior:
/** * Called during initialization to set the form's validity based on stored data * @param {boolean} valid - Initial form validity state */ this.args.setIsPreValid(valid); /** * Called after each field update to set the current form validity * @param {boolean} valid - Current form validity state */ this.args.setIsValid(allValid);app/components/new-join-steps/new-step-one.hbs (2)
22-22: Verify the product decision for immutable role selection.Line 22 states: "it won't be changed later" regarding the role choice. This is a very restrictive constraint that could lead to user frustration if they select the wrong role or their circumstances change.
Ensure this is an intentional product decision and consider:
- Whether users can contact support to change their role
- Whether the warning should be more prominent (e.g., a modal confirmation before proceeding)
- Adding documentation about the implications of role selection
This is a UX concern that should be validated with product/design stakeholders. If the role cannot be changed, consider making the warning more prominent or adding a confirmation step.
45-63: The suggested solution uses non-existent component parameters. However, error messages ARE missing and need to be added.The
Reusables::InputBoxandReusables::Dropdowncomponents don't support an@errorMessageparameter. Instead, the codebase uses a different pattern—errors are displayed in separate conditional blocks outside the component, as shown instep-two.hbs:{{#if this.errorMessage.fieldName}} <div class='error__message'>{{this.errorMessage.fieldName}}</div> {{/if}}Since
new-step-one.jsextendsBaseStepComponentwith validationMap entries forstate,city, andcountry, error messages will be tracked but not displayed. Add error display blocks after each field:<Reusables::InputBox @field='State' @name='state' @placeHolder='Your State e.g. Karnataka' @type='text' @required={{true}} @value={{this.data.state}} @onInput={{this.inputHandler}} /> {{#if this.errorMessage.state}} <div class='error__message'>{{this.errorMessage.state}}</div> {{/if}} <Reusables::InputBox @field='City' @name='city' @placeHolder='Your City e.g. Bengaluru' @type='text' @required={{true}} @value={{this.data.city}} @onInput={{this.inputHandler}} /> {{#if this.errorMessage.city}} <div class='error__message'>{{this.errorMessage.city}}</div> {{/if}} <Reusables::Dropdown @field='Your Country' @name='country' @placeHolder='Choose your country' @required={{true}} @value={{this.data.country}} @options={{this.countries}} @onChange={{this.inputHandler}} /> {{#if this.errorMessage.country}} <div class='error__message'>{{this.errorMessage.country}}</div> {{/if}}Likely an incorrect or invalid review comment.
MayankBansal12
force-pushed
the
feat/new-step-1
branch
from
8147e2f to
00fd80e
Compare
MayankBansal12
force-pushed
the
feat/new-step-1
branch
from
00fd80e to
e5edda0
Compare
MayankBansal12
force-pushed
the
feat/new-step-1
branch
from
7ecf73e to
1b8f477
Compare
prakashchoudhary07
left a comment
prakashchoudhary07
left a comment
There was a problem hiding this comment.
As discussed @MayankBansal12 , will be raising a separate PR for the suggested changes
…1084) * feat: add new step two for introduction * feat: add step three for interests * feat: add base step class to handle input validation and local storage sync * fix: postIntialization in new step one * fix: grid styling for step two and three * refactor: remove redundant max words * fix: lint issue and form headings * refactor: use constant for storage keys * feat: add new step four and five for social links and whyRds [Onboarding-Form] (#1086) * feat: add new step for social links * feat: add snew step four for social links * feat: add new step fivve for stepper * fix: styling for selectt in new step five * refactor: replace hardcoded storage keys with constants * feat: add review step for new stepper [Onboarding-Form] (#1087) * feat: add review step for new stepper * feat: add thank you screen for new stepper * refactor: use structure for step data and replace hardcoded storage keys * refactor: use getter func for review step button
Date: 03-11-25
Developer Name: @MayankBansal12
Issue Ticket Number:-
Description:
Is Under Feature Flag
Database changes
Breaking changes (If your feature is breaking/missing something please mention pending tickets)
Is Development Tested?
Tested in staging?
Add relevant Screenshot below ( e.g test coverage etc. )
screencast
Screencast.from.2025-11-08.18-32-03.mp4
tests