Attachments target octet-stream content type

[nathancolgate]

Currently the parseHTML function for attachments only uses a figure[data-trix-attachment] selector. This makes sense as the Rhino editor is heavily geared towards Trix attachments being images.

However, while implementing the tiptap mention extension, we ran into a challenge where we want to capture/deliver mentions as Action Text Attachments.

Due to the broad selector above, we had to disable the attachments extension altogether to stop the editor from trying to turn our @mentions into image attachments.

rhinoEditor.starterKitOptions = {
    ...rhinoEditor.starterKitOptions,
    rhinoGallery: false,
    rhinoAttachment: false,
    rhinoFigcaption: false,
    rhinoImage: false,
  }

This PR targets the octet-stream content type specifically, which is what Rails delivers by default.

Merging this PR would allow users to keep the image attachment functionality of Rhino, while also adding custom content types for other attachments/extensions via the attachable_content_type method on objects. For example:

class User < ApplicationRecord
  def attachable_content_type
    "application/vnd.rails.user"
  end
end

[changeset-bot]

⚠️ No Changeset found

Latest commit: cf14d66

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
rhino-editor	❌ Failed (Inspect)		Sep 12, 2023 8:28pm

[KonnorRogers]

Hmmm....I'm not sure overriding the MimeType is the right way to go here.

I'm not 100%, but I think this would break anyone using the old "CustomAttachment" functionality from Trix.

however, perhaps we do something slightly different.

would something like using data-rhino-serialize="false" perhaps be a better way to get around this issue?

the other option is to do something like: <div content=" "></div> and make it a blank string with a space.

content use the old escape hatch that Trix used to prevent turning attachments into images and what's used in Rhino as well

rhino-editor/src/exports/extensions/attachment.ts

Line 330 in a942226

if (!content) {

[nathancolgate]

🤷 I'm not sure either, but for us it was the easiest way to differentiate different kinds of attachments.

• content - we didn't want to sniff around here and create some kind of dependency between the view and the javascript powering the editor (the designers just want to modify the views and not worry about something breaking)
• sgid - i'm not sure if the trix JS can parse things. I know rails can reverse engineer what object type it is. This might be cool.
• contentType - low hanging fruit.

Check out how we're doing mention support over in #113 . It might shed some light on where we're coming from, and how we're trying to handle all kinds of "attachments".

Thanks again for your work on this! It's been a fun week exploring.

[KonnorRogers]

@nathancolgate

1.) honestly, I agree. TipTap having an editable schema renders the content attribute an ugly hack that exists purely for backwards compatibility.

2.) right now SGID is purely an attribute we need to send back to Rails. Rhino doesn't do any introspection.

3.) fair point. IIRC contentType may actually be HTML for custom attachments so this may be okay...

[nathancolgate]

@KonnorRogers Rails uses the application/octet-stream by default, but provides an easy way to over-ride it.

# app/models/user.rb
class User < ApplicationRecord
  # A custom content type for easy querying
  def attachable_content_type
    "application/vnd.active_record.user"
  end
end

I couldn't find anywhere else in Rails where this is used, so hopefully there aren't any adverse side effects.

[KonnorRogers]

@nathancolgate I just confirmed, rendering custom attachments does use the content-type="application/octet-stream"

I may need to walk this one back 🤔

[nathancolgate]

@KonnorRogers Even after changing the attachable_content_type override? Hmmm... that's not what we're seeing.

[KonnorRogers]

@nathancolgate Changing the attachable_content_type works. But for a new application with no changes is what I meant. The goal is to out of the box support an existing Trix implementation. I haven't figured out if there's a better way to handle this. Just spitballing right now.