Add realtime site address validation by shiki · Pull Request #10255 · wordpress-mobile/WordPress-Android

shiki · 2019-07-19T15:39:48Z

Closes #9720.

This adds realtime validation for the site address in Login or Add Site. This is based on the iOS implementation described in wordpress-mobile/WordPress-iOS#10294 (comment).

The error is only shown if the user has not typed anything after 2 seconds
The Next button is only enabled if the URL is valid

To implement this, I moved all the site address logic to a new class, LoginSiteAddressValidator. The class encapsulates the validation, cleaning, and error reporting using debounce. A corresponding unit test, LoginSiteAddressValidatorTest, has also been added.

Known Issues

The LoginSiteAddressFragment doesn't retain any error message shown below the text field after configuration changes. I didn't attempt to fix this. I thought it would be a huge change.

Testing

Adding a Site

Click on Sites → Switch Site.
Click on the + button at the top right.
Enter a site address.

Logging in with a self-hosted site

On Login page, click on Login.
Click on Log in by entering your site address.
Enter a self-hosted site address.

On both scenarios, please test against the specs described above. Please also test that there are no regressions in logging in with a valid site.

Reviewing

Only 1 reviewer is needed but anyone can review.

Editorial

We're hoping to get a review for the error message that is shown when the URL is invalid:

Enter a valid URL eg. example.com.

Release Notes

If there are user-facing changes, I have added an item to RELEASE-NOTES.txt.

Tasks

Review
Merge if approved
Merge changes back to WordPress-Login-Flow-Android
- Submitted Merge latest changes from WordPress-Android WordPress-Login-Flow-Android#24
Apply copy changes to iOS
- Created an issue Update error message when entering an invalid site address WordPress-iOS#12188

This time, the _Next_ button is not enabled until the user has entered an invalid site address.

The invalid address error message will only be shown 2 seconds after the user stopped typing.

This is to match the new iOS error message.

peril-wordpress-mobile · 2019-07-19T15:39:54Z

	Messages
📖	This PR contains changes in the subtree `libs/login/`. It is your responsibility to ensure these changes are merged back into `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 issue/9720-site-url-validation` `git subtree push --prefix=libs/login/ https://github.com/wordpress-mobile/WordPress-Login-Flow-Android.git merge/WordPress-Android/10255` Browse to https://github.com/wordpress-mobile/WordPress-Login-Flow-Android/pull/new/merge/WordPress-Android/10255 and open a new PR.

Generated by 🚫 dangerJS

shiki · 2019-07-19T20:39:02Z

...n/WordPressLoginFlow/src/main/java/org/wordpress/android/login/LoginSiteAddressFragment.java

            } else {
                AppLog.e(T.API, "onDiscoveryResponse has error: " + event.error.name()
-                        + " - " + event.error.toString());
+                                + " - " + event.error.toString());


Auto-formatted. ¯\_(ツ)_/¯

benhuberman · 2019-07-20T05:38:53Z

For the error message, I'd go with something that explains the error a bit more fully and avoids abbreviations (like e.g.). For example:

Please enter a complete website address, like example.com.

This doesn't strike me as significantly longer than the current version, but let me know if it creates a length issue.

This is based on Editorial’s advice.

shiki · 2019-07-22T17:37:39Z

Thank you, @benhuberman. I've applied your suggestion. This is what it looks like now:

benhuberman · 2019-07-22T17:49:32Z

I like how it looks, @shiki.

I'm sorry I didn't catch this when reviewing this the first time, but I just saw that the first sentence on that screen could also use a small tweak -- the your/you sounds redundant (and it's strongly assumed that you'd only ever connect your own site). I'd go with this instead:

Enter the address of the WordPress site you'd like to connect.

shiki · 2019-07-22T18:08:31Z

No worries at all @benhuberman. Updated:

AmandaRiu

Tested through the noted scenarios and reviewed the code. LGTM

shiki added 6 commits July 18, 2019 21:30

Move LoginSiteAddress validation to a class

fe396ca

This time, the _Next_ button is not enabled until the user has entered an invalid site address.

Do not react to keyboard enter if site is invalid

00fdc98

Site Address: Show an error message after 2 sec

3020aec

The invalid address error message will only be shown 2 seconds after the user stopped typing.

Do not report an error if the text is empty

244ffe2

Replace login_invalid_site_url

495db16

This is to match the new iOS error message.

Delete unused R.string.login_empty_site_url

d3d4752

shiki added the [Type] Enhancement label Jul 19, 2019

shiki added this to the 13.0 milestone Jul 19, 2019

Add tests for LoginSiteAddressValidator

e350913

shiki commented Jul 19, 2019

View reviewed changes

shiki changed the title ~~Issue/9720 site url validation~~ Add realtime site address validation Jul 19, 2019

shiki mentioned this pull request Jul 19, 2019

Groundskeeping (Android) Master Issue: Jul 15 to Jul 19, 2019 #10217

Closed

shiki marked this pull request as ready for review July 19, 2019 21:02

shiki added the [Status] Needs Copy Review label Jul 19, 2019

shiki requested review from AmandaRiu and aforcier July 19, 2019 21:07

Update login_invalid_site_url

69661d1

This is based on Editorial’s advice.

shiki removed the [Status] Needs Copy Review label Jul 22, 2019

Fix redundancy in R.string.enter_site_address

2401e41

AmandaRiu assigned AmandaRiu and unassigned AmandaRiu Jul 22, 2019

AmandaRiu approved these changes Jul 22, 2019

View reviewed changes

shiki merged commit 2a0c965 into develop Jul 22, 2019

shiki deleted the issue/9720-site-url-validation branch July 22, 2019 19:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add realtime site address validation#10255

Add realtime site address validation#10255
shiki merged 9 commits intodevelopfrom
issue/9720-site-url-validation

shiki commented Jul 19, 2019 •

edited

Loading

Uh oh!

peril-wordpress-mobile bot commented Jul 19, 2019

Uh oh!

shiki Jul 19, 2019

Uh oh!

benhuberman commented Jul 20, 2019

Uh oh!

shiki commented Jul 22, 2019 •

edited

Loading

Uh oh!

benhuberman commented Jul 22, 2019

Uh oh!

shiki commented Jul 22, 2019

Uh oh!

AmandaRiu left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

shiki commented Jul 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Known Issues

Testing

Adding a Site

Logging in with a self-hosted site

Reviewing

Editorial

Release Notes

Tasks

Uh oh!

peril-wordpress-mobile bot commented Jul 19, 2019

Uh oh!

shiki Jul 19, 2019

Choose a reason for hiding this comment

Uh oh!

benhuberman commented Jul 20, 2019

Uh oh!

shiki commented Jul 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benhuberman commented Jul 22, 2019

Uh oh!

shiki commented Jul 22, 2019

Uh oh!

AmandaRiu left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shiki commented Jul 19, 2019 •

edited

Loading

shiki commented Jul 22, 2019 •

edited

Loading