hotwired / turbo-rails

Use Turbo in your Ruby on Rails app
https://turbo.hotwired.dev
MIT License
2.07k stars 318 forks source link

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

Open BaggioGiacomo opened 9 months ago

BaggioGiacomo commented 9 months ago

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

<%= turbo_refreshes_with method: :morph, scroll: :preserve %>
<%= yield :head %>

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

adrienpoly commented 9 months ago

I did a test also with a Trix editor and had similar issue. This relates to this more general issue https://github.com/hotwired/turbo/issues/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 commented 9 months ago

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 commented 9 months ago

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.

BaggioGiacomo commented 9 months ago

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 commented 8 months ago

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

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.
SylarRuby commented 7 months ago

Strange. I can't reproduce this issue.

doits commented 4 months ago

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

doits commented 4 months ago

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 commented 4 months ago

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 commented 3 months ago

@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?