Premature prop validation can break the UI

Created on 16 September 2024, 5 months ago
Updated 19 September 2024, 5 months ago

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.

Proposed resolution

It might be better to just display an inline error message and wait to reload the component until it passes, for example.

User interface changes

πŸ› Bug report
Status

Active

Component

Page builder

Created by

πŸ‡ΊπŸ‡ΈUnited States traviscarden

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @traviscarden
  • First commit to issue fork.
  • Assigned to bnjmnm
  • πŸ‡ΊπŸ‡ΈUnited States bnjmnm Ann Arbor, MI
  • πŸ‡ΊπŸ‡ΈUnited States bnjmnm Ann Arbor, MI

    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.

  • πŸ‡ΊπŸ‡ΈUnited States bnjmnm Ann Arbor, MI

    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.

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Ted knows that test the best.

  • First commit to issue fork.
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA

    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

  • πŸ‡ΊπŸ‡ΈUnited States bnjmnm Ann Arbor, MI

    Sorry for the chungus MR, but it is ready.

  • πŸ‡¬πŸ‡§United Kingdom jessebaker

    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"
  • πŸ‡ΊπŸ‡ΈUnited States bnjmnm Ann Arbor, MI
  • πŸ‡ΊπŸ‡ΈUnited States effulgentsia
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA

    needed to merge 0.x but now test are failing

  • πŸ‡ΊπŸ‡ΈUnited States bnjmnm Ann Arbor, MI
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA
  • First commit to issue fork.
  • πŸ‡³πŸ‡±Netherlands balintbrews Amsterdam, NL
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA
  • Pipeline finished with Skipped
    3 months ago
    #336785
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA

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

  • Pipeline finished with Skipped
    3 months ago
    #337545
    • bnjmnm β†’ committed 31e46aa5 on 0.x
      Issue #3474732 by bnjmnm, tedbow, jessebaker, balintbrews, traviscarden...
  • πŸ‡ΊπŸ‡ΈUnited States bnjmnm Ann Arbor, MI
  • Issue was unassigned.
  • Status changed to Fixed 3 months ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
Production build 0.71.5 2024