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

Trix editor is disabled and the toolbar disappears on page "update" using turbo 8 beta #533

Open
BaggioGiacomo opened this issue Nov 30, 2023 · 10 comments

Comments

@BaggioGiacomo
Copy link

BaggioGiacomo commented Nov 30, 2023

I have a task page where the user can comment using a trix editor. When I create a comment, it goes on the comments list above correctly, without reloading the page but the trix editor is disabled and the toolbar isn't there anymore.

Preview

Initial status:
image

Let's create a comment
image

This is the situation after clicking the post button
image

Some code

  • The application.html.erb contains those lines just before the </head>:
<%= turbo_refreshes_with method: :morph, scroll: :preserve %>
<%= yield :head %>
  • The comment model has belongs_to :commentable, polymorphic: true, touch: true where commentable is the task where I've commented in this case.

  • The task model has broadcasts_refreshes

I think the problem is that trix is not being re-initialized after the form submission.

@adrienpoly
Copy link
Member

I did a test also with a Trix editor and had similar issue. This relates to this more general issue hotwired/turbo#1083.

What worked for me was to put the Trix editor in a data-turbo-permanent and reset the form on turbo:morph but this means that any morph update occurring on that page would trigger a form reset so it is not bullet proof.

@BaggioGiacomo
Copy link
Author

BaggioGiacomo commented Dec 1, 2023

Thanks @adrienpoly for linking the more general issue.

What worked for me was to put the Trix editor in a data-turbo-permanent and reset the form on turbo:morph but this means that any morph update occurring on that page would trigger a form reset so it is not bullet proof.

Yeah, I've done the same thing to temporarily fix the problem!

@brunoprietog
Copy link
Contributor

