[TASK-9934] feat: Slim down setup flow #770
Conversation
Removed success screens for each setup step, went down to 4 steps each of which the user has to make a choice or interact in some form: - Welcome, here the user can Sign Up or Login - SignUp, here the user will choose their handle - Passkey, here the user will setup their passkey - PWA, here the user will install the PWA Now installing the PWA is not skipeable, also fixed prompting for platforms that support it (Android and Desktop), the problem was that the event that we have to handle is fired on first page load, so now we add the listener on the first render of the page.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis PR updates the setup flow across multiple components. The dependency in Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant SP as SetupPage
participant SW as SetupWrapper
participant IP as InstallPWA
U->>SP: Trigger "beforeinstallprompt" event
SP->>SP: Prevent default & update state (deferredPrompt, canInstall)
SP->>SW: Pass deferredPrompt and canInstall props
SW->>IP: Clone InstallPWA with new props
U->>IP: Click "Install" button
IP->>deferredPrompt: Invoke prompt() method
deferredPrompt-->>IP: Return user choice
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
jjramirezn
changed the title
|
Made installing the first step on the setup flow and also make desktop install skippeable |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/Setup/Views/InstallPWA.tsx (1)
78-82: Minimalistic prompt logic.The
handleInstallmethod properly invokesdeferredPrompt.prompt()if available. Consider capturinguserChoiceif you need to track whether the user accepted or dismissed the install prompt.+ deferredPrompt.prompt() + const { outcome } = await deferredPrompt.userChoice + console.log("PWA installation outcome:", outcome)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/(setup)/setup/page.tsx(2 hunks)src/components/Setup/Setup.consts.tsx(1 hunks)src/components/Setup/Views/InstallPWA.tsx(4 hunks)src/components/Setup/Views/SetupPasskey.tsx(3 hunks)src/components/Setup/components/SetupWrapper.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Setup/components/SetupWrapper.tsx
🧰 Additional context used
🧬 Code Definitions (4)
src/components/Setup/Views/SetupPasskey.tsx (1)
src/utils/general.utils.ts (1)
getFromLocalStorage(106-121)
src/app/(setup)/setup/page.tsx (2)
src/hooks/useSetupFlow.ts (1)
useSetupFlow(6-46)src/components/Setup/Setup.types.ts (1)
BeforeInstallPromptEvent(45-52)
src/components/Setup/Views/InstallPWA.tsx (2)
src/components/Setup/Setup.types.ts (1)
BeforeInstallPromptEvent(45-52)src/hooks/useSetupFlow.ts (1)
useSetupFlow(6-46)
src/components/Setup/Setup.consts.tsx (1)
src/components/Setup/Setup.types.ts (1)
ISetupStep(33-43)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (15)
src/components/Setup/Views/SetupPasskey.tsx (3)
9-10: Imports look good.These imports cleanly bring in
getFromLocalStorageanduseRouter, aligning with the new redirect logic. No issues found here.
21-23: Local storage-based redirect is appropriate.Reading the redirect URL from local storage and using a fallback of
/homeis a straightforward approach. Ensure the saved redirect path is always valid and does not inadvertently introduce an open redirect vulnerability.Please confirm that only trusted values are stored in localStorage for
redirect. If needed, sanitize or validate the domain/URI to avoid security gaps.
33-33: Router-based navigation replaces step-based flow.Previously,
handleNext()might have stacked additional steps in the process. With this direct call torouter.push(redirect), subsequent steps in the flow may be bypassed. Confirm that skipping other setup steps is intended.Would you like to keep the old step flow logic or confirm that immediately redirecting the user is correct?
src/components/Setup/Views/InstallPWA.tsx (5)
6-8: New imports for callback and event type.Importing
useCallbackoptimizes your event handling. TheBeforeInstallPromptEventtype helps keep the prompt usage strictly typed.
52-55: Clear prop definition for PWA install logic.The component’s interface now explicitly documents each prop. This clarifies usage of deferred prompts and device types. Great job.
73-73: Potential user flow concern with forced reload.
window.location.href = window.location.origin + '/setup'forcibly reloads the app. This ensures the newly installed PWA starts fresh, but reloading can momentarily disrupt the user flow. Validate that reloading is desired over a smoother client-side transition.Ensure that reloading does not cause data loss or an abrupt user experience issue. If needed, consider a client-side route transition using the router.
157-159: Conditional logic for install button.Good approach: if
canInstallistrue, callhandleInstall. Otherwise, check if installation is already complete or show the modal. This logic is simple, ensuring the correct user flow.
189-199: “Got it!” button elegantly handles non-installable devices.By rendering this fallback button only when
canInstallis false, you allow users to dismiss the modal. No issues found here.src/components/Setup/Setup.consts.tsx (3)
3-3: Import of mobile asset.Importing
PEANUTMAN_MOBILEensures the correct image is used for the PWA install step. Looks good.
5-5: Consolidated imports.Grouping
InstallPWA,SetupPasskey,SignupStep, andWelcomeStepin a single import improves clarity. No concerns.
8-18: New “pwa-install” step integrated.Introducing a distinct step for installing the PWA is a clean solution. Hiding back and skip buttons ensures users can’t bypass mandatory installation. This aligns with the streamlined flow described in the PR.
src/app/(setup)/setup/page.tsx (4)
7-7: Importing typed event fosters clarity.Including
BeforeInstallPromptEventfor the prompt is consistent with other references. Good practice.
14-16: Essential states for installation and environment detection.Storing
deferredPrompt,canInstall, anddeviceTypein state neatly organizes the installation logic. This ensures a smoother user experience.
18-48: Comprehensive effect for capturing install prompt and device type.
- Cancels the default prompt, capturing it for later use.
- Differentiates between desktop, iOS, and Android for specialized installation flows.
- Maintains a clean cleanup for event listeners.
Ensure that the user agent checks handle edge cases (e.g., Chrome on iPad). Testing across devices is recommended.
Would you like a script to systematically parse the user agent for iPadOS, which often masks itself as a Mac or desktop in certain contexts?
76-78: Prop propagation to SetupWrapper.Passing
deferredPrompt,canInstall, anddeviceTypeensures thorough coverage of the new functionality in child components. Good approach for modular design.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/components/Setup/Views/InstallPWA.tsx (2)
128-128: Address TODO comment about setup instructionsThe TODO comment indicates incomplete functionality related to setup instructions after login.
This should be addressed as part of the setup flow streamlining. Would you like me to help implement these post-login setup instructions?
146-146: Remove console.log statementThere's a console.log statement that should be removed before production deployment.
const getInstructions = () => { - console.log('Showing instructions for:', deviceType) if (deviceType === 'desktop') {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/manifest.ts(1 hunks)src/components/Setup/Views/InstallPWA.tsx(4 hunks)src/components/Setup/components/SetupWrapper.tsx(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/app/manifest.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Setup/components/SetupWrapper.tsx
🧰 Additional context used
🧬 Code Definitions (1)
src/components/Setup/Views/InstallPWA.tsx (2)
src/components/Setup/Setup.types.ts (1)
BeforeInstallPromptEvent(45-52)src/hooks/useSetupFlow.ts (1)
useSetupFlow(6-46)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (5)
src/components/Setup/Views/InstallPWA.tsx (5)
6-9: Import refinements align with component changesThe additional imports properly support the new functionality:
useCallbackfor performance optimization,BeforeInstallPromptEventtype for better type safety, and Redux utilities for state management.
53-61: Props-based architecture improves component reusabilityThe refactoring to accept external props (
canInstall,deferredPrompt, anddeviceType) instead of managing this state internally is a good architectural change. This makes the component more reusable and testable, while aligning with the PR objective to streamline the setup flow.
159-175: Button state management improvementThe button now properly handles loading states using the
isLoadingproperty fromuseSetupFlow, ensuring a better user experience by disabling the button during installation.
163-165: Installation flow prioritization enhances UXThe updated conditional logic prioritizes installation when
canInstallis true, aligning with the PR objective to make PWA installation mandatory in the setup flow.
194-205: Conditional modal button improves user flowThe "Got it!" button now only appears when the user cannot install the app, providing clearer guidance based on their device capabilities.
| const handleInstall = useCallback(async () => { | ||
| if (!deferredPrompt) return | ||
|
|
||
| dispatch(setupActions.setLoading(true)) | ||
| // Show the install prompt | ||
| deferredPrompt.prompt() | ||
|
|
||
| // Wait for the user to respond to the prompt | ||
| const { outcome } = await deferredPrompt.userChoice | ||
|
|
||
| if (outcome === 'accepted') { | ||
| deferredPrompt = null | ||
| } | ||
| } | ||
| await deferredPrompt.prompt() | ||
| dispatch(setupActions.setLoading(false)) | ||
| }, [deferredPrompt]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Installation handler lacks error handling
The handleInstall function now correctly manages loading state through Redux but doesn't include error handling if the prompt fails or is dismissed by the user.
Consider adding proper error handling:
const handleInstall = useCallback(async () => {
if (!deferredPrompt) return
dispatch(setupActions.setLoading(true))
// Show the install prompt
try {
await deferredPrompt.prompt()
+ // Optionally track user action
+ const { outcome } = await deferredPrompt.userChoice
+ if (outcome === 'dismissed') {
+ console.log('User dismissed the install prompt')
+ }
} catch (error) {
+ console.error('Installation prompt error:', error)
} finally {
dispatch(setupActions.setLoading(false))
}
}, [deferredPrompt])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleInstall = useCallback(async () => { | |
| if (!deferredPrompt) return | |
| dispatch(setupActions.setLoading(true)) | |
| // Show the install prompt | |
| deferredPrompt.prompt() | |
| // Wait for the user to respond to the prompt | |
| const { outcome } = await deferredPrompt.userChoice | |
| if (outcome === 'accepted') { | |
| deferredPrompt = null | |
| } | |
| } | |
| await deferredPrompt.prompt() | |
| dispatch(setupActions.setLoading(false)) | |
| }, [deferredPrompt]) | |
| const handleInstall = useCallback(async () => { | |
| if (!deferredPrompt) return | |
| dispatch(setupActions.setLoading(true)) | |
| // Show the install prompt | |
| try { | |
| await deferredPrompt.prompt() | |
| // Optionally track user action | |
| const { outcome } = await deferredPrompt.userChoice | |
| if (outcome === 'dismissed') { | |
| console.log('User dismissed the install prompt') | |
| } | |
| } catch (error) { | |
| console.error('Installation prompt error:', error) | |
| } finally { | |
| dispatch(setupActions.setLoading(false)) | |
| } | |
| }, [deferredPrompt]) |
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| setShowModal(false) | ||
| // Try to open the PWA | ||
| window.location.href = window.location.origin + redirect | ||
| window.location.href = window.location.origin + '/setup' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the redirection path is correct
The redirection to /setup after installation might create a navigation loop if users are already on the setup page.
🏁 Script executed:
#!/bin/bash
# Check for other occurrences of similar redirections and the current routes
rg -A 3 -B 3 "window.location.(href|origin)" --type ts --type tsx
rg -A 3 -B 3 "'/setup'" --type ts --type tsxLength of output: 176
Action: Validate Setup Redirection to Avoid a Possible Navigation Loop
The code in src/components/Setup/Views/InstallPWA.tsx (line 75) unconditionally redirects to /setup by setting
window.location.href = window.location.origin + '/setup'This may lead to a navigation loop if the current page is already /setup.
Recommendations:
- Confirm the redirection behavior:
Ensure that no other part of the codebase triggers an automatic redirection back to/setupwhen the user is already on that page. - Enhance the redirection logic:
Consider adding a guard to check the current URL. For example:if (window.location.pathname !== '/setup') { window.location.href = window.location.origin + '/setup'; }
- Verification Tip:
When searching the codebase, note that the initial RG commands produced errors for thetsxfile type. You might use an updated command such as:This should help confirm whether any duplicate or conflicting redirections exist.# Search for redirection usage in .tsx files using a glob pattern rg -g "*.tsx" -A 3 -B 3 "window.location.href\\s*=" . rg -g "*.tsx" -A 3 -B 3 "/setup" .
Please verify these points to ensure the redirection won’t trap users in a loop.
Below is a revised shell script to further verify the redirection logic in the file (using a glob pattern for TSX files) and check for any guard condition against a potential navigation loop. Please run this script to gather the required details:
Once you have the output from this script, we can confirm if a safeguard (such as verifying that the current pathname isn’t already /setup) exists and ensure the redirection won’t create a loop.
🏁 Script executed:
#!/bin/bash
# Verify the redirection usage in InstallPWA.tsx and check for a guard condition to avoid navigation loops
echo "Inspecting redirection usage in src/components/Setup/Views/InstallPWA.tsx..."
rg -g "*.tsx" -A 5 -B 5 "window.location.href" src/components/Setup/Views/InstallPWA.tsx
echo "Checking for potential guard conditions (e.g., checking window.location.pathname) in the file..."
rg -g "*.tsx" -A 5 -B 5 "if\s*\(window\.location\.pathname" src/components/Setup/Views/InstallPWA.tsxLength of output: 1053
Action: Prevent Potential Navigation Loop in PWA Setup Redirection
After verifying the redirection logic in src/components/Setup/Views/InstallPWA.tsx, we found that the current code unconditionally redirects to /setup:
window.location.href = window.location.origin + '/setup'There is no guard condition in place to check if the current page is already /setup. This can potentially cause a navigation loop if users are on the setup page.
Recommendations:
-
Add a Guard Condition: Update the redirection logic to first verify that the current pathname is not already
/setup, for example:if (window.location.pathname !== '/setup') { window.location.href = window.location.origin + '/setup'; }
-
Review and Test: Ensure that this change does not interfere with any intended navigation flow and confirm that the redirection behaves correctly across different scenarios.
| navigator.userAgent | ||
| ) | ||
|
|
||
| // For desktop, default to iOS if on Mac, otherwise Android |
There was a problem hiding this comment.
wat? this comment is wrong no?
| const handleInstall = useCallback(async () => { | ||
| if (!deferredPrompt) return | ||
|
|
||
| dispatch(setupActions.setLoading(true)) | ||
| // Show the install prompt | ||
| deferredPrompt.prompt() | ||
|
|
||
| // Wait for the user to respond to the prompt | ||
| const { outcome } = await deferredPrompt.userChoice | ||
|
|
||
| if (outcome === 'accepted') { | ||
| deferredPrompt = null | ||
| } | ||
| } | ||
| await deferredPrompt.prompt() | ||
| dispatch(setupActions.setLoading(false)) | ||
| }, [deferredPrompt]) |
Removed success screens for each setup step, went down to 4 steps each of which the user has to make a choice or interact in some form:
Now installing the PWA is not skipeable, also fixed prompting for platforms that support it (Android and Desktop), the problem was that the event that we have to handle is fired on first page load, so now we add the listener on the first render of the page.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor