-
Notifications
You must be signed in to change notification settings - Fork 424
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
Adds pixels to VPN tips #3629
Adds pixels to VPN tips #3629
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,48 +31,9 @@ struct NetworkProtectionStatusView: View { | |
@ObservedObject | ||
public var statusModel: NetworkProtectionStatusViewModel | ||
|
||
// MARK: - Tips | ||
|
||
let geoswitchingTip: VPNGeoswitchingTip = { | ||
let tip = VPNGeoswitchingTip() | ||
|
||
if #available(iOS 17.0, *) { | ||
if tip.shouldDisplay { | ||
Task { | ||
for await status in tip.statusUpdates { | ||
if case .invalidated = status { | ||
await VPNSnoozeTip.geolocationTipDismissedEvent.donate() | ||
await VPNAddWidgetTip.geolocationTipDismissedEvent.donate() | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
return tip | ||
}() | ||
|
||
let snoozeTip: VPNSnoozeTip = { | ||
let tip = VPNSnoozeTip() | ||
|
||
if #available(iOS 17.0, *) { | ||
if tip.shouldDisplay { | ||
Task { | ||
for await status in tip.statusUpdates { | ||
if case .invalidated = status { | ||
await VPNAddWidgetTip.snoozeTipDismissedEvent.donate() | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
return tip | ||
}() | ||
|
||
let widgetTip: VPNAddWidgetTip = { | ||
VPNAddWidgetTip() | ||
}() | ||
var tipsModel: VPNTipsModel { | ||
statusModel.tipsModel | ||
} | ||
Comment on lines
+34
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Placing the tips in a model to separate view and other logic. |
||
|
||
// MARK: - View | ||
|
||
|
@@ -106,6 +67,16 @@ struct NetworkProtectionStatusView: View { | |
.sheet(isPresented: $statusModel.showAddWidgetEducationView) { | ||
widgetEducationSheet() | ||
} | ||
.onAppear { | ||
if #available(iOS 18.0, *) { | ||
tipsModel.handleStatusViewAppear() | ||
} | ||
} | ||
.onDisappear { | ||
if #available(iOS 18.0, *) { | ||
tipsModel.handleStatusViewDisappear() | ||
} | ||
} | ||
} | ||
|
||
@ViewBuilder | ||
|
@@ -359,11 +330,26 @@ struct NetworkProtectionStatusView: View { | |
@ViewBuilder | ||
private func geoswitchingTipView() -> some View { | ||
if statusModel.canShowTips { | ||
|
||
TipView(geoswitchingTip) | ||
TipView(tipsModel.geoswitchingTip) | ||
.removeGroupedListStyleInsets() | ||
.tipCornerRadius(0) | ||
.tipBackground(Color(designSystemColor: .surface)) | ||
.onAppear { | ||
tipsModel.handleGeoswitchingTipShown() | ||
} | ||
.task { | ||
var previousStatus = tipsModel.geoswitchingTip.status | ||
|
||
for await status in tipsModel.geoswitchingTip.statusUpdates { | ||
if case .invalidated(let reason) = status { | ||
if case .available = previousStatus { | ||
Comment on lines
+343
to
+345
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bit nasty. I copied this logic from macOS. Tips don't have a simple action hook for when they're dismissed through the "X" button, unless we reimplement them from scratch. Since I didn't want to recreate the standard tip UI, I am observing the tip status and when I detect a change from |
||
tipsModel.handleGeoswitchingTipInvalidated(reason) | ||
} | ||
} | ||
|
||
previousStatus = status | ||
} | ||
} | ||
} | ||
} | ||
|
||
|
@@ -373,10 +359,26 @@ struct NetworkProtectionStatusView: View { | |
if statusModel.canShowTips, | ||
statusModel.hasServerInfo { | ||
|
||
TipView(snoozeTip, action: statusModel.snoozeActionHandler(action:)) | ||
TipView(tipsModel.snoozeTip, action: statusModel.snoozeActionHandler(action:)) | ||
.removeGroupedListStyleInsets() | ||
.tipCornerRadius(0) | ||
.tipBackground(Color(designSystemColor: .surface)) | ||
.onAppear { | ||
tipsModel.handleSnoozeTipShown() | ||
} | ||
.task { | ||
var previousStatus = tipsModel.snoozeTip.status | ||
|
||
for await status in tipsModel.snoozeTip.statusUpdates { | ||
if case .invalidated(let reason) = status { | ||
if case .available = previousStatus { | ||
tipsModel.handleSnoozeTipInvalidated(reason) | ||
} | ||
} | ||
|
||
previousStatus = status | ||
} | ||
} | ||
} | ||
} | ||
|
||
|
@@ -386,10 +388,26 @@ struct NetworkProtectionStatusView: View { | |
if statusModel.canShowTips, | ||
!statusModel.isNetPEnabled && !statusModel.isSnoozing { | ||
|
||
TipView(widgetTip, action: statusModel.widgetActionHandler(action:)) | ||
TipView(tipsModel.widgetTip, action: statusModel.widgetActionHandler(action:)) | ||
.removeGroupedListStyleInsets() | ||
.tipCornerRadius(0) | ||
.tipBackground(Color(designSystemColor: .surface)) | ||
.onAppear { | ||
tipsModel.handleWidgetTipShown() | ||
} | ||
.task { | ||
var previousStatus = tipsModel.widgetTip.status | ||
|
||
for await status in tipsModel.widgetTip.statusUpdates { | ||
if case .invalidated(let reason) = status { | ||
if case .available = previousStatus { | ||
tipsModel.handleWidgetTipInvalidated(reason) | ||
} | ||
} | ||
|
||
previousStatus = status | ||
} | ||
} | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,6 +113,8 @@ final class NetworkProtectionStatusViewModel: ObservableObject { | |
@Published | ||
var showAddWidgetEducationView: Bool = false | ||
|
||
let tipsModel: VPNTipsModel | ||
|
||
// MARK: Error | ||
|
||
struct ErrorItem { | ||
|
@@ -138,7 +140,7 @@ final class NetworkProtectionStatusViewModel: ObservableObject { | |
didSet { | ||
if #available(iOS 17.0, *) { | ||
if isNetPEnabled { | ||
VPNGeoswitchingTip.donateVPNConnectedEvent() | ||
VPNGeoswitchingTip.vpnEnabledOnce = true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a change to keep this as a flag, rather than an ever growing counter. We just need to know this happened once. |
||
} | ||
|
||
VPNSnoozeTip.vpnEnabled = isNetPEnabled | ||
|
@@ -184,6 +186,7 @@ final class NetworkProtectionStatusViewModel: ObservableObject { | |
errorObserver: ConnectionErrorObserver = ConnectionErrorObserverThroughSession(), | ||
locationListRepository: NetworkProtectionLocationListRepository, | ||
usesUnifiedFeedbackForm: Bool) { | ||
|
||
self.tunnelController = tunnelController | ||
self.settings = settings | ||
self.statusObserver = statusObserver | ||
|
@@ -199,6 +202,11 @@ final class NetworkProtectionStatusViewModel: ObservableObject { | |
|
||
self.dnsSettings = settings.dnsSettings | ||
|
||
self.tipsModel = VPNTipsModel( | ||
isTipFeatureEnabled: featureFlagger.isFeatureOn(.networkProtectionUserTips), | ||
statusObserver: statusObserver, | ||
vpnSettings: settings) | ||
Comment on lines
+205
to
+208
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spawning the tips model. |
||
|
||
updateViewModel(withStatus: statusObserver.recentValue) | ||
|
||
setUpIsConnectedStatePublishers() | ||
|
@@ -477,8 +485,8 @@ final class NetworkProtectionStatusViewModel: ObservableObject { | |
return | ||
} | ||
|
||
if #available(iOS 17.0, *) { | ||
VPNSnoozeTip().invalidate(reason: .actionPerformed) | ||
if #available(iOS 18.0, *) { | ||
tipsModel.handleUserSnoozedVPN() | ||
} | ||
|
||
let defaultDuration: TimeInterval = .minutes(20) | ||
|
@@ -577,30 +585,30 @@ final class NetworkProtectionStatusViewModel: ObservableObject { | |
|
||
// MARK: - UI Events handling | ||
|
||
@available(iOS 17.0, *) | ||
@available(iOS 18.0, *) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may seem confusing, but the VPN tips feature was initially meant for iOS 17, but due to some unexpected crashes that seem to be on Apple, we've decided to move them to iOS 18. This just needed to be updated. |
||
func snoozeActionHandler(action: Tips.Action) { | ||
if action.id == VPNSnoozeTip.ActionIdentifiers.learnMore.rawValue { | ||
let url = URL(string: "https://duckduckgo.com/duckduckgo-help-pages/privacy-pro/vpn/troubleshooting/")! | ||
UIApplication.shared.open(url, options: [:], completionHandler: nil) | ||
} | ||
} | ||
|
||
@available(iOS 17.0, *) | ||
@available(iOS 18.0, *) | ||
@MainActor | ||
func widgetActionHandler(action: Tips.Action) { | ||
if action.id == VPNAddWidgetTip.ActionIdentifiers.addWidget.rawValue { | ||
showAddWidgetEducationView = true | ||
|
||
VPNAddWidgetTip().invalidate(reason: .actionPerformed) | ||
tipsModel.handleUserOpenedWidgetLearnMore() | ||
} | ||
} | ||
|
||
/// The user opened the VPN locations view | ||
/// | ||
func handleUserOpenedVPNLocations() { | ||
if #available(iOS 17.0, *) { | ||
if #available(iOS 18.0, *) { | ||
Task { @MainActor in | ||
VPNGeoswitchingTip().invalidate(reason: .actionPerformed) | ||
tipsModel.handleUserOpenedLocations() | ||
} | ||
} | ||
} | ||
|
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.
New pixels