Contextual Onboarding dialog improvements (#3105)

Co-authored-by: Alessandro Boron <[email protected]>
duckduckgo · Jul 24, 2024 · 274cdf1 · 274cdf1
1 parent 1a08ef8
commit 274cdf1
Show file tree

Hide file tree

Showing 9 changed files with 75 additions and 27 deletions.
diff --git a/DuckDuckGo/DaxDialogs.swift b/DuckDuckGo/DaxDialogs.swift
@@ -38,6 +38,7 @@ protocol NewTabDialogSpecProvider {
 
 protocol ContextualOnboardingLogic {
     func setFireEducationMessageSeen()
+    func setFinalOnboardingDialogSeen()
 }
 
 extension ContentBlockerRulesManager: EntityProviding {
@@ -315,6 +316,11 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic {
         settings.fireButtonEducationShownOrExpired = true
     }
 
+    func setFinalOnboardingDialogSeen() {
+        guard isNewOnboarding else { return }
+        settings.browsingFinalDialogShown = true
+    }
+
     func nextBrowsingMessageIfShouldShow(for privacyInfo: PrivacyInfo) -> BrowsingSpec? {
         guard privacyInfo.url != lastURLDaxDialogReturnedFor else { return nil }
 
@@ -445,12 +451,10 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic {
             return .subsequent
         }
 
-        if firstBrowsingMessageSeen && !finalDaxDialogSeen {
-            // Ensure we don't show the final dialog again in context when the user sees it and vice-versa.
-            settings.browsingFinalDialogShown = true
+        if settings.fireButtonEducationShownOrExpired && !finalDaxDialogSeen {
             return .final
         }
-
+        
         return nil
     }
 

diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift
@@ -753,7 +753,7 @@ class MainViewController: UIViewController {
             addToContentContainer(controller: controller)
             viewCoordinator.logoContainer.isHidden = true
         } else {
-            let newTabDaxDialogFactory = NewTabDaxDialogFactory(delegate: self)
+            let newTabDaxDialogFactory = NewTabDaxDialogFactory(delegate: self, contextualOnboardingLogic: DaxDialogs.shared)
             let homePageDependencies = HomePageDependencies(homePageConfiguration: homePageConfiguration,
                                                             model: tabModel,
                                                             favoritesViewModel: favoritesViewModel,

diff --git a/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/NewTabDaxDialogFactory.swift b/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/NewTabDaxDialogFactory.swift
@@ -27,9 +27,11 @@ protocol NewTabDaxDialogProvider {
 
 class NewTabDaxDialogFactory: NewTabDaxDialogProvider {
     var delegate: OnboardingNavigationDelegate?
+    var contextualOnboardingLogic: ContextualOnboardingLogic
 
-    init(delegate: OnboardingNavigationDelegate?) {
+    init(delegate: OnboardingNavigationDelegate?, contextualOnboardingLogic: ContextualOnboardingLogic) {
         self.delegate = delegate
+        self.contextualOnboardingLogic = contextualOnboardingLogic
     }
 
     @ViewBuilder
@@ -91,6 +93,9 @@ class NewTabDaxDialogFactory: NewTabDaxDialogProvider {
                 }).padding()
             }
         }
+        .onAppear { [weak self] in
+            self?.contextualOnboardingLogic.setFinalOnboardingDialogSeen()
+        }
     }
 }
 

diff --git a/DuckDuckGo/OnboardingHelpers/OnboardingSuggestedSitesProvider.swift b/DuckDuckGo/OnboardingHelpers/OnboardingSuggestedSitesProvider.swift
@@ -107,7 +107,7 @@ struct OnboardingSuggestedSitesProvider: OnboardingSuggestionsItemsProviding {
         case .germany: site = "https://www.duden.de/rechtschreibung/Ente"
         case .netherlands: site = "https://www.woorden.org/woord/eend"
         case .sweden: site = "https://www.synonymer.se/sv-syn/anka"
-        default: site = "britannica.com/animal/duck"
+        default: site = "https:britannica.com/animal/duck"
         }
         return ContextualOnboardingListItem.surprise(title: site)
     }

diff --git a/DuckDuckGo/TabSwitcherViewController.swift b/DuckDuckGo/TabSwitcherViewController.swift
@@ -317,11 +317,16 @@ class TabSwitcherViewController: UIViewController {
 
     @IBAction func onFirePressed(sender: AnyObject) {
         Pixel.fire(pixel: .forgetAllPressedTabSwitching)
-
-        if DaxDialogs.shared.shouldShowFireButtonPulse {
+        let isNewOnboarding = DefaultVariantManager().isSupported(feature: .newOnboardingIntro)
+
+        if !isNewOnboarding
+            && DaxDialogs.shared.shouldShowFireButtonPulse {
             let spec = DaxDialogs.shared.fireButtonEducationMessage()
             performSegue(withIdentifier: "ActionSheetDaxDialog", sender: spec)
         } else {
+            if isNewOnboarding {
+                ViewHighlighter.hideAll()
+            }
             let alert = ForgetDataAlert.buildAlert(forgetTabsAndDataHandler: { [weak self] in
                 self?.forgetAll()
             })

diff --git a/DuckDuckGoTests/ContextualOnboardingNewTabDialogFactoryTests.swift b/DuckDuckGoTests/ContextualOnboardingNewTabDialogFactoryTests.swift
@@ -25,19 +25,27 @@ class ContextualOnboardingNewTabDialogFactoryTests: XCTestCase {
 
     var factory: NewTabDaxDialogFactory!
     var mockDelegate: CapturingOnboardingNavigationDelegate!
+    var contextualOnboardingLogicMock: ContextualOnboardingLogicMock!
     var onDismissCalled: Bool!
+    var window: UIWindow!
 
     override func setUp() {
         super.setUp()
         mockDelegate = CapturingOnboardingNavigationDelegate()
+        contextualOnboardingLogicMock = ContextualOnboardingLogicMock()
         onDismissCalled = false
-        factory = NewTabDaxDialogFactory(delegate: mockDelegate)
+        factory = NewTabDaxDialogFactory(delegate: mockDelegate, contextualOnboardingLogic: contextualOnboardingLogicMock)
+        window = UIWindow(frame: UIScreen.main.bounds)
+        window.makeKeyAndVisible()
     }
 
     override func tearDown() {
+        window.isHidden = true
+        window = nil
         factory = nil
         mockDelegate = nil
         onDismissCalled = nil
+        contextualOnboardingLogicMock = nil
         super.tearDown()
     }
 
@@ -73,23 +81,28 @@ class ContextualOnboardingNewTabDialogFactoryTests: XCTestCase {
 
     func testCreateFinalDialogCreatesAnOnboardingFinalDialog() {
         // Given
+        let expectation = XCTestExpectation(description: "action triggered")
+        contextualOnboardingLogicMock.expectation = expectation
         var onDismissedRun = false
         let homeDialog = DaxDialogs.HomeScreenSpec.final
         let onDimsiss = { onDismissedRun = true }
 
         // When
         let view = factory.createDaxDialog(for: homeDialog, onDismiss: onDimsiss)
         let host = UIHostingController(rootView: view)
+        window.rootViewController = host
         XCTAssertNotNil(host.view)
 
         // Then
         let finalDialog = find(OnboardingFinalDialog.self, in: host)
         XCTAssertNotNil(finalDialog)
         finalDialog?.highFiveAction()
         XCTAssertTrue(onDismissedRun)
+        wait(for: [expectation], timeout: 5.0)
+        XCTAssertTrue(contextualOnboardingLogicMock.didCallsetFinalOnboardingDialogSeen)
     }
 
-    func testCreateAddFavoriteDialogCreatesAnContextualDaxDialog() {
+    func testCreateAddFavoriteDialogCreatesAContextualDaxDialog() {
         // Given
         let homeDialog = DaxDialogs.HomeScreenSpec.addFavorite
 

diff --git a/DuckDuckGoTests/DaxDialogsNewTabTests.swift b/DuckDuckGoTests/DaxDialogsNewTabTests.swift
@@ -28,7 +28,8 @@ final class DaxDialogsNewTabTests: XCTestCase {
 
     override func setUp() {
         settings = MockDaxDialogsSettings()
-        daxDialogs = DaxDialogs(settings: settings, entityProviding: MockEntityProvider())
+        let mockVariantManager = MockVariantManager(isSupportedReturns: true)
+        daxDialogs = DaxDialogs(settings: settings, entityProviding: MockEntityProvider(), variantManager: mockVariantManager)
     }
 
     override func tearDown() {
@@ -70,44 +71,57 @@ final class DaxDialogsNewTabTests: XCTestCase {
         // GIVEN
         settings.browsingAfterSearchShown = true
         settings.browsingMajorTrackingSiteShown = true
-        XCTAssertFalse(settings.browsingFinalDialogShown)
+        settings.fireButtonEducationShownOrExpired = true
 
         // WHEN
         let homeScreenMessage = daxDialogs.nextHomeScreenMessageNew()
 
         // THEN
         XCTAssertEqual(homeScreenMessage, .final)
-        XCTAssertTrue(settings.browsingFinalDialogShown)
     }
 
-    func testIfBrowsingAfterSearchShown_andBrowsingWithTrackersShown_OnNextHomeScreenMessageNew_ReturnsFinal() {
+    func testIfBrowsingAfterSearchShown_andBrowsingWithTrackersShown_andFireAnimationShown_OnNextHomeScreenMessageNew_ReturnsFinal() {
         // GIVEN
         settings.browsingAfterSearchShown = true
         settings.browsingWithTrackersShown = true
-        XCTAssertFalse(settings.browsingFinalDialogShown)
+        settings.fireButtonEducationShownOrExpired = true
 
         // WHEN
         let homeScreenMessage = daxDialogs.nextHomeScreenMessageNew()
 
         // THEN
         XCTAssertEqual(homeScreenMessage, .final)
-        XCTAssertTrue(settings.browsingFinalDialogShown)
     }
 
-    func testIfBrowsingAfterSearchShown_andBrowsingWithoutTrackersShown_OnNextHomeScreenMessageNew_ReturnsFinal() {
+    func testIfBrowsingAfterSearchShown_andBrowsingWithoutTrackersShown_andFireAnimationShown_OnNextHomeScreenMessageNew_ReturnsFinal() {
         // GIVEN
         settings.browsingAfterSearchShown = true
         settings.browsingWithoutTrackersShown = true
-        XCTAssertFalse(settings.browsingFinalDialogShown)
+        settings.fireButtonEducationShownOrExpired = true
 
         // WHEN
         let homeScreenMessage = daxDialogs.nextHomeScreenMessageNew()
 
         // THEN
         XCTAssertEqual(homeScreenMessage, .final)
-        XCTAssertTrue(settings.browsingFinalDialogShown)
     }
 
+    func testIfBrowsingAfterSearchShown_andTrackersDialogsShown_andFirreButtonFialogNotShown_OnNextHomeScreenMessageNew_ReturnsNil() {
+        // GIVEN
+        settings.browsingAfterSearchShown = true
+        settings.browsingWithoutTrackersShown = true
+        settings.browsingMajorTrackingSiteShown = true
+        settings.browsingWithTrackersShown = true
+
+        // WHEN
+        let homeScreenMessage = daxDialogs.nextHomeScreenMessageNew()
+
+        // THEN
+        XCTAssertNil(homeScreenMessage)
+        XCTAssertFalse(settings.browsingFinalDialogShown)
+    }
+
+
     func testIfBrowsingAfterSearchShown_andBrowsingMajorTrackingSiteShown_andFinalDialogAlreadyShown_OnNextHomeScreenMessageNew_ReturnsNil() {
         // GIVEN
         settings.browsingAfterSearchShown = true

diff --git a/DuckDuckGoTests/OnboardingSuggestedSitesProviderTests.swift b/DuckDuckGoTests/OnboardingSuggestedSitesProviderTests.swift
@@ -35,7 +35,7 @@ class OnboardingSuggestedSitesProviderTests: XCTestCase {
         XCTAssertEqual(sitesList[0], ContextualOnboardingListItem.site(title: scheme + "bolasport.com"))
         XCTAssertEqual(sitesList[1], ContextualOnboardingListItem.site(title: scheme + "kompas.com"))
         XCTAssertEqual(sitesList[2], ContextualOnboardingListItem.site(title: scheme + "tokopedia.com"))
-        XCTAssertEqual(sitesList[3], ContextualOnboardingListItem.surprise(title: "britannica.com/animal/duck"))
+        XCTAssertEqual(sitesList[3], ContextualOnboardingListItem.surprise(title: scheme + "britannica.com/animal/duck"))
     }
 
     func testSuggestedSitesForGB() {
@@ -50,7 +50,7 @@ class OnboardingSuggestedSitesProviderTests: XCTestCase {
         XCTAssertEqual(sitesList[0], ContextualOnboardingListItem.site(title: scheme + "skysports.com"))
         XCTAssertEqual(sitesList[1], ContextualOnboardingListItem.site(title: scheme + "bbc.co.uk"))
         XCTAssertEqual(sitesList[2], ContextualOnboardingListItem.site(title: scheme + "eBay.com"))
-        XCTAssertEqual(sitesList[3], ContextualOnboardingListItem.surprise(title: "britannica.com/animal/duck"))
+        XCTAssertEqual(sitesList[3], ContextualOnboardingListItem.surprise(title: scheme + "britannica.com/animal/duck"))
     }
 
     func testSuggestedSitesForGermany() {
@@ -80,7 +80,7 @@ class OnboardingSuggestedSitesProviderTests: XCTestCase {
         XCTAssertEqual(sitesList[0], ContextualOnboardingListItem.site(title: scheme + "tsn.ca"))
         XCTAssertEqual(sitesList[1], ContextualOnboardingListItem.site(title: scheme + "cbc.ca"))
         XCTAssertEqual(sitesList[2], ContextualOnboardingListItem.site(title: scheme + "canadiantire.ca"))
-        XCTAssertEqual(sitesList[3], ContextualOnboardingListItem.surprise(title: "britannica.com/animal/duck"))
+        XCTAssertEqual(sitesList[3], ContextualOnboardingListItem.surprise(title: scheme + "britannica.com/animal/duck"))
     }
 
     func testSuggestedSitesForNetherlands() {
@@ -110,7 +110,7 @@ class OnboardingSuggestedSitesProviderTests: XCTestCase {
         XCTAssertEqual(sitesList[0], ContextualOnboardingListItem.site(title: scheme + "afl.com.au"))
         XCTAssertEqual(sitesList[1], ContextualOnboardingListItem.site(title: scheme + "abc.net.au"))
         XCTAssertEqual(sitesList[2], ContextualOnboardingListItem.site(title: scheme + "eBay.com"))
-        XCTAssertEqual(sitesList[3], ContextualOnboardingListItem.surprise(title: "britannica.com/animal/duck"))
+        XCTAssertEqual(sitesList[3], ContextualOnboardingListItem.surprise(title: scheme + "britannica.com/animal/duck"))
     }
 
     func testSuggestedSitesForSweden() {
@@ -140,7 +140,7 @@ class OnboardingSuggestedSitesProviderTests: XCTestCase {
         XCTAssertEqual(sitesList[0], ContextualOnboardingListItem.site(title: scheme + "skysports.com"))
         XCTAssertEqual(sitesList[1], ContextualOnboardingListItem.site(title: scheme + "bbc.co.uk"))
         XCTAssertEqual(sitesList[2], ContextualOnboardingListItem.site(title: scheme + "eBay.com"))
-        XCTAssertEqual(sitesList[3], ContextualOnboardingListItem.surprise(title: "britannica.com/animal/duck"))
+        XCTAssertEqual(sitesList[3], ContextualOnboardingListItem.surprise(title: scheme + "britannica.com/animal/duck"))
     }
 
     func testSuggestedSitesForUS() {
@@ -155,7 +155,7 @@ class OnboardingSuggestedSitesProviderTests: XCTestCase {
         XCTAssertEqual(sitesList[0], ContextualOnboardingListItem.site(title: scheme + "ESPN.com"))
         XCTAssertEqual(sitesList[1], ContextualOnboardingListItem.site(title: scheme + "yahoo.com"))
         XCTAssertEqual(sitesList[2], ContextualOnboardingListItem.site(title: scheme + "eBay.com"))
-        XCTAssertEqual(sitesList[3], ContextualOnboardingListItem.surprise(title: "britannica.com/animal/duck"))
+        XCTAssertEqual(sitesList[3], ContextualOnboardingListItem.surprise(title: scheme + "britannica.com/animal/duck"))
     }
 
     func testSuggestedSitesForUnknownCountry() {
@@ -170,6 +170,6 @@ class OnboardingSuggestedSitesProviderTests: XCTestCase {
         XCTAssertEqual(sitesList[0], ContextualOnboardingListItem.site(title: scheme + "ESPN.com"))
         XCTAssertEqual(sitesList[1], ContextualOnboardingListItem.site(title: scheme + "yahoo.com"))
         XCTAssertEqual(sitesList[2], ContextualOnboardingListItem.site(title: scheme + "eBay.com"))
-        XCTAssertEqual(sitesList[3], ContextualOnboardingListItem.surprise(title: "britannica.com/animal/duck"))
+        XCTAssertEqual(sitesList[3], ContextualOnboardingListItem.surprise(title: scheme + "britannica.com/animal/duck"))
     }
 }
diff --git a/DuckDuckGoTests/TabViewControllerDaxDialogTests.swift b/DuckDuckGoTests/TabViewControllerDaxDialogTests.swift
@@ -119,10 +119,17 @@ final class TabViewControllerDaxDialogTests: XCTestCase {
 }
 
 final class ContextualOnboardingLogicMock: ContextualOnboardingLogic {
+    var expectation: XCTestExpectation?
     private(set) var didCallSetFireEducationMessageSeen = false
+    private(set) var didCallsetFinalOnboardingDialogSeen = false
 
     func setFireEducationMessageSeen() {
         didCallSetFireEducationMessageSeen = true
     }
 
+    func setFinalOnboardingDialogSeen() {
+        didCallsetFinalOnboardingDialogSeen = true
+        expectation?.fulfill()
+    }
+
 }