Skip to content

Commit

Permalink
Privacy Dashboard 8.1.0: Improved breakage form (#3729)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/0/1209127604215386/f
CC: @jaceklyp 

**Description**:

Upgrades Privacy Dashboard to 8.0.0, which contains the new site
breakage flow as can be previewed
[here](https://duckduckgo.github.io/privacy-dashboard/app-debug/html/iframe.html?platforms=android%2Cios%2Cmacos%2Cwindows%2Cbrowser&screen=breakageForm)

Figma:
https://www.figma.com/design/eeVdakW2pjijbDlv8HUSgZ/O-E---Report-Broken-Site-Flow-(Design-Updates-2024-09)?node-id=4024-6272&m=dev

**Steps to test this PR**:

1. Trigger the breakage form in the two possible ways:
- Open the Privacy Dashboard and click on "Report a problem with this
site"
- Open the app menu ••• and click on “Send Feedback > Report Broken
Site” (macOS)

2. Confirm that the breakage form matches the Figma or the [web preview
](https://duckduckgo.github.io/privacy-dashboard/app-debug/html/iframe.html?platforms=android%2Cios%2Cmacos%2Cwindows%2Cbrowser&screen=breakageForm)

3. Attempt to complete a broken site report from both entry points
above. Things to keep an eye on:
- The “See what’s sent” toggle expands and collapses normally
- When the category is “Something else”, trying to submit the form
without a description should raise an alert
- The description is optional for all other categories 

<!--
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.
-->

**Definition of Done**:

* [ ] Does this PR satisfy our [Definition of
Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)?

---
###### 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)

---------

Co-authored-by: Jacek Łyp <[email protected]>
  • Loading branch information
mgurgel and jaceklyp authored Jan 20, 2025
1 parent 9c1ff3c commit e47e5ad
Show file tree
Hide file tree
Showing 12 changed files with 54 additions and 32 deletions.
2 changes: 1 addition & 1 deletion DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -15386,7 +15386,7 @@
repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit";
requirement = {
kind = exactVersion;
version = 225.1.0;
version = 226.0.0;
};
};
9FF521422BAA8FF300B9819B /* XCRemoteSwiftPackageReference "lottie-spm" */ = {
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" : {
"revision" : "fb315acdbbea9dee949cc066fcafbb1a1f3618f4",
"version" : "225.1.0"
"revision" : "d1275238d88f25cd31a29be2b4f20bbba5de9aa1",
"version" : "226.0.0"
}
},
{
Expand Down Expand Up @@ -104,8 +104,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/duckduckgo/privacy-dashboard",
"state" : {
"revision" : "bea4d750913ef82c10cd06e791686501c8e648e4",
"version" : "7.6.0"
"revision" : "c52bd5d851b1f8f0482e82b8720852670f525497",
"version" : "8.1.0"
}
},
{
Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGo/Menus/MainMenuActions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,7 @@ extension MainViewController: FindInPageDelegate {
extension AppDelegate: PrivacyDashboardViewControllerSizeDelegate {

func privacyDashboardViewControllerDidChange(size: NSSize) {
privacyDashboardWindow?.setFrame(NSRect(origin: .zero, size: size), display: true, animate: true)
privacyDashboardWindow?.setFrame(NSRect(origin: .zero, size: size), display: true, animate: false)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ final class PrivacyDashboardViewController: NSViewController {

struct Constants {
static let initialContentHeight: CGFloat = 489.0
static let reportBrokenSiteInitialContentHeight = 587.0 + 28.0
static let reportBrokenSiteInitialContentHeight = 406.0 + 28.0
static let initialContentWidth: CGFloat = 360.0
}

Expand Down Expand Up @@ -62,8 +62,6 @@ final class PrivacyDashboardViewController: NSViewController {
}, keyValueStoring: UserDefaults.standard)
}()

private let eventMapping = EventMapping<PrivacyDashboardEvents> { _, _, _, _ in }

private let permissionHandler = PrivacyDashboardPermissionHandler()
private var preferredMaxHeight: CGFloat = Constants.initialContentHeight
func setPreferredMaxHeight(_ height: CGFloat) {
Expand All @@ -73,6 +71,20 @@ final class PrivacyDashboardViewController: NSViewController {
var sizeDelegate: PrivacyDashboardViewControllerSizeDelegate?
private weak var tabViewModel: TabViewModel?

private let privacyDashboardEvents = EventMapping<PrivacyDashboardEvents> { event, _, parameters, _ in
let domainEvent: NonStandardPixel
switch event {
case .showReportBrokenSite: domainEvent = .brokenSiteReportShown
case .reportBrokenSiteShown: domainEvent = .brokenSiteReportShown
case .reportBrokenSiteSent: domainEvent = .brokenSiteReportSent
}
if let parameters {
PixelKit.fire(NonStandardEvent(domainEvent), withAdditionalParameters: parameters)
} else {
PixelKit.fire(NonStandardEvent(domainEvent))
}
}

init(privacyInfo: PrivacyInfo? = nil,
entryPoint: PrivacyDashboardEntryPoint = .dashboard,
privacyConfigurationManager: PrivacyConfigurationManaging = ContentBlocking.shared.privacyConfigurationManager) {
Expand All @@ -81,9 +93,8 @@ final class PrivacyDashboardViewController: NSViewController {
let toggleReportingManager = ToggleReportingManager(feature: toggleReportingFeature)
self.privacyDashboardController = PrivacyDashboardController(privacyInfo: privacyInfo,
entryPoint: entryPoint,
variant: .control,
toggleReportingManager: toggleReportingManager,
eventMapping: eventMapping)
eventMapping: privacyDashboardEvents)
super.init(nibName: nil, bundle: nil)
}

Expand All @@ -103,7 +114,7 @@ final class PrivacyDashboardViewController: NSViewController {
}

override func loadView() {
view = NSView(frame: NSRect(x: 0, y: 0, width: 360, height: 489))
view = ColorView(frame: NSRect(x: 0, y: 0, width: 360, height: 489), backgroundColor: NSColor(named: "PopoverBackgroundColor"))
initWebView()
}

Expand All @@ -128,6 +139,7 @@ final class PrivacyDashboardViewController: NSViewController {
configuration.preferences.setValue(true, forKey: "developerExtrasEnabled")
#endif
let webView = PrivacyDashboardWebView(frame: .zero, configuration: configuration)
webView.setValue(false, forKey: "drawsBackground")
self.webView = webView
view.addAndLayout(webView)

Expand Down Expand Up @@ -187,15 +199,6 @@ final class PrivacyDashboardViewController: NSViewController {

extension PrivacyDashboardViewController: PrivacyDashboardControllerDelegate {

func privacyDashboardController(_ privacyDashboardController: PrivacyDashboardController,
didSelectBreakageCategory category: String) {
// Not used in macOS
}

func privacyDashboardControllerDidRequestShowReportBrokenSite(_ privacyDashboardController: PrivacyDashboardController) {
// Not used in macOS: PixelKit.fire(GeneralPixel.privacyDashboardReportBrokenSite)
}

func privacyDashboardController(_ privacyDashboardController: PrivacyDashboardController,
didChangeProtectionSwitch protectionState: ProtectionState,
didSendReport: Bool) {
Expand Down Expand Up @@ -246,12 +249,9 @@ extension PrivacyDashboardViewController: PrivacyDashboardControllerDelegate {
dismiss()
}

func privacyDashboardControllerDidRequestShowAlertForMissingDescription(_ privacyDashboardController: PrivacyDashboardController) {
// Not used in macOS
}

func privacyDashboardControllerDidRequestShowGeneralFeedback(_ privacyDashboardController: PrivacyDashboardController) {
// Not used in macOS
dismiss()
FeedbackPresenter.presentFeedbackForm()
}

func privacyDashboardController(_ privacyDashboardController: PrivacyDashboardController,
Expand Down
6 changes: 6 additions & 0 deletions DuckDuckGo/Statistics/NonStandardPixel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ import Configuration
enum NonStandardPixel: PixelKitEventV2 {

case brokenSiteReport
case brokenSiteReportShown
case brokenSiteReportSent
case privacyDashboardReportBrokenSite
case emailEnabled
case emailDisabled
case emailUserPressedUseAddress
Expand All @@ -35,6 +38,9 @@ enum NonStandardPixel: PixelKitEventV2 {
var name: String {
switch self {
case .brokenSiteReport: return "epbf_macos_desktop"
case .brokenSiteReportSent: return "m_report-broken-site_sent"
case .brokenSiteReportShown: return "m_report-broken-site_shown"
case .privacyDashboardReportBrokenSite: return "mp_rb"
case .emailEnabled: return "email_enabled_macos_desktop"
case .emailDisabled: return "email_disabled_macos_desktop"
case .emailUserPressedUseAddress: return "email_filled_main_macos_desktop"
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/DataBrokerProtection/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ let package = Package(
targets: ["DataBrokerProtection"])
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "225.1.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "226.0.0"),
.package(path: "../SwiftUIExtensions"),
.package(path: "../AppKitExtensions"),
.package(path: "../XPCHelper"),
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/FeatureFlags/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ let package = Package(
targets: ["FeatureFlags"]),
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "225.1.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "226.0.0"),
],
targets: [
// Targets are the basic building blocks of a package, defining a module or a test suite.
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/NetworkProtectionMac/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ let package = Package(
.library(name: "VPNAppLauncher", targets: ["VPNAppLauncher"]),
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "225.1.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "226.0.0"),
.package(url: "https://github.com/airbnb/lottie-spm", exact: "4.4.3"),
.package(path: "../AppLauncher"),
.package(path: "../UDSHelper"),
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/NewTabPage/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ let package = Package(
targets: ["NewTabPage"]),
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "225.1.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "226.0.0"),
.package(path: "../WebKitExtensions"),
.package(path: "../Utilities"),
],
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/SubscriptionUI/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ let package = Package(
targets: ["SubscriptionUI"]),
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "225.1.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "226.0.0"),
.package(path: "../SwiftUIExtensions"),
.package(path: "../FeatureFlags")
],
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/WebKitExtensions/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ let package = Package(
),
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "225.1.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "226.0.0"),
.package(path: "../AppKitExtensions")
],
targets: [
Expand Down
16 changes: 16 additions & 0 deletions UnitTests/WebsiteBreakageReport/WebsiteBreakageReportTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,22 @@ class WebsiteBreakageReportTests: XCTestCase {
line: #line)
}

func testReportBrokenSiteShownPixel() {
fire(NonStandardEvent(NonStandardPixel.brokenSiteReportShown),
frequency: .standard,
and: .expect(pixelName: "m_report-broken-site_shown"),
file: #filePath,
line: #line)
}

func testReportBrokenSiteSentPixel() {
fire(NonStandardEvent(NonStandardPixel.brokenSiteReportSent),
frequency: .standard,
and: .expect(pixelName: "m_report-broken-site_sent"),
file: #filePath,
line: #line)
}

func testCommonSetOfFields() throws {
let breakage = BrokenSiteReport(
siteUrl: URL(string: "https://example.test/")!,
Expand Down

0 comments on commit e47e5ad

Please sign in to comment.