Overview

If you start typing in a prop field and pause long enough or just type slowly enough for it to try to re-render the component, it can cause a premature validation error and break the UI. This is especially acute for link fields, for example, which require non-trivial typing before they become valid.

Screenshot

Proposed resolution

  • Add clientside HTML5 and JSON Schema Validation
  • Refactor iri/uri props to not use the url widget because that creates an input with type="url" which has HTML5 validation expecting an absolute path. The JSON schema validation will take care of what we need

  • Refactor inputBehaviors entirely - move logic into multiple files that separate various concerns.

User interface changes

Command icon 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

traviscarden created an issue. See original summary.

bnjmnm made their first commit to this issue’s fork.

bnjmnm’s picture

Assigned: Unassigned » bnjmnm
bnjmnm’s picture

This will likely be dormant for a week or so while DrupalCon happens. The plan is nything in the schemas that can be enforced by html5 validation will be added as attributes to the form inputs. Some things such as type enforcement in some element types can't be achieved this way (select is just gonna store strings even if we want numbers) - but letting the browser enforce where possible should help minimize the overall spaghetti.

bnjmnm’s picture

Version: » 0.x-dev
Assigned: bnjmnm » Unassigned

99% done - FieldTypeUninstallValidatorTest is failing and I'm not yet sure why. Unassigning myself to open this up to contributors who have had more exposure to that test and might be able to figure it out more quickly. In the meantime I'll keep debugging to see what is happening.

wim leers’s picture

Assigned: Unassigned » tedbow

Ted knows that test the best.

tedbow made their first commit to this issue’s fork.

tedbow’s picture

Assigned: tedbow » bnjmnm
Status: Active » Needs work

FieldTypeUninstallValidatorTest was failing it tests we can uninstall the link module after we remove all uses in XB fields. Now that link is a dependency of XB we get an error after remove all the links uses because it tries to also uninstall XB but its fields are in use on bundles.

Fix the test to make sure we don't get "link" at ll in the uninstall error message at after we have removed all the uses of the link field

there is merge error now in one of the .js tests so assigning back to @bnjmnm

bnjmnm’s picture

Assigned: bnjmnm » Unassigned
Status: Needs work » Needs review

Sorry for the chungus MR, but it is ready.

jessebaker’s picture

StatusFileSize
new420.58 KB

I've done a bit of manual testing and found some issues around entering numbers. Would you prefer to address here, or else I can raise them as new issues?

On a required, string, field (E.g. the Heading field on the default Hero component)

  1. Entering a number fails validation but I would expect my entered value to be cast to a string. What if I want a heading that says "911" for example.
  2. If I select all the text in an input and press space, the value is immediately changed to a 0 and then the app crashes.
  3. Typing in -0 also crashes the app. (possibly same issue as 2)
  4. After typing a number, I can't press space (e.g. If I wanted the heading to be "4 person menu"

On a URI field (E.g. the CTA 1 Link field on the default Hero component)

  1. Typing 0 crashes the app.
  2. After typing a number, I can't press space (e.g. If I wanted the heading to be "4 person menu"
bnjmnm’s picture

Status: Needs review » Reviewed & tested by the community
effulgentsia’s picture

Assigned: Unassigned » tedbow
tedbow’s picture

Status: Reviewed & tested by the community » Needs work

needed to merge 0.x but now test are failing

bnjmnm’s picture

Issue summary: View changes
Status: Needs work » Needs review
tedbow’s picture

Assigned: tedbow » balintbrews
tedbow’s picture

tedbow’s picture

Status: Needs review » Needs work

balintbrews made their first commit to this issue’s fork.

balintbrews’s picture

Assigned: balintbrews » Unassigned
Status: Needs work » Reviewed & tested by the community
tedbow’s picture

tedbow’s picture

Assigned: Unassigned » f.mazeikis

Looks like this needs @wim leers or @f.mazeikis approval to merge

  • bnjmnm committed 31e46aa5 on 0.x
    Issue #3474732 by bnjmnm, tedbow, jessebaker, balintbrews, traviscarden...
bnjmnm’s picture

Assigned: f.mazeikis » Unassigned
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

wim leers’s picture

Component: Page builder » Redux-integrated field widgets