I wonder if the right place to fix this is Trix itself or Action Text. Is it correct that Trix knows about these cases? Or should it be bulletproof and reset itself every time trix-editor element is changed. I had initially thought of marking trix-toolbar as data-turbo-permanent but if you say that the editor is disabled I don't think that will be enough.

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Dec 3, 2023
Follow-up to [9944490][]
Related to [hotwired#1083]
Related to [@hotwired/turbo-railshotwired#533][]

The problem
---

Some client-side plugins are losing their state when elements are
morphed.

Without resorting to `MutationObserver` instances to determine when a
node is morphed, uses of those plugins don't have the ability to prevent
(without `[data-turbo-permanent]`) or respond to the morphing.

The proposal
---

This commit introduces a `turbo:before-morph` event that'll dispatch as
part of the Idiomorph `beforeNodeMorphed` callback. It'll give
interested parties access to the nodes before and after a morph. If that
event is cancelled via `event.preventDefault()`, it'll skip the morph as
if the element were marked with `[data-turbo-permanent]`.

Similarly, this commit re-purposes the new `turbo:morph` event to be
dispatched for every morphed node (via Idiomorph's `afterNodeMorphed`
callback). The original implementation dispatched the event for the
`<body>` element as part of `MorphRenderer`'s lifecycle. That event will
still be dispatched, since `<body>` is the first element the callback
will fire for. In addition to that event, each individual morphed node
will dispatch one.

This commit re-introduced test coverage for a Stimulus controller to
demonstrate how an interested party might respond. It isn't immediately
clear with that code should live, but once we iron out the details, it
could be part of a `@hotwired/turbo/stimulus` package, or a
`@hotwired/stimulus/turbo` package that users (or
`@hotwired/turbo-rails`) could opt-into.

[9944490]: hotwired@9944490
[hotwired#1083]: hotwired#1083
[@hotwired/turbo-railshotwired#533]: hotwired/turbo-rails#533
@BaggioGiacomo
Copy link
Author

BaggioGiacomo commented Dec 4, 2023

I had initially thought of marking trix-toolbar as data-turbo-permanent but if you say that the editor is disabled I don't think that will be enough

With disabled I mean that I am unable to write anything, and I cannot focus either. With data-turbo-permanent + the form.reset() on submit end, it works great!

@seanpdoyle
Copy link
Contributor

seanpdoyle commented Dec 6, 2023

After further investigation, I believe there are two issues at-play:

  • Action Text renders the <trix-editor> element without any innerHTML content. When morphed, the stateful editor that already exists in the document has content, and the new <trix-editor> has no content. By default, the new editor has precedence, so the innerHTML content is removed after the morph. Action Text renders a corresponding <input type="hidden"> element that has the new content, but that's only automatically used to populate the editor once JavaScript is available and aware that the element connects to the document. Since it's a morph and not a removal or addition, the element isn't connecting, disconnecting, or reconnecting.
  • Action Text renders the <trix-editor> element without the [contenteditable] attribute, since Trix is responsible for setting that attribute on the element when JavaScript is available and ready to execute. Since the server-sent response is missing the [contenteditable] attribute, and the server-sent response has precedence, the [contenteditable] attribute is removed from the existing editor element.

There are, of course, some other issues. For example, Trix will respond to an omitted [toolbar] attribute by rendering a new <trix-toolbar> on the client-side. I wonder if Trix could add a MutationObserver to observe when its [toolbar] attribute changes or its referenced <trix-toolbar> element matches the trix-toolbar:empty CSS selector, then re-render or copy the content prior to the change. There are probably some other attributes it'd need to monitor in a similar way.

As an initial step, I propose that Action Text make two corresponding changes:

  1. extend Action Text's rich_text_area and rich_text_area_tag helpers to render the existing content into both the <input type="hidden"> element, as well as the <trix-editor> element itself
  2. extend Action Text's rich_text_area and rich_text_area_tag helpers to render the <trix-editor> element with contenteditable: true by default.

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Dec 7, 2023
Follow-up to [9944490][]
Related to [hotwired#1083]
Related to [@hotwired/turbo-railshotwired#533][]

The problem
---

Some client-side plugins are losing their state when elements are
morphed.

Without resorting to `MutationObserver` instances to determine when a
node is morphed, uses of those plugins don't have the ability to prevent
(without `[data-turbo-permanent]`) or respond to the morphing.

The proposal
---

This commit introduces a `turbo:before-morph` event that'll dispatch as
part of the Idiomorph `beforeNodeMorphed` callback. It'll give
interested parties access to the nodes before and after a morph. If that
event is cancelled via `event.preventDefault()`, it'll skip the morph as
if the element were marked with `[data-turbo-permanent]`.

Similarly, this commit re-purposes the new `turbo:morph` event to be
dispatched for every morphed node (via Idiomorph's `afterNodeMorphed`
callback). The original implementation dispatched the event for the
`<body>` element as part of `MorphRenderer`'s lifecycle. That event will
still be dispatched, since `<body>` is the first element the callback
will fire for. In addition to that event, each individual morphed node
will dispatch one.

This commit re-introduced test coverage for a Stimulus controller to
demonstrate how an interested party might respond. It isn't immediately
clear with that code should live, but once we iron out the details, it
could be part of a `@hotwired/turbo/stimulus` package, or a
`@hotwired/stimulus/turbo` package that users (or
`@hotwired/turbo-rails`) could opt-into.

[9944490]: hotwired@9944490
[hotwired#1083]: hotwired#1083
[@hotwired/turbo-railshotwired#533]: hotwired/turbo-rails#533
@SylarRuby
Copy link

Strange. I can't reproduce this issue.

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Jan 25, 2024
Follow-up to [9944490][]
Related to [hotwired#1083]
Related to [@hotwired/turbo-railshotwired#533][]

The problem
---

Some client-side plugins are losing their state when elements are
morphed.

Without resorting to `MutationObserver` instances to determine when a
node is morphed, uses of those plugins don't have the ability to prevent
(without `[data-turbo-permanent]`) or respond to the morphing.

The proposal
---

This commit introduces a `turbo:before-morph` event that'll dispatch as
part of the Idiomorph `beforeNodeMorphed` callback. It'll give
interested parties access to the nodes before and after a morph. If that
event is cancelled via `event.preventDefault()`, it'll skip the morph as
if the element were marked with `[data-turbo-permanent]`.

Along with `turbo:before-morph`, this commit also introduces a
`turbo:before-morph-attribute` to correspond to the
`beforeAttributeUpdated` callback that Idiomorph provides. When
listeners (like an `HTMLDetailsElement`, an `HTMLDialogElement`, or a
Stimulus controller) want to preserve the state of an attribute, they
can cancel the `turbo:before-morph-attribute` event that corresponds
with the attribute name (through `event.detail.attributeName`).

Similarly, this commit re-purposes the new `turbo:morph` event to be
dispatched for every morphed node (via Idiomorph's `afterNodeMorphed`
callback). The original implementation dispatched the event for the
`<body>` element as part of `MorphRenderer`'s lifecycle. That event will
still be dispatched, since `<body>` is the first element the callback
will fire for. In addition to that event, each individual morphed node
will dispatch one.

This commit re-introduced test coverage for a Stimulus controller to
demonstrate how an interested party might respond. It isn't immediately
clear with that code should live, but once we iron out the details, it
could be part of a `@hotwired/turbo/stimulus` package, or a
`@hotwired/stimulus/turbo` package that users (or
`@hotwired/turbo-rails`) could opt-into.

[9944490]: hotwired@9944490
[hotwired#1083]: hotwired#1083
[@hotwired/turbo-railshotwired#533]: hotwired/turbo-rails#533
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Jan 27, 2024
Follow-up to [9944490][]
Related to [hotwired#1083]
Related to [@hotwired/turbo-railshotwired#533][]

The problem
---

Some client-side plugins are losing their state when elements are
morphed.

Without resorting to `MutationObserver` instances to determine when a
node is morphed, uses of those plugins don't have the ability to prevent
(without `[data-turbo-permanent]`) or respond to the morphing.

The proposal
---

This commit introduces a `turbo:before-morph-element` event that'll
dispatch as part of the Idiomorph `beforeNodeMorphed` callback. It'll
give interested parties access to the nodes before and after a morph. If
that event is cancelled via `event.preventDefault()`, it'll skip the
morph as if the element were marked with `[data-turbo-permanent]`.

Along with `turbo:before-morph-element`, this commit also introduces a
`turbo:before-morph-attribute` to correspond to the
`beforeAttributeUpdated` callback that Idiomorph provides. When
listeners (like an `HTMLDetailsElement`, an `HTMLDialogElement`, or a
Stimulus controller) want to preserve the state of an attribute, they
can cancel the `turbo:before-morph-attribute` event that corresponds
with the attribute name (through `event.detail.attributeName`).

Similarly, this commit adds a new `turbo:morph-element` event to be
dispatched for every morphed node (via Idiomorph's `afterNodeMorphed`
callback). The original implementation dispatched the event for the
`<body>` element as part of `MorphRenderer`'s lifecycle. That event will
still be dispatched, since `<body>` is the first element the callback
will fire for. In addition to that event, each individual morphed node
will dispatch one.

This commit re-introduced test coverage for a Stimulus controller to
demonstrate how an interested party might respond. It isn't immediately
clear with that code should live, but once we iron out the details, it
could be part of a `@hotwired/turbo/stimulus` package, or a
`@hotwired/stimulus/turbo` package that users (or
`@hotwired/turbo-rails`) could opt-into.

[9944490]: hotwired@9944490
[hotwired#1083]: hotwired#1083
[@hotwired/turbo-railshotwired#533]: hotwired/turbo-rails#533
@doits
Copy link

doits commented Apr 17, 2024

Is just stumbled across this. now that there the hooks available, is there a solution for the trix editor?

@doits
Copy link

doits commented Apr 17, 2024

I think I got it working like this:

addEventListener("turbo:before-morph-attribute", (event) => {
  const { target } = event;

  if (target.tagName == "TRIX-EDITOR") {
    // save new value as default so that `reset()` doesn't reset to old content
    target.defaultValue = target.value;
  }
});

addEventListener("turbo:morph-element", (event) => {
  const { target } = event;

  if (target.tagName == "TRIX-EDITOR") {
    // after morphing `value` is `null`, so set value to default which was saved before
    target.reset();

    // disconnect because we call `connectedCallback()` below
    target.disconnectedCallback();

    // remove old `editorController`, otherwise `connectedCallback` will skip some initialization
    target.editorController = null;

    // connect it
    target.connectedCallback();
  }
});

I haven't tested it extensively though yet, and not with stuff like attachments and images inside it.

What do you think?

@doits
Copy link

doits commented Apr 18, 2024

I just noticed my solution has a serious performance drawback. On one page I have 188 trix-editor elements. It works fine when loading the page, but on morphing it triggers the callbacks for every one of these. The connectedCallback makes the page hang for seconds with so many trix editors. Not sure why, since on initial page load the performance is fine (even with 188 editors).

Anyway it looks like I got a solution that is easier: Simply prevent morphing for the editor and toolbar.

addEventListener("turbo:before-morph-attribute", (event) => {
  const { target } = event;

  if (target.tagName == "TRIX-EDITOR" || target.tagName == "TRIX-TOOLBAR") {
    event.preventDefault();
  }
});

addEventListener("turbo:before-morph-element", (event) => {
  const { target } = event;

  if (target.tagName == "TRIX-EDITOR" || target.tagName == "TRIX-TOOLBAR") {
    event.preventDefault();
  }
});

A drawback is though that if the request actually changed the editor (e.g. different content got returned) it will not be reflected. I think there is nothing to do about it for now.

@ghiculescu
Copy link
Contributor

@jorgemanrubia sorry to tag you out of the blue, but I'd love your thoughts on this one. I'm sure the Hey calendar uses Trix somewhere, so I imagine this is something you guys would have come up against already?

domchristie pushed a commit to domchristie/turbo that referenced this issue Jul 20, 2024
Follow-up to [9944490][]
Related to [hotwired#1083]
Related to [@hotwired/turbo-railshotwired#533][]

The problem
---

Some client-side plugins are losing their state when elements are
morphed.

Without resorting to `MutationObserver` instances to determine when a
node is morphed, uses of those plugins don't have the ability to prevent
(without `[data-turbo-permanent]`) or respond to the morphing.

The proposal
---

This commit introduces a `turbo:before-morph-element` event that'll
dispatch as part of the Idiomorph `beforeNodeMorphed` callback. It'll
give interested parties access to the nodes before and after a morph. If
that event is cancelled via `event.preventDefault()`, it'll skip the
morph as if the element were marked with `[data-turbo-permanent]`.

Along with `turbo:before-morph-element`, this commit also introduces a
`turbo:before-morph-attribute` to correspond to the
`beforeAttributeUpdated` callback that Idiomorph provides. When
listeners (like an `HTMLDetailsElement`, an `HTMLDialogElement`, or a
Stimulus controller) want to preserve the state of an attribute, they
can cancel the `turbo:before-morph-attribute` event that corresponds
with the attribute name (through `event.detail.attributeName`).

Similarly, this commit adds a new `turbo:morph-element` event to be
dispatched for every morphed node (via Idiomorph's `afterNodeMorphed`
callback). The original implementation dispatched the event for the
`<body>` element as part of `MorphRenderer`'s lifecycle. That event will
still be dispatched, since `<body>` is the first element the callback
will fire for. In addition to that event, each individual morphed node
will dispatch one.

This commit re-introduced test coverage for a Stimulus controller to
demonstrate how an interested party might respond. It isn't immediately
clear with that code should live, but once we iron out the details, it
could be part of a `@hotwired/turbo/stimulus` package, or a
`@hotwired/stimulus/turbo` package that users (or
`@hotwired/turbo-rails`) could opt-into.

[9944490]: hotwired@9944490
[hotwired#1083]: hotwired#1083
[@hotwired/turbo-railshotwired#533]: hotwired/turbo-rails#533
sfnelson added a commit to katalyst/content that referenced this issue Aug 27, 2024
toddsundsted added a commit to toddsundsted/ktistec that referenced this issue Nov 20, 2024
Problems with the Trix editor blocked this before. I didn't identify
problems with any other components. The issue seems to be cases where
a component adds elements and the morph removes them but does not
reinitialize the component so it can fix things up.

See: hotwired/turbo-rails#533
See: hotwired/turbo#1083
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

7 participants