duckduckgo / iOS

DuckDuckGo iOS Application
https://itunes.apple.com/us/app/duckduckgo-privacy-browser/id663592361?mt=8
Apache License 2.0
1.82k stars 414 forks source link

Onboarding Intro experiment setup #3028

Closed alessandroboron closed 3 months ago

alessandroboron commented 3 months ago

Task/Issue URL: https://app.asana.com/0/0/1207723831937951/f CC: @SabrinaTardio

Description:

This PR is to setup the experiment variants and to show a different onboarding intro based on the variant.

Steps to test this PR:

Scenario 1 - Old Onboarding

Screenshot 2024-07-03 at 7 30 03 PM
  1. Setup the variant by using Edit Scheme -> Arguments -> Environment Variables as per attached image.
  2. Launch the App Expected Result: The old onboarding should be presented to the user.

Scenario 2 - New Onboarding

Screenshot 2024-07-03 at 7 29 50 PM
  1. Setup the variant by using Edit Scheme -> Arguments -> Environment Variables as per attached image.
  2. Launch the App Expected Result: The new onboarding should be presented to the user.

<!— Before submitting a PR, please ensure you have tested the combinations you expect the reviewer to test, then delete configurations you know do not need explicit testing.

Using a simulator where a physical device is unavailable is acceptable. —>

Definition of Done (Internal Only):

Copy Testing:

Orientation Testing:

Device Testing:

OS Testing:

Theme Testing:

Internal references:

Software Engineering Expectations Technical Design Template

github-actions[bot] commented 3 months ago

:no_entry_sign: The Asana task linked in the PR description is not added to iOS App Board project.

  1. Verify that the correct task is linked in the PR.
    • :warning: Please use the actual implementation task, rather than the Code Review subtask.
  2. Verify that the task is added to iOS App Board project.
  3. When ready, remove the bot: not in app board label to retrigger the check.
alessandroboron commented 3 months ago

Wasn't too keen on Onboarding taking on the UIViewController protocol, but I see why you did it.

An alternative would be:

    var controller: (Onboarding & UIViewController)?

    if DefaultVariantManager().isSupported(feature: .newOnboardingIntro) {
        controller = OnboardingIntroViewController()
    } else {
        let storyboard = UIStoryboard(name: "DaxOnboarding", bundle: nil)
        controller = storyboard.instantiateInitialViewController(creator: { coder in
            DaxOnboardingViewController(coder: coder)
        })
    }

    controller?.delegate = self // Note: moved this above the let guard
    guard let controller else { return assertionFailure() }
    controller.modalPresentationStyle = .overFullScreen
    present(controller, animated: false)

However, that's just being picky, so LGTM

I see where you're coming from! I made the change so I avoid enforcing the Onboarding protocol being implementable only by UIViewControllers. 👍