bouncer icon indicating copy to clipboard operation
bouncer copied to clipboard

only use event.preventDefault() in submitHandler if form invalid

Open xini opened this issue 2 years ago • 6 comments

This makes sure that the original submit event is passed on and the submit button name is submitted to the backend.

xini avatar Oct 11 '21 09:10 xini

Can you say a bit more? Open to this PR, just want to understand it.

cferdinandi avatar Oct 11 '21 14:10 cferdinandi

Hi Chris,

Yes, of course. Sorry.

The current submitHandler catches the default submit event and creates a new submit event using js if the form is valid. This causes two issues:

  1. When a form has multiple submit buttons, because the original submit event is caught and replaced with a generic js event, the name of the submit button that was clicked is not submitted with the request.

  2. Say you want to submit the form via AJAX. Because the original submit event was caught and replaced with a js event, any other event listener can't intercept that event anymore. JS submits can't be intercepted, which means that when you use the validator, you can't have AJAX forms.

Does this make it any clearer?

I have tested my changes and as far as I can tell they work fine.

Cheers Flo

xini avatar Oct 11 '21 23:10 xini

Thank you. Yes, that makes perfect sense, and I appreciate the clarification.

I'll test this locally, run an updated build, and merge later when I get a spare moment. Thanks for this!

cferdinandi avatar Oct 12 '21 00:10 cferdinandi

Thank you very much.

xini avatar Oct 12 '21 01:10 xini

Is this problem solved? I have a form with multiple submits (formaction). The submits also have GET parameters (e.g. ?nextPage=43535) that are not submitted.

var submitHandler = function (event) {

  // Only run on matching elements
  if (!event.target.matches(selector)) return;
  
  // Prevent form submission
  event.preventDefault(); // <------------ does this kill my GET parameter?
  
  // Validate each field
  var errors = publicAPIs.validateAll(event.target);
  
  // If there are errors, focus on the first one
  if (errors.length > 0) {
  // event.preventDefault(); // <------------ Shouldn't that be here?
  errors[0].focus();
  emitEvent(event.target, 'bouncerFormInvalid', {errors: errors});
  return;
  }
  
  // Otherwise, submit if not disabled
  if (!settings.disableSubmit) {
  event.target.submit();
 }

  
  // Emit custom event
  if (settings.emitEvents) {
  emitEvent(event.target, 'bouncerFormValid');
  }
  
};

QJan84 avatar Apr 27 '22 05:04 QJan84

Thank you. Yes, that makes perfect sense, and I appreciate the clarification.

I'll test this locally, run an updated build, and merge later when I get a spare moment. Thanks for this!

Can you please add this to the master brunch?

QJan84 avatar Apr 27 '22 10:04 QJan84