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

Add message to allow user to login manually in the enter email screen. #1693

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

notandyvee
Copy link
Contributor

Fix #1689

This PR adds a new label on the screen to enter your email for a magic link. That lable will have a link to login manually, meaning with password instead.

Test

  • On app launch click login.
  • Under the "Log In With Email" button, there is a new label, "We'll email you a code to log in, or you can log in manually".
  • The "log in manually" is hyperlinked.
  • Clicking the hyperlink should take you to login with password screen.

Release

N/A

@notandyvee notandyvee added the task This issue is some sort of task or a spike of time not directly related to a feature. label Sep 11, 2024
@notandyvee notandyvee added this to the 2.35 milestone Sep 11, 2024
@dangermattic
Copy link
Collaborator

dangermattic commented Sep 11, 2024

1 Warning
⚠️ This PR is assigned to the milestone 2.34 ❄️. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Sep 11, 2024

📲 You can test the changes from this Pull Request in Simplenote Android by scanning the QR code below to install the corresponding build.

App Name Simplenote Android
Build TypeDebug
Commitf1a36ae
Direct Downloadsimplenote-android-prototype-build-pr1693-f1a36ae-0191ed52-2066-419b-8108-359aa96ea3ed.apk

@notandyvee notandyvee changed the base branch from trunk to release/2.34 September 11, 2024 19:45
@notandyvee notandyvee modified the milestones: 2.35, 2.34 ❄️ Sep 11, 2024
Copy link
Contributor

@roundhill roundhill left a comment

Choose a reason for hiding this comment

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

Worked pretty well, requesting a change to support localizing the linked text.

Also, when you press the back arrow from the password field, you end up at the welcome screen. Is that expected? I think it works ok but I expected to be back to the email login form.

@@ -101,6 +104,17 @@ class SignInFragment: MagicLinkableFragment() {
"wpcc_button_press_signin_activity"
)
}
val manualLoginTextView = view.findViewById<TextView>(R.id.sign_in_login_manually)
val message = getString(R.string.signin_login_with_email_manually);
val span = StrUtils.generateClickableSpannableString(LOGIN_MANUALLY_SUBSTRING, message
Copy link
Contributor

@roundhill roundhill Sep 11, 2024

Choose a reason for hiding this comment

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

I don't think this will work for localized strings... can it instead work like this string does?

<string name="simperium_footer_signup">By creating an account, you agree to our %1$s%2$s%3$sTerms and Conditions%4$s</string>

And the code that formats it:

String.format(
                if (this.isLogin) this.resources.getString(R.string.simperium_footer_login) else this.resources.getString(
                    R.string.simperium_footer_signup
                ), "<span style=\"color:#", colorLink, "\">", "</span>"
            )

Something like that?

@notandyvee
Copy link
Contributor Author

Also, when you press the back arrow from the password field, you end up at the welcome screen. Is that expected?

Unfortunately it is. This is a remnant of the old way of doing things. I can update it, but would require me to replace the user and password screen with one that isn't made by simperium. Or we can ticket it to fix it. Basically a bigger fix.

I don't think this will work for localized strings... can it instead work like this string does?

And great catch! This was an oversight. You are correct it wouldn't work for localized strings. The fix could be to localize the substring as well. Not sure if there is risk with the people translating and needing to translate the last part.

As to why I opted out of doing what you are suggesting is because the entire TextView is clickable. It felt off to highlight part of the text but make the whole thing clickable. But if you don't mind, I can do it that way instead. You let me know @roundhill .

@roundhill
Copy link
Contributor

As to why I opted out of doing what you are suggesting is because the entire TextView is clickable. It felt off to highlight part of the text but make the whole thing clickable. But if you don't mind, I can do it that way instead. You let me know @roundhill .

Ah, I see. I think it's ok to make it all clickable so that the localization can work properly.

@notandyvee
Copy link
Contributor Author

@roundhill I made the changes. Using the old method for now. Will create an issue to explore using a ClickableSpan instead of a click action on the text view.

@roundhill roundhill self-requested a review September 16, 2024 15:02
Copy link
Contributor

@roundhill roundhill left a comment

Choose a reason for hiding this comment

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

Looks good to me, and tested well!

@notandyvee notandyvee merged commit 1c149f9 into release/2.34 Sep 16, 2024
15 checks passed
@notandyvee notandyvee deleted the andy/simplenote-android-issue-1689 branch September 16, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
task This issue is some sort of task or a spike of time not directly related to a feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add log in with email label with option to sign in with password
4 participants