Feat/marketing page improvements#22
Conversation
❌ Deploy Preview for tradiaai failed.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7ad54814f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| hasAccount: false, | ||
| hasTrades: false, | ||
| hasPreferences: false, |
There was a problem hiding this comment.
For users who follow the primary CTAs, these flags never get updated from the actual account/trade state, so the onboarding flow gets stuck. I checked that OnboardingModal is only mounted on the dashboard page, while the Add Account and Import Trades flows redirect to /dashboard/trade-history after success; when the user later returns to /dashboard, hasAccount/hasTrades are reinitialized to false, so the modal still shows “Add Trading Account”/“Import Trades” instead of allowing them to continue. Please initialize or subscribe these flags from AccountContext/TradeContext (or persist completion when those flows succeed) rather than keeping them as local-only booleans.
Useful? React with 👍 / 👎.
| const ONBOARDING_KEY = "tradia_onboarding_complete"; | ||
| const ONBOARDING_STEP_KEY = "tradia_onboarding_step"; |
There was a problem hiding this comment.
Because these localStorage keys are global to the browser, completing or skipping onboarding as one signed-in user suppresses it for every later user on the same device/browser profile. I checked the dashboard uses NextAuth sessions, but this hook never includes the session/user id in the key or stores state server-side, so a fresh account can land on /dashboard and have onboarding hidden solely because a previous account set tradia_onboarding_complete=true.
Useful? React with 👍 / 👎.
| const stored = localStorage.getItem(ONBOARDING_KEY); | ||
| const step = localStorage.getItem(ONBOARDING_STEP_KEY) as OnboardingStep | null; |
There was a problem hiding this comment.
If tradia_onboarding_step contains anything outside the OnboardingStep union (for example from stale/corrupt localStorage or a browser extension/user edit), this cast accepts it and OnboardingModal then indexes STEPS_CONFIG[currentStep], making config undefined and crashing when rendering config.icon. Please guard the stored value against the allowed step list before putting it in state, and fall back to welcome for invalid values.
Useful? React with 👍 / 👎.
| hasAccount: false, | ||
| hasTrades: false, | ||
| hasPreferences: false, |
There was a problem hiding this comment.
For users who follow the primary CTAs, these flags never get updated from the actual account/trade/settings state, so the onboarding flow gets stuck. I checked that OnboardingModal is only mounted on the dashboard page, while the Add Account and Import Trades flows redirect to /dashboard/trade-history after success; when the user later returns to /dashboard, hasAccount/hasTrades/hasPreferences are reinitialized to false, so the modal still shows the same setup CTA instead of allowing them to continue. Please initialize or subscribe these flags from the relevant contexts (or persist completion when those flows succeed) rather than keeping them as local-only booleans.
Useful? React with 👍 / 👎.
1 participant
No description provided.