avo icon indicating copy to clipboard operation
avo copied to clipboard

Improve trix and action text support

Open Paul-Bob opened this issue 1 year ago • 9 comments

Feature

Trix field have some limitations:

  1. File attachment is possible only on persistent records https://github.com/avo-hq/avo/issues/2713
  2. File attachment is possible only whit explicit attachment_key
  3. File attachments are corrupt except images on action text. (has_rich_text) https://github.com/avo-hq/avo/issues/2538

Let's explore this possibility:

  1. When attachment_key is set that means we need a persistent record with that attachment_key association. So limitation points 1 and 2 still applying.
  2. When attachment_key is not set that means the user do not want to associate the attachments to a fixed key/association. We can generate a random key like here https://trix-editor.org/js/attachments.js and in this case the limitation points 1 and 2 will not apply, i.e there is no need for a record and neither attachment_key.

Paul-Bob avatar Apr 22 '24 07:04 Paul-Bob

Editing an ActionText field with Trix field separates captions from images.

  1. Add an image
  2. Click and change the caption
  3. Save
  4. Edit

In the trix edit view, the caption is rendered as regular text (a couple
line breaks and plain text). Saving the field causes that caption to become part of the body. Rendering the ActionText elsewhere also shows the text as part of the body - no longer attached to the image.

davekruse avatar May 02 '24 14:05 davekruse

I think a helpful first step would be to ensure models using has_rich_text are added to the test suite and to the demo website. Currently the use of has_rich_text does not seem to be tested/demonstrated/documented anywhere. See issue.

I can also confirm that captions are broken as noted by @davekruse.

sedubois avatar May 07 '24 06:05 sedubois

Should a separate issue be opened to fix the broken captions?

sedubois avatar May 23 '24 11:05 sedubois

Yes, please open a new issue, but first, can you please test if a new app with Trix and AS works well and the captions aren't broken?

adrianthedev avatar May 23 '24 11:05 adrianthedev

@adrianthedev your new demo app shows the issue:

sedubois avatar May 23 '24 11:05 sedubois

I wrote on the previous PR that that will require us to digg in deeper for such an edge-case. Would you be able to sponsor that fix?

adrianthedev avatar May 28 '24 09:05 adrianthedev

@adrianthedev could you please clarify what you mean by edge case? Maybe there is a misunderstanding. Using has_rich_text is to my knowledge the official way to use Action Text (and, by extension, Trix).

Screenshot 2024-05-28 at 12 04 35

What I consider to be an edge (a.k.a unofficial) case is to use the Trix editor on top of a plain text DB column without using has_rich_text, as is currently done throughout the Avo documentation and demo code.

Currently I am trying to add Avo to a production system but in my free time, because it is just being evaluated; so in turn I am not able to sponsor a fix although I do try to support the project by documenting the issues found with reproduction steps.

I hope it's clearer this way. Although I can't sponsor I am happy to support the project in a more modest way and wish you much success.

sedubois avatar May 28 '24 10:05 sedubois

Following you last comment I am talking about captions on attachments with has_rich_text.

What I mean by digging deeper is that we first need to see if they work on a regular Trix and Action Text setup. It's not a given that the captions just work in a non-Avo setup. I found that out many times. That takes time. We can't assign that time right now to this issue.

One thing that we'd love some help with and definitely could help faster is if we had that validation done by you (or someone else). That means setting up a rails repo with action text and trix installed and try to see if the captions bug still manifests itself in that way.

Then, if we find out that it's an Avo bug, we can take action and try to fix it (ourselves, you, or together).

The reason why I'm saying it's an edgecase is that it works fine with a text DB column. Something is a bit off.

I sincerely appreciate you trying out Avo and insisting to add it to your infrastructure! I really do!

Of course I'm not expecting you personally to sponsor this development. If that's the vibe I gave, I'm sorry. I feel (and correct me if I'm wrong) that the company is getting a lot of value from using Avo. It's not that out of place to sponsor something like that.

So, let's try to fix this together. Would you be able to create that repo (or use https://github.com/avo-hq/trix-field-demo) to test out in a non-Avo environemtn if this is still happening?

adrianthedev avatar May 28 '24 10:05 adrianthedev

Editing an ActionText field with Trix field separates captions from images.

  1. Add an image
  2. Click and change the caption
  3. Save
  4. Edit

In the trix edit view, the caption is rendered as regular text (a couple line breaks and plain text). Saving the field causes that caption to become part of the body. Rendering the ActionText elsewhere also shows the text as part of the body - no longer attached to the image.

This has been fixed with https://github.com/avo-hq/avo/pull/2958

adrianthedev avatar Jul 08 '24 19:07 adrianthedev

Hi everyone, I've been digging deeper into this issue and have done a lot of research in the Rails codebase. It seems Rails handles attachment uploads a bit differently. I’d like to contribute to resolving this issue.

Is there any particular reason why we shouldn't stick with Rails' direct upload solution for handling attachments? @adrianthedev , @Paul-Bob

I've initiated the solution mentioned above in this PR: https://github.com/avo-hq/avo/pull/3609.

Hi @PedroAugustoRamalhoDuarte, thanks for looking into this.

Is there any particular reason why we shouldn't stick with Rails' direct upload solution for handling attachments?

One of the reasons that we handle the upload request on a custom controller is authorization.


It seems to me that all the issues mentioned in the original description have been resolved. What exactly are you trying to fix?

Paul-Bob avatar Jan 27 '25 07:01 Paul-Bob

I just noticed that for creation we're not applying the authorization so using the solution from the PR shouldn't introduce any regression on creation. Is it possible to keep using the custom controller and redirect to the rails direct upload URL around here

Paul-Bob avatar Jan 27 '25 10:01 Paul-Bob

@Paul-Bob Thanks for the fast response, i am having problem with the third one, i am attaching a pdf, but when i render this pdf the attachment is missing.