[LiveComponent] Should input fields honor browser validation?

[NicoHaase]

In a LiveComponent, I use an input field with required attribute. The action is triggered manually by a button. As a shortened example, my component markup looks like the following:

<input type="text" id="ean" name="ean" value="{{ ean }}" data-model="norender|ean"
   required="required" class="form-control" autofocus>
<input type="submit" class="btn btn-primary" data-action="live#action" data-action-name="checkIn"
   data-loading="addAttribute(disabled)" value="Submit">

Even if I leave the input field empty, the AJAX request is sent out. Shouldn't the JS code first run the browser's built-in form validation to check whether all fields are valid?

[weaverryan]

Hmm, potentially. But is triggering the native browser validation manually possible? And even if it is, how would we determine whether or not to do it (we wouldn't want to automatically trigger it for any live action that's inside of a form)?

[NicoHaase]

Triggering it manually seems possible, as documented on https://developer.mozilla.org/en-US/docs/Learn/Forms/Form_validation#validating_forms_using_javascript. For the given field with a required attribute, we could check it through

document.getElementById('ean').reportValidity()

If this returns false, sending the form could be avoided. Also, this triggers the browser's own form validation stuff, such that a popup is shown at the validated form element.

After all, this could also be opt-in, like the norender modifier?

[weaverryan]

Hmm, that seems reasonable enough. Probably it would be something like:

<input
    data-action="live#action"
    data-action-name="validate|checkIn"
    value="Submit"
>

The validate modified would cause it to find the parent form element and call reportValidity() on it, as you said. I've never used this method before, but if it works as expected, then this seems simple enough.

PR welcome :). The directives for actions are parsed around here:

ux/src/LiveComponent/assets/src/live_controller.ts

Lines 182 to 217 in dd07ad6

	directive.modifiers.forEach((modifier) => {
	switch (modifier.name) {
	case 'prevent':
	event.preventDefault();
	break;
	case 'stop':
	event.stopPropagation();
	break;
	case 'self':
	if (event.target !== event.currentTarget) {
	return;
	}
	break;
	case 'debounce': {
	const length: number = modifier.value ? parseInt(modifier.value) : this.getDefaultDebounce();
	// clear any pending renders
	if (this.actionDebounceTimeout) {
	clearTimeout(this.actionDebounceTimeout);
	this.actionDebounceTimeout = null;
	}
	this.actionDebounceTimeout = window.setTimeout(() => {
	this.actionDebounceTimeout = null;
	_executeAction();
	}, length);
	handled = true;
	break;
	}
	default:
	console.warn(`Unknown modifier ${modifier.name} in action ${rawAction}`);
	}
	});

Cheers!

[NicoHaase]

@weaverryan I've started working on this in a branch, but I'm struggling with generating the dist files. Is there any trick to do that? The error messages on my system look like those from Github Actions at https://github.com/symfony/ux/runs/8202892022?check_suite_focus=true

[weaverryan]

Are you just getting that error from chart.js? If so, don’t just worry about it - the chart.js build is currently broken - we’re debugging in another PR. If it’s blocking your new code from building, the “hack” would be to open up the rollup config file and, temporarily, change the files variable - https://github.com/symfony/ux/blob/2.x/rollup.config.js#L41 - to point directly (hardcode the path, no * needed) to this ONE component. Then building will build only this one component. But if you still have problems, don’t worry about it - you can push the updated src files to a PR and I can help with the dist files there - no biggie :)

[Kocal]

Is this issue still relevant even after #2926?