Skip to content

feat: Better parsing of external HTML attributes & inline styles #1605

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -116,29 +116,30 @@ function serializeBlock<
block.type as any
].implementation.toExternalHTML({ ...block, props } as any, editor as any);

const blockContentDataAttributes = [
...attrs,
...Array.from(ret.dom.attributes),
];

let listType = undefined;
if (orderedListItemBlockTypes.has(block.type!)) {
listType = "OL";
} else if (unorderedListItemBlockTypes.has(block.type!)) {
listType = "UL";
}

const elementFragment = doc.createDocumentFragment();
if (ret.dom.classList.contains("bn-block-content")) {
const blockContentDataAttributes = [
...attrs,
...Array.from(ret.dom.attributes),
].filter(
(attr) =>
attr.name.startsWith("data") &&
attr.name !== "data-content-type" &&
attr.name !== "data-file-block" &&
attr.name !== "data-node-view-wrapper" &&
attr.name !== "data-node-type" &&
attr.name !== "data-id" &&
attr.name !== "data-index" &&
attr.name !== "data-editable"
);

// ret.dom = ret.dom.firstChild! as any;
for (const attr of blockContentDataAttributes) {
(ret.dom.firstChild! as HTMLElement).setAttribute(attr.name, attr.value);
if (!listType) {
for (const attr of blockContentDataAttributes) {
(ret.dom.firstChild! as HTMLElement).setAttribute(
attr.name,
attr.value
);
}
}

addAttributesAndRemoveClasses(ret.dom.firstChild! as HTMLElement);
addAttributesAndRemoveClasses(ret.dom as HTMLElement);
elementFragment.append(...Array.from(ret.dom.childNodes));
} else {
elementFragment.append(ret.dom);
Expand All @@ -155,13 +156,6 @@ function serializeBlock<
ret.contentDOM.appendChild(ic);
}

let listType = undefined;
if (orderedListItemBlockTypes.has(block.type!)) {
listType = "OL";
} else if (unorderedListItemBlockTypes.has(block.type!)) {
listType = "UL";
}

if (listType) {
if (fragment.lastChild?.nodeName !== listType) {
const list = doc.createElement(listType);
Expand All @@ -172,6 +166,9 @@ function serializeBlock<
fragment.append(list);
}
const li = doc.createElement("li");
for (const attr of blockContentDataAttributes) {
li.setAttribute(attr.name, attr.value);
}
li.append(elementFragment);
fragment.lastChild!.appendChild(li);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ const NumberedListItemBlockContent = createStronglyTypedTiptapNode({
const startIndex =
parseInt(parent.getAttribute("start") || "1") || 1;

if (element.previousSibling || startIndex === 1) {
if (element.previousElementSibling || startIndex === 1) {
return {};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import { updateBlockCommand } from "../../api/blockManipulation/commands/updateB
import { InputRule } from "@tiptap/core";

export const quotePropSchema = {
...defaultProps,
backgroundColor: defaultProps.backgroundColor,
textColor: defaultProps.textColor,
};

export const QuoteBlockContent = createStronglyTypedTiptapNode({
Expand Down
53 changes: 53 additions & 0 deletions packages/core/src/blocks/defaultProps.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Attribute } from "@tiptap/core";

import type { Props, PropSchema } from "../schema/index.js";

// TODO: this system should probably be moved / refactored.
Expand All @@ -22,3 +24,54 @@ export type DefaultProps = Props<typeof defaultProps>;
// `blockContent` nodes. Ensures that they are not redundantly added to
// a custom block's TipTap node attributes.
export const inheritedProps = ["backgroundColor", "textColor"];

export const getBackgroundColorAttribute = (
attributeName = "backgroundColor"
): Attribute => ({
default: defaultProps.backgroundColor.default,
parseHTML: (element) => {
if (element.hasAttribute("data-background-color")) {
return element.getAttribute("data-background-color");
}

if (element.style.backgroundColor) {
return element.style.backgroundColor;
}

return defaultProps.backgroundColor.default;
},
renderHTML: (attributes) => {
if (attributes[attributeName] === defaultProps.backgroundColor.default) {
return {};
}

return {
"data-background-color": attributes[attributeName],
};
},
});

export const getTextColorAttribute = (
attributeName = "textColor"
): Attribute => ({
default: defaultProps.textColor.default,
parseHTML: (element) => {
if (element.hasAttribute("data-text-color")) {
return element.getAttribute("data-text-color");
}

if (element.style.color) {
return element.style.color;
}

return defaultProps.textColor.default;
},
renderHTML: (attributes) => {
if (attributes[attributeName] === defaultProps.textColor.default) {
return {};
}
return {
"data-text-color": attributes[attributeName],
};
},
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Extension } from "@tiptap/core";
import { defaultProps } from "../../blocks/defaultProps.js";
import { getBackgroundColorAttribute } from "../../blocks/defaultProps.js";

export const BackgroundColorExtension = Extension.create({
name: "blockBackgroundColor",
Expand All @@ -9,24 +9,7 @@ export const BackgroundColorExtension = Extension.create({
{
types: ["blockContainer", "tableCell", "tableHeader"],
attributes: {
backgroundColor: {
default: defaultProps.backgroundColor.default,
parseHTML: (element) =>
element.hasAttribute("data-background-color")
? element.getAttribute("data-background-color")
: defaultProps.backgroundColor.default,
renderHTML: (attributes) => {
if (
attributes.backgroundColor ===
defaultProps.backgroundColor.default
) {
return {};
}
return {
"data-background-color": attributes.backgroundColor,
};
},
},
backgroundColor: getBackgroundColorAttribute(),
},
},
];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
import { Mark } from "@tiptap/core";
import { getBackgroundColorAttribute } from "../../blocks/defaultProps.js";
import { createStyleSpecFromTipTapMark } from "../../schema/index.js";

const BackgroundColorMark = Mark.create({
name: "backgroundColor",

addAttributes() {
return {
stringValue: {
default: undefined,
parseHTML: (element) => element.getAttribute("data-background-color"),
renderHTML: (attributes) => ({
"data-background-color": attributes.stringValue,
}),
},
stringValue: getBackgroundColorAttribute("stringValue"),
};
},

Expand All @@ -25,10 +20,11 @@ const BackgroundColorMark = Mark.create({
return false;
}

if (element.hasAttribute("data-background-color")) {
return {
stringValue: element.getAttribute("data-background-color"),
};
if (
element.hasAttribute("data-background-color") ||
element.style.backgroundColor
) {
return {};
}

return false;
Expand Down
18 changes: 2 additions & 16 deletions packages/core/src/extensions/TextColor/TextColorExtension.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Extension } from "@tiptap/core";
import { defaultProps } from "../../blocks/defaultProps.js";
import { getTextColorAttribute } from "../../blocks/defaultProps.js";

export const TextColorExtension = Extension.create({
name: "blockTextColor",
Expand All @@ -9,21 +9,7 @@ export const TextColorExtension = Extension.create({
{
types: ["blockContainer", "tableCell", "tableHeader"],
attributes: {
textColor: {
default: defaultProps.textColor.default,
parseHTML: (element) =>
element.hasAttribute("data-text-color")
? element.getAttribute("data-text-color")
: defaultProps.textColor.default,
renderHTML: (attributes) => {
if (attributes.textColor === defaultProps.textColor.default) {
return {};
}
return {
"data-text-color": attributes.textColor,
};
},
},
textColor: getTextColorAttribute(),
},
},
];
Expand Down
14 changes: 5 additions & 9 deletions packages/core/src/extensions/TextColor/TextColorMark.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
import { Mark } from "@tiptap/core";
import { getTextColorAttribute } from "../../blocks/defaultProps.js";
import { createStyleSpecFromTipTapMark } from "../../schema/index.js";

const TextColorMark = Mark.create({
name: "textColor",
priority: 1000,

addAttributes() {
return {
stringValue: {
default: undefined,
parseHTML: (element) => element.getAttribute("data-text-color"),
renderHTML: (attributes) => ({
"data-text-color": attributes.stringValue,
}),
},
stringValue: getTextColorAttribute("stringValue"),
};
},

Expand All @@ -25,8 +21,8 @@ const TextColorMark = Mark.create({
return false;
}

if (element.hasAttribute("data-text-color")) {
return { stringValue: element.getAttribute("data-text-color") };
if (element.hasAttribute("data-text-color") || element.style.color) {
return {};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return {}?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we don't need to parse the color because it's already handled in the parseHTML function of the stringValue attribute.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we omit line 25 / 26 then entirely? it's still a bit confusing right? (maybe tiptap issue)

Copy link
Collaborator Author

@matthewlipski matthewlipski Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because return {} tells TipTap that the element should get parsed as a textColor mark, and to just use the default attribute values for that mark (here it is equivalent to return { textColor: defaultProps.textColor.default }). The parseHTML function of the stringValue attribute then does its thing, parsing the actual value of stringValue and replacing the default one. Returning false instead means the mark never gets parsed at all.

This is a kind of weird TipTap thing especially with return {}, but it does mostly make sense.

We could choose to parse the actual value of stringValue within the parseHTML function of TextColorMark, but this would result in duplicated code between TextColorMark and TextColorExtension.

}

return false;
Expand Down
17 changes: 13 additions & 4 deletions packages/core/src/pm-nodes/BlockContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import { mergeCSSClasses } from "../util/browser.js";

// Object containing all possible block attributes.
const BlockAttributes: Record<string, string> = {
blockColor: "data-block-color",
blockStyle: "data-block-style",
textColor: "data-text-color",
backgroundColor: "data-background-color",
id: "data-id",
depth: "data-depth",
depthChange: "data-depth-change",
Expand All @@ -31,7 +31,10 @@ export const BlockContainer = Node.create<{
parseHTML() {
return [
{
tag: "div",
// Not only `div`s as this way props from exported HTML can also be
// parsed correctly.
tag: "*",
priority: 500,
getAttrs: (element) => {
if (typeof element === "string") {
return false;
Expand All @@ -44,12 +47,18 @@ export const BlockContainer = Node.create<{
}
}

if (element.getAttribute("data-node-type") === "blockContainer") {
if (
element.getAttribute("data-node-type") === "blockContainer" ||
element.getAttribute("data-node-type") === "blockOuter"
) {
return attrs;
}

return false;
},
// Allows exported HTML to be parsed as both a `blockContainer` with a
// `blockContent` child, preserving all block data.
consuming: false,
},
];
},
Expand Down
Loading
Loading