Choose contentDOM wrapper more carefully and add display: contents

nytimes · May 9, 2023 · f997b0c · f997b0c
1 parent 5f319c5
commit f997b0c
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 9 deletions.
diff --git a/.yarn/versions/c193871f.yml b/.yarn/versions/c193871f.yml
@@ -0,0 +1,2 @@
+releases:
+  "@nytimes/react-prosemirror": patch
diff --git a/src/nodeViews/createReactNodeViewConstructor.tsx b/src/nodeViews/createReactNodeViewConstructor.tsx
@@ -8,7 +8,6 @@ import type {
 } from "prosemirror-view";
 import React, {
   Dispatch,
-  ReactHTML,
   SetStateAction,
   forwardRef,
   useImperativeHandle,
@@ -17,6 +16,8 @@ import React, {
 import type { ComponentType, ReactNode } from "react";
 import { createPortal } from "react-dom";
 
+import { phrasingContentTags } from "./phrasingContentTags.js";
+
 export interface NodeViewComponentProps {
   decorations: readonly Decoration[];
   getPos: () => number;
@@ -106,10 +107,16 @@ export function createReactNodeViewConstructor(
 
     const { dom, contentDOM, component: ReactComponent } = reactNodeView;
 
-    const ContentDOMElementType = contentDOM?.tagName.toLocaleLowerCase() as
-      | keyof ReactHTML
-      | undefined;
-
+    // Use a span if the provided contentDOM is in the "phrasing" content
+    // category. Otherwise use a div. This is our best attempt at not
+    // breaking the intended content model, for now.
+    //
+    // https://developer.mozilla.org/en-US/docs/Web/HTML/Content_categories#phrasing_content
+    const ContentDOMWrapper =
+      contentDOM &&
+      (phrasingContentTags.includes(contentDOM.tagName.toLocaleLowerCase())
+        ? "span"
+        : "div");
     /**
      * Wrapper component to provide some imperative handles for updating
      * and re-rendering its child. Takes and renders an arbitrary ElementType
@@ -152,10 +159,10 @@ export function createReactNodeViewConstructor(
           decorations={decorations}
           isSelected={isSelected}
         >
-          {ContentDOMElementType && (
-            <ContentDOMElementType
-              // @ts-expect-error There are too many HTML tags, so typescript won't compute this union type
-              ref={(nextContentDOMWrapper: HTMLElement | null) => {
+          {ContentDOMWrapper && (
+            <ContentDOMWrapper
+              style={{ display: "contents" }}
+              ref={(nextContentDOMWrapper) => {
                 setContentDOMWrapper(nextContentDOMWrapper);
               }}
             />

diff --git a/src/nodeViews/phrasingContentTags.ts b/src/nodeViews/phrasingContentTags.ts
@@ -0,0 +1,49 @@
+export const phrasingContentTags = [
+  "abbr",
+  "audio",
+  "b",
+  "bdo",
+  "br",
+  "button",
+  "canvas",
+  "cite",
+  "code",
+  "data",
+  "datalist",
+  "dfn",
+  "em",
+  "embed",
+  "i",
+  "iframe",
+  "img",
+  "input",
+  "kbd",
+  "keygen",
+  "label",
+  "mark",
+  "math",
+  "meter",
+  "noscript",
+  "object",
+  "output",
+  "picture",
+  "progress",
+  "q",
+  "ruby",
+  "s",
+  "samp",
+  "script",
+  "select",
+  "small",
+  "span",
+  "strong",
+  "sub",
+  "sup",
+  "svg",
+  "textarea",
+  "time",
+  "u",
+  "var",
+  "video",
+  "wbr",
+];