Optionally provide innerHtml and textContent on StimulusReflex::Element

[julianrubisch]

Enhancement

Description

This PR allows to optionally transmit the reflexElement's innerHTML and/or textContent when calling a reflex.

This can be done:

1. by passing includeInnerHTML: true or includeTextContent: true as an option to this.stimulate
   OR
2. by declaring data-reflex-include-html or data-reflex-include-text on the reflex HTML element

This PR also includes a refactoring of client-side reflexData handling into its own class, analog to the Ruby side, along with tests.

Why should this be added

This PR is inspired by working with contenteditable divs (i.e., a "Notion"-like interface).

Prior to this, you would have to put the innerHTML into a data attribute manually like this:

    currentTarget.dataset.content = currentTarget.innerHTML;

    this.stimulate("ContentElementsReflex#save", currentTarget);

and I figured this should be taken care of at the framework level.

Checklist

• My code follows the style guidelines of this project
• Checks (StandardRB & Prettier-Standard) are passing
• This is not a documentation update

Please note that the best way to suggest changes or updates to the documentation is to join Discord and leave a note in the #docs channel. Any documentation updates posted as PRs cannot be accepted at this time. ❤️

[leastbad]

This is really exciting! 🦖

[assuntaw]

This is so useful @julianrubisch !

The note-taking feature I'm working on has contenteditable divs all over the place, and there's a lot going on in the corresponding Stimulus controllers. Not having to assign the innerHTML to the dataset before saving would be super helpful. Thanks for taking this on 🎉

[julianrubisch]

We'll have to hold this back a bit, I might have found a bug around checkbox list values not being pulled correctly from attrs

[leastbad]

What if you call a succession of selector/nothing Reflexes, using the same instance of an SR controller, and the dataset/selectors change between invocations?

[julianrubisch]

hmmm so all I can say is this mimicks behavior as it was before. I was seeing errors because I was pulling attrs of reflexElement which changed its checked status midway (in the setTimeout in stimulate), and suddenly the wrong set of values was sent back to the server

In other words: the state of ReflexData was mutable, whereas prior to that PR it was just stored as a simple object (const data = { .... }) that did not mutate.

Now we're back to "as before"

[leastbad]

That's a great answer, thank-you.

With that insight in mind, I amend my review to LGTM.

Each stimulate invocation calls new ReflexData so there is no risk of stale values.

[julianrubisch]

Exactly!

[hopsoft]

This looks great. I propose that we make the data attributes more explicit to match the stimulate args. Even though it's annoyingly verbose.

• data-reflex-include-inner-html
• data-reflex-include-text-content