search

colinaut / alpinejs-plugin-simple-validate

Simple Alpine form validation plugin
97 stars 4 forks source link

`disabled` fieldsets are validated #14

Closed QJan84 closed 6 months ago

QJan84 commented 1 year ago

fieldsets with disabled should not be validated with. I have a MultiStep form on one page in one form and put the single steps in a fieldset. All inactive fieldsets have the attribute disabled and therefore should not be used for validation.

However, it is still validated. When I get to the next step, I already see the error messages here.

mozilla Docs:

If this Boolean attribute is set, all form controls that are descendants of the fieldset, are disabled, meaning they are not editable and won't be submitted along with the form. They won't receive any browsing events, like mouse clicks or focus-related events. By default browsers display such controls grayed out.

Can this be fixed or am I wrong here?

colinaut commented 1 year ago

Ah yeah that is an issue. Disabled fields/fieldsets should be ignored by validation. I'll look to add that.

colinaut commented 1 year ago

And yes you are right in your other issue #13 that I am incorrectly using the disabled attribute in my steps.html example. A class or x-show would be better. I'm going to close that issue and fold it into this one. Thank you for bringing this to my attention.

colinaut commented 1 year ago

This is fixed in the latest Release 1.7.19. Disabled fieldsets or fields are automatically considered valid.

QJan84 commented 1 year ago

@colinaut https://www.novanetz.de/bestellen/bestellformular/index.php?validate=yes It doesn't quite seem to work that way. My disabled fieldset's are validated anyway. Is my code wrong?

colinaut commented 1 year ago

Ok yeah you are correct. I mistakenly only tested static disabled attribute not set by Alpine. When disabled is set by Alpine the script is missing it due to the order of operations. Alpine runs in order of the DOM so x-validate does it's initial setup on the form element and it doesn't initially see that disabled is set on the fieldset since that is set after. I need to reengineer it somehow to be more reactive to changes in the DOM. I'll look into fixing this.

colinaut commented 1 year ago

@QJan84 I just pushed a new version 1.7.20 which should hopefully fix the issue for you. I added a MutationObserver for the disabled and the required attributes so any changes to them are automatically updated in the x-validate data. This allows setting those attributes with AlpineJS :disabled or :required

Let me know if this works for you.

QJan84 commented 1 year ago

Super! This works wonderfully.

But there still seems to be a problem with x-model. I have already entered data here, which are stored in the LocalStorage (for example: zipcode: this.$persist(null)). On page load and submit also already filled inputs are not recognized and marked with an error message.

See also: https://www.novanetz.de/bestellen/bestellformular/index.php?validate=yes

error

https://github.com/colinaut/alpinejs-plugin-simple-validate/assets/18675776/8fa364de-9382-46dc-8455-e49c2f156898

colinaut commented 1 year ago

Ah. Issue is that x-validate runs when it encounters the form element and the x-model runs when it hits the field elements later so the values are updated via javascript and x-validate does not see the change. This plugin was not designed with x-model in mind. I might be able to add some sort of option so it checks later or uses MutationObserver to watch for the change. Not sure when I'll have time for that as it's a bigger fix.

In the mean time, what should work is adding x-validate attribute on the individual fields. This will cause it to run when it hits the field. It should grab the updated value then since it's running at the same time as the x-model.

QJan84 commented 1 year ago

@QJan84 I just pushed a new version 1.7.20 which should hopefully fix the issue for you. I added a MutationObserver for the disabled and the required attributes so any changes to them are automatically updated in the x-validate data. This allows setting those attributes with AlpineJS :disabled or :required

Let me know if this works for you.

My parent fieldset is disabled and my child fieldset is not. Should the inputs in the child fieldset NOT be used for validation? Currently I have to disable child fieldsets as well.

colinaut commented 1 year ago

Ah well I never anticipated nested fieldsets. I honestly didn't think that was valid html until you mentioned it here and I looked it up. I've been at this HTML game for a while. You'd think I'd know that. Anyways… I'll need to review my code and look at how to have it handle nested fieldsets. I likely won't have time to look at this till September as I'm busy with work and other stuff right now.

QJan84 commented 6 months ago

Some time has passed :-) It would be great if this error is fixed.

colinaut commented 6 months ago

Ok thanks for the poke. I'm working on this now. It's taking some rewiring but I have a working version. I need to do some QA to make sure it doesn't break anything

colinaut commented 6 months ago

Fixed in Release 1.8: plugin now recognizes nested fieldsets for both magic functions and setting disabled on the fieldset. It uses mutation observer to spot any dynamic changes to disabled.