fix: is onboarding ready evaluation #4165
Conversation
sshanzel
requested review from
rebelchris,
omBratteng,
ilasw,
capJavert,
nensidosari and
AmarTrebinjac
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
I will run the critical flows both on preview and prod deployment 🙏 |
| }, [ | ||
| isOnboardingReady, | ||
| router, |
There was a problem hiding this comment.
Router has been known to an unstable dependency, hence removed.
There was a problem hiding this comment.
its not unstable since we upgraded to next 14 code, it no longer changes on each render, it only changes when page changes, which is supposed to happen
There was a problem hiding this comment.
Ah, finally. It is quite annoying to have them commented in the past.
| completeStep: (action: ActionType) => void; | ||
| } | ||
|
|
||
| export const useOnboardingActions = (): UseOnboardingActions => { |
There was a problem hiding this comment.
can we move it to separate hook file?
There was a problem hiding this comment.
I'm not really sure this is the right fix though...
Why can't we just export the way it was and inside onboarding add that 1 extra variable?
Isn't that the same result, but without having to copy a whole hook?
| @@ -78,15 +80,17 @@ function InternalApp({ Component, pageProps, router }: AppProps): ReactElement { | |||
|
|
|||
| useEffect(() => { | |||
| if ( | |||
| isOnboardingReady && | |||
| isOnboardingActionsReady && | |||
There was a problem hiding this comment.
I mean here, why don't we just do isOnboardingReady && !user (or whatever we changed in the hook)
There was a problem hiding this comment.
The problem with that then is the isAuthReady can still be false. I've updated everything now to make it simpler.
|
@rebelchris @capJavert I've updated it to make the affected changes smaller. They are basically the same previously but kept within the same hook. |
Changes
useOnboardinghook to its original form and introduce a new one that will handle the evaluation of actions performed related to onboarding.Events
Did you introduce any new tracking events?
Experiment
Did you introduce any new experiments?
Manual Testing
Caution
Please make sure existing components are not breaking/affected by this PR
Preview domain
https://fix-isonboarding.preview.app.daily.dev