-
-
Notifications
You must be signed in to change notification settings - Fork 511
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
feat: an attempt at parsing markdown on paste #1490
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff!
What do you think about making this pluggable? i.e.: you could register a paste handler / preprocessor, and our "default preprocessor" changes the mimetype to text/markdown
if we think it's a better fit.
(just realized; we actually also have a client requesting Event listeners for "paste" events so we can do our own manipulation before inserting into the document
)
packages/core/src/api/clipboard/fromClipboard/pasteExtension.ts
Outdated
Show resolved
Hide resolved
packages/core/src/api/clipboard/fromClipboard/pasteExtension.ts
Outdated
Show resolved
Hide resolved
packages/core/src/api/clipboard/fromClipboard/pasteExtension.ts
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,60 @@ | |||
// Headings H1-H6. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious how many false positives we get. I didn't review the regexpes obviously.
Figured this might be useful: https://chatgpt.com/share/67d12910-3d78-8009-8bc0-ddd23dd7cba9
@@ -66,6 +76,8 @@ The hook takes two optional parameters: | |||
|
|||
`initialContent:` The content that should be in the editor when it's created, represented as an array of [Partial Blocks](/docs/manipulating-blocks#partial-blocks). | |||
|
|||
`pasteHandler`: A function that can be used to override the default paste behavior. See [Custom Paste Behavior](/docs/advanced/custom-paste-behavior) for more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Want to add a demo
/** | ||
* Custom paste handler that can be used to override the default paste behavior. | ||
* @returns The function should return `true` if the paste event was handled, otherwise it should return `false` if it should be canceled or `undefined` if it should be handled by another handler. | ||
* | ||
* @example | ||
* ```ts | ||
* pasteHandler: ({ defaultPasteHandler }) => { | ||
* return defaultPasteHandler({ pasteBehavior: "prefer-html" }); | ||
* } | ||
* ``` | ||
*/ | ||
pasteHandler?: (context: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also was considering making this into an onPaste, and expose it on the blocknoteview, but it would make it a lot harder to implement than the existing onChange stuff, because you expect there to only be one entity which is handling a paste.
I started doing it here:
diff --git i/docs/pages/docs/editor-basics/setup.mdx w/docs/pages/docs/editor-basics/setup.mdx
index 9c8bfef1f..07aa4e5cc 100644
--- i/docs/pages/docs/editor-basics/setup.mdx
+++ w/docs/pages/docs/editor-basics/setup.mdx
@@ -33,16 +33,6 @@ type BlockNoteEditorOptions = {
class?: string;
}) => Plugin;
initialContent?: PartialBlock[];
- pasteHandler?: (context: {
- view: EditorView;
- event: ClipboardEvent;
- editor: BlockNoteEditor;
- defaultPasteHandler: (context: {
- pasteBehavior?: "prefer-markdown" | "prefer-html";
- }) => boolean | undefined;
- convertHtmlToBlockNoteHtml: (html: string) => string;
- convertMarkdownToBlockNoteHtml: (markdown: string) => Promise<string>;
- }) => boolean | undefined;
resolveFileUrl: (url: string) => Promise<string>
schema?: BlockNoteSchema;
setIdAttribute?: boolean;
@@ -76,8 +66,6 @@ The hook takes two optional parameters:
`initialContent:` The content that should be in the editor when it's created, represented as an array of [Partial Blocks](/docs/manipulating-blocks#partial-blocks).
-`pasteHandler`: A function that can be used to override the default paste behavior. See [Custom Paste Behavior](/docs/advanced/custom-paste-behavior) for more.
-
`resolveFileUrl:` Function to resolve file URLs for display/download. Useful for creating authenticated URLs or implementing custom protocols.
`resolveUsers`: Function to resolve user information for comments. See [Comments](/docs/collaboration/comments) for more.
@@ -130,6 +118,16 @@ export type BlockNoteViewProps = {
editable?: boolean;
onSelectionChange?: () => void;
onChange?: () => void;
+ onPaste?: (context: {
+ view: EditorView;
+ event: ClipboardEvent;
+ editor: BlockNoteEditor;
+ defaultPasteHandler: (context: {
+ pasteBehavior?: "prefer-markdown" | "prefer-html";
+ }) => boolean | undefined;
+ convertHtmlToBlockNoteHtml: (html: string) => string;
+ convertMarkdownToBlockNoteHtml: (markdown: string) => Promise<string>;
+ }) => boolean | undefined;
theme?:
| "light"
| "dark"
@@ -158,6 +156,8 @@ export type BlockNoteViewProps = {
`onChange`: Callback fired when the editor content (document) changes.
+`onPaste`: A function that can be used to override the default paste behavior.
+
`theme`: The editor's theme, see [Themes](/docs/styling-theming/themes) for more about this.
`formattingToolbar`: Whether the [Formatting Toolbar](/docs/ui-components/formatting-toolbar) should be enabled.
diff --git i/packages/core/src/api/clipboard/fromClipboard/pasteExtension.ts w/packages/core/src/api/clipboard/fromClipboard/pasteExtension.ts
index 1f6e8bd1c..40901a3e7 100644
--- i/packages/core/src/api/clipboard/fromClipboard/pasteExtension.ts
+++ w/packages/core/src/api/clipboard/fromClipboard/pasteExtension.ts
@@ -125,7 +125,7 @@ export const createPasteFromClipboardExtension = <
return;
}
- return editor.settings.pasteHandler({
+ return editor.onPaste({
view,
event,
editor,
diff --git i/packages/core/src/editor/BlockNoteEditor.ts w/packages/core/src/editor/BlockNoteEditor.ts
index aab8824c0..2c07f1d52 100644
--- i/packages/core/src/editor/BlockNoteEditor.ts
+++ w/packages/core/src/editor/BlockNoteEditor.ts
@@ -218,12 +218,12 @@ export type BlockNoteEditorOptions<
*
* @example
* ```ts
- * pasteHandler: ({ defaultPasteHandler }) => {
+ * onPaste: ({ defaultPasteHandler }) => {
* return defaultPasteHandler({ pasteBehavior: "prefer-html" });
* }
* ```
*/
- pasteHandler?: (context: {
+ onPaste?: (context: {
view: EditorView;
event: ClipboardEvent;
editor: BlockNoteEditor<BSchema, ISchema, SSchema>;
@@ -475,6 +475,14 @@ export class BlockNoteEditor<
private onUploadStartCallbacks: ((blockId?: string) => void)[] = [];
private onUploadEndCallbacks: ((blockId?: string) => void)[] = [];
+ /**
+ * Custom paste handler that can be used to override the default paste behavior.
+ */
+ public readonly onPaste: Exclude<
+ BlockNoteEditorOptions<BSchema, ISchema, SSchema>["onPaste"],
+ undefined
+ >;
+
public readonly resolveFileUrl?: (url: string) => Promise<string>;
public readonly resolveUsers?: (userIds: string[]) => Promise<User[]>;
/**
@@ -487,10 +495,6 @@ export class BlockNoteEditor<
cellTextColor: boolean;
headers: boolean;
};
- pasteHandler: Exclude<
- BlockNoteEditorOptions<any, any, any>["pasteHandler"],
- undefined
- >;
};
public static create<
@@ -538,14 +542,6 @@ export class BlockNoteEditor<
cellTextColor: options?.tables?.cellTextColor ?? false,
headers: options?.tables?.headers ?? false,
},
- pasteHandler:
- options.pasteHandler ||
- ((context: {
- defaultPasteHandler: (context: {
- pasteBehavior?: "prefer-markdown" | "prefer-html";
- }) => boolean | undefined;
- }) =>
- context.defaultPasteHandler({ pasteBehavior: "prefer-markdown" })),
};
// apply defaults
@@ -630,6 +626,11 @@ export class BlockNoteEditor<
};
}
+ this.onPaste =
+ newOptions.onPaste ||
+ ((context) =>
+ context.defaultPasteHandler({ pasteBehavior: "prefer-markdown" }));
+
this.resolveFileUrl = newOptions.resolveFileUrl;
this.headless = newOptions._headless;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like, it is somewhere between the purely runtime event of onChange, and something you'd need to know upfront like uploadFile.
This distinction isn't super clear to me what should be on the view side or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think current approach is ok. You could do something like return true / false
to enable chaining but at this point I'm not sure it's worthwhile (don't see a lot of scenarios where that would be useful)
defaultPasteHandler: (context: { | ||
pasteBehavior?: "prefer-markdown" | "prefer-html"; | ||
}) => boolean | undefined; | ||
convertHtmlToBlockNoteHtml: (html: string) => string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API feedback:
- what about exposing
pasteHTML
pasteMarkdown
andpasteText
on the editor instead of the conversion functions? this way consumers also don't need to call into prosemirrorview.paste...
functions. - I'm not sure I like passing the
defaultPasteHandler
vs exporting that function directly (still in doubt)
} | ||
|
||
if (format === "text/html") { | ||
if (pasteBehavior === "prefer-markdown") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code style feedback:
instead of having these checks spread here and below (L95), I feel like it would be cleaner to check the flag earlier (or even in a separate "middleware" function) that would just set the format to "text/markdown" and then let the default behavior paste that. makes sense / wdyt? (I didn't completely think it out)
return true; | ||
} | ||
|
||
if (pasteBehavior === "prefer-markdown" && isMarkdown(data)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this single flag is what we want. We now always turn on "detection" and for both markdown and HTML. is that desirable?
Other options I can think of what the user might want:
- always paste plain text as markdown (i.e.: paste markdown instead of plain text at L103), regardless of detection
- Only run the detection for plain text, not for HTML (i.e.: always prefer regular HTML paste)?
- (Other combinations like only run the detection for HTML, not for plain text. Not sure if that makes sense)
Happy to think this through together. I don't think we need all options to be possible via different flags, especially since users can now add their own paste handlers. But I do think there should at least be a way to not rely on the auto-detection yet still paste text as markdown
/** | ||
* Custom paste handler that can be used to override the default paste behavior. | ||
* @returns The function should return `true` if the paste event was handled, otherwise it should return `false` if it should be canceled or `undefined` if it should be handled by another handler. | ||
* | ||
* @example | ||
* ```ts | ||
* pasteHandler: ({ defaultPasteHandler }) => { | ||
* return defaultPasteHandler({ pasteBehavior: "prefer-html" }); | ||
* } | ||
* ``` | ||
*/ | ||
pasteHandler?: (context: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think current approach is ok. You could do something like return true / false
to enable chaining but at this point I'm not sure it's worthwhile (don't see a lot of scenarios where that would be useful)
This was just a really quick stab at an approach to paste markdown