Skip to content

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

Open
xini wants to merge 1 commit into
cferdinandi:masterfrom
xini:patch-1
Open

only use event.preventDefault() in submitHandler if form invalid#76
xini wants to merge 1 commit into
cferdinandi:masterfrom
xini:patch-1

Conversation

@xini

@xini xini commented Oct 11, 2021

Copy link
Copy Markdown

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

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

Copy link
Copy Markdown
Owner

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

@xini

xini commented Oct 11, 2021

Copy link
Copy Markdown
Author

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

@cferdinandi

Copy link
Copy Markdown
Owner

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!

@xini

xini commented Oct 12, 2021

Copy link
Copy Markdown
Author

Thank you very much.

xini added a commit to xini/silverstripe-form-validation that referenced this pull request Oct 12, 2021
@QJan84

QJan84 commented Apr 27, 2022

Copy link
Copy Markdown

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

QJan84 commented Apr 27, 2022

Copy link
Copy Markdown

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 QJan84 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants