Page MenuHomePhabricator

[EPIC] Add support for native Constraint Validation API to form associated components
Open, Needs TriagePublic

Description

Background

Currently, Codex components that use submittable elements and other form associated HTML tags do not directly support exposing the Constraint Validation API methods or emitting associated events such as 'invalid'. While all of these can be accessed by querying the inner elements of the component, or setting up native event listeners. This creates quite a lot of boilerplate for usage that is available natively by well-supported DOM APIs. Adding support for this API will allow Codex users to leverage constraint validation attributes such as min, max, pattern, etc. and enable them to better implement more semantic HTML. In Wikidata Query Builder, for example, we either have to forego usage of a type=number input and utilize home baked validation rules in the update method (this can be seen in the current code with Wikit, and in a recent patch to migrate to Codex) or jerry-rig the component to allow us to leverage native constraint validation (see pastes in comment below: T373872#10112820).

This issue proposes to add support for native constraint validation in all Codex components that are meant to be used as form controls, beginning with CdxTextInput. These components are:

  • TextInput.vue
  • TextArea.vue
  • Checkbox.vue
  • Radio.vue
  • ToggleSwitch.vue
  • SearchInput.vue
  • Combobox.vue
  • ChipInput.vue
  • Lookup.vue
  • Select.vue(?)

Acceptance criteria

  • An ADR is published detailing how we intend to support the constraint validation API in Codex
  • The components listed above are updated to expose the native properties and methods provided by the constraint validation API
  • A composable is provided to make it easy to work with these properties and methods within Codex
  • Use of the API and the composable is documented on the Codex docs site

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change #1070241 had a related patch set uploaded (by Itamar Givon; author: Itamar Givon):

[design/codex@main] Input: Emit the native DOM 'invalid' event

https://gerrit.wikimedia.org/r/1070241

Change #1070242 had a related patch set uploaded (by Itamar Givon; author: Itamar Givon):

[design/codex@main] Input: Expose native constraint Validation API DOM methods

https://gerrit.wikimedia.org/r/1070242

Change #1070243 had a related patch set uploaded (by Itamar Givon; author: Itamar Givon):

[design/codex@main] Input: Add validation public getters

https://gerrit.wikimedia.org/r/1070243

Code Samples with Constraint Validation API:

1<template>
2 <div class="querybuilder-limit">
3 <CdxCheckbox
4 id="limit"
5 v-model:model-value="useLimit"
6 class="querybuilder-limit__checkbox"
7 >
8 {{ $i18n( 'query-builder-limit-number-results-description' ) }}
9 </CdxCheckbox>
10 <CdxField
11 class="querybuilder-limit__field"
12 :status="error ? error.type : null"
13 :messages="error ? { [error.type]: $i18n(error.message) } : {}"
14 hide-label
15 >
16 <CdxTextInput
17 class="querybuilder-limit__input"
18 ref="inputContainer"
19 input-type="number"
20 v-model:model-value="limit"
21 :disabled="!useLimit"
22 :placeholder="$i18n( 'query-builder-limit-number-placeholder' )"
23 :required="useLimit"
24 :min="1"
25 @input="validate"
26 @blur="validate"
27 />
28 </CdxField>
29 </div>
30</template>
31
32<script setup lang="ts">
33import { storeToRefs } from 'pinia';
34import {
35 onMounted,
36 onBeforeUnmount,
37 ref,
38 Ref
39} from 'vue';
40
41import QueryBuilderError from '@/data-model/QueryBuilderError';
42import { useStore } from '@/store/index';
43import {
44 CdxCheckbox,
45 CdxField,
46 CdxTextInput
47} from '@wikimedia/codex';
48
49const inputContainer: Ref<null | InstanceType<typeof CdxTextInput>> = ref( null );
50const error: Ref<null | QueryBuilderError> = ref( null );
51const { limit, useLimit } = storeToRefs(useStore());
52
53function validate(e: InputEvent | FocusEvent){
54 if (!(e.target instanceof HTMLInputElement)) return;
55
56 e.target.checkValidity()
57}
58
59function handleError(e: Event){
60 if (!(e.target instanceof HTMLInputElement)) return;
61
62 const { validity } = e.target;
63
64 if (validity.valueMissing) {
65 // Limit distinguishes between undefined if the input is empty (which
66 // resets the limit to 100 on submit)...
67 limit.value = undefined;
68 } else {
69 // ... and null if the input is invalid for any other reason.
70 // See: T270179
71 limit.value = null;
72 }
73
74 error.value = {
75 type: 'error',
76 message: 'query-builder-limit-number-error-message',
77 };
78}
79
80onMounted(() => {
81 if (!inputContainer.value) return;
82
83 inputContainer.value.$el
84 .querySelector('input')
85 ?.addEventListener('invalid', handleError);
86});
87
88onBeforeUnmount(() => {
89 if (!inputContainer.value) return;
90
91 inputContainer.value.$el
92 .querySelector('input')
93 ?.removeEventListener('invalid', handleError);
94});
95</script>

1<template>
2 <div class="querybuilder-limit">
3 <CdxCheckbox
4 id="limit"
5 v-model:model-value="useLimit"
6 class="querybuilder-limit__checkbox"
7 >
8 {{ $i18n( 'query-builder-limit-number-results-description' ) }}
9 </CdxCheckbox>
10 <CdxField
11 class="querybuilder-limit__field"
12 :status="error ? error.type : null"
13 :messages="error ? { [error.type]: $i18n(error.message) } : {}"
14 hide-label
15 >
16 <CdxTextInput
17 class="querybuilder-limit__input"
18 ref="inputContainer"
19 input-type="number"
20 v-model:model-value="limit"
21 :disabled="!useLimit"
22 :placeholder="$i18n( 'query-builder-limit-number-placeholder' )"
23 :required="useLimit"
24 :min="1"
25 @input="validate"
26 @blur="validate"
27 @invalid="handleError"
28 />
29 </CdxField>
30 </div>
31</template>
32
33<script setup lang="ts">
34import { storeToRefs } from 'pinia';
35import {
36 onMounted,
37 onBeforeUnmount,
38 ref,
39 Ref
40} from 'vue';
41
42import QueryBuilderError from '@/data-model/QueryBuilderError';
43import { useStore } from '@/store/index';
44import {
45 CdxCheckbox,
46 CdxField,
47 CdxTextInput
48} from '@wikimedia/codex';
49
50const inputContainer: Ref<null | InstanceType<typeof CdxTextInput>> = ref( null );
51const error: Ref<null | QueryBuilderError> = ref( null );
52const { limit, useLimit } = storeToRefs(useStore());
53
54const input = inputContainer.value
55const validate = input.checkValidity
56
57function handleError(){
58 const { validity } = input;
59
60 if (validity.valueMissing) {
61 // Limit distinguishes between undefined if the input is empty (which
62 // resets the limit to 100 on submit)...
63 limit.value = undefined;
64 } else {
65 // ... and null if the input is invalid for any other reason.
66 // See: T270179
67 limit.value = null;
68 }
69
70 error.value = {
71 type: 'error',
72 message: 'query-builder-limit-number-error-message',
73 };
74}
75</script>

Hi @ItamarWMDE, thanks for this task and the associated patches. I've filed T373941 to represent the fact that the DST engineering team will spend some time discussing and considering your suggestions over the next 1-2 weeks, and we'll get back to you.

It would be helpful to know more about your anticipated use-case for this as well as what you see as its priority in your work – feel free to elaborate here or in the sub-task in a comment. Thanks!

Hi @egardner, thanks for the prompt reply. The current use case is described and linked in the ticket itself. The idea is to not have to replicate logic that is already afforded to us by browsers and is part of the WHATWG standards. In particular, to be able to use constraint attributes on codex components and leverage the Constraint Validation API instead of fashion our own validation checks (or use 3rd party libraries). Each of the components can adhere to different constraints, of course, as described in the linked WHATWG HTML Standard: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#constraints

Hi @egardner, thanks for the prompt reply. The current use case is described and linked in the ticket itself. The idea is to not have to replicate logic that is already afforded to us by browsers and is part of the WHATWG standards. In particular, to be able to use constraint attributes on codex components and leverage the Constraint Validation API instead of fashion our own validation checks (or use 3rd party libraries). Each of the components can adhere to different constraints, of course, as described in the linked WHATWG HTML Standard: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#constraints

Hey @ItamarWMDE – just a quick update from the DST engineers here. We chatted about this and we all agreed to move forward with this apporach. Thanks for providing patches – they look good, but we want to spend a little more time reviewing them next week (mostly to think about whether/how any of the native validation logic should interact with the custom logic that Codex components already provide – what if there are disagreements between the validation status according to the component vs the element inside the component, etc).

Anyway, I expect that we can review and merge these patches in our next sprint, meaning that this feature will make it's way into Codex in about 3 weeks (not next release but the one right after).

Thanks for this contribution!

@egardner sorry for the late reply, I was on holiday. Sure thing, looking forward to the review, and thank you for the speedy turnaround!

This looks awesome! I essentially was going for the same thing at https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1074256/1/resources/src/mediawiki.special.block/components/ValidatingTextInput.js

One possible improvement: should we provide a prop or option such that when changes are made to the TextInput, it automatically updates the status prop? Use of the status prop is standardized in Codex, but setting it is not. Because these patches we leverage native constraint validation, we don't need status "messages" (the browser provides them), but we still want to highlight the field in red. I'm doing that now by setting the status prop in the onInvalid event handler, but I wonder Codex should use the :invalid selector.

I envision every form behaving in the same way. Form submission calls reportValidity() (as it would do natively, but usually in Vue we're intercepting form submission with preventDefault()), then all fields validated by the browser are highlighted red, and change to the default state once the validation passes.

I guess the above is more of a product proposal. I bring it up as my team is building the very first Vue application in Core, so I figured now was the time to discuss standardizing how we implement forms. I think in an ideal world, the next engineer who builds a Vue/Codex form in MediaWiki doesn't have to do anything with respect to validations (assuming nothing is needed beyond what the browser can do natively). Wouldn't that be nice!? :)

Thanks for the compliments 😊. Your suggestion sounds fair to me, especially utilizing the :invalid selector. But I think you're right, it's probably a bigger product idea.

However, there is still a way where you can utilize browser messages without having to leverage "reportValidity()" directly. It is a bit of a slog to replicate logic already offered to us by the browser, but you can leverage the validationMessage (mdn) getter that is already being exposed in codex/+/1070243. This way you can use the browser provided localized message in the messages prop, and you can even get to decide its display severity (warning, error) in status (which is not afforded by the browser).

The added bonus here is that you can use native browser messages without extending the design principles around error and warning message rendering that Codex already provides us with.

@MusikAnimal I sketched out a composable that might make your life easier without having to create another component, but enable you to use native validation and messages together with codex's status and messages props codex/+/1075582

I see CdxLookup is not in component list, does that mean the lookup component is not meant for capturing input? In CommunityConfiguration there are a couple of components (PageTitleControl and CommonsFileControl) that use it for capturing page titles and file names based on a API search. Ideally these two components would also use native validation (eg: min length) but if they are built on top of a component that will not have the feature available (for valid reasons) it may be more suitable to change the underlying Codex component to use for a TextInput and Menu or SearchInput (?). Thoughts? @ItamarWMDE @egardner

Probably just an oversight on my part, since I see it actually uses an underlying TextInput. I'll add it to the list.

@ItamarWMDE I've spent some time looking at your patches, learning about HTML constraint validation, and playing around with demos and other ideas. First off - this is an excellent idea, and I think the way you've constructed things in your patches makes a lot of sense and is the direction we should take to support native validation in Codex. Many thanks for raising this issue and for contributing code!

I've listed some thoughts below so we can discuss here/in the patches. We could also use next week's Front-end Standards Group meeting to continue the discussion if you and others would find that valuable.

(Comments are welcome from anyone, btw!)


Implementation

I think the implementation in @ItamarWMDE's patches is great, and that the composable is the best way to provide developers with an easy way to implement this in their features.

I've created some demos that use different features implemented in the patches, some applying them manually and one using the composable. IMO, the developer experience is quite simple, especially with the composable. We could take @MusikAnimal's suggestion and apply error styles to the TextInput using the :invalid selector as well, which would mean you wouldn't even have to bind the status prop if you're using TextInput outside of a Field (although we do recommend using it inside a Field if possible).

One thing we might add to the composable is a resetStatus method to enable you to reset the status to "default", e.g. when the user is trying to fix the issue (like in this demo). I wonder if there are any other helpers that would be useful.

Other components

I've been reviewing the list of other components and have some questions, plus some thoughts about how useful this would be outside TextInput and TextArea.

  • Binary inputs (Checkbox, Radio, and ToggleSwitch): as far as I can tell, the only attribute relevant to these input types is required, and that only applies to a single Checkbox or ToggleSwitch that must be checked (because we can't use HTML constraint validation on a group of binary inputs). Is it worth implementing this in Checkbox and ToggleSwitch for this one purpose?
  • Lookup/MultiselectLookup: Is this useful for these components given you have to select an item from the menu? A selected item should never be wrong, since the list of options from which it was selected was provided by the feature. Text left over in the input without a selection being made can constitute a warning or error state, so we could use native validation to test if there is input on blur or submission (in this demo, type something then blur out of the input without making a selection to see what I mean). Is it useful enough to implement native validation here? Lookup contains a TextInput, so we'd just need to make sure the events bubbled up and the user could access the TextInput's DOM methods.
  • TypeaheadSearch: I don't think we need to implement this here, since you either select something from the menu or go to the search page with the current input set as the search query. Any objections?
  • Button/ToggleButton: How could this be used in these components?

The DST engineers do think we should implement this in TextArea, and see a case for Combobox since it accepts arbitrary input in addition to a selected item, but that one seems less important. Perhaps we can finish up the implementation in TextInput and add it to TextArea as well, then consider other components if there's a compelling use case. I'm interested to hear what everyone thinks of this.

Design considerations

I plan to ask the DST designers what they think of the native UI for displaying validation messages. I think we'll want to recommend using the Field component and passing the native validation messages into Field for display. However, I want to know if there are cases where it's acceptable to use the native UI. If you all have any known cases of this, please let me know.

Path forward

Here's a plan for moving this work forward:

  1. Discuss the details here and in gerrit
  2. Potentially discuss this in the Front-end Standards Group meeting next week
  3. Write an ADR documenting our decisions about supporting HTML constraint validation (I'm happy to do this)
  4. Finish the existing patches to implement this for TextInput and document usage
  5. Consider building this into other components (at least TextArea)

Feedback on all of this is very welcome!

  • Binary inputs (Checkbox, Radio, and ToggleSwitch): as far as I can tell, the only attribute relevant to these input types is required, and that only applies to a single Checkbox or ToggleSwitch that must be checked (because we can't use HTML constraint validation on a group of binary inputs). Is it worth implementing this in Checkbox and ToggleSwitch for this one purpose?

If it takes the required attribute (or any native constraint validation), then I'd say yes :)

  • Lookup/MultiselectLookup: Is this useful for these components given you have to select an item from the menu? A selected item should never be wrong, since the list of options from which it was selected was provided by the feature. Text left over in the input without a selection being made can constitute a warning or error state, so we could use native validation to test if there is input on blur or submission (in this demo, type something then blur out of the input without making a selection to see what I mean). Is it useful enough to implement native validation here? Lookup contains a TextInput, so we'd just need to make sure the events bubbled up and the user could access the TextInput's DOM methods.

For Multiblocks we have a Lookup component that takes arbitrary input, not just from the selections (I can elaborate as to why, but it's not relevant I don't think). The input in our case is required, so we ended up manually calling checkValidity() on the field and applying the error status and message. I did this with anticipation that one day Lookup could handle required on its own. But if nothing else, being able to access the inner TextInput as you say would be great.

I plan to ask the DST designers what they think of the native UI for displaying validation messages. I think we'll want to recommend using the Field component and passing the native validation messages into Field for display. However, I want to know if there are cases where it's acceptable to use the native UI. If you all have any known cases of this, please let me know.

We ended up feeding the native validation messages to Field component, so they display as our designs prescribe. You can see this in action at Special:Block on Beta enwiki. Here's a screenshot for reference:

Screenshot from 2024-10-17 17-44-29.png (149×840 px, 17 KB)

We do this by listening to the invalid event and emitting update:status to the parent component with the status and message (example). Maybe it would make sense for Codex to do the same. Perhaps even automatically have the nearest parent Field component listen to it and show the messages accordingly? Just an idea.

Thanks @AnneT for this great write-up.

I have a small suggestion in terms of the API of the proposed useNativeValidation composable.

There are a few other places in Codex where we use composables as wrappers around different, non-Vue APIs. For example, in useIntersectionObserver, that composable contains a bunch of logic that pertains to the IntersectionObserver API. The composable returns a boolean ref that auto-updates based on whether the given element is or is not intersecting with the viewport.

We do something similar with the useFloatingMenu composable. In this case the composable encapsulates some FloatingUI-related code.

But in both cases we have a workflow where you pass in a Vue thing (a template ref to the underlying HTML element you care about) and get back other Vue things (useFloatingMenu doesn't actually return anything at all, but it sets up watchers a side-effect). This minimizes the presence of non-Vue API calls in Codex components and helps with context switching I think.

I'd like to propose that we do something similar here – maybe we should re-work useNativeValidation slightly so that it takes a template ref (in this case to some kind of HTML input element) and returns refs or computed properties that automatically give you the latest status, error message, etc. The composable could also set a watcher in the background to call checkValidity() on input or change events potentially.

Otherwise the approach you outlined wouldn't really change – I'm just thinking of ways to make the Vue usage as ergonomic as possible.

We do this by listening to the invalid event and emitting update:status to the parent component with the status and message (example). Maybe it would make sense for Codex to do the same. Perhaps even automatically have the nearest parent Field component listen to it and show the messages accordingly? Just an idea.

Thanks for the example - I want to play around with this and see if there's a sensible way for us to automatically set the status in the TextInput and Field. One issue I see is that we'd also want an automatic way to reset the status to default - IMO we should either handle both of these things automatically, or neither. It's obvious when the error state should be set - when the invalid event is emitted. But when do we reset the status to default? Is there a standard event or interaction for this? In my demos, I'm resetting the status on input, because that indicates that the user may be trying to fix the error. But I doubt that behavior is universal.

FWIW, even if you do have to set the status on the field yourself, the code is very simple with @ItamarWMDE's composable (and you only need to set the status on the Field, since Field passes it down to TextInput).

Also, thanks for all of the info on your use cases! I agree with should include binary inputs and Lookup.

@egardner thanks for your helpful suggestions here - I agree that we should consider building more of the API into the composable. You're right that passing in a ref to a composable is common, and people would need access to the ref to use the native methods directly anyway, so passing it into a composable is no big deal. I think we should open a separate task for the composable and we can iron out the details there.

CCiufo-WMF renamed this task from Add support for native Constraint Validation API to form associated components to [EPIC] Add support for native Constraint Validation API to form associated components.Oct 24 2024, 5:48 PM
CCiufo-WMF assigned this task to AnneT.
CCiufo-WMF triaged this task as High priority.

Change #1082833 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] docs: ADR for native constraint validation

https://gerrit.wikimedia.org/r/1082833

Change #1082852 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] [proof of concept] Native validation in Select

https://gerrit.wikimedia.org/r/1082852

CCiufo-WMF changed the task status from Open to In Progress.Oct 28 2024, 4:19 PM

Change #1082833 merged by jenkins-bot:

[design/codex@main] docs: ADR for native constraint validation

https://gerrit.wikimedia.org/r/1082833

Test wiki created on Patch demo by ATomasevich (WMF) using patch(es) linked to this task:
http://patchdemo.wmcloud.org/wikis/a5d0a31f72/w/

Test wiki on Patch demo by ATomasevich (WMF) using patch(es) linked to this task was deleted:

http://patchdemo.wmcloud.org/wikis/a5d0a31f72/w/

CCiufo-WMF changed the task status from In Progress to Open.Dec 2 2024, 6:21 PM
CCiufo-WMF raised the priority of this task from High to Needs Triage.
CCiufo-WMF moved this task from Now to Parking Lot on the Design-System-Team (Roadmap) board.

Native browser validation methods are now available for the TextInput and TextArea. We'd like to see how the TextInput and TextArea features get used in real products before focusing on enabling this for other components, or working on the proposed composable (T378023), so I'm moving this Epic to the Parking Lot of our Roadmap board for the time being. Others are welcome to pick it up though!

CCiufo-WMF removed a project: Patch-For-Review.