Skip to content
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

Conversation

BohdanOne
Copy link
Contributor

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

  • A comprehensive Linear ticket, providing sufficient context and details to facilitate the review of the PR, is linked to the PR.
  • If my changes require tests, I added them.
  • If my changes affect backward compatibility, it has been discussed.
  • If my changes require an update to the CONTRIBUTING.md guide, I updated it.

Preview

How to QA 1

Footnotes

  1. Please use these labels when submitting a review:
    ❓ #ask: Ask a question.
    💡 #idea: Suggest an idea.
    ⚠️ #issue: Strongly suggest a change.
    🎉 #nice: Share a compliment.

@BohdanOne BohdanOne requested a review from a team as a code owner December 18, 2024 14:51
Copy link

linear bot commented Dec 18, 2024

Copy link

vercel bot commented Dec 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
slice-machine ✅ Ready (Inspect) Visit Preview Dec 19, 2024 8:18am

Copy link
Collaborator

@xrutayisire xrutayisire left a 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;
Copy link
Collaborator

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:

Suggested change
if (!isVisible) return null;
if (!isVisible || !isSharedExperimentEligible) return null;

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true, changed

@xrutayisire
Copy link
Collaborator

Btw I guess you confirm with Product the old onboarding guide should never come back.

@BohdanOne
Copy link
Contributor Author

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.
I did some QA locally, will still do more on alpha.

@BohdanOne BohdanOne merged commit 847e00d into main Dec 19, 2024
33 checks passed
@BohdanOne BohdanOne deleted the BI/dt-2524-aapm-i-can-see-a-new-experiment-with-the-shared-onboarding branch December 19, 2024 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants