Skip to content
This repository has been archived by the owner on Apr 27, 2023. It is now read-only.

Validation on reviewer details entry #1290

Open
chris-huggins opened this issue Aug 11, 2020 · 2 comments
Open

Validation on reviewer details entry #1290

chris-huggins opened this issue Aug 11, 2020 · 2 comments
Labels
UX xpub-parity Required for reaching parity with xpub
Milestone

Comments

@chris-huggins
Copy link
Collaborator

When entering details to suggest reviewers, the email input shows a validation error as soon as a single character of the name is entered. Validation should happen on text entry within the email field, but the validation of the email input when entering names could happen when focus is moved from/no longer on the name field instead.

This is how it appears to work in xPub.

In Reviewer:

image

In xPub:
image

@chris-huggins chris-huggins added UX xpub-parity Required for reaching parity with xpub labels Aug 11, 2020
@hdrury1 hdrury1 added this to the Release milestone Aug 11, 2020
@norisalsaadie
Copy link
Contributor

Simplified attempt here https://github.com/libero/reviewer-client/tree/1290-auto-validation-bug

The issue is that the current onChange triggers validation, which causes the whole schema to validate. The first change it to allow the validation to be triggered dynamically via the onChange (https://github.com/libero/reviewer-client/blob/1290-auto-validation-bug/src/initial-submission/components/EditorsForm.tsx#L286) - which allows the ExpandingEmailField component to make decisions on when to trigger validation.

Pending the above change, the updatePeople routine in ExpandingEmailField would then need to make sensible decisions on when to trigger validation, ideally only on the input field which has been interacted. Perhaps this can be achieve via react-hook-form.

Parking issue for now in favour of more pressing issues.

@norisalsaadie norisalsaadie self-assigned this Sep 23, 2020
@norisalsaadie
Copy link
Contributor

Update: following on from the previous comment on this issue. The suggested solution is not so simple and wouldn't work as the validation schema would validate all fields even if triggered manually.

The following solution was attempted, in EditorForm.tsx, in the onChange function passed to <ExpandingEmailField>, manually loop the personArray and validate each item, if one item is invalid then invoke the validation as part of the setValue call, though this causes all fields to be validated leading to false validation issues. This also introduces some negative performance consequences as the onChange is called too often as it's based on user input.

There is some promise with the newer version of the form library, specifically the trigger function (https://react-hook-form.com/api/#trigger) but for this to make sense the component structure needs to be reworked. The simplest solution would be that each ExpandingEmailField is passed validation rules and ExpandingEmailField would handle the validation, leaving EditorForm.tsx to focus on higher level validation.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
UX xpub-parity Required for reaching parity with xpub
Projects
None yet
Development

No branches or pull requests

3 participants