PM-10267 PM-12988 - Fix saving master password generation and match swipe/close gesture ux

phil-livefront commented 1 month ago

🎟️ Tracking

PM-12988 PM-10267

📔 Objective

The close button and swipe-down gesture in the Create Account flow led to inconsistent navigation, landing users on different screens. This issue stemmed from how we were using dismissPresented in the AuthCoordinator. When navigating via an email verification deep link, it wasn't a problem because the stack was cleared before showing the view. However, during QA testing—where the email verification step was bypassed—this bug surfaced.
The same root cause affected the "Save" button functionality, where users weren’t being returned to the CompleteRegistrationView. Since we were using presentedViewController, all current modals were dismissed, sending testers back to the Login screen.
In this solution we are using the same UINavigationController used in the MasterPasswordGuidanceView and pushing the MasterPasswordGeneratorView on top of the existing stack instead of making a new one. This allowed us to use the new implemented dismissPresented function which dismisses only the top VC as opposed to closing more than 1.
I discussed the proposed solution with design, and they've confirmed it effectively resolves all the issues.

📸 Screenshots

https://github.com/user-attachments/assets/1515484f-acda-4a64-aa72-ff05d730d7bb

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 89.12%. Comparing base (cc8039a) to head (184fdb3). Report is 1 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1010 +/- ## ========================================== + Coverage 89.10% 89.12% +0.02% ========================================== Files 657 657 Lines 41317 41319 +2 ========================================== + Hits 36814 36825 +11 + Misses 4503 4494 -9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 1 month ago

Checkmarx One – Scan Summary & Details – 9fadfb87-2007-428c-b6da-e634b3b70f37

bitwarden / ios