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

Gh 3293/update safe settings #3304

Merged
merged 5 commits into from
Aug 24, 2023
Merged

Conversation

MouazAlzahabi
Copy link
Contributor

@MouazAlzahabi MouazAlzahabi commented Aug 22, 2023

Handles #3290 #3293 #3294 #3295

Changes proposed in this pull request:

  • Create All screens related to MFA
  • Tracking will be fixed in another pr
  • Navigation is not accurate, we need to integrate the SDK first

@MouazAlzahabi MouazAlzahabi self-assigned this Aug 22, 2023
@DmitryBespalov
Copy link
Contributor

pr is too big

@MouazAlzahabi MouazAlzahabi marked this pull request as ready for review August 23, 2023 15:30
@codecov-commenter
Copy link

codecov-commenter commented Aug 23, 2023

Codecov Report

Merging #3304 (c09380b) into main (8197b41) will decrease coverage by 1.85%.
Report is 1791 commits behind head on main.
The diff coverage is 0.53%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff            @@
##            main   #3304      +/-   ##
========================================
- Coverage   8.75%   6.90%   -1.85%     
========================================
  Files        620     779     +159     
  Lines      31233   42295   +11062     
========================================
+ Hits        2733    2920     +187     
- Misses     28500   39375   +10875     
Files Changed Coverage Δ
Multisig/App/AppDelegate+Messaging.swift 0.00% <0.00%> (ø)
Multisig/App/AppDelegate.swift 0.00% <0.00%> (ø)
Multisig/App/SceneDelegate.swift 0.00% <0.00%> (-52.71%) ⬇️
Multisig/App/RemoteNotificationHandler.swift 7.90% <1.66%> (-11.66%) ⬇️

... and 458 files with indirect coverage changes

@DmitryBespalov
Copy link
Contributor

DmitryBespalov commented Aug 23, 2023

If we leave it like this, you'll get the design bug issue , so here are some minor of the differences I found:

Set up MFA

  • missing "Learn More"

Set Password

  • do we need the text under the title? (in figma there is a text)

Success

  • title style is different from the design
  • the rows are different from the design - needs more space inside the borders

@MouazAlzahabi
Copy link
Contributor Author

MouazAlzahabi commented Aug 23, 2023

@DmitryBespalov all solved except the missing url, it's actually intended because we don't have the url yet, that's why the button is hidden

@MouazAlzahabi MouazAlzahabi merged commit ae11b1e into main Aug 24, 2023
@MouazAlzahabi MouazAlzahabi deleted the GH-3293/Update-Safe-settings branch August 24, 2023 09:06
@github-actions github-actions bot locked and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants