search

eclipsesource / jsonforms-vuetify-renderers

https://jsonforms-vuetify-renderers.netlify.app/
Other
25 stars 26 forks source link

Touch error filtering #107

Closed yaffol closed 1 month ago

yaffol commented 3 months ago

This PR adds a touched ref that is bound to controls from useVuetifyControl and a handleBlur composable that updates the touched state after an input is blurred.

It also takes account of a new config setting enableFilterErrorsBeforeTouch which allows the forcing of all errors to display, such as when a user attempts to submit a form which is not completely valid.

netlify[bot] commented 3 months ago

Deploy Preview for jsonforms-vuetify-renderers ready!

Name Link
Latest commit d2d23f483b4b3791b66af0b3f532c406452bec3a
Latest deploy log https://app.netlify.com/sites/jsonforms-vuetify-renderers/deploys/665497f06d4b670008519483
Deploy Preview https://deploy-preview-107--jsonforms-vuetify-renderers.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

yaffol commented 3 months ago

I had a shallow look. Looks good to me for a first implementation.

One drawback which I noticed is that we don't have any clue about which errors are shown in child controls, so container renderers like object and arrays at the moment can't determine which errors to show in an error summary. Is that a use case for you?

Thanks! Container errors are a use case, but as discussed handling the display of those errors seems quite complicated.

Therefore I've updated the behaviour so that, when enableFilterErrorsBeforeTouch is true, container errors are not shown. When enableFilterErrorsBeforeTouch becomes false, all errors - including container errors - are shown.

This covers my primary use case of someone attempting to submit a form which is not valid, when they attempt to submit, all the errors are shown to them, so they can properly correct them.

manuelmeister commented 2 months ago

What is missing to get this merged? Can I help?

sdirix commented 2 months ago

Hi @manuelmeister, I'll review it very soon. My plan is to review, merge and release within the month. However as this is outside of sponsored development, I can't make any promises for a delivery date.

In case you want to actively help: It's always useful to have additional testers who integrated the changes themselves and can confirm that they work. This gives considerably more confidence than when it's just me testing and making sure myself that I checked all edge cases.