A4A-Partner Directory: Highlight invalid form fields
Resolves https://github.com/Automattic/automattic-for-agencies-dev/issues/716
Proposed Changes
This PR highlights the invalid field values. In this case, we check/validate the URLs and the email.
The validator highlight text will be displayed if you type at least three (3) characters.
Note
In a follow-up PR, the Agency Details form could be saved at any moment without completing the whole form, even with no valid fields.
- In another PR, on each field update, the details form will be saved.
Why are these changes being made?
Testing Instructions
- Check the code
- Try to introduce an invalid URL (Any form field for URL, in the Details and Expertise form)
- Try to introduce an invalid Email (Company email, in the Details form)
Pre-merge Checklist
- [ ] Has the general commit checklist been followed? (PCYsg-hS-p2)
- [ ] Have you written new tests for your changes?
- [ ] Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
- [ ] Have you checked for TypeScript, React or other console errors?
- [ ] Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
- [ ] Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
- [ ] For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?
Jetpack Cloud live (direct link)
|
|
https://calypso.live?image=registry.a8c.com/calypso/app:build-112953&env=jetpack |
Automattic for Agencies live (direct link)
|
|
https://calypso.live?image=registry.a8c.com/calypso/app:build-112953&env=a8c-for-agencies |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:
Sections (~1111 bytes added 📈 [gzipped])
name parsed_size gzip_size
a8c-for-agencies-partner-directory +3724 B (+0.8%) +1059 B (+0.7%)
a8c-for-agencies-sites +230 B (+0.0%) +52 B (+0.0%)
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.
Legend
What is parsed and gzip size?
Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.
Generated by performance advisor bot at iscalypsofastyet.com.
Looks good so far, @cleacos.
Have a few comments on the behavior and on styling though, so leaving it up to you whether to address here or on a separate PR.
Behavior
The fields are marked as soon as it detects something is off, which is not a great experience. In other words, it points out the error even before a mistake happens. See this example where I'm trying to add a URL:
https://github.com/Automattic/wp-calypso/assets/390760/8dc86b0c-6609-42f2-9a50-58bb5ca75a88
Now check this example where it delays the error for when the input is de-selected (or onblur). Please note that this was the first example I found, but the particular implementation on this Codepen does not matter for our case.
https://github.com/Automattic/wp-calypso/assets/390760/9df950ea-55ed-4a70-acbb-a3d9bcbdb8d4
From: https://codepen.io/bramus/pen/ExNYBOK
Styling
In terms of styling, I think we can get rid of the parenthesis and also outline the entire field with the red color. That would look something like this:
Additionally, and if possible at all, we should try to position the error message below the field so that it's more obvious something needs attention. This is what we're doing in Calypso:
Thanks, Clemente!
I completely agree with using the Calypso pattern to ensure consistency when displaying errors. We have already implemented this pattern on the 'Add Payment Method' page.
@keoshi @cleacos @jkguidaven Can you give it a go. I've updated to follow signup pattern.
Thank you @andrii-lysenko for working on it. I'll take a look and take over on it.
This PR modifies the release build for the following Calypso Apps:
For info about this notification, see here: PCYsg-OT6-p2
- blaze-dashboard
- command-palette-wp-admin
- help-center
- notifications
- odyssey-stats
- wpcom-block-editor
To test WordPress.com changes, run install-plugin.sh $pluginSlug add/a4a/partner-directory/highlight-invalid-form-fields on your sandbox.
Would still try to move validation to onblur on each field instead of having to click the Submit button
I was working on it, I prefer that solution too, onBlur... Since this PR is not super prioritize, I'll push this for tomorrow.
After talking with @andrii-lysenko, we will address the right way to this, using the onBlur property and wrapping all this logic inside of the new A4AFormFieldset component.
With this version, the user cannot save the profile details until all the data is valid. The ideal behavior is that the user can save the profile information, but until we don't have the validation per each field instead of the full form, we don't have a way to highlight the wrong field. It's a short form, it's the MVP version, and highlighting what is the wrong field seems more important. We will address the right way in a follow-up PR. cc: @jeffgolenski
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/16058124
Some locales (Hebrew) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday.
Thank you @cleacos for including a screenshot in the description! This is really helpful for our translators.
Translation for this Pull Request has now been finished.