Overview
In that issue it was identified that HTML5 errors aren't displayed in the UI once the user blurs the input
Proposed resolution
Push HTML5 errors into the formState slice
User interface changes
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | html5-validation-page-data.png | 658.29 KB | bnjmnm |
Issue fork experience_builder-3537946
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
larowlanComment #4
bnjmnmHaving the form display / enforce HTML5 Errors was implemented very early on - in a panic - to prevent preview updates that include preview-crashing values on the props form Since then, we've added JSON schema validation that serves this purpose more robustly. This has me wondering if the html5 validation is necessary at all in props
On a surface level, it can be confusing to have a mixture of schema and HTML5 validation on the same form.
We've also already had to create a workaround due to HTML5 and JSON schema disagreeing about what constitutes a required field violation, opting to go with JSON schema's definition.
If there's something that HTML5 validation enforces that the AJV schema checking is not, then perhaps this issue is correct to promote it's role within both forms. If the HTML5 validation is at best replicating what AJV already does, then we're probably better off removing the HTML5 validation logic from the props form, so it's only present on the page data form, which doesn't do schema validation.
There is one bit we'd need to change in this condition, though:
Here,
newValueshould be(newValue || newValue === '')so empty strings get the schema validation, too.Comment #5
larowlanThat works for me, the main issue here was we weren't writing the errors to the store
Comment #8
bnjmnmWith the
3537946-page-data-onlyMR:Comment #9
larowlanLooks great - thanks!
Comment #10
bnjmnmI'm going to vote no on doing this, although I think it was completely reasonable to suggest. The areas being changed in the MR are happening at an earlier stage than anything that would impact the appearance of those messages. Keeping it in its own issue avoids this being delayed by surprise test failures due to the changes, and gives us an issue where there's the opportunity for community members to discuss concerns about the designs (however unlikely those are to occur - the designs seem pretty good)
Comment #11
wim leers@bnjmnm's comment in #8 is 🤩👏 — thanks so much for that!
→ the server-side changes #3529788: Required `type: string` (without other constraints, so "prose strings") props should fall back to empty string (even though invalid) while Content Author is typing, to allow matching live preview in most cases made to make this possible are specifically only for the component instance form. But the client-side changes in #3529788 were generic.
#10++ too.
Needs reroll because #3537945: Narrow the conditions under which we allow a prop to be an empty string landed.
Comment #12
bnjmnmComment #14
wim leers