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

Add geoswitching usage pixels #2222

Closed
wants to merge 2 commits into from
Closed

Conversation

graeme
Copy link
Contributor

@graeme graeme commented Dec 5, 2023

Task/Issue URL: https://app.asana.com/0/0/1206107031155633/f

Description:

Adds the Pixels described in the parent of the linked PR task to monitor Geoswitching during the Waitlist beta

Steps to test this PR:

  1. Run the app in the debugger
  2. Make sure you’re an internal user and you’ve used an invite code to access NetP
  3. Go to Settings -> Network Protection -> VPN Settings -> VPN Location
  4. You should see the Pixel for opening Geoswitching in the debugger
  5. Select a location
  6. ** You should see the Pixel for selecting custom int the debugger
  7. Select Nearest
  8. ** You should see the Pixel for selecting nearest in the debugger
  9. There is also a pixel for if there is no list returned. Perhaps get creative with how you test that, but my guess is we won’t get many of these due to the fact they’ll be most likely to occur when offline.

Copy Testing:

  • Use of correct apostrophes in new copy, ie rather than

Orientation Testing:

  • Portrait
  • Landscape

Device Testing:

  • iPhone SE (1st Gen)
  • iPhone 8
  • iPhone X
  • iPhone 14 Pro
  • iPad

OS Testing:

  • iOS 14
  • iOS 15
  • iOS 16

Theme Testing:

  • Light theme
  • Dark theme

Internal references:

Software Engineering Expectations
Technical Design Template

@graeme graeme requested a review from samsymons December 5, 2023 20:26
@@ -68,6 +72,9 @@ final class NetworkProtectionVPNLocationViewModel: ObservableObject {
@MainActor
private func reloadList() async {
guard let list = try? await locationListRepository.fetchLocationList() else { return }
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about firing off a debug error pixel when this call throws?

Copy link
Contributor

@samsymons samsymons left a comment

Choose a reason for hiding this comment

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

Changes look good, I've left one optional comment about adding an additional pixel for errors when fetching the location list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants