-
Notifications
You must be signed in to change notification settings - Fork 53
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(onboarding): enable another iteration of onboarding guide experiment #1514
feat(onboarding): enable another iteration of onboarding guide experiment #1514
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
LGTM, just one small comment.
Love to see that many removals, huge win!
I guess this removal required some QA to confirm nothing is broken. Even if we have e2e tests.
import { useUpdateAvailable } from "@/hooks/useUpdateAvailable"; | ||
|
||
import { SharedOnboardingGuide } from "./SharedOnboardingGuide"; | ||
import { SliceMachineOnboardingGuide } from "./SliceMachineOnboardingGuide/SliceMachineOnboardingGuide"; | ||
import { useSharedOnboardingExperiment } from "./useSharedOnboardingExperiment"; | ||
|
||
export function OnboardingGuide() { | ||
const isVisible = useIsOnboardingGuideVisible(); | ||
const isSharedExperimentEligible = useSharedOnboardingExperiment().eligible; | ||
|
||
if (!isVisible) return null; |
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.
💡 You could put all null
cases all together:
if (!isVisible) return null; | |
if (!isVisible || !isSharedExperimentEligible) return null; |
WDYT?
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.
true, changed
Btw I guess you confirm with Product the old onboarding guide should never come back. |
Yes, asked Come and he confirmed we don't want to keep any of the older versions of onboarding. |
Resolves: DT-2524
Description
In another iteration of onboarding guide experiment we display either shared onboarding guide or nothing.
I've updated key of the experiment to match new setup in Amplitude and removed code related to old guides that are no longer needed.
Checklist
Preview
How to QA 1
Footnotes
Please use these labels when submitting a review:
⚠️ #issue: Strongly suggest a change.
❓ #ask: Ask a question.
💡 #idea: Suggest an idea.
🎉 #nice: Share a compliment. ↩