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

v0.8.14 broke Umbraco Forms or "how to exclude forms" #127

Open
jrunestone opened this issue Sep 5, 2024 · 6 comments
Open

v0.8.14 broke Umbraco Forms or "how to exclude forms" #127

jrunestone opened this issue Sep 5, 2024 · 6 comments

Comments

@jrunestone
Copy link

jrunestone commented Sep 5, 2024

HI!

I realize I haven't updated the package in a year, but I got around to it and now our generated "Umbraco Forms" (a UI form builder for Umbraco) forms don't submit in versions above 0.8.13 (nothing happens on click, no errors).
"disabled" is added to the submit button when I click it but I suspect it might be Umbraco Forms that does some debouncing thing to prevent multiple submissions.

If I remove the creation of the ValidationService along with the call to bootstrap() the Umbraco Forms form will submit again, so it's something with the lib I think.

Anyway I tracked it down to 0.8.14 and there's a suspect fix there but I can't be sure of course. It seems to be happening in most browsers, not just FF.

This brings me to a question that I had while trying to fix this problem on my end - can I exclude certain forms from being considered by the ValidationService at all?

EDIT: I realize I haven't provided a repro, but it requires a full Umbraco site setup + Umbraco Forms. But maybe you have some ideas anyway.

@jrunestone
Copy link
Author

I found the undocumented ValidationService::remove(form) but how is it supposed to work? I'm trying to remove my Umbraco Forms form completely from the service with that method but it still has entries in the ValidationService.formInputs array and the form still won't submit.

@dahlbyk
Copy link
Collaborator

dahlbyk commented Sep 16, 2024

Anyway I tracked it down to 0.8.14 and there's a suspect fix there but I can't be sure of course. It seems to be happening in most browsers, not just FF.

The suspect PR does seem most likely, as it changes validation from often synchronous to always asynchronous. But I have no idea why it would be a specific problem in an Umbraco world in a way that doesn't seem to be causing problems more generally. Presumably its relationship with some sort of Umbraco Forms event handling.

Shot in the dark: does behavior change if you rearrange where the bootstrap() happens relative to initializing Umbraco Forms? I assume the issue is fixed specifically by avoiding bootstrap(); just creating ValidationService should be harmless.

I found the undocumented ValidationService::remove(form) but how is it supposed to work?

AFAIK the old jQuery Validation didn't have any form/input caching, so remove(form) behavior is largely undefined/untested. But I would generally expect it to completely revert the form's behavior to as if the form had never been scanned/bootstrapped.

#110 in particular tried to fix that up, but I'm sure there are gaps.


Are you initializing your ValidationService with a logger, e.g. new ValidationService(console)? Any hints in the logs as to what is or isn't happening when you try to submit?

@jrunestone
Copy link
Author

jrunestone commented Sep 16, 2024

Thanks for replying!

I also believe it is due to Umbraco Forms including aspnet-client-validation on their end, but the fact remains it can be worked around by leaving out bootstrap and it's working before that particular version I mentioned.

Anyhow I would be happy to just be able to exclude certain forms from even being bootstrapped by my initialization, because right now I can't have 2 forms (UC/non-UC) on the same page or they will conflict :) The remove(form) doesn't seem to do anything. Is it too far fetched of an idea to have the ability to config a selector for which forms I want the validation service to target? E.g form-selector: '[data-validate-form]'.

I'll try the v0.10.0 release using remove(form) again to see if that works.
I've also not used the logger yet, I'll see what I can find.

@jrunestone
Copy link
Author

I attached a logger and the behavior is a little more sinister than I thought :) Upon clicking submit on an Umbraco Forms form the validation service I created (not Umbraco Forms) goes into an infinite validation loop for some reason (it doesn't crash the browser though).

image

I did however try the latest package, 0.11.0, and did remove(form) after bootstrapping, and now I'm able to reliably remove the Umbraco Forms forms from the validation service and they work as they should. It's a bit backwards and I would prefer to be able to to just select the forms I want to validate at config, but this works.

@dahlbyk
Copy link
Collaborator

dahlbyk commented Sep 16, 2024

It's a bit backwards and I would prefer to be able to to just select the forms I want to validate at config, but this works.

We should support "don't scan" better. Ideas:

  1. Update options.root to accept null to opt out of initial scan
  2. Add options.scanOnInit (default: true) to explicitly opt out

bootstrap(options?: Partial<ValidationServiceOptions>) {
Object.assign(this.options, options);
this.addMvcProviders();
let document = window.document;
const root = this.options.root;
const init = () => {
this.scan(root);
// Watch for further mutations after initial scan
if (this.options.watch) {
this.watch(root);
}
}

In the meantime, you can specify a different root to scan on bootstrap, e.g. document.head shouldn't have any forms.

@jrunestone
Copy link
Author

Sure, I think option 2 is the better of the two. Thanks for the tip, it looks like I can then manually call scan(form) for each form I want to enable validation for. That would be an acceptable solution, albeit slightly unintuitive to new users.. Maybe if we had a scanForms(<selector>) or something ;)

This should work:
v.bootstrap({ root: document.head })
myForms.forEach(v.scan);

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

No branches or pull requests

2 participants