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

buttons in forms prevents submit #401

Open
lubomirblazekcz opened this issue May 3, 2024 · 3 comments
Open

buttons in forms prevents submit #401

lubomirblazekcz opened this issue May 3, 2024 · 3 comments
Labels

Comments

@lubomirblazekcz
Copy link

Bug Report

Before Naja 3.1.0 it was possible to send form via naja (button without type="submit") and process custom submit event listeners on form.

But now naja prevents all form submissions on every button type except type="button". Is this a good solution? Wouldn't be better if the button only sent new CustomEvent('submit', { cancelable: true }) onto form and let the form submission proceed naturally?

My usecase is that I have submit event listener on form that checks errors like this and prevents submission
https://getbootstrap.com/docs/5.3/forms/validation/ or https://winduum.dev/docs/components/form.html

Or recaptcha submit event listener on form, that checks recaptcha score.
https://cloud.google.com/recaptcha-enterprise/docs/instrument-web-pages#user-action

Naja prevents the form submission on buttons, so these listeners never execute. Current workaround is to use type="button" and add execute custom submit event on form so the events bubble naturally.

Is there any particular reason why naja binds event listeners on form buttons? There is already an event listener on form that listens for submit event?

And yes you could write an extension for validation or recaptcha, but I don't like that solution. You should be able to easy enhance current form functionality with naja without such workarounds.

@filiphlm
Copy link
Contributor

filiphlm commented May 15, 2024

Is there any particular reason why naja binds event listeners on form buttons? There is already an event listener on form that listens for submit event?

Well, this was/is pretty common practice because the submitter has to somehow get into the data submitted by the (ajx) form, but you're right, Naja does it in a way that makes the standard submit event almost irrelevant.

Although it has a fairly straightforward solution (SubmitEvent.submitter and the submitter parameter of the FormData constructor), it may not be to everyone's liking. It would introduce several BC breaks in the UIHandler), besides I am not able to figure it out without using event delegation – judge for yourself, no polyfills included.

There is quite decent cross-browser support for FormData(form, submitter), and it's really "guaranteed to work in the latest versions of Chromium (Chrome and Edge), Firefox, and WebKit (Safari)" 😉, for the rest (Chrome/Edge 112<, Safari 16.4<, Firefox 111<) there is a polyfill. If you also need to support Safari 15.4<, there's another polyfill for SubmitEvent.submitter. (Or all together.)

@jiripudil : Should I prepare a PR request or is it a bit too much? I understand that this can be quite controversial, but personally I'd rather put up with a working polyfill of an established standard temporarily than a complex workaround(s) that might have unwanted side effects.

@jiripudil
Copy link
Member

Hi, yep, this is mainly for historical reasons. Today, with SubmitEvent.submitter and FormData(data, submitter) (and form.requestSubmit()) widely available (and polyfill-able if needed), Naja could listen solely to form.submit, and if the form or the submitter matches the configured selector, process the form submission asynchronously.

I might need to think this through more thoroughly, but I think that if UIHandler uses the submitter for the element in the interaction event, this could even come without BC breaks 🤔

@filiphlm
Copy link
Contributor

filiphlm commented May 16, 2024

You're right, maybe we can get by with a pretty minimal edit:

UIHandler.ts#L53-L58

if (element.tagName === 'FORM') {
	bindForm(element as HTMLFormElement);
} else {
	const forms = element.querySelectorAll('form');
	forms.forEach((form) => bindForm(form as HTMLFormElement));
}

(Well, it's not exactly ideal, but I can't see a better solution.)

UIHandler.ts#L75-L77

if (event.type === 'submit') {
	const {submitter} = (event as SubmitEvent);
	if ((element as HTMLFormElement).matches(this.selector) || submitter?.matches(this.selector)) {
		this.submitForm(element as HTMLFormElement, options, event).catch(ignoreErrors);
	}

UIHandler.ts#L116-L121

public async submitForm(form: HTMLFormElement, options: Options = {}, event?: SubmitEvent): Promise<Payload> {
	const submitter = event?.submitter;
	const method = (submitter?.getAttribute('formmethod') || form.getAttribute('method') || 'GET').toUpperCase();
	const url = submitter?.getAttribute('formaction') ?? form.getAttribute('action') ?? window.location.pathname + window.location.search;
	const data = new FormData(form, submitter);

	return this.processInteraction(submitter || form, method, url, data, options, event);
}

filiphlm added a commit to filiphlm/naja that referenced this issue May 17, 2024
jiripudil pushed a commit that referenced this issue Jul 7, 2024
* SubmitEvent.submitter & FormData(form, submitter)

Solves #401

* Update test.yml

* Update UIHandler.ts

* UIHandler: Coding style

* UIHandler.submitForm: Accepts *any* event

* UIHandler.clickElement: Handles click events on button|input elements

* UIHandler.bindUI: Logical order

* UIHandler: tests

* UiHandler: submitForm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants