Integrate with ElementInternals

[seanpdoyle]

Related to #1143
Closes #1023

Integrate with <form> elements directly through built-in support for ElementInternals.

According to the Form-associated custom elements section of More capable form controls, various behaviors that the <trix-editor> element was recreating are provided out of the box.

For example, the <label> element support can be achieved through ElementInternals.labels. Similarly, a formResetCallback() will fire whenever the associated <form> element resets.

For now, keep the changes minimal. Future changes will handle integrating with more parts of ElementInternals.

TODO after merging:

• Integrate with ElementInternals.willValidate, ElementInternals.validity, ElementInternals.validationMessage
• Form callbacks like void formDisabledCallback(disabled) to support [disabled]
• Instance properties included from ARIA

[seanpdoyle]

@jorgemanrubia @brunoprietog are either of you able to review this?

[jorgemanrubia]

Looks like a great change @seanpdoyle

[jorgemanrubia]

The "disabled" bit is separated from the "element internals" change, right? I like both, but are they related?

[seanpdoyle]

The integration with ElementInternals provides access to the formDisabledCallback(disabled) lifecycle hook. Through that integration the element is given an opportunity to disable or re-enable its associated <input>. The [disabled] attribute is a common attribute among form controls, and provides the server an opportunity to render the element with an initially disabled state. However, the [disabled] attribute is only one aspect of disabling an element. If it's rendered within a <fieldset disabled>, the formDisabledCallback(disabled) will fire (and the associated <input> will be disabled anyway if its a child of the <fieldset> whether ElementInternals is integrated or not). There is also a :disabled CSS pseudo-selector that incorporates both [disabled] on the element itself and :disabled of any ancestor <fieldset> element.

The .disabled property itself is an enhancement that's separate from the bare necessities.

I've included it here because applications that want to introspect whether or not the element is disabled are unlikely to get it right when incorporating [disabled] on the element, :disabled on its <fieldset> ancestors (or their HTMLFieldSetElement.disabled property).

Since the <input> element is built-in, it already accounts for all these permutations. The current formDisabledCallback(disabled) implementation already manages the HTMLInputElement.disabled property, so the <input> felt like a good delegate for the time being. If the <trix-editor> is ever changed to managed its own value in a way that makes the <input> no longer necessary, this property could be changed to handle the rest of the logic.

[jorgemanrubia]

Is this just for testing purposes? Could you replace with just instanceof in tests?

[seanpdoyle]

It isn't solely for testing. I've added the property because it's mentioned in the Defining a form-associated custom element section of the More capable form controls article.

Having said that, it's implemented using Element.localName (and not hard-coded like I've done here). It also doesn't seem to be mentioned by the MDN documentation for ElementInternals, so it isn't clear how necessary it is to start.

[jorgemanrubia]

Thanks @seanpdoyle

[seanpdoyle]

I've opened #1190 to add support for [required] and setCustomValidity(message).

[jorgemanrubia]

@seanpdoyle I'm noticing that this change will make Trix require Safari 16.4+ because of attachInternals. That's a version of March of 2023. Safari users upgrade browsers much slower than Chrome ones and we still have a decent base of users using Safari 14,15 in Basecamp. In HEY, where we use attachInternals in some components, we upgraded the minimum version to 17, but with Basecamp we need to move more carefully. I'm sorry but I'll have to revert this 🙏.

I'd be happy to consider a version that uses attachInternals but that it doesn't break if the browser does not support it. I think this was a great change, as well as the constraints one based on this one.

[seanpdoyle]

Originally, I had opened #1132 and #1128 (depends on #1132) to add conditional support.

#1128 was more ambitious, and replaced the <input type="hidden"> with directly reading from the <trix-editor> element's content.

I'm dissatisfied with all of the machinery involved in conditionally running the appropriate code in CI depending on the browser support, but if that's what is necessary to ship these features, we can take that approach.

Since #1188 is viable, I'll incorporate those changes into a new PR that progressively enhances toward ElementInternals based on a Trix.config value that will default to whether or not the browser supports `ElementInternals.