feat: add new stepper and welcome screen for form #1081
Conversation
|
Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment
|
|
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 new onboarding join flow components: a welcome screen with terms-and-conditions modal, a new stepper component managing step progression with localStorage persistence and URL state sync, and an application-terms service tracking user acceptance. Updates join controller and template to conditionally render the new stepper alongside existing flow. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WelcomeScreen
participant TermsModal
participant Service as applicationTerms Service
participant NewStepper
User->>WelcomeScreen: render at step 0
WelcomeScreen->>WelcomeScreen: display unchecked checkbox
alt User clicks checkbox (terms not accepted)
User->>WelcomeScreen: click checkbox
WelcomeScreen->>TermsModal: showModal = true
TermsModal->>User: display terms & conditions
User->>TermsModal: click Accept button
TermsModal->>Service: setTermsAcceptance(true)
Service->>Service: hasUserAcceptedTerms = true
TermsModal->>WelcomeScreen: onAccept → closeModal
WelcomeScreen->>WelcomeScreen: checkbox now checked
end
User->>NewStepper: click Start button (now enabled)
NewStepper->>Service: verify hasUserAcceptedTerms
NewStepper->>NewStepper: save user details to localStorage
NewStepper->>NewStepper: increment currentStep
NewStepper->>User: render next step
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Deploying www-rds with
|
| Latest commit: |
d693fed
|
| Status: | 🚫 Build failed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (15)
app/components/new-join-steps/welcome-screen.hbs(1 hunks)app/components/new-join-steps/welcome-screen.js(1 hunks)app/components/new-stepper.hbs(1 hunks)app/components/new-stepper.js(1 hunks)app/components/terms-modal.hbs(1 hunks)app/components/terms-modal.js(1 hunks)app/controllers/join.js(1 hunks)app/services/application-terms.js(1 hunks)app/styles/app.css(1 hunks)app/styles/new-stepper.module.css(1 hunks)app/templates/join.hbs(1 hunks)tests/integration/components/new-join-steps/welcome-screen-test.js(1 hunks)tests/integration/components/new-stepper-test.js(1 hunks)tests/integration/components/terms-modal-test.js(1 hunks)tests/unit/controllers/join-test.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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:
tests/integration/components/new-stepper-test.jstests/integration/components/new-join-steps/welcome-screen-test.jsapp/components/new-stepper.hbs
📚 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:
tests/integration/components/new-stepper-test.jsapp/components/new-stepper.hbs
🧬 Code graph analysis (3)
tests/integration/components/new-stepper-test.js (2)
tests/integration/components/new-join-steps/welcome-screen-test.js (1)
terms(48-48)tests/integration/components/terms-modal-test.js (2)
terms(29-29)terms(52-52)
tests/integration/components/new-join-steps/welcome-screen-test.js (2)
tests/integration/components/new-stepper-test.js (1)
terms(30-30)tests/integration/components/terms-modal-test.js (2)
terms(29-29)terms(52-52)
tests/integration/components/terms-modal-test.js (2)
tests/integration/components/new-join-steps/welcome-screen-test.js (1)
terms(48-48)tests/integration/components/new-stepper-test.js (1)
terms(30-30)
⏰ 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). (1)
- GitHub Check: build (18.x)
🔇 Additional comments (27)
app/controllers/join.js (1)
25-27: LGTM! String comparison is appropriate for query parameters.The implementation correctly compares the query param value as a string, which is the proper approach since query parameters are always strings.
app/styles/app.css (1)
12-12: LGTM! CSS import added correctly.The new stepper styles are imported in a logical location alongside the existing stepper styles.
tests/integration/components/new-join-steps/welcome-screen-test.js (1)
1-59: LGTM! Comprehensive test coverage for WelcomeScreen component.The test suite covers all critical user interactions:
- Initial rendering state
- Modal opening on checkbox click
- Terms acceptance flow
- Unchecking after acceptance
The tests follow Ember testing best practices and use appropriate data-test attributes for element selection.
tests/unit/controllers/join-test.js (1)
24-24: LGTM! Test updated correctly.The test expectation now matches the updated
queryParamsarray in the controller.app/components/terms-modal.js (1)
1-6: LGTM! Simple component with service injection.This component correctly injects the
applicationTermsservice to make it accessible in the template. The minimal implementation is appropriate for a presentational component.app/templates/join.hbs (1)
41-53: LGTM! Conditional rendering logic is correct.The template correctly implements the three-path routing:
dev=true→ New stepper flow (NewStepper)oldOnboarding=true→ Old join form (StepperSignup)- Default → Current production flow (Stepper)
This aligns with the PR objectives and maintains backward compatibility.
app/components/new-stepper.hbs (2)
13-13: LGTM! Start button correctly disabled until terms are accepted.The button is properly disabled when
hasUserAcceptedTermsis false, ensuring users must accept terms before proceeding. The use of thenothelper is correct.
2-2: No issues found—form submission is correctly implemented.The
nextStepmethod inapp/components/new-stepper.js(line 62) properly callse.preventDefault()as its first statement, preventing page reload. ThestartHandlermethod also exists (line 53). The form submission behavior is secure and will not cause unwanted navigation.app/components/new-join-steps/welcome-screen.hbs (2)
25-36: LGTM!The checkbox implementation correctly binds the checked state to the service and wires the change handler. The wrapping label pattern is a valid accessibility approach.
39-45: LGTM!The TermsModal integration correctly uses conditional rendering and properly wires the callbacks for accept and close actions.
tests/integration/components/terms-modal-test.js (3)
9-25: LGTM!The test correctly verifies basic modal rendering when opened.
27-49: LGTM!The test correctly verifies the Accept Terms button appears when terms are not accepted and properly validates the action callback. Good use of the step pattern for assertion.
51-69: LGTM!The test correctly validates that the Accept Terms button is hidden when terms have already been accepted.
app/components/terms-modal.hbs (3)
1-12: LGTM!The modal header is properly structured with an accessible close button that correctly wires the onClose callback.
14-36: LGTM!The terms sections are well-structured and clearly communicate the requirements and data handling policies.
38-48: LGTM!The conditional footer correctly renders the Accept Terms button only when terms haven't been accepted, and properly wires the onAccept callback.
app/styles/new-stepper.module.css (3)
1-52: LGTM!The welcome screen styles are well-structured, use modern CSS practices with flexbox, and include appropriate spacing and responsive considerations.
53-96: LGTM!The modal styles are well-designed with appropriate sizing constraints, scrolling for overflow, and good interactive feedback on the close button.
98-127: LGTM!The responsive design appropriately adjusts spacing, sizing, and layout for tablet and mobile viewports.
app/components/new-stepper.js (2)
29-35: LGTM!The computed getters correctly use optional chaining to safely access potentially undefined properties.
65-70: LGTM!The nextStep action correctly prevents default form submission and resets the validation state for the next step.
tests/integration/components/new-stepper-test.js (3)
9-20: LGTM!The test properly verifies that the welcome screen renders at step 0 with all expected elements.
22-27: LGTM!The test correctly verifies the Start button is disabled by default when terms have not been accepted.
29-37: LGTM!The test correctly validates that the Start button becomes enabled when terms are accepted by manipulating the service state.
app/components/new-join-steps/welcome-screen.js (3)
11-19: LGTM!The checkbox handler correctly implements the UX pattern where users must accept terms through the modal rather than directly checking the box. The DOM manipulation on line 16 is necessary to prevent the checkbox from checking before modal acceptance.
21-24: LGTM!The closeModal action correctly hides the modal.
26-30: LGTM!The acceptTerms action correctly closes the modal and updates the service to mark terms as accepted, which will reactively update the checkbox state.
![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 |
|---|---|---|
 |
Generic modal state name ▹ view | |
 |
Missing Input Validation ▹ view | |
|
Mixed DOM and State Management Responsibilities ▹ view | |
 |
Empty component with no modal functionality ▹ view | |
|
Direct DOM manipulation breaks data binding ▹ view | |
|
Non-descriptive event parameter name ▹ view | |
|
Ambiguous service name ▹ view | |
|
Missing input validation in setTermsAcceptance ▹ view | |
|
Unclear parameter name in setTermsAcceptance ▹ view | |
|
Missing Terms Acceptance Persistence ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| app/components/terms-modal.js | ✅ |
| app/services/application-terms.js | ✅ |
| app/components/new-join-steps/welcome-screen.js | ✅ |
| app/controllers/join.js | ✅ |
| app/components/new-stepper.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.
iamitprakash
left a comment
iamitprakash
left a comment
There was a problem hiding this comment.
please help me with this https://github.com/Real-Dev-Squad/website-www/pull/1081/files#r2484858922
can we use darker shade of this color for list items for better visibility
|
updated, thanks for correcting! |
Date: 30-10-25
Developer Name: @MayankBansal12
Issue Ticket Number:-
Description:
dev=trueparamoldOnboarding=trueinstead ofdev=truestep=0, more steps to be added in next PRsIs 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-07.20-21-01.mp4
tests
Description by Korbit AI
What change is being made?
Add a new stepper flow and welcome screen for the join form, including a terms modal and acceptance flow, plus support for an old onboarding path via a new query param and conditional rendering upgrades.
Why are these changes being made?
Introduce a streamlined, componentized onboarding flow with a welcome screen and a modal-based terms agreement, while preserving existing old onboarding behavior for backward compatibility. The changes enable a dev-mode/new-stepper experience while keeping the older onboarding path available when requested.