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

Improve trix and action text support #2715

Open
Paul-Bob opened this issue Apr 22, 2024 · 14 comments · May be fixed by #3609
Open

Improve trix and action text support #2715

Paul-Bob opened this issue Apr 22, 2024 · 14 comments · May be fixed by #3609
Assignees
Labels
Bug Something isn't working Field

Comments

@Paul-Bob
Copy link
Contributor

Feature

Trix field have some limitations:

  1. File attachment is possible only on persistent records Avo demo error: "You can't upload files into the Trix editor until you save the resource" #2713
  2. File attachment is possible only whit explicit attachment_key
  3. File attachments are corrupt except images on action text. (has_rich_text) After uploading File in ActionText (Trix) it can not be found when rendering (missing attachable) #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.
@davekruse
Copy link
Contributor

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.

@sedubois
Copy link
Contributor

sedubois commented May 7, 2024

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
Copy link
Contributor

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

@adrianthedev
Copy link
Collaborator

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?

@sedubois
Copy link
Contributor

sedubois commented May 23, 2024

@adrianthedev your new demo app shows the issue:

@adrianthedev
Copy link
Collaborator

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?

@sedubois
Copy link
Contributor

sedubois commented May 28, 2024

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

@adrianthedev
Copy link
Collaborator

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 adrianthedev moved this from Next up to To Do in Issues Jun 21, 2024
@adrianthedev
Copy link
Collaborator

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 #2958

@PedroAugustoRamalhoDuarte
Copy link
Contributor

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

@PedroAugustoRamalhoDuarte PedroAugustoRamalhoDuarte linked a pull request Jan 25, 2025 that will close this issue
4 tasks
@PedroAugustoRamalhoDuarte
Copy link
Contributor

I've initiated the solution mentioned above in this PR: #3609.

@Paul-Bob
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@PedroAugustoRamalhoDuarte
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Field
Projects
Status: To Do
Development

Successfully merging a pull request may close this issue.

5 participants