This repository was archived by the owner on Feb 24, 2025. It is now read-only.
Update onboarding-related error states #1504
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
) Task/Issue URL: https://app.asana.com/0/0/1205247616211793/f Tech Design URL: CC: ## Stack PR - Feature branch: `diego/feature-network-protection-onboarding` - #1465 (this PR) - #1466 ## Description Just adds entitlements to two agent Apps in REVIEW and CI configurations, since we'll use `UserDefaults` for sharing data between them. This was disabled initially for NetP as we weren't supporting builds other than DEBUG and RELEASE.
… the tunnel view. (#1466) Task/Issue URL: https://app.asana.com/0/0/1205247622252423/f Tech Design URL: CC: ## Stack PR: - Feature branch: `diego/feature-network-protection-onboarding` - #1465 - #1466 (this PR) ## Description: `NetworkProtectionStatusView` has grown to become quite hard to maintain, and the same applies for the view's model. I need to make some changes to be able to disable specifically the tunnel controller part of the view, and these changes make it a bit easier and cleaner. This PR splits it into a parent composite view, and a child `TunnelControllerView`. ## Scope: This PR concentrates refactoring work, meaning that none of the changes should affect functionality in any way. The only intentional change has been to use the menu item attributes that were missing. I'll mark this change with a comment.
Task/Issue URL: https://app.asana.com/0/0/1205257341292466/f Tech Design URL: CC: ## Stack: - Feature branch: `diego/feature-network-protection-onboarding` - #1474 (this PR) - #1464 ## Description: Implements the shared `UserDefaults` that will store the onboarding status, and the debug menu options to test it.
Task/Issue URL: https://app.asana.com/0/0/1205257644127197/f ## Stack - Feature branch: `diego/feature-network-protection-onboarding` - #1474 - #1464 (this PR) ## Description Implements the new onboarding UI/UX.
Task/Issue URL: https://app.asana.com/0/0/1205268328062294/f ## Description Addresses design adjustments requested for the new Network Protection onboarding views. Work done: - Update to the latest design changes - Update to the latest assets - Update to the latest copy - The Allow-VPN step will now only trigger if the user disallows creating the VPN configuration. - BigSur-specific onboarding screenshot
Contributor
Task/Issue URL: https://app.asana.com/0/0/1205275221447702/f ## Description: This PR offers a solution to the System Extension activation issues we currently have. It also modifies the startup flow: now when the user enables the system extension they'll still need to start the tunnel manually.
samsymons
reviewed
Aug 20, 2023
Comment on lines
+149
to
+150
| // Please note that shared defaults MUST have a name that matches exactly their value, | ||
| // or else KVO will just not work as of 2023-08-07 |
Collaborator
There was a problem hiding this comment.
Are we able to add more context here as to why the date is important? (i.e. link to a PR or Asana task with more info)
samsymons
reviewed
Aug 20, 2023
| ] | ||
|
|
||
| let popover = NetworkProtectionPopover(controller: controller, statusReporter: statusReporter, menuItems: menuItems) | ||
| let onboardingStatusPublisher = UserDefaults.shared!.networkProtectionOnboardingStatusPublisher |
Collaborator
There was a problem hiding this comment.
Are we sure that UserDefaults.shared can never be nil here? If so, could we move this into the UserDefaults extension where shared is declared?
samsymons
approved these changes
Aug 20, 2023
Collaborator
samsymons
left a comment
samsymons
left a comment
There was a problem hiding this comment.
Looks good, we've reviewed this via stacked PRs but I gave it one more pass, everything works well on my end. Nice work!
Contributor
Author
|
Thank you @samsymons. I haven't replied yet because I'm quite busy with the maintenance rotation but I'll address your comments. I may also need to introduce a few more changes, so if that's the case I'll ping you back. |
samsymons
added a commit
that referenced
this pull request
Aug 23, 2023
# By Diego Rey Mendez (7) and others # Via Sam Symons (2) and others * develop: (26 commits) Improve Sync-related database cleaning logic (#1529) Update onboarding-related error states (#1504) Prevents launching our menu agent without an auth code. (#1516) Autofill UI letter icons (#1535) Cleans up some code (#1517) Revert "Autofill Letter Icons" (#1534) Adds remote pre-commit installer, which includes automatic fix for linter (#1369) Autofill Letter Icons (#1475) change context menu for mailto links (#1513) Updates the version to 1.53.1 Updated the embedded files for 1.53.1 Update the phased rollout tester to avoid caching the config (#1520) Require Duck Player scheme URL to be passed from YouTube Overlay User Script (#1519) Add pixels related to Duck Player usage (#1515) only allow error reloads on http(s) urls (#1523) Standardize TDS Loading Error handling (#1524) Move pixel sender logic into the main view controller (#1528) Update the phased rollout tester to avoid caching the config (#1520) Set version to 1.52.3. Move pixel sender logic into the main view controller (#1528) ... # Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved # DuckDuckGo/AppDelegate/AppDelegate.swift # DuckDuckGo/Common/Localizables/UserText.swift # DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Task/Issue URL: https://app.asana.com/0/0/1205301030177476/f
Description:
Implements the new onboarding for Network Protection.
Incremental PR reviews
This is a list of the individual PRs (and reviews) that make up this feature:
Testing:
The design review is complete, and we're currently doing a ship review, so this PR should mostly be about signing off what's already been reviewed.
Internal references:
Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation