-
Notifications
You must be signed in to change notification settings - Fork 116
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
[Login] Use magic link for suspicious emails #15444
base: trunk
Are you sure you want to change the base?
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Generated by 🚫 Danger |
|
7eecc65
to
1102b53
Compare
1102b53
to
f3ea590
Compare
|
||
// MARK: UI Setup | ||
private extension MagicLinkRequestViewController { | ||
private func configureStackView() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't use a xib file, as I still don't feel familiar with XCode UI for them, and I didn't use SwiftUI just to make sure the content is correctly styled to match the other screens. If you have any remarks about this please share them.
f3ea590
to
e033e5f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this, Hicham! Great job navigating through the authenticator library code. 👏
I have a the following questions about the UI before approving this PR.

- Would the wording "store credentials" in the above screen create confusion about where to enter their store's wporg credentials or WPCOM username and password?
- I see that we pre-fill the email in the above screen. AFAIK, logging in using WPCOM email and password will not work for a suspicious email. Should we avoid pre-filling the email.

The title of "Use username and password instead" button isn't fully visible while using large fonts.
) | ||
} | ||
|
||
private func fallbackButtonTitle() -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private func fallbackButtonTitle() -> String { | |
func fallbackButtonTitle() -> String { |
nit: This method is already inside a private extension.
pinSubviewToHorizontalEdges(primaryButton) | ||
} | ||
|
||
private func createHeader() -> UIView { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: We can remote the private keyword as this method is already inside a private extension. Applies to other methods inside this extension.
private func createHeader() -> UIView { | ||
let headerStackView = UIStackView() | ||
headerStackView.spacing = 16 | ||
headerStackView.layoutMargins = UIEdgeInsets(top: 24, left: 16, bottom: 24, right: 16) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest moving the spacing and padding related constants used in this view inside a Layout enum like we follow in SwiftUI views. WDYT?
Closes: #15442
Description
This PR implements the first part of the project, updating the suspicious email flow to use magic links in the login.
In my spike initially, I suggested reusing the screen
LoginLinkRequestViewController
, but when working on this, I found out that this screen is not part of the unified login flow, so I added a new screen, and I matched its design to the Android one.Steps to reproduce
TC1: Confirming navigation to magic link request screen
For all the following TCs, apply the following patch
TC2: Magic Link login flow
TC3: Fallback to username/password
Testing information
TC1: Confirm that the app shows the Magic Link request screen.
TC2: Confirm that the email was sent, and that the following was tracked:
TC3: Confirm that signing using site credentials work, and that the following was tracked after tapping on the fallback button:
Screenshots
Simulator.Screen.Recording.-.iPhone.16.-.2025-03-28.at.01.56.28.mp4
RELEASE-NOTES.txt
if necessary.Reviewer (or Author, in the case of optional code reviews):
Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement: