Skip to content

Commit

Permalink
Fix dax dialog presentation (#1035)
Browse files Browse the repository at this point in the history 
Add a check to make sure the dax dialog is only displayed if the current URL is the same as the one who originated it

Task/Issue URL: https://app.asana.com/0/414709148257752/1201620790053163/f
PR Link: #1035
  • Loading branch information
@Bunn
Bunn authored Feb 7, 2022
1 parent 8c90f83 commit 37f3330
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 24 deletions.
56 changes: 40 additions & 16 deletions DuckDuckGo/DaxDialogs.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import Foundation
import Core
import TrackerRadarKit

// swiftlint:disable type_body_length

class DaxDialogs {

struct MajorTrackers {
Expand All @@ -33,61 +35,85 @@ class DaxDialogs {
}

struct HomeScreenSpec: Equatable {

static let initial = HomeScreenSpec(message: UserText.daxDialogHomeInitial, accessibilityLabel: nil)
static let subsequent = HomeScreenSpec(message: UserText.daxDialogHomeSubsequent, accessibilityLabel: nil)
static let addFavorite = HomeScreenSpec(message: UserText.daxDialogHomeAddFavorite,
accessibilityLabel: UserText.daxDialogHomeAddFavoriteAccessible)

let message: String
let accessibilityLabel: String?

}

func overrideShownFlagFor(_ spec: BrowsingSpec, flag: Bool) {
switch spec.type {
case .withMultipleTrackers, .withOneTracker :
settings.browsingWithTrackersShown = flag
case .afterSearch:
settings.browsingAfterSearchShown = flag
case .withoutTrackers:
settings.browsingWithoutTrackersShown = flag
case .siteIsMajorTracker, .siteOwnedByMajorTracker:
settings.browsingMajorTrackingSiteShown = flag
settings.browsingWithoutTrackersShown = flag
}
}

struct BrowsingSpec: Equatable {

// swiftlint:disable nesting

enum SpecType {
case afterSearch
case withoutTrackers
case siteIsMajorTracker
case siteOwnedByMajorTracker
case withOneTracker
case withMultipleTrackers
}
// swiftlint:enable nesting

static let afterSearch = BrowsingSpec(message: UserText.daxDialogBrowsingAfterSearch,
cta: UserText.daxDialogBrowsingAfterSearchCTA,
highlightAddressBar: false,
pixelName: .daxDialogsSerp)
pixelName: .daxDialogsSerp, type: .afterSearch)

static let withoutTrackers = BrowsingSpec(message: UserText.daxDialogBrowsingWithoutTrackers,
cta: UserText.daxDialogBrowsingWithoutTrackersCTA,
highlightAddressBar: false,
pixelName: .daxDialogsWithoutTrackers)
pixelName: .daxDialogsWithoutTrackers, type: .withoutTrackers)

static let siteIsMajorTracker = BrowsingSpec(message: UserText.daxDialogBrowsingSiteIsMajorTracker,
cta: UserText.daxDialogBrowsingSiteIsMajorTrackerCTA,
highlightAddressBar: false,
pixelName: .daxDialogsSiteIsMajor)
pixelName: .daxDialogsSiteIsMajor, type: .siteIsMajorTracker)

static let siteOwnedByMajorTracker = BrowsingSpec(message: UserText.daxDialogBrowsingSiteOwnedByMajorTracker,
cta: UserText.daxDialogBrowsingSiteOwnedByMajorTrackerCTA,
highlightAddressBar: false,
pixelName: .daxDialogsSiteOwnedByMajor)
pixelName: .daxDialogsSiteOwnedByMajor, type: .siteOwnedByMajorTracker)

static let withOneTracker = BrowsingSpec(message: UserText.daxDialogBrowsingWithOneTracker,
cta: UserText.daxDialogBrowsingWithOneTrackerCTA,
highlightAddressBar: true,
pixelName: .daxDialogsWithTrackers)
pixelName: .daxDialogsWithTrackers, type: .withOneTracker)

static let withMutipleTrackers = BrowsingSpec(message: UserText.daxDialogBrowsingWithMultipleTrackers,
static let withMultipleTrackers = BrowsingSpec(message: UserText.daxDialogBrowsingWithMultipleTrackers,
cta: UserText.daxDialogBrowsingWithMultipleTrackersCTA,
highlightAddressBar: true,
pixelName: .daxDialogsWithTrackers)
pixelName: .daxDialogsWithTrackers, type: .withMultipleTrackers)

let message: String
let cta: String
let highlightAddressBar: Bool
let pixelName: PixelName
let type: SpecType

func format(args: CVarArg...) -> BrowsingSpec {
return BrowsingSpec(message: String(format: message, arguments: args),
cta: cta,
highlightAddressBar: highlightAddressBar,
pixelName: pixelName)
pixelName: pixelName,
type: type)
}

}

struct ActionSheetSpec: Equatable {
Expand Down Expand Up @@ -228,7 +254,6 @@ class DaxDialogs {
}

func nextHomeScreenMessage() -> HomeScreenSpec? {

if nextHomeScreenMessageOverride != nil {
return nextHomeScreenMessageOverride
}
Expand Down Expand Up @@ -298,11 +323,10 @@ class DaxDialogs {

default:
settings.browsingWithTrackersShown = true
return BrowsingSpec.withMutipleTrackers.format(args: entitiesBlocked.count - 2,
return BrowsingSpec.withMultipleTrackers.format(args: entitiesBlocked.count - 2,
entitiesBlocked[0].displayName ?? "",
entitiesBlocked[1].displayName ?? "")
}

}

private func entitiesBlocked(_ siteRating: SiteRating) -> [Entity]? {
Expand All @@ -322,5 +346,5 @@ class DaxDialogs {
let entity = currentTrackerData.findEntity(forHost: host) else { return nil }
return entity.domains?.contains(where: { MajorTrackers.domains.contains($0) }) ?? false ? entity : nil
}

}
// swiftlint:enable type_body_length
9 changes: 9 additions & 0 deletions DuckDuckGo/TabViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,16 @@ extension TabViewController: WKNavigationDelegate {

isShowingFullScreenDaxDialog = true
scheduleTrackerNetworksAnimation(collapsing: !spec.highlightAddressBar)
let daxDialogSourceURL = self.url

DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { [weak self] in
// https://app.asana.com/0/414709148257752/1201620790053163/f
if self?.url != daxDialogSourceURL {
DaxDialogs.shared.overrideShownFlagFor(spec, flag: false)
self?.isShowingFullScreenDaxDialog = false
return
}

self?.chromeDelegate?.omniBar.resignFirstResponder()
self?.chromeDelegate?.setBarsHidden(false, animated: true)
self?.performSegue(withIdentifier: "DaxDialog", sender: spec)
Expand Down
10 changes: 5 additions & 5 deletions DuckDuckGoTests/DaxDialogTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ class DaxDialogTests: XCTestCase {
// swiftlint:disable line_length
let testCases = [
(urls: [ URLs.google ], expected: DaxDialogs.BrowsingSpec.withOneTracker.format(args: "Google"), line: #line),
(urls: [ URLs.google, URLs.amazon ], expected: DaxDialogs.BrowsingSpec.withMutipleTrackers.format(args: 0, "Google", "Amazon.com"), line: #line),
(urls: [ URLs.amazon, URLs.ownedByFacebook ], expected: DaxDialogs.BrowsingSpec.withMutipleTrackers.format(args: 0, "Facebook", "Amazon.com"), line: #line),
(urls: [ URLs.facebook, URLs.google ], expected: DaxDialogs.BrowsingSpec.withMutipleTrackers.format(args: 0, "Google", "Facebook"), line: #line),
(urls: [ URLs.facebook, URLs.google, URLs.amazon ], expected: DaxDialogs.BrowsingSpec.withMutipleTrackers.format(args: 1, "Google", "Facebook"), line: #line),
(urls: [ URLs.facebook, URLs.google, URLs.amazon, URLs.tracker ], expected: DaxDialogs.BrowsingSpec.withMutipleTrackers.format(args: 2, "Google", "Facebook"), line: #line)
(urls: [ URLs.google, URLs.amazon ], expected: DaxDialogs.BrowsingSpec.withMultipleTrackers.format(args: 0, "Google", "Amazon.com"), line: #line),
(urls: [ URLs.amazon, URLs.ownedByFacebook ], expected: DaxDialogs.BrowsingSpec.withMultipleTrackers.format(args: 0, "Facebook", "Amazon.com"), line: #line),
(urls: [ URLs.facebook, URLs.google ], expected: DaxDialogs.BrowsingSpec.withMultipleTrackers.format(args: 0, "Google", "Facebook"), line: #line),
(urls: [ URLs.facebook, URLs.google, URLs.amazon ], expected: DaxDialogs.BrowsingSpec.withMultipleTrackers.format(args: 1, "Google", "Facebook"), line: #line),
(urls: [ URLs.facebook, URLs.google, URLs.amazon, URLs.tracker ], expected: DaxDialogs.BrowsingSpec.withMultipleTrackers.format(args: 2, "Google", "Facebook"), line: #line)
]
// swiftlint:enable line_length

Expand Down
6 changes: 3 additions & 3 deletions DuckDuckGoTests/DaxDialogsBrowsingSpecTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class DaxDialogsBrowsingSpecTests: XCTestCase {
let majorTracker1 = "TestTracker1"
let majorTracker2 = "TestTracker2"
let count = 1
let message = DaxDialogs.BrowsingSpec.withMutipleTrackers.format(args: count, majorTracker1, majorTracker2).message
let message = DaxDialogs.BrowsingSpec.withMultipleTrackers.format(args: count, majorTracker1, majorTracker2).message
XCTAssertTrue(message.contains(majorTracker1))
XCTAssertTrue(message.contains(majorTracker2))
XCTAssertTrue(message.contains("\(count)"))
Expand All @@ -61,7 +61,7 @@ class DaxDialogsBrowsingSpecTests: XCTestCase {
let majorTracker1 = "TestTracker1"
let majorTracker2 = "TestTracker2"
let count = 6
let message = DaxDialogs.BrowsingSpec.withMutipleTrackers.format(args: count, majorTracker1, majorTracker2).message
let message = DaxDialogs.BrowsingSpec.withMultipleTrackers.format(args: count, majorTracker1, majorTracker2).message
XCTAssertTrue(message.contains(majorTracker1))
XCTAssertTrue(message.contains(majorTracker2))
XCTAssertTrue(message.contains("\(count)"))
Expand All @@ -72,7 +72,7 @@ class DaxDialogsBrowsingSpecTests: XCTestCase {
func testWhenTwoTrackersThenMessageContainsBothTrackers() {
let majorTracker1 = "TestTracker1"
let majorTracker2 = "TestTracker2"
let message = DaxDialogs.BrowsingSpec.withMutipleTrackers.format(args: 0, majorTracker1, majorTracker2).message
let message = DaxDialogs.BrowsingSpec.withMultipleTrackers.format(args: 0, majorTracker1, majorTracker2).message
XCTAssertTrue(message.contains(majorTracker1))
XCTAssertTrue(message.contains(majorTracker2))
XCTAssertTrue(message.contains("\n"))
Expand Down

0 comments on commit 37f3330

Please sign in to comment.