Don't disable button elements if form is using browser validation + invalid

[tcannonfodder]

Description

• If a form is using the browser's Constraint Validation API to provide interactive validation errors, we should not disable a button on the form as part of the Mrujs remote submission.
  • This is because the browser will automatically prevent submission on its own, and present the errors for the user to correct
• If we do not early-return in disableFormElement in this case, the form becomes locked and cannot be submitted because its submit buttons are disabled.
• We need to check if the form has its noValidate set to true, otherwise we do not disable the button if the form has bypassed the constraint validation API (and is still invalid according toform.checkValidity)

I added test forms to the demo page for various mixtures of constraint validation + the submit button being a button or input[type=submit]

Status

• (Needs Review)

Related Issue(s)

• forms with data-remote + data-disable that use constraint validation are locked out when browser's "submit by pressing enter" behavior is used #219

Additional Notes

• I tried writing tests for this, but was unable to find the right mixture of code to test that the element was not actually disabled

[netlify]

✅ Deploy Preview for mrujs canceled.

Name	Link
🔨 Latest commit	2da4c1f
🔍 Latest deploy log	https://app.netlify.com/projects/mrujs/deploys/6950743033aa620008d41fdc

[kaspth]

Don't have any ideas about the implementation but did have some drive by thoughts 😅

[kaspth]

Wonder if we can boil the comment down a bit? Here's a pitch:

Suggested change

	// If the form is using the browser's default constraint validation, and the form is not valid, we should
	// not attempt to disable the button because the browser will prevent errors and prevent submission
	// When browser constraint validation using forms are invalid, the browser prevents errors & submission

[kaspth]

Seems like we should match return style above:

Suggested change

	if (form?.noValidate == false && form?.checkValidity() == false) { return }
	if (form?.noValidate == false && form?.checkValidity() == false) return

[kaspth]

Would this make more sense below the comment since it's closely tied to this one specific early return condition?

I'm thinking like:

  // When browser constraint validation using forms are invalid, the browser prevents errors & submission.
  const form = element?.form
  if (form?.noValidate == false && form?.checkValidity() == false) return

[tcannonfodder]

Took @kaspth's suggestions (good ones!) and rebased them into my commit

[KonnorRogers]

So this is kind of a low level utility and the wrong spot for this as its for generalized disabling.

This is the better probably a better place to check this stuff:

https://github.com/KonnorRogers/mrujs/blob/main/src/formSubmitDispatcher.ts#L40-L41

mrujs/src/formSubmitDispatcher.ts

Lines 60 to 71 in 1331574

	function startFetchRequest (event: CustomEvent): void {
	const { element, fetchRequest, request, submitter }: AjaxEventDetail = event.detail
	if (event.defaultPrevented || shouldNotSubmit(element) || shouldNotSubmit(submitter)) {
	dispatchStopped(event)
	return
	}
	dispatch.call(element, AJAX_EVENTS.ajaxBeforeSend, {
	detail: { element, fetchRequest, request, submitter }
	})
	}

[tcannonfodder]

Ahhh, thank you! I actually think this might make sense as a check in shouldNotSubmit, because...well, we should not submit.

Proposed version:

function shouldNotSubmit (element?: HTMLElement | null): boolean {
  if(element?.dataset.ujsSubmit === 'false') return true

  // If an invalid form uses the default constraint validation behavior,
  // the browser prevents submission, so we should not disable the element
  const form = element?.form
  return (form?.noValidate == false && form?.checkValidity() == false)
}

[netlify]

✅ Deploy Preview for mrujs canceled.

Name	Link
🔨 Latest commit	bb6a0f8
🔍 Latest deploy log	https://app.netlify.com/projects/mrujs/deploys/69d6ee86e8aba00008c3e0eb