feat: allow custom constraint validation errors

[blm768]

Fixes #928

• Inputs have a new customValidity prop, which allows setting custom constraint validation errors. Unlike the validationMessage prop, this new prop actually marks the element as invalid, preventing form submission.

[netlify]

✅ Deploy Preview for oruga-documentation-preview ready!

Name	Link
🔨 Latest commit	8d36756
🔍 Latest deploy log	https://app.netlify.com/sites/oruga-documentation-preview/deploys/66b4c0d5a954ba0007d3f910
😎 Deploy Preview	https://deploy-preview-1023--oruga-documentation-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

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

[blm768]

It might be feasible to merge the customValidity and validationMessage props, as their behavior somewhat overlaps. That would be a breaking change, though, so it would require some further discussion.

[blm768]

I'll need to extend this to cover other input types if/when we determine whether this is the right API.

[mlmoravek]

Hmm, OK, I get it now.

But maybe our customValidity should be a function that takes the validityState (input.validity) as an argument and has to return the validation message as string. Then this function, if given, will be called when the validity check returns false. This allows the user to create different validation messages based on the validityState.

How about this?

[blm768]

But maybe our customValidity should be a function that takes the validityState (input.validity) as an argument and has to return the validation message as string. Then this function, if given, will be called when the validity check returns false. This allows the user to create different validation messages based on the validityState.

That could work, but it would be more complicated than strictly necessary for my main use case. The main goal here isn't to override the validation message for inputs that already fail a built-in validation check (e.g. required); the existing validationMessage prop generally does that well enough. The goal is to add checks that can't be expressed using the normal validation attributes. For instance:

<script setup lang="ts">
import { OInput } from '@oruga-ui/oruga-next';
import { computed, ref } from 'vue';

const val = ref<string>('');
const validation = computed(() => {
  const value = val.value;
  const valid = value === '' || Number.parseInt(value) % 2 == 0;
  return valid ? '' : 'Enter a number divisible by two';
});

const onSubmit = (e: Event) => { alert('Submitted'); e.preventDefault(); };

</script>

<template>
  <form @submit="onSubmit">
    <OField label="Value">
      <OInput v-model="val" type="number" required :customValidity="validation" />
    </OField>
  </form>
</template>

In this example, any number that isn't a multiple of two will fail the validation checks, preventing form submission.

Your suggested API does seem like it would be an improvement for the existing validationMessage prop, and it might make the distinction between the two use cases clearer. I have to wonder if we should just merge the props, though, since the new customValidity prop can also override the validation message in cases where some other constraint validation check is failing.

Perhaps the right solution (if we're OK with a breaking change) is to merge the props and allow the merged prop to take any one of the following:

1. A string.
2. A function taking no arguments and returning a string (or undefined).
3. A function taking a ValidityState and returning a string (or undefined).

If we want to minimize breakage, we could make it so the prop doesn't set a custom validation error (via setCustomValidity) for otherwise valid inputs when it's given just a string, which would make its behavior for that specific case pretty much match the existing behavior. In the long run, though, I think it's cleanest to call setCustomValidity even if there's not a built-in validation error.

[mlmoravek]

I am just trying to stay open to a wider range of usecases where possible.

With the current validationMessage prop, you can only set the error message, but you cannot set different error messages for different validation errors.

We are currently on a breaking change release with 0.9.0, so it is fine to merge both features into one new prop and implement it as a breaking change.

So the type would be:
customValidity?: string | (validityState: ValidityState) => string | undefined; ?

Yes, we should call setCustomValidity when the prop is given and an error occurs.
Do we need to clean it up when the input is valid again? Like setCustomValidity("") after calling it once with a message?

[blm768]

Do we need to clean it up when the input is valid again? Like setCustomValidity("") after calling it once with a message?

Yes; without cleaning up the custom message, the control will still think it's in an invalid state.

I've pushed up a new version that supports the function-based syntax.

[blm768]

The diff is (unfortunately) a bit of a mess with the restructuring, but I've added some changes that should make this more robust. This seems to hold up in the test cases I've been able to think of so far.

[mlmoravek]

This seems to hold up in the test cases I've been able to think of so far.

Maybe you can add some unit test for this composable?

[blm768]

Maybe you can add some unit test for this composable?

It definitely needs some tests, but implementing them outside of a browser context may be tricky. I don't think the current DOM polyfill supports the constraint validation API.

[blm768]

Should we consider just adding browser-based testing back in for these kinds of cases? This definitely seems like it's getting complicated enough that it needs browser-level tests.

It looks like Vitest does have experimental browser support.

[mlmoravek]

Should we consider just adding browser-based testing back in for these kinds of cases? This definitely seems like it's getting complicated enough that it needs browser-level tests.

I've never done browser-based testing before. Feel free to add a new PR with the configuration changes and some primitive tests to demonstrate.

[mlmoravek]

Thanks mate, looks good now, great work!

Will you finish it up with all the other components? Or should i do that?

[blm768]

I've never done browser-based testing before. Feel free to add a new PR with the configuration changes and some primitive tests to demonstrate.

Sure; I can do that. Would you like me to do that after this PR, or should I consider the browser-based tests a prerequisite for this change?

Will you finish it up with all the other components? Or should i do that?

I'll get the rest of the components taken care of. It should be pretty quick to sort them out.

[mlmoravek]

Would you like me to do that after this PR, or should I consider the browser-based tests a prerequisite for this change?

Maybe just finish this one before opening another bigger topic.

I'll get the rest of the components taken care of. It should be pretty quick to sort them out.

Gool!

[blm768]

Rebased to make the next stage of review easier. I'll extend these changes to all input types next.

[blm768]

I've pushed up some new code changes, When it comes to the documentation, I was trying to build the documentation to test my edits and discovered that the Markdown files seem to be automatically generated. Should I make any updates to the documentation, or should I just let that process take care of things?

[mlmoravek]

The documentation is generated by npm run gen to generate whatever is generated, or just run npm run gen:docs to generate the docs, or npm run dev:watch to start a local documentation dev server with hmr for the oruga package and one time markdown files generation.

Please generate the docs and push the documentation changes to this branch as well.

[mlmoravek]

Looks good to me. Remove the draft stage when you think it`s ready to merge :)

[mlmoravek]

It seems that you have an older version of the oruga theme installed.

I will fix this for you! Just learned how to commit to another once pull request :D

[blm768]

Thanks for sorting out the documentation! I'll keep that in mind for my next PR.