-
Notifications
You must be signed in to change notification settings - Fork 51
Feat/signup-msg #1085
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/signup-msg #1085
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Updates onboarding welcome messaging and related styling cleanup.
- Changes greeting text from "Captain" to "Cap'n".
- Removes unused styled components and a button import in Welcome.tsx.
- Removes default fallback name "Sailor" for waitlist users with no first name.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/web/src/views/Onboarding/steps/welcome/WelcomeScreen.tsx | Adjusts greeting string to new copy. |
| packages/web/src/views/Onboarding/steps/welcome/Welcome.tsx | Cleans up imports and removes unused styled components. |
| packages/web/src/views/Onboarding/steps/waitlist/WaitlistCheck.tsx | Removes fallback name, now sets empty string when firstName is absent. |
| const data = await WaitlistApi.getWaitlistStatus(processedInput); | ||
| setWaitlistStatus(data); | ||
| setFirstName(data.firstName ?? "Sailor"); | ||
| setFirstName(data.firstName ?? ""); |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replacing the previous fallback with an empty string means downstream UI (e.g. the welcome message) will render as 'Welcome, Cap'n ' with a trailing space and no name when firstName is missing. Consider retaining a meaningful fallback (e.g. 'Sailor') or conditionally omitting the space/comma when the name is absent.
| setFirstName(data.firstName ?? ""); | |
| setFirstName(data.firstName ?? "Sailor"); |
| prevBtnDisabled={true} | ||
| > | ||
| <OnboardingText>Welcome, Captain {firstName}</OnboardingText> | ||
| <OnboardingText>Welcome, Cap'n {firstName}</OnboardingText> |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Using ' here is unnecessary in JSX text and slightly reduces readability; you can safely use a plain apostrophe: Welcome, Cap'n {firstName}. This avoids HTML entity decoding and keeps the string consistent with typical JSX text content.
| <OnboardingText>Welcome, Cap'n {firstName}</OnboardingText> | |
| <OnboardingText>Welcome, Cap'n {firstName}</OnboardingText> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| const data = await WaitlistApi.getWaitlistStatus(processedInput); | ||
| setWaitlistStatus(data); | ||
| setFirstName(data.firstName ?? "Sailor"); | ||
| setFirstName(data.firstName ?? ""); |
Copilot
AI
Oct 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting firstName to an empty string when data.firstName is null/undefined will result in 'Welcome, Cap'n ' (with trailing space) being displayed. Consider using a default name or handle the empty case in the welcome message.
| setFirstName(data.firstName ?? ""); | |
| setFirstName(data.firstName ?? "matey"); |
|
This work is invalided by the recent waitlist changes in #1071 |
No description provided.