Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

data-user-(in)valid attributes set too early #1175

Closed
xdev1 opened this issue Feb 3, 2023 · 5 comments
Closed

data-user-(in)valid attributes set too early #1175

xdev1 opened this issue Feb 3, 2023 · 5 comments
Assignees
Labels
bug Things that aren't working right in the library.

Comments

@xdev1
Copy link
Contributor

xdev1 commented Feb 3, 2023

Please go to the Shoelace example "Styling Invalid Form Controls" and enter something in field Name.
The border of the Name field will become green immediately. This is a bug. It should become green after blur or submit. Same for the data-user-invalid attribute.

Please find here an additional demo to compare with original input behavior (please use Firefox):
https://jsfiddle.net/hts05oaq/3/

@xdev1 xdev1 added the bug Things that aren't working right in the library. label Feb 3, 2023
@claviska
Copy link
Member

claviska commented Feb 6, 2023

I'm not seeing any colors show up in that fiddle, but the example in the docs shows that user-valid | user-invalid states don't show up until you type into the field.

The spec isn't clear on the heuristics, saying only the user must "significantly [interact] with the element."

Looking back to this RFC, I chose to activate it on input instead of blur because the former felt more natural and the latter caused validation errors to show up when tabbing through the controls, which was jarring.

Perhaps we should move to a combination of input + blur before applying user-valid | user-invalid.

Any thoughts on that?

@xdev1
Copy link
Contributor Author

xdev1 commented Feb 6, 2023

I'm not seeing any colors show up in that fiddle

Strange, did you use Firefox as mentioned?

@claviska
Copy link
Member

claviska commented Feb 6, 2023

Whoops, forgot support wasn't that great yet. Nevertheless, the rest of my reply stands.

@xdev1
Copy link
Contributor Author

xdev1 commented Feb 6, 2023

Perhaps we should move to a combination of input + blur before applying user-valid | user-invalid.

This is without any doubt a way better solution than now (the best IMHO). Currently, if the app uses nice styles for different validation states, you cannot even enter your email address without the app telling you immediately after the first key stroke that you've done something wrong (which in most cases you have not). UI libraries seem to have different strategies, but not all of them are particular user-friendly.

A bit similar to the current Firefox behavior, but subtle differences (editing a required field for the first time but deleting all characters before blur will not trigger an validation warning):
https://jqueryvalidation.org/files/demo/

Similar to current Shoelace behavior (bad UX):
https://ant.design/~demos/components-form-demo-nest-messages

Completely different to the previous ones:
https://snippet.webix.com/gallery/core/a47681e7

claviska added a commit that referenced this issue Feb 7, 2023
@claviska
Copy link
Member

claviska commented Feb 7, 2023

It took two rounds of me trying to solve this. 😅

The first approach wasn't very flexible, as it required both sl-input and sl-blur to be emitted. Not all form controls emit those events, though. For example, buttons don't emit sl-input or input events.

Since form controls vary in that way — and in the way users interact with them — I found it more useful to provide a customizable list of events that must be emitted before user-valid | user-invalid states will be applied.

The list is passed to the FormControlController as an optional parameter, defaulting to only sl-input:

private readonly formControlController = new FormControlController(this, {
  // …
  assumeInteractionOn: ['sl-blur', 'sl-input']
});

The controller tracks interactions and only applies user-valid | user-invalid once all have been met. On reset, all interactions are cleared.

In the future, this can be extended even further to allow more complex interactions as needed. For now, it seems to work much better than what we currently have. You can test a couple right now in this example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Things that aren't working right in the library.
Projects
None yet
Development

No branches or pull requests

2 participants