feat: formAssociated element should treat boolean "disabled" Prop differently

[danyball]

Prerequisites

• [X] I have read the Contributing Guidelines.
• [X] I agree to follow the Code of Conduct.
• [X] I have searched for existing issues that already include this feature request, without success.

Describe the Feature Request

If the component has set formAssociated: true and a boolean disabled Prop the component should not add this as a attribute if disabled=false is set.

Describe the Use Case

We have formAssociated elements which should emit the click event if the user clicks on it. This should also work if disabled=false is set. For formAssociated elements the value of disabled is not respected like described here: https://html.spec.whatwg.org/dev/form-control-infrastructure.html#attr-fe-disabled:~:text=custom%20element%2C-,and%20the,attribute%20is%20specified%20on%20this%20element%20(regardless%20of%20its%20value),-%3B%20or

Describe Preferred Solution

If formAssociated: true and disabled=false is set, the attribute should not be present.

Describe Alternatives

No response

Related Code

Reproducer: https://github.com/danyball/stencil-noevent.git

Additional Information

Closed Mozilla bug ticket (not sure if related): https://bugzilla.mozilla.org/show_bug.cgi?id=1818287

[alicewriteswrongs]

Hey @danyball, thanks for filing this issue and glad to hear you've been making use of our support for form-associated custom elements!

I just want to check a little bit to make sure I'm understanding the ask here. Right now Stencil and the form-associated custom elements standard interact in a bit of an undesirable way, where if you set a disabled prop that will be added to the element as an attribute, and since for form controls the disabled state is based on whether or not the attribute is present, rather than its value, this results in the form control being disabled whether disabled is set to true or false.

If I've understood that correctly, the request is then to change Stencil's behavior from something like:

flowchart LR
    A[Stencil component has 'disabled' prop] --> B[set disabled attr on element to prop value]

to something like this:

flowchart TD
    A[Stencil component has 'disabled' prop]
    B[Is component form-associated?]
    C[set disabled attr on element to prop value]
    D[What's the value for `disabled`?]
    E[Remove `disabled` attr from element]
    A ---> B
    B --->|yes| D
    B --->|no| C
    D --->|true| C
    D --->|false| E

If I have that all right, then there's a few things to say about this. One is that if we applied this across the board it would be a breaking change, since we'd be changing the behavior of Stencil components where other developers might be depending on the current behavior. On the other hand, we could make this an opt-in change with a plan to make it the default in a future breaking change. Curious to hear your thoughts!

[danyball]

Hi. Yes correct. (nice graphic). Related to "breaking change": This is already breaking at our consumers of our stencil lib: They are using a "sdx-select" element with disabled prop. Now we made this component form-associated and disabled=false now breaks these apps. So yes, its breaking for you. But it is a "breaking change bugfix", because the behavior is wrong compared to the html standard. Sorry, I noticed I have opened up this ticket as "feature request". Maybe better label it as a bug?

[alicewriteswrongs]

Hmm this is something to noodle on a little bit!

I meant breaking change in the sense of a change in Stencil's behavior which we can't do without a major version bump - if we wanted to change this behavior more quickly we'd need to add some sort of configuration option in stencil.config.ts or in the @Component decorator.

I could be misunderstanding but I'm not sure I understand where Stencil isn't respecting the standard here. It seems as if Stencil is setting disabled on the component regardless of its value, but as the whatwg document says, an element should be disabled (functionally) if:

the element is a button, input, select, textarea, or form-associated custom element, and the disabled attribute is specified on this element (regardless of its value); or

so if you set disabled=false onto a form-associated custom element my reading of the standard is that the value should be set on the component, and the component should be disabled. If I'm understanding you correctly, the issue is that switching from a normal Stencil component to use form-association has meant that setting disabled=false now functionally causes the component to be disabled for users in situations where it didn't previously, preventing an event from firing when it's clicked. I understand how for consumers of your library this could constitute a breaking change. But given what the standard says, isn't this exactly what should happen if you set disabled=false?

I could be missing something here! If I am sorry for misunderstanding. But if that's all true then I think adding an option to prevent setting disabled=false on form-associated Stencil components would then be effectively having an option to opt-out from standards-compliant behavior. But anyway, just some more thoughts on the question, lmk what you think!

[danyball]

It seems as if Stencil is setting disabled on the component regardless of its value, but as the whatwg document says, an element should be disabled (functionally) if:

Stencil should set disabled regardless of its value. But not for formAssociated components. In this case Stencil also set "disabled=false". For non-form components this should not disable anything. Because its "false". But the standard wants something different: just set the disabled attribute if the element should really be disabled. Otherwise just do not set this attribute. Because "disabled=false" is interpreted as "disabled attribute present = disable it". And exactly this is what we have observed.

In short:

• Stencil meaning of disabled=false: do not disable anything, because value is false
• Standard meaning of disabled=false (for form elements): disabled attr is present, so disable it (disable events)

In my opinion you have to release this breaking change to be aligned with the standard. Adding an option would add code to your codebase which is not needed. You already have everything you should know: formAssociated: true. Maybe you could add a hint to your documentation - but a Dev should know how this attr acts like ;)

[alicewriteswrongs]

Gotcha, I think there is something here for us to noodle on more as a team. We have a longer term effort to improve the correctness of Stencil's handling of boolean attributes, and I think there's work for us to do here. I'm going to ingest this so the team can prioritize and refine it internally. Thanks for reporting!