Issue/10930 email address error is preserved on rotation by FernandaCG · Pull Request #11003 · wordpress-mobile/WordPress-Android

FernandaCG · 2019-12-20T22:54:54Z

Fixes #10930

Description:
When rotate the device, the email error is preserved now.

Testing instructions:

Press Log In
Enter Email address and press Next. An error message will be shown
Rotate the screen.

I also test it in an physical device.

PR submission checklist:

I have considered adding unit tests where possible.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

peril-wordpress-mobile · 2019-12-20T22:55:18Z

	Messages
📖	This PR contains changes in the subtree `libs/login/`. It is your responsibility to ensure these changes are merged back into `wordpress-mobile/WordPress-Login-Flow-Android`. Follow these handy steps! WARNING: Make sure your git version is 2.19.x or lower - there is currently a bug in later versions that will corrupt the subtree history! `cd WordPress-Android` `git checkout fix/10930-email-error-dissapears-on-rotation` `git subtree push --prefix=libs/login/ https://github.com/wordpress-mobile/WordPress-Login-Flow-Android.git merge/WordPress-Android/11003` Browse to https://github.com/wordpress-mobile/WordPress-Login-Flow-Android/pull/new/merge/WordPress-Android/11003 and open a new PR.

Generated by 🚫 dangerJS

jd-alexander · 2020-01-06T14:48:08Z

Hi @FernandaCG the changes look good! I tested it and it works! However, due to the nature of these changes, they have to be pushed to the subtree project as stated in the comment above by peril-wordpress-mobile bot. So once you follow those instructions, your changes will be merged. Let me know if you need any help!

libs/login/WordPressLoginFlow/src/main/java/org/wordpress/android/login/LoginEmailFragment.java

jd-alexander · 2020-01-06T16:20:47Z

libs/login/WordPressLoginFlow/src/main/java/org/wordpress/android/login/LoginEmailFragment.java

    private String mGoogleEmail;
    private String mRequestedEmail;
    private boolean mIsSocialLogin;
+    private boolean mIsValidEmail = true;


Why set the email as true since you won't know if it is valid until it's checked. I think it's better to leave the default value of false.

FernandaCG · 2020-01-06T23:34:34Z

Hi @jd-alexander! I followed the instructions of peril-wordpress-mobile but when I execute the git subtree push command, I receive a 403 error.

jd-alexander · 2020-01-06T23:43:10Z

@FernandaCG Thanks! I just realized I will have to do it. I will do another review of your changes later today! Thank you so much for your patience 😄

jd-alexander

Thanks again @FernandaCG Once these final changes are done I will create the PR in the subtree project and get this merged in!

jd-alexander · 2020-01-09T17:20:12Z

libs/login/WordPressLoginFlow/src/main/java/org/wordpress/android/login/LoginEmailFragment.java

