Skip to content
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

Privacy Dashboard 8.0.0: Improved breakage form #1160

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

mgurgel
Copy link
Collaborator

@mgurgel mgurgel commented Jan 14, 2025

Required:

Task/Issue URL: https://app.asana.com/0/0/1209127604215386/f
iOS PR: duckduckgo/iOS#3802
macOS PR: duckduckgo/macos-browser#3729
What kind of version bump will this require?: Minor

Description:

Upgrades Privacy Dashboard to 8.0.0, which contains the new site breakage flow as can be previewed here

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 "Report Broken Site” (iOS) or “Send Feedback > Report Broken Site” (macOS)
  1. Confirm that the breakage form matches the Figma or the web preview

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

OS Testing:

  • iOS 14
  • iOS 15
  • iOS 16
  • macOS 10.15
  • macOS 11
  • macOS 12

Internal references:

Software Engineering Expectations
Technical Design Template

@afterxleep afterxleep self-requested a review January 15, 2025 14:29
Copy link
Collaborator

@afterxleep afterxleep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM to me. Both iOS and macOS working as described.

I do see some removals related to a previous experiment. I'm just mentioning to confirm it's intentional as is not mentioned in the description.

jaceklyp and others added 11 commits January 17, 2025 14:35
<!--
Note: This checklist is a reminder of our shared engineering
expectations.
-->

Please review the release process for BrowserServicesKit
[here](https://app.asana.com/0/1200194497630846/1200837094583426).

**Required**:

Task/Issue URL:
https://app.asana.com/0/1206594217596623/1208729801503622/f
iOS PR: 
macOS PR: 
What kind of version bump will this require?: Major/Minor/Patch

**Optional**:

CC: @jaceklyp 

**Description**:


When triggering `window.onGetToggleReportOptionsResponse` on the Privacy
Dashboard, the native side would create a variable `json` in the global
scope before sending it as a parameter to the callback. If
GetToggleReportOptionsResponse was even invoked again in the same
webview, the attempt to declare another variable with the same name
(`json`) would result in a fatal error.

This PR removes the declaration of the `json` variable, instead passing
the data directly as a parameter to the callback.

**Steps to test this PR**:
1. Check out branch `jacek/new-breakage-form` of the iOS app
2. Set BSK to point to `mgurgel/new-breakage-form`
3. Run the app
4. Click on three dots menu 
5. Select Report Broken Site
6. Choose a category and a subcategory
7. The form should be visible
8. Navigate back to category selection page
9. Choose a category again
10. The form should still be visible

<!—
Before submitting a PR, please ensure you have tested the combinations
you expect the reviewer to test, then delete configurations you *know*
do not need explicit testing.

Using a simulator where a physical device is unavailable is acceptable.
—>

**OS Testing**:

* [ ] iOS 14
* [ ] iOS 15
* [ ] iOS 16
* [ ] macOS 10.15
* [ ] macOS 11
* [ ] macOS 12

—
###### Internal references:
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
Remove experimental code
Update resolved

Remove unused method
Update Resolved
@mgurgel mgurgel force-pushed the mgurgel/improved-breakage-form branch from c77e36e to 13ccadd Compare January 17, 2025 14:36
**Required**:

Task/Issue URL:
https://app.asana.com/0/1206594217596623/1209173355503842/f
iOS PR: duckduckgo/iOS#3822
macOS PR: duckduckgo/macos-browser#3744
What kind of version bump will this require?: Minor

**Description**:

Implements Privacy Dashboard message `ReportBrokenSiteShown` which will
trigger a pixel on macOS and iOS

This PR contains a fix for
#1160 which is why
it is stacked on it. Please let me know if this is not the best way to
have this code reviewed.

**Steps to test this PR**:
1. Invoke 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 "Report Broken Site” (iOS)

2. Confirm that the pixel `m_report-broken-site_shown` was fired
3. Submit a report
4. Confirm that the pixels `epbf` (iOS) / `epbf_macos_desktop` (macOS)
and `m_report-broken-site_sent` were fired

<!--
Before submitting a PR, please ensure you have tested the combinations
you expect the reviewer to test, then delete configurations you *know*
do not need explicit testing.

Using a simulator where a physical device is unavailable is acceptable.
-->

**OS Testing**:

* [ ] iOS 14
* [ ] iOS 15
* [ ] iOS 16
* [ ] macOS 10.15
* [ ] macOS 11
* [ ] macOS 12

---
###### Internal references:
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants