trix icon indicating copy to clipboard operation
trix copied to clipboard

Replace hidden `<input>` with `ElementInternals` integration

Open seanpdoyle opened this issue 1 year ago • 6 comments

Closes #1023

Replace the requirement for an <input type="hidden"> element with direct <form> integration 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 <input type="hidden">-[input] attribute pairing can be achieved through ElementInternals.setFormValue. Similarly, the <label> element support can be achieved through ElementInternals.labels.

TODO before merging:

  • [x] Control through browser feature-detection and a Trix.config.editor object

TODO after merging:

seanpdoyle avatar Jan 30 '24 03:01 seanpdoyle

@afcapel do you have any thoughts or ideas about the TODO before merging list in the PR description?

seanpdoyle avatar Feb 02 '24 16:02 seanpdoyle

@seanpdoyle thanks for kicking this off. ElementInternals is certainly the way of the future and it makes the whole setup look much simpler. It's also exciting that we will be able to add attributes such as required to the <trix-editor> without needing any extra JS.

A couple of things to consider:

  • ElementInternals is only available since Safari 16.4 (released 2023-03-27)
  • We should also look for the migration path for existing projects. We probably can update the markup in actiontext to discard the hidden input and start using the <trix-editor> element itself. But what's the story for projects that integrated Trix without actiontext?

Given that, do you think it's possible for the two systems (ElementInternals and hidden input) to coexist for a while to ease the migration?

afcapel avatar Feb 06 '24 14:02 afcapel

Given that, do you think it's possible for the two systems to coexist for a while to ease the migration?

I do. I'd created this item in the PR description to vaguely capture that:

  • [ ] Control through browser feature-detection and a Trix.config.editor object

I'd originally explored this work by introducing two delegate classes (something like (ElementInternalsDelegate and LegacyDelegate). I abandoned that in favor of demonstrating the possibilities in a straightforward way. Now that the approach is proven, I'm curious of what kind of configuration options are available.

On the JS-only side, a Trix.config value is straightforward. I'm not sure what to name it, but it could be as simple as a Trix.config.legacy = false boolean to use the new stuff.

On the Action Text side, I'm unsure about how to best forward that server-side decision along. A <meta> tag? Guidance to set a global window.Trix.config value? A Rails helper method to do the behind the scenes work for you?

Like you mentioned, there are Form Builder implications to that setting, since using the newer machinery would mean that the <input type="hidden"> is no longer necessary. It'd be crucial to ensure that the setting is respected all the way through from an application's config/ file to its server-generated HTML to its client-side Trix instance.

Do you have any thoughts on what that configuration might entail?

seanpdoyle avatar Feb 06 '24 14:02 seanpdoyle

@afcapel I've explored a Trix.config.editor.formAssociated JavaScript configuration that's pre-populated by the presence of a <meta name="trix-editor-formAssociated" content="true"> element in the <head>.

The configuration key is provisional open to renaming. The idea is that Action Text could provide a configuration key that would then utilize the new yield :head standard in the layout to push that node when Element Internals is enabled.

To set the stage for these changes and to ease the particulars of the Element Internals code review, I've opened https://github.com/basecamp/trix/pull/1132 to introduce the idea of a delegate and to make some of the less controversial changes ahead of these.

Both https://github.com/basecamp/trix/pull/1132 and https://github.com/basecamp/trix/pull/1128 are ready for review.

seanpdoyle avatar Feb 09 '24 04:02 seanpdoyle

@afcapel, any chance this can be re-evaluated? It is a common pain when working with Trix. Is there anything we can do to move this forward somehow?

MatheusRich avatar Aug 19 '24 17:08 MatheusRich