+    }
+
+    private void showErrorIfEmailInvalid() {
+        if (!mIsValidEmail && mEmailInput.getEditText().getText().length() > 0) {


Hi @FernandaCG Did you check the length of the EditText so that the warning doesn't get shown when the activity is loaded? I tested this and that is the behavior I saw. Good job 😄 We can take this a bit further though and utilize the Boolean object wrapper so that the initial value can be null before the email validation is done because if you click next and it shows the warning and you rotate the screen the warning disappears and we don't want that to happen 😄

I'm not so sure how to do that. I try validating the mEmailInput.getEditText().getText() == null or setting the mEmailInput to null, but not works.
I need some help 😄

Oh I meant to just change the private boolean mIsValidEmail = false; to private Boolean mIsValidEmail;so now you can check if it's null before showing the error. Makes sense? Let me know.

I understand, thank you for your help.
I just commit my changes

jd-alexander · 2020-01-09T17:20:48Z

libs/login/WordPressLoginFlow/src/main/java/org/wordpress/android/login/LoginEmailFragment.java

    private String mGoogleEmail;
    private String mRequestedEmail;
    private boolean mIsSocialLogin;
+    private boolean mIsValidEmail = false;


The default value of a boolean is already false so you don't have to specify 😄

jd-alexander · 2020-01-24T22:07:24Z

@FernandaCG Hi, are you still going to be working on this? Let me know. Thanks 😄

FernandaCG · 2020-01-24T22:32:58Z

Hi! Yes, I'm still working on that 😄

jd-alexander · 2020-02-03T03:55:45Z

libs/login/WordPressLoginFlow/src/main/java/org/wordpress/android/login/LoginEmailFragment.java

            mIsSocialLogin = savedInstanceState.getBoolean(KEY_IS_SOCIAL);
            mIsDisplayingEmailHints = savedInstanceState.getBoolean(KEY_IS_DISPLAYING_EMAIL_HINTS);
            mHasDismissedEmailHints = savedInstanceState.getBoolean(KEY_HAS_DISMISSED_EMAIL_HINTS);
+            if (savedInstanceState.getBoolean(VALIDITY_EMAIL) || !(savedInstanceState


Hi again! I don't think this if statement is needed. Similar to Boolean values above you can just grab the value and that will take care of it 😄 because if it's true or false then it will be set and if no value is detected it will be null.

So just having this line below works.
mIsValidEmail = savedInstanceState.getBoolean(VALIDITY_EMAIL);

jd-alexander

Hi! Thanks for the changes 😄You are almost there. I have some suggestions that are tagged above and below!

Also, when testing, if the error is shown and then you modify the contents of the EditText it's cleared so you shouldn't see it when the app gets rotated anymore. So you can clear the mIsValidEmail here

WordPress-Android/libs/login/WordPressLoginFlow/src/main/java/org/wordpress/android/login/LoginEmailFragment.java

Line 397 in 009f66e

mEmailInput.setError(null);

jd-alexander · 2020-02-03T04:00:04Z

libs/login/WordPressLoginFlow/src/main/java/org/wordpress/android/login/LoginEmailFragment.java

        outState.putBoolean(KEY_IS_DISPLAYING_EMAIL_HINTS, mIsDisplayingEmailHints);
        outState.putBoolean(KEY_HAS_DISMISSED_EMAIL_HINTS, mHasDismissedEmailHints);
-        outState.putBoolean(VALIDITY_EMAIL, mIsValidEmail);
+        if (mIsValidEmail != null) {


You don't have to check if this is null, you can simply just put it similar to the others above 😄

@jd-alexander Unfortunately that's not entirely correct, because it looks like mIsValidEmail is a Boolean and not a boolean. I think that created a bit of a confusion here.

@FernandaCG Could you update the mIsValidEmail's type to boolean and remove the null check here? You should be able to follow the example of mIsSocialLogin for the other parts as it should work similarly.

Let me know if either of you have any questions.

Thanks @oguzkocer I just realized my mistake 🤦‍♂ Yes, since Boolean is the wrapper type so the check can be made. I was looking at the values above and got confused.

jkmassel · 2020-02-10T22:22:51Z

We're freezing 14.2 today, so this PR is being bumped to 14.3. If you'd like it to ship with 14.2, please merge it into release/14.2 and ping me – I'll be happy to cut a new beta release.

oguzkocer · 2020-02-24T21:00:36Z

We're freezing 14.3 today, so this PR is being bumped to 14.4. If you'd like it to ship with 14.3, please merge it into release/14.3 and ping me – I'll be happy to cut a new beta release.

oguzkocer · 2020-03-09T21:51:46Z

We're freezing 14.4 today, so this PR is being bumped to 14.5. If you'd like it to ship with 14.4, please merge it into release/14.4 and ping me – I'll be happy to cut a new beta release.

@FernandaCG Could you follow up on this PR when you get a chance?

oguzkocer · 2020-03-23T18:03:32Z

This issue is targeting the current frozen milestone which is about to be released. I am bumping it to 14.6 which is when a PR that lands to develop in the next 2 weeks will be released. If it's more urgent than that, please make sure to target release/14.5 branch which will be available later today and update the milestone for the issue accordingly.

@jd-alexander It looks like @FernandaCG is not available to follow up on this PR, would you mind finishing it up instead?

jd-alexander · 2020-03-24T21:01:50Z

@jd-alexander It looks like @FernandaCG is not available to follow up on this PR, would you mind finishing it up instead?

I don't mind at all. I will add it to my tasks. Thanks 🙏

Issue/10930 email address error is preserved on rotation

026d8aa

Remove whitespaces

2b0653b

jd-alexander self-assigned this Jan 6, 2020

jd-alexander added [Type] Enhancement Login labels Jan 6, 2020

jd-alexander added this to the 14.0 milestone Jan 6, 2020

jd-alexander suggested changes Jan 6, 2020

View reviewed changes

libs/login/WordPressLoginFlow/src/main/java/org/wordpress/android/login/LoginEmailFragment.java Outdated Show resolved Hide resolved

jd-alexander reviewed Jan 6, 2020

View reviewed changes

libs/login/WordPressLoginFlow/src/main/java/org/wordpress/android/login/LoginEmailFragment.java Outdated Show resolved Hide resolved

jd-alexander reviewed Jan 6, 2020

View reviewed changes

Add requested changes

d6d8e95

jd-alexander suggested changes Jan 9, 2020

View reviewed changes

jkmassel modified the milestones: 14.0, 14.1 Jan 14, 2020

maxme modified the milestones: 14.1, 14.2 Jan 27, 2020

Fix validation in input of Email

009f66e

jd-alexander reviewed Feb 3, 2020

View reviewed changes

jd-alexander suggested changes Feb 3, 2020

View reviewed changes

jd-alexander reviewed Feb 3, 2020

View reviewed changes

jkmassel modified the milestones: 14.2, 14.3 Feb 10, 2020

oguzkocer removed this from the 14.3 ❄️ milestone Feb 24, 2020

oguzkocer added this to the 14.4 milestone Feb 24, 2020

oguzkocer modified the milestones: 14.4, 14.5 Mar 9, 2020

oguzkocer modified the milestones: 14.5, 14.6 Mar 23, 2020

jd-alexander mentioned this pull request Mar 26, 2020

Fix/10930 email error dissapears on rotation - continued #11537

Merged

3 tasks

ashiagr closed this in #11537 Apr 2, 2020

Conversation

FernandaCG commented Dec 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Dec 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jd-alexander commented Jan 6, 2020

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FernandaCG commented Jan 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jd-alexander commented Jan 6, 2020

Uh oh!

jd-alexander left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jd-alexander commented Jan 24, 2020

Uh oh!

FernandaCG commented Jan 24, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jd-alexander left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jd-alexander Feb 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkmassel commented Feb 10, 2020

Uh oh!

oguzkocer commented Feb 24, 2020

Uh oh!

oguzkocer commented Mar 9, 2020

Uh oh!

oguzkocer commented Mar 23, 2020

Uh oh!

jd-alexander commented Mar 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

FernandaCG commented Dec 20, 2019 •

edited

Loading

peril-wordpress-mobile bot commented Dec 20, 2019 •

edited

Loading

FernandaCG commented Jan 6, 2020 •

edited

Loading

jd-alexander left a comment •

edited

Loading

jd-alexander Feb 21, 2020 •

edited

Loading