fix previewable and non-previewable attachments (#125)

* add changeseT

* prettier

* remove console.log

* remove console.log

* remove console.log

* make TS happy

* prettier

.changeset/swift-walls-lay.md

@@ -0,0 +1,6 @@
+---
+"rhino-editor": patch
+---
+
+- Fixed a bug in rendering of non-previewable attachments not having an `sgid` or `url`
+- Fixed a bug in previewable attachments not having their height + width attached

docs/package.json

@@ -23,8 +23,8 @@
     "@hotwired/turbo": "^7.3.0",
     "@konnorr/bridgetown-quick-search": "^3.7.4",
     "@shoelace-style/shoelace": "^2.7.0",
-    "@tiptap/extension-character-count": "2.1.7",
-    "@tiptap/extension-code-block-lowlight": "2.1.7",
+    "@tiptap/extension-character-count": "~2.1.7",
+    "@tiptap/extension-code-block-lowlight": "~2.1.7",
     "linkifyjs": "^4.1.1",
     "lowlight": "^2.9.0",
     "prosemirror-view": "1.28.2",

package.json

@@ -66,13 +66,13 @@
     "@rails/activestorage": "^7.0.6",
     "@tiptap/core": "2.1.7",
     "@tiptap/extension-code-block": "2.1.7",
-    "@tiptap/extension-focus": "2.1.7",
-    "@tiptap/extension-image": "2.1.7",
-    "@tiptap/extension-link": "2.1.7",
-    "@tiptap/extension-placeholder": "2.1.7",
-    "@tiptap/extension-strike": "2.1.7",
-    "@tiptap/pm": "2.1.7",
-    "@tiptap/starter-kit": "2.1.7",
+    "@tiptap/extension-focus": "~2.1.7",
+    "@tiptap/extension-image": "~2.1.7",
+    "@tiptap/extension-link": "~2.1.7",
+    "@tiptap/extension-placeholder": "~2.1.7",
+    "@tiptap/extension-strike": "~2.1.7",
+    "@tiptap/pm": "~2.1.7",
+    "@tiptap/starter-kit": "~2.1.7",
     "linkifyjs": "^4.1.1",
     "lit": "^2.8.0",
     "prosemirror-utils": "1.2.1-0",

src/exports/attachment-manager.ts

@@ -17,6 +17,8 @@ export interface AttachmentManagerAttributes {
   content?: Maybe<string>;
   caption?: Maybe<string>;
   url?: Maybe<string>;
+  width?: Maybe<number>;
+  height?: Maybe<number>;
 }
 
 /**
@@ -27,6 +29,15 @@ export class AttachmentManager implements AttachmentManagerAttributes {
   attributes: AttachmentManagerAttributes;
   editorView: EditorView;
 
+  static get previewableRegex() {
+    return /^image(\/(gif|png|jpe?g)|$)/;
+  }
+
+  static isPreviewable(str: string) {
+    // (this || AttachmentManager) works around a strange bug in ESBuild v0.14.17 around how it transpiles static functions.
+    return (this || AttachmentManager).previewableRegex.test(str);
+  }
+
   constructor(obj: AttachmentManagerAttributes, editorView: EditorView) {
     this.editorView = editorView;
     this.attributes = {
@@ -45,34 +56,65 @@ export class AttachmentManager implements AttachmentManagerAttributes {
     }
   }
 
-  setAttributes(obj: Omit<AttachmentManagerAttributes, "src" | "file">) {
-    this.attributes.sgid = obj.sgid;
+  setAttributes(obj: Partial<AttachmentManagerAttributes>) {
+    this.attributes = Object.assign(this.attributes, obj);
+
+    /**
+     * These are the old Trix custom attachment APIs.
+     */
+    if (obj.content) {
+      this.setNodeMarkup({
+        sgid: this.attributes.sgid,
+        content: this.attributes.content,
+      });
 
-    if (obj.content == null && obj.url) {
+      return;
+    }
+
+    /**
+     * Sometimes we don't have a URL. We need that.
+     */
+    if (!obj.url) {
+      return;
+    }
+
+    const isPreviewable = (
+      this.constructor as unknown as typeof AttachmentManager
+    ).isPreviewable;
+
+    const contentType = this.contentType;
+
+    if (contentType && isPreviewable(contentType)) {
       /** This preloads the image so we don't show a big flash. */
       const image = new Image();
 
-      this.attributes.url = obj.url;
-
       image.src = obj.url;
 
       image.onload = () => {
+        this.attributes.width = image.naturalWidth;
+        this.attributes.height = image.naturalHeight;
+
         this.setNodeMarkup({
           sgid: this.attributes.sgid,
           url: this.attributes.url,
           src: this.attributes.url,
           href: this.attributes.url + "?content-disposition=attachment",
-          width: image.naturalWidth,
-          height: image.naturalHeight,
+          width: this.attributes.width,
+          height: this.attributes.height,
+          contentType: this.contentType,
         });
         image.remove();
       };
       return;
     }
 
+    /**
+     * These are non-previewable assets like CSVs, Word docs, etc.
+     */
     this.setNodeMarkup({
       sgid: this.attributes.sgid,
-      content: this.attributes.content,
+      url: this.attributes.url,
+      contentType: this.contentType,
     });
   }
 
@@ -141,6 +183,14 @@ export class AttachmentManager implements AttachmentManagerAttributes {
     this.attributes.content = val;
   }
 
+  get height() {
+    return this.attributes.height;
+  }
+
+  get width() {
+    return this.attributes.width;
+  }
+
   get caption(): string {
     return toDefaultCaption({
       fileName: this.fileName,

src/exports/extensions/attachment.ts

@@ -97,11 +97,8 @@ function handleCaptions(
   return modified;
 }
 
-/** https://github.com/basecamp/trix/blob/main/src/trix/models/attachment.coffee#L4 */
-const isPreviewable = /^image(\/(gif|png|jpe?g)|$)/;
-
 function canPreview(previewable: Boolean, contentType: Maybe<string>): Boolean {
-  return previewable || contentType?.match(isPreviewable) != null;
+  return previewable || AttachmentManager.isPreviewable(contentType || "");
 }
 
 function toExtension(fileName: Maybe<string>): string {

tests/fixtures/addresses.csv

@@ -0,0 +1,6 @@
+John,Doe,120 jefferson st.,Riverside, NJ, 08075
+Jack,McGinnis,220 hobo Av.,Phila, PA,09119
+"John ""Da Man""",Repici,120 Jefferson St.,Riverside, NJ,08075
+Stephen,Tyler,"7452 Terrace ""At the Plaza"" road",SomeTown,SD, 91234
+,Blankman,,SomeTown, SD, 00298
+"Joan ""the bone"", Anne",Jet,"9th, at Terrace plc",Desert City,CO,00123

tests/rails/app/frontend/controllers/tip_tap_mirror_controller.js

@@ -6,9 +6,11 @@ export default class TipTapMirrorController extends Controller {
 	}
 	connect () {
 		const editor = this.element
-		replaceWithWrapper(this.trixInput, "value", (_obj, _property, value) => {
-			editor.editor.commands.setContent(value)
-		})
+		setTimeout(() => {
+		  replaceWithWrapper(this.trixInput, "value", (_obj, _property, value) => {
+			  editor.editor.commands.setContent(value)
+		  })
+		}, 30)
 	}
 }
 

tests/rails/package.json

@@ -1,7 +1,7 @@
 {
   "devDependencies": {
-    "playwright": "~1.28.0",
     "@playwright/test": "~1.28.0",
+    "playwright": "~1.28.0",
     "vite": "^4.2.0",
     "vite-plugin-ruby": "^3.2.0"
   },
@@ -11,15 +11,15 @@
     "@rails/actioncable": "^7.0.0",
     "@rails/actiontext": "^7.0.0",
     "@rails/activestorage": "^7.0.0",
-    "@tiptap/extension-collaboration": "2.0.3",
-    "@tiptap/extension-collaboration-cursor": "2.0.3",
-    "@y-rb/actioncable": "^0.1.5",
+    "@tiptap/extension-collaboration": "~2.1.7",
+    "@tiptap/extension-collaboration-cursor": "~2.1.7",
+    "@y-rb/actioncable": "^0.2.1",
     "rhino-editor": "link:../../",
-    "trix": "^1.3",
+    "trix": "^2.0.5",
     "y-prosemirror": "1.2.1",
-    "y-protocols": "^1.0.1",
+    "y-protocols": "^1.0.6",
     "y-websocket": "1.5.0",
-    "yjs": "13.6.0"
+    "yjs": "13.6.8"
   },
   "pnpm": {
     "overrides": {