Skip to content

Commit

Permalink
VPN screen improvements #3: Data volume + Menu items (#2611)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/0/1206942473732464/f
Tech Design URL:
CC:

**Description**:

Adds the Data Volume item & reorder the menu items.

**Steps to test this PR**:

1. Open the VPN screen on either the status bar or main browser
2. Check if data volume is correct
3. Also, verify the menu item ordering. Open DDG should go last.

<!--
Tagging instructions
If this PR isn't ready to be merged for whatever reason it should be
marked with the `DO NOT MERGE` label (particularly if it's a draft)
If it's pending Product Review/PFR, please add the `Pending Product
Review` label.

If at any point it isn't actively being worked on/ready for
review/otherwise moving forward (besides the above PR/PFR exception)
strongly consider closing it (or not opening it in the first place). If
you decide not to close it, make sure it's labelled to make it clear the
PRs state and comment with more information.
-->

---
###### Internal references:
[Pull Request Review
Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f)
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
[Pull Request
Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f)
  • Loading branch information
quanganhdo authored Apr 19, 2024
1 parent 2864ba9 commit c7ecdbb
Show file tree
Hide file tree
Showing 34 changed files with 296 additions and 85 deletions.
2 changes: 1 addition & 1 deletion DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -14568,7 +14568,7 @@
isa = XCRemoteSwiftPackageReference;
repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit";
requirement = {
branch = "anh/netp/location-selection";
branch = "anh/netp/screen-improvements";
kind = branch;
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/duckduckgo/BrowserServicesKit",
"state" : {
"branch" : "anh/netp/location-selection",
"revision" : "dc6725de9e7019ef7e2b0bb7438b9d98010808f8"
"branch" : "anh/netp/screen-improvements",
"revision" : "3b354a70cdfff5d46d2c8f525748dee63a36ad88"
}
},
{
Expand Down
2 changes: 2 additions & 0 deletions DuckDuckGo/Application/URLEventHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ final class URLEventHandler {
WindowControllersManager.shared.showPreferencesTab(withSelectedPane: .vpn)
case AppLaunchCommand.shareFeedback.launchURL:
WindowControllersManager.shared.showShareFeedbackModal()
case AppLaunchCommand.justOpen.launchURL:
WindowControllersManager.shared.showMainWindow()
case AppLaunchCommand.showVPNLocations.launchURL:
WindowControllersManager.shared.showPreferencesTab(withSelectedPane: .vpn)
WindowControllersManager.shared.showLocationPickerSheet()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ extension UserText {
static let networkProtectionInviteSuccessMessage = "DuckDuckGo's VPN secures all of your device's Internet traffic anytime, anywhere."

// MARK: - Navigation Bar Status View
// "network.protection.navbar.status.view.share.feedback" - Menu item for 'Send VPN Feedback' in the VPN status view that's shown in the navigation bar
static let networkProtectionNavBarStatusViewShareFeedback = "Send VPN Feedback…"
// "network.protection.navbar.status.view.share.feedback" - Menu item for 'Share VPN Feedback' in the VPN status view that's shown in the navigation bar
static let networkProtectionNavBarStatusViewShareFeedback = "Share VPN Feedback…"
// "network.protection.status.menu.vpn.settings" - The status menu 'VPN Settings' menu item
static let networkProtectionNavBarStatusMenuVPNSettings = "VPN Settings…"
// "network.protection.status.menu.faq" - The status menu 'FAQ' menu item
Expand Down Expand Up @@ -299,7 +299,7 @@ extension UserText {
// "vpn.location.description.nearest" - Nearest city setting description
static let vpnLocationNearest = "Nearest"
// "vpn.location.description.nearest.available" - Nearest available location setting description
static let vpnLocationNearestAvailable = "Nearest available"
static let vpnLocationNearestAvailable = "Nearest Location"
// "vpn.location.nearest.available.title" - Subtitle underneath the nearest available vpn location preference text.
static let vpnLocationNearestAvailableSubtitle = "Automatically connect to the nearest server we can find."

Expand Down
3 changes: 2 additions & 1 deletion DuckDuckGo/MainWindow/MainViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ final class MainViewController: NSViewController {
serverInfoObserver: ipcClient.ipcServerInfoObserver,
connectionErrorObserver: ipcClient.ipcConnectionErrorObserver,
connectivityIssuesObserver: connectivityIssuesObserver,
controllerErrorMessageObserver: controllerErrorMessageObserver
controllerErrorMessageObserver: controllerErrorMessageObserver,
dataVolumeObserver: ipcClient.ipcDataVolumeObserver
)
}()

Expand Down
6 changes: 6 additions & 0 deletions DuckDuckGo/NavigationBar/View/NetPPopoverManagerMock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ final class IPCClientMock: NetworkProtectionIPCClient {
}
var ipcControllerErrorMessageObserver: any NetworkProtection.ControllerErrorMesssageObserver = ControllerErrorMesssageObserverMock()

final class DataVolumeObserverMock: NetworkProtection.DataVolumeObserver {
var publisher: AnyPublisher<DataVolume, Never> = PassthroughSubject().eraseToAnyPublisher()
var recentValue: DataVolume = .init()
}
var ipcDataVolumeObserver: any NetworkProtection.DataVolumeObserver = DataVolumeObserverMock()

func start() {}

func stop() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ protocol NetworkProtectionIPCClient {
var ipcStatusObserver: ConnectionStatusObserver { get }
var ipcServerInfoObserver: ConnectionServerInfoObserver { get }
var ipcConnectionErrorObserver: ConnectionErrorObserver { get }
var ipcDataVolumeObserver: DataVolumeObserver { get }

func start()
func stop()
Expand All @@ -41,6 +42,7 @@ extension TunnelControllerIPCClient: NetworkProtectionIPCClient {
public var ipcStatusObserver: any NetworkProtection.ConnectionStatusObserver { connectionStatusObserver }
public var ipcServerInfoObserver: any NetworkProtection.ConnectionServerInfoObserver { serverInfoObserver }
public var ipcConnectionErrorObserver: any NetworkProtection.ConnectionErrorObserver { connectionErrorObserver }
public var ipcDataVolumeObserver: any NetworkProtection.DataVolumeObserver { dataVolumeObserver }
}

final class NetworkProtectionNavBarPopoverManager: NetPPopoverManager {
Expand Down Expand Up @@ -69,7 +71,8 @@ final class NetworkProtectionNavBarPopoverManager: NetPPopoverManager {
serverInfoObserver: ipcClient.ipcServerInfoObserver,
connectionErrorObserver: ipcClient.ipcConnectionErrorObserver,
connectivityIssuesObserver: ConnectivityIssueObserverThroughDistributedNotifications(),
controllerErrorMessageObserver: ControllerErrorMesssageObserverThroughDistributedNotifications()
controllerErrorMessageObserver: ControllerErrorMesssageObserverThroughDistributedNotifications(),
dataVolumeObserver: ipcClient.ipcDataVolumeObserver
)

let onboardingStatusPublisher = UserDefaults.netP.networkProtectionOnboardingStatusPublisher
Expand Down
3 changes: 2 additions & 1 deletion DuckDuckGo/VPNFeedbackForm/VPNMetadataCollector.swift
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ final class DefaultVPNMetadataCollector: VPNMetadataCollector {
serverInfoObserver: ipcClient.serverInfoObserver,
connectionErrorObserver: ipcClient.connectionErrorObserver,
connectivityIssuesObserver: ConnectivityIssueObserverThroughDistributedNotifications(),
controllerErrorMessageObserver: ControllerErrorMesssageObserverThroughDistributedNotifications()
controllerErrorMessageObserver: ControllerErrorMesssageObserverThroughDistributedNotifications(),
dataVolumeObserver: ipcClient.dataVolumeObserver
)

// Force refresh just in case. A refresh is requested when the IPC client is created, but distributed notifications don't guarantee delivery
Expand Down
7 changes: 7 additions & 0 deletions DuckDuckGo/Windows/View/WindowControllersManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,13 @@ extension WindowControllersManager {
}
}

func showMainWindow() {
guard WindowControllersManager.shared.lastKeyMainWindowController == nil else { return }
let tabCollection = TabCollection(tabs: [])
let tabCollectionViewModel = TabCollectionViewModel(tabCollection: tabCollection)
_ = WindowsManager.openNewWindow(with: tabCollectionViewModel)
}

func showLocationPickerSheet() {
let locationsViewController = VPNLocationsHostingViewController()
let locationsWindowController = locationsViewController.wrappedInWindowController()
Expand Down
4 changes: 2 additions & 2 deletions DuckDuckGoDBPBackgroundAgent/UserText.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ import Foundation
final class UserText {
// MARK: - Status Menu

static let networkProtectionStatusMenuShareFeedback = NSLocalizedString("network.protection.status.menu.share.feedback", value: "Send VPN Feedback...", comment: "The status menu 'Send VPN Feedback' menu item")
static let networkProtectionStatusMenuOpenDuckDuckGo = NSLocalizedString("network.protection.status.menu.open.duckduckgo", value: "Open DuckDuckGo...", comment: "The status menu 'Open DuckDuckGo' menu item")
static let networkProtectionStatusMenuShareFeedback = NSLocalizedString("network.protection.status.menu.share.feedback", value: "Share VPN Feedback", comment: "The status menu 'Share VPN Feedback' menu item")
static let networkProtectionStatusMenuOpenDuckDuckGo = NSLocalizedString("network.protection.status.menu.open.duckduckgo", value: "Open DuckDuckGo", comment: "The status menu 'Open DuckDuckGo' menu item")
}
14 changes: 10 additions & 4 deletions DuckDuckGoVPN/DuckDuckGoVPNAppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,18 @@ final class DuckDuckGoVPNAppDelegate: NSObject, NSApplicationDelegate {
platformNotificationCenter: NSWorkspace.shared.notificationCenter,
platformDidWakeNotification: NSWorkspace.didWakeNotification)

let dataVolumeObserver = DataVolumeObserverThroughSession(
tunnelSessionProvider: tunnelController,
platformNotificationCenter: NSWorkspace.shared.notificationCenter,
platformDidWakeNotification: NSWorkspace.didWakeNotification)

return DefaultNetworkProtectionStatusReporter(
statusObserver: statusObserver,
serverInfoObserver: serverInfoObserver,
connectionErrorObserver: errorObserver,
connectivityIssuesObserver: DisabledConnectivityIssueObserver(),
controllerErrorMessageObserver: ControllerErrorMesssageObserverThroughDistributedNotifications()
controllerErrorMessageObserver: ControllerErrorMesssageObserverThroughDistributedNotifications(),
dataVolumeObserver: dataVolumeObserver
)
}()

Expand Down Expand Up @@ -245,12 +251,12 @@ final class DuckDuckGoVPNAppDelegate: NSObject, NSApplicationDelegate {
StatusBarMenu.MenuItem(name: UserText.networkProtectionStatusMenuFAQ, action: { [weak self] in
await self?.appLauncher.launchApp(withCommand: .showFAQ)
}),
StatusBarMenu.MenuItem(name: UserText.networkProtectionStatusMenuShareFeedback, action: { [weak self] in
await self?.appLauncher.launchApp(withCommand: .shareFeedback)
}),
StatusBarMenu.MenuItem(name: UserText.networkProtectionStatusMenuOpenDuckDuckGo, action: { [weak self] in
await self?.appLauncher.launchApp(withCommand: .justOpen)
}),
StatusBarMenu.MenuItem(name: UserText.networkProtectionStatusMenuShareFeedback, action: { [weak self] in
await self?.appLauncher.launchApp(withCommand: .shareFeedback)
})
]
},
agentLoginItem: nil,
Expand Down
10 changes: 10 additions & 0 deletions DuckDuckGoVPN/TunnelControllerIPCService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ final class TunnelControllerIPCService {
subscribeToErrorChanges()
subscribeToStatusUpdates()
subscribeToServerChanges()
subscribeToDataVolumeUpdates()

server.serverDelegate = self
}
Expand Down Expand Up @@ -84,6 +85,15 @@ final class TunnelControllerIPCService {
}
.store(in: &cancellables)
}

private func subscribeToDataVolumeUpdates() {
statusReporter.dataVolumeObserver.publisher
.subscribe(on: DispatchQueue.main)
.sink { [weak self] dataVolume in
self?.server.dataVolumeUpdated(dataVolume)
}
.store(in: &cancellables)
}
}

// MARK: - Requests from the client
Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGoVPN/UserText.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,5 @@ final class UserText {
static let networkProtectionStatusMenuVPNSettings = NSLocalizedString("network.protection.status.menu.vpn.settings", value: "VPN Settings…", comment: "The status menu 'VPN Settings' menu item")
static let networkProtectionStatusMenuFAQ = NSLocalizedString("network.protection.status.menu.faq", value: "Frequently Asked Questions…", comment: "The status menu 'FAQ' menu item")
static let networkProtectionStatusMenuOpenDuckDuckGo = NSLocalizedString("network.protection.status.menu.vpn.open-duckduckgo", value: "Open DuckDuckGo…", comment: "The status menu 'Open DuckDuckGo' menu item")
static let networkProtectionStatusMenuShareFeedback = NSLocalizedString("network.protection.status.menu.share.feedback", value: "Send VPN Feedback…", comment: "The status menu 'Send VPN Feedback' menu item")
static let networkProtectionStatusMenuShareFeedback = NSLocalizedString("network.protection.status.menu.share.feedback", value: "Share VPN Feedback…", comment: "The status menu 'Share VPN Feedback' menu item")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//
// DataVolumeObserverThroughIPC.swift
//
// Copyright © 2024 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Combine
import Foundation
import NetworkProtection

public final class DataVolumeObserverThroughIPC: DataVolumeObserver {

private let subject = CurrentValueSubject<DataVolume, Never>(.init())

// MARK: - DataVolumeObserver

public lazy var publisher = subject.eraseToAnyPublisher()

public var recentValue: DataVolume {
subject.value
}

// MARK: - Publishing Updates

func publish(_ dataVolume: DataVolume) {
subject.send(dataVolume)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public protocol IPCClientInterface: AnyObject {
func errorChanged(_ error: String?)
func serverInfoChanged(_ serverInfo: NetworkProtectionStatusServerInfo)
func statusChanged(_ status: ConnectionStatus)
func dataVolumeUpdated(_ dataVolume: DataVolume)
}

/// This is the XPC interface with parameters that can be packed properly
Expand All @@ -34,6 +35,7 @@ protocol XPCClientInterface {
func errorChanged(error: String?)
func serverInfoChanged(payload: Data)
func statusChanged(payload: Data)
func dataVolumeUpdated(payload: Data)
}

public final class TunnelControllerIPCClient {
Expand All @@ -47,6 +49,7 @@ public final class TunnelControllerIPCClient {
public var serverInfoObserver = ConnectionServerInfoObserverThroughIPC()
public var connectionErrorObserver = ConnectionErrorObserverThroughIPC()
public var connectionStatusObserver = ConnectionStatusObserverThroughIPC()
public var dataVolumeObserver = DataVolumeObserverThroughIPC()

/// The delegate.
///
Expand All @@ -65,7 +68,8 @@ public final class TunnelControllerIPCClient {
clientDelegate: self.clientDelegate,
serverInfoObserver: self.serverInfoObserver,
connectionErrorObserver: self.connectionErrorObserver,
connectionStatusObserver: self.connectionStatusObserver
connectionStatusObserver: self.connectionStatusObserver,
dataVolumeObserver: self.dataVolumeObserver
)

xpc = XPCClient(
Expand Down Expand Up @@ -97,15 +101,18 @@ private final class TunnelControllerXPCClientDelegate: XPCClientInterface {
let serverInfoObserver: ConnectionServerInfoObserverThroughIPC
let connectionErrorObserver: ConnectionErrorObserverThroughIPC
let connectionStatusObserver: ConnectionStatusObserverThroughIPC
let dataVolumeObserver: DataVolumeObserverThroughIPC

init(clientDelegate: IPCClientInterface?,
serverInfoObserver: ConnectionServerInfoObserverThroughIPC,
connectionErrorObserver: ConnectionErrorObserverThroughIPC,
connectionStatusObserver: ConnectionStatusObserverThroughIPC) {
connectionStatusObserver: ConnectionStatusObserverThroughIPC,
dataVolumeObserver: DataVolumeObserverThroughIPC) {
self.clientDelegate = clientDelegate
self.serverInfoObserver = serverInfoObserver
self.connectionErrorObserver = connectionErrorObserver
self.connectionStatusObserver = connectionStatusObserver
self.dataVolumeObserver = dataVolumeObserver
}

func errorChanged(error: String?) {
Expand All @@ -130,6 +137,15 @@ private final class TunnelControllerXPCClientDelegate: XPCClientInterface {
connectionStatusObserver.publish(status)
clientDelegate?.statusChanged(status)
}

func dataVolumeUpdated(payload: Data) {
guard let dataVolume = try? JSONDecoder().decode(DataVolume.self, from: payload) else {
return
}

dataVolumeObserver.publish(dataVolume)
clientDelegate?.dataVolumeUpdated(dataVolume)
}
}

// MARK: - Outgoing communication to the server
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,20 @@ extension TunnelControllerIPCServer: IPCClientInterface {
client.statusChanged(payload: payload)
}
}

public func dataVolumeUpdated(_ dataVolume: DataVolume) {
let payload: Data

do {
payload = try JSONEncoder().encode(dataVolume)
} catch {
return
}

xpc.forEachClient { client in
client.dataVolumeUpdated(payload: payload)
}
}
}

// MARK: - Incoming communication from a client
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@
import Foundation

public enum NetworkProtectionAsset: String, CaseIterable {
case ipAddressIcon = "IP-16"
case serverLocationIcon = "Server-Location-16"
case vpnDisabledImage = "VPNDisabled"
case vpnEnabledImage = "VPN"
case vpnIcon = "VPN-16"
case nearestAvailable = "VPNLocation"
case dataReceived = "VPNDownload"
case dataSent = "VPNUpload"

// Apple Icons
case appleVaultIcon = "apple-vault-icon"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ extension Color {
///
enum NetworkProtectionColor: String {
case defaultText = "TextColor"
case secondaryText = "SecondaryColor"
case linkColor = "LinkBlueColor"
case onboardingButtonBackgroundColor = "OnboardingButtonBackgroundColor"
#if swift(<5.9)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"colors" : [
{
"color" : {
"color-space" : "srgb",
"components" : {
"alpha" : "0.600",
"blue" : "0x00",
"green" : "0x00",
"red" : "0x00"
}
},
"idiom" : "universal"
},
{
"appearances" : [
{
"appearance" : "luminosity",
"value" : "dark"
}
],
"color" : {
"color-space" : "srgb",
"components" : {
"alpha" : "0.850",
"blue" : "0xFF",
"green" : "0xFF",
"red" : "0xFF"
}
},
"idiom" : "universal"
}
],
"info" : {
"author" : "xcode",
"version" : 1
}
}
Loading

0 comments on commit c7ecdbb

Please sign in to comment.