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

Exclude Disabled Fieldset Descendent Controls #110

Closed
kevinslodic opened this issue Jun 16, 2020 · 10 comments
Closed

Exclude Disabled Fieldset Descendent Controls #110

kevinslodic opened this issue Jun 16, 2020 · 10 comments
Labels

Comments

@kevinslodic
Copy link

According to the W3C spec, a disabled <fieldset> element should cause all descendent form controls, excluding those of the first <legend> element, to also become disabled.

Looking through the source that iterates over the form's elements, I didn't see a check for the above. To have parity with the native implementation of FormData, this sounds like a good addition.

Happy to help contribute myself if you don't have the bandwidth.

@jimmywarting
Copy link
Owner

crap, You are right. Missed it. That means iterating over all fields makes it slightly more annoying since we probably have to start looking at the DOM tree :[

Happy to help contribute myself if you don't have the bandwidth.

That would be nice, just contributing with an idea of how to filter out fields inside a disabled fieldset would be 👍

@jimmywarting
Copy link
Owner

hmm, maybe could use matches + :disabled

elm.disabled || elm.matches(':disabled')

@jimmywarting
Copy link
Owner

@jimmywarting
Copy link
Owner

jimmywarting commented Jun 16, 2020

any other suggestions or alternative way? must deal with prefixed versions of matches otherwise

@kevinslodic
Copy link
Author

kevinslodic commented Jun 17, 2020

I don't think looking at elm.matches(':disabled') will work since the input itself doesn't become disabled, it just inherently acts disabled based on the <fieldset>; I think that's a different problem altogether, though.

I stand corrected above; interesting that it triggers :disabled, but not the DOM property.

What about keeping a reference to the disabled <fieldset> elements and simply checking if the element is a descendent of that node?

// since this primarily is driven from <= IE 11's lack of support, can't use Array.from without adding a polyfill
const disabledFieldsets = Array.prototype.slice.call(form.querySelectorAll('fieldset:disabled'));

each(form.elements, elm => {
  if (!elm.name || elm.disabled || elm.type === 'submit' || elm.type === 'button' || disabledFieldsets.some(f => f.contains(elm)) return
});

@jimmywarting
Copy link
Owner

Are you sure elm.matches(':disabled') would not work? i like it better... and it seems to work

otherwise an alternetive would be elm.matches('fieldset[disabled] *')

@kevinslodic
Copy link
Author

No, you're right; I had spoken too soon (I updated my comment above to reflect). I had figured it wouldn't work based on elm.disabled still reporting as false, so poorly assuming it wouldn't match the :disabled pseudo-class.

Agreed--it's a much cleaner implementation!

@kevinslodic
Copy link
Author

kevinslodic commented Jun 17, 2020

I think elm.matches('fieldset[disabled] *') would work best; I ran elm.matches(':disabled') in an IE 11 VM and it didn't seem to register correctly (screenshot below):

ie11-matches

@jimmywarting
Copy link
Owner

jimmywarting commented Jun 17, 2020

fixed, and publish to npm. it's funny how no new features are added or that there is no breaking changes. my patch version is on 20 now :P never had it that high on any application

I'm amazed that ppl still are able to find tiny bugs that have gone unnoticed by so many users upon till this day. I would think more ppl would stop using this since most browser have proper support now days

@kevinslodic
Copy link
Author

Cool--thanks for the effort! Unfortunately, IE 11 still has just enough presence that it's too hard to ignore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants