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

Implement barebones NetP UI #1840

Merged
merged 8 commits into from
Jul 19, 2023
Merged

Conversation

graeme
Copy link
Contributor

@graeme graeme commented Jul 18, 2023

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

Description:

This will be the final PR of the iOS: Update NetworkProtection package to support iOS project. It provides the absolute minimum UI required to be able to test the operation of the NetworkProtectionPacketTunnelProvider and related components on iOS.

Note that the UI here is very much temporary, behind feature flags and will be almost entirely replaced in iOS: MVP TestFlight Build.

Steps to test this PR:

  1. Check out this branch
  2. Embed the PacketTunnelProvider extension in the DuckDuckGo app target’s build phases.
  3. Run the app in debug
  4. Ensure you are an internal user
  5. Navigate to Settings -> Network Protection
  6. Enter an invite code
  7. On successful invite code redemption, toggle the switch to activate Network Protection
  8. Allow the system permissions prompt.
  9. The status should update to Connected
  10. Navigate to iOS Settings -> General -> VPN & Device Management -> VPN
  11. Ensure that the VPN is connected
  12. Navigate back to DuckDuckGo
  13. Use an IP address checker website to ensure that the location is that of our ID3 servers (probably Rotterdam)

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

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

}

@MainActor
func clear() async {
Copy link
Contributor Author

@graeme graeme Jul 18, 2023

Choose a reason for hiding this comment

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

Only for testing while still behind a feature flag. In the final implementation, this will be moved to an item in the Debug menu

@graeme
Copy link
Contributor Author

graeme commented Jul 18, 2023

This currently depends on #1837 to allow it to full function according to the test steps.

@graeme graeme force-pushed the graeme/implement-barebones-netp-ui branch from 30d9de4 to 7d98062 Compare July 18, 2023 15:23
@graeme graeme changed the base branch from develop to diego/remove-distributed-notification-code-from-netp-bsk July 18, 2023 15:24
@graeme graeme force-pushed the graeme/implement-barebones-netp-ui branch from 7d98062 to 30d9de4 Compare July 18, 2023 19:22
@graeme graeme changed the base branch from diego/remove-distributed-notification-code-from-netp-bsk to develop July 18, 2023 19:23
@graeme graeme marked this pull request as ready for review July 18, 2023 19:23
@samsymons samsymons self-assigned this Jul 18, 2023
@graeme graeme requested a review from samsymons July 18, 2023 19:27
HStack {
if let status = statusModel.statusMessage {
Text(status)
.foregroundColor(statusModel.isNetPEnabled ? .green : .red)
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to anyone else reviewing this: this is a dev UI that we're using for NetP, the next project will build the final UI and will be using DRK rather than hardcoded colours.

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.

LGTM!

@samsymons samsymons removed their assignment Jul 19, 2023
@@ -608,7 +608,6 @@ In addition to the details entered into this form, your app issue report will co
// MARK: Network Protection

public static let netPNavTitle = NSLocalizedString("netP.title", value: "Network Protection", comment: "Title for the Network Protection feature")
public static let netPCellDetail = NSLocalizedString("netP.cell.detail", value: "Hide your location and conceal your online activity", comment: "Detail string describing what NetP is")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed offline: this will not go into the final feature so does not need to be translated

@graeme graeme merged commit 127333b into develop Jul 19, 2023
6 checks passed
@graeme graeme deleted the graeme/implement-barebones-netp-ui branch July 19, 2023 16:18
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.

2 participants