Skip to content
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

Add RichTextEditor component #1

Closed
wants to merge 22 commits into from

Conversation

emilhe
Copy link

@emilhe emilhe commented Mar 11, 2025

This PR adds a new RichTextEditor, a wrapping of the most recent RTE componnent from Mantine based on tiptap. Since this is a (rather large) extension, this branch is based on the (ongoing) async work by @AnnMarieW (to enable async loading).

Design considerations

Plugins

The tiptap component comes with a huge number of extensions. In the current design, support is hardcoded for a number of predefined plugins, which can then be selectively enabled by passing a list of strings from the Python layer with the names of the plugins to enable. Note however that all supported plugins are included in the component chunk (!). I would love to hear if anyone has ideas for a better design (the currently added plugins are small though, so at present bundle size increase due to plugins is not a huge issue).

Toolbar

The toolbar construction is performed in React using a ReactNode hierarchy (i.e. components). My initial idea was to pass a such hierarchy from Dash direcly (to keep the design as aligned with Mantine as possible), but as there are so many controls, that would require the creation > 10 components just for toolbar configuration. As an alternative, I have drafted a design, where a JSON-like structure with the component name(s) is passed instead (similar to how the plugins are passed).

Content

The content of the component can be extracted either as html or (ProseMirror) json. Per default content tracking is disabled to improve performance. Setting the format property enables content tracking (i.e. the Dash content property is updated as the content changes), either in html or json format depending on its value. This design deviates from the Mantine design, where content tracking is achieve through events. If you have an idea for a better design, please chip in.

Bundle size

The RichTextEditor component supports async loading. With the current design, the increase in the (main) bundle is therefore small (about 3kB, which I believe is acceptable). The component itself (including currently added plugins) yields a chunk > 300 kB size. As more plugins are added, the size will increase. At some point, we may consider splitting the plugins into chunks.

Example(s)

The following (Dash 3) example is a reproduction of the first/main example in the Mantine docs,

image

from dash import Dash, Input, html

import dash_mantine_components as dmc

# Content from the Mantine example.
content = """<h2 style="text-align: center;">Welcome to Mantine rich text editor</h2><p><code>RichTextEditor</code> component focuses on usability and is designed to be as simple as possible to bring a familiar editing experience to regular users. <code>RichTextEditor</code> is based on <a href="https://tiptap.dev/" rel="noopener noreferrer" target="_blank">Tiptap.dev</a> and supports all of its features:</p><ul><li>General text formatting: <strong>bold</strong>, <em>italic</em>, <u>underline</u>, <s>strike-through</s> </li><li>Headings (h1-h6)</li><li>Sub and super scripts (<sup>&lt;sup /&gt;</sup> and <sub>&lt;sub /&gt;</sub> tags)</li><li>Ordered and bullet lists</li><li>Text align&nbsp;</li><li>And all <a href="https://tiptap.dev/extensions" target="_blank" rel="noopener noreferrer">other extensions</a></li></ul>"""

# Setup the Dash app.
app = Dash(__name__, external_stylesheets=dmc.styles.ALL)
app.layout = dmc.MantineProvider(
    html.Div(
        dmc.RichTextEditor(
            content=content,  # can be a JSON object or an HTML string
            format="json",  # format of the content to be returned to Dash
            extensions=[
                "StarterKit",
                "Underline",
                "Link",
                "Superscript",
                "SubScript",
                "Highlight",
                "TextAlign",
            ],
            toolbar={
                "sticky": True,
                "controlsGroups": [
                    [
                        "Bold",
                        "Italic",
                        "Underline",
                        "Strikethrough",
                        "ClearFormatting",
                        "Highlight",
                        "Code",
                    ],
                    ["H1", "H2", "H3", "H4"],
                    [
                        "Blockquote",
                        "Hr",
                        "BulletList",
                        "OrderedList",
                        "Subscript",
                        "Superscript",
                    ],
                    ["Link", "Unlink"],
                    ["AlignLeft", "AlignCenter", "AlignJustify", "AlignRight"],
                    ["Undo", "Redo"],
                ],
            },
            id="rte",
        ),
        style=dict(margin="5px"),
    )
)


# Read the content from the editor.
@app.callback(Input("rte", "content"))
def update_content(content: str):
    print(content)


if __name__ == "__main__":
    app.run(debug=True)

TODO

  • Clean up the PR (remove TODO, bundles example(s) etc.)
  • Add more plugins?
  • Get the async branch merged (and merge any new changes into this branch)
  • Make it possible to update content after component creation

"@plotly/dash-component-plugins": "^1.2.0",
"@tiptap/extension-highlight": "^2.11.5",
Copy link
Owner

Choose a reason for hiding this comment

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

It's better to use tiptap version 2.9.1 -- the one that is used and tested with Mantine 7.17.1. We've had issues when using different versions of the dependencies (like with carousel)

https://github.com/mantinedev/mantine/blob/master/package.json#L77

Copy link
Author

@emilhe emilhe Mar 11, 2025

Choose a reason for hiding this comment

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

That's a great point! Now that I am thinking about it - wouldn't it be better to remove the (explicit) dependencies all together and let npm would figure out the correct versions itself? I guess the same gues for day-js, recharts, and embla-carousel-..., unless you use something (very) version specific that is not synchronized with Mantine itself - which seems unlikely... Do you remember why the carousel dependency was added in the first place?

Copy link
Owner

@AnnMarieW AnnMarieW Mar 11, 2025

Choose a reason for hiding this comment

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

Hmm - it probably is better to leave the mantine dependencies specified (except Carousel).

We upgraded Carousel to use Embla V8 to enable the autoScroll feature. Initially, updating to a new major version seemed like it would be OK, but it unexpectedly broke some props. (These props are removed in Mantine V8 which uses Embla 8) You can see the discussion in this PR.

Now we will have to keep Embla pinned to 8 so it doesn't break the autoScroll as discussed in this issue

Copy link
Author

Choose a reason for hiding this comment

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

Ah, I just realised that the packages are also needed for the type definitions. Hence they cannot be remove. I'll change the package version instead, as you suggested ;)

@AnnMarieW
Copy link
Owner

Any security issues with allowing raw html in the text editor? Does TipTap sanitize it on their end?

@AnnMarieW
Copy link
Owner

Sending the content back to dash could be an issue. It updates on every keystroke and when it's json format, there is a new dict for each keystroke.

@emilhe
Copy link
Author

emilhe commented Mar 12, 2025

Any security issues with allowing raw html in the text editor? Does TipTap sanitize it on their end?

As I understand, the rendering in tiptap is controlled by the extensions, and any non-matching tags are filtered out. In other words, tiptap itself does not pose any security issues - but a plugin might. Looking at the currently added plugins, the most obious security issues would be with the link extension. That seems to have been fixed though, so at the top of my head, I don't see any issues. It is a point worth considering though every time we add a new extension.

@emilhe
Copy link
Author

emilhe commented Mar 12, 2025

Sending the content back to dash could be an issue. It updates on every keystroke and when it's json format, there is a new dict for each keystroke.

I was concerned about the same thing, which is why I have disabled content tracking by default. I haven't made and performance tests though, so I may be doing premature optimiztion ;). In the current design with content tracking enabled, there are a few common Dash patterns to limit data exchanged wih the server,

  • Use the content property as State in a callback instead of Input and trigger the callback using e.g. a button instead
  • Use the content property in a clientside callback, and only relay data to a Python callback when needed

That being said, there are of couse ways to improve performance from the component itself. One approach would be to add an (optional) debounce so that the user can control how often callbacks are triggered. Do you have other ideas? :)

EDIT: I have just added a debounce property wíth a default value of 100 ms to adress the performance consideration above

@emilhe
Copy link
Author

emilhe commented Mar 12, 2025

Just a thought - maybe we should loade all extensions per default? That would save people a few lines of code. And in most cases, I don't think the overhead would be noticeable (at least not with the currently bundled extensions). What do you think, @AnnMarieW ?

} as const;

// Simple debounce function. Maybe move to utils?
function debounceCall(func, wait) {
Copy link
Owner

Choose a reason for hiding this comment

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

No need to write your own debounce function - Mantine has a hook. Also if we can add an onBlur, we could also add a boolean debounce so it's consistent with other inputs like https://github.com/snehilvj/dash-mantine-components/blob/master/src/ts/components/core/input/Textarea.tsx

Copy link
Author

@emilhe emilhe Mar 12, 2025

Choose a reason for hiding this comment

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

I was considering using that hook as first. My first design was along these lines,

  // Local state to store the content from the editor
  const [content, setContent] = useState('');

  // Debounce the content value by 500ms
  const [debouncedContent] = useDebouncedValue(content, 500);

  // Update props whenever debounced content changes
  useEffect(() => {
    if (debouncedContent) {
      setProps({ content: debouncedContent });
    }
  }, [debouncedContent, setProps]);

  // The onUpdate function updates the local state immediately
  const onUpdate = ({ editor }) => {
    if (format === 'json') {
      setContent(editor.getHTML());
    } else if (format === 'html') {
      setContent(editor.getHTML());
    }
  };

In this design i do debounce the setProps call - but not the getHTML() / getHTML() calls (that keeps getting invoked on each key stoke). So I figured the implementation without the hook would be more performant (although I am not sure of the difference). I did come up with a solution that avoids the call,

  const editorRef = useRef(null);
  // A counter to trigger updates
  const [trigger, setTrigger] = useState(0);
  // Debounce the trigger so the effect runs only once after 500ms of inactivity
  const [debouncedTrigger] = useDebouncedValue(trigger, 500);

  const onUpdate = ({ editor }) => {
    // Save the latest editor instance
    editorRef.current = editor;
    // Increment trigger to indicate an update
    setTrigger((prev) => prev + 1);
  };

  useEffect(() => {
    if (!editorRef.current) return;
    // Call the conversion function only after the debounce delay
    const content =
      format === 'json'
        ? editorRef.current.getJSON()
        : editorRef.current.getHTML();
    setProps({ content });
  }, [debouncedTrigger, format, setProps]);

But it seems more complicated that the (simple) manual debounce function to me :D. Maybe it's just because I'am not so well versed with React hooks.

@AnnMarieW
Copy link
Owner

Just a thought - maybe we should loade all extensions per default?

Yes, I think that's a good idea. It would be good to have a minimal default toolbar as well.

@AnnMarieW
Copy link
Owner

Thanks for adding the test! I see it passes, but when I run the usage_rte.py file I see the following error in the console

hook.js:608 Warning: Each child in a list should have a unique "key" prop.

Do you see the same thing? It might be related to snehilvj#511

@AnnMarieW
Copy link
Owner

AnnMarieW commented Mar 12, 2025

I got this to work by applying the debounce method from the TextArea component. Also referred to the Mantine docs for a controlled RTE component, Also found that the console warning was from the toolbar, and added a key prop there

import { RichTextEditor as MantineRichTextEditor, Link } from "@mantine/tiptap";
import { useDebouncedValue, useDidUpdate } from "@mantine/hooks";
import React, { useState, useMemo } from "react";
import { Props }  from "../RichTextEditor"

import { useEditor } from '@tiptap/react';
import Highlight from '@tiptap/extension-highlight';
import StarterKit from '@tiptap/starter-kit';
import Underline from '@tiptap/extension-underline';
import TextAlign from '@tiptap/extension-text-align';
import Superscript from '@tiptap/extension-superscript';
import SubScript from '@tiptap/extension-subscript';
import { getLoadingState } from "../../../../utils/dash3";

// Import all extensions directly
const extensionMap = {
    StarterKit,
    Underline,
    Link,
    Superscript,
    SubScript,
    Highlight,
    TextAlign: TextAlign.configure({ types: ['heading', 'paragraph'] })
} as const;


/** RichTextEditor */
const RichTextEditor = (props: Props) => {
    const { setProps, loading_state, value, format, variant, extensions, toolbar, n_blur = 0, debounce = 0, ...others } = props;

    // Construct the toolbar.
    const mantineToolbar = useMemo(() => {
        if (!toolbar) return undefined;
        return (
          <MantineRichTextEditor.Toolbar sticky={toolbar.sticky}>
            {toolbar.controlsGroups.map((controlGroup, index) => (
              <MantineRichTextEditor.ControlsGroup key={index}>
                {controlGroup.map((control, i) =>
                  React.createElement(MantineRichTextEditor[control], { key: i })
                )}
              </MantineRichTextEditor.ControlsGroup>
            ))}
          </MantineRichTextEditor.Toolbar>
        );
    }, [toolbar]);

    const [val, setVal] = useState(value);
    const debounceValue = typeof debounce === 'number' ? debounce : 0;
    const [debounced] = useDebouncedValue(val, debounceValue);

    const onChange = (value: string) => {
        setVal(value);
    };

    const onUpdate = ({ editor }) => {
        if (!editor) return;
        const newValue = format === "json" ? editor.getJSON() : editor.getHTML();
        onChange(newValue);
    };

    useDidUpdate(() => {
        if (typeof debounce === 'number' || debounce === false) {
             if (format === "json") {
              setProps({ value: debounced });
            }
            if (format === "html") {
              setProps({ value: debounced });
            }
        }
    }, [debounced]);

    useDidUpdate(() => {
        setVal(value);
    }, [value]);

     const handleBlur = () => {
        setProps({
            n_blur: n_blur + 1,
            ...(debounce === true && { value: val })
        });
    };

    let mantineExtensions = [];
    if(extensions !== undefined){
        mantineExtensions = extensions.map(extension => extensionMap[extension]);
    }
    // Create the editor.
    const editor = useEditor({
        extensions: mantineExtensions,
        content: value,
        onUpdate,
        onBlur: handleBlur,
      });
    // Render the component tree.
    return (
        <MantineRichTextEditor variant={variant} editor={editor} data-dash-is-loading={getLoadingState(loading_state) || undefined} {...others}>
          {mantineToolbar}
          <MantineRichTextEditor.Content />
        </MantineRichTextEditor>
      );
};

export default RichTextEditor;

I also updated the non-fragment component to use the DebounceProps, but we don't need the onSubmit prop so could exclude that one.
import { DashBaseProps, DebounceProps } from "props/dash";

Edit
Also changed the content prop to value for the controlled prop.

@AnnMarieW
Copy link
Owner

@emilhe - just FYI - I'm going to release DMC V1.0.0, then we can add this component to 1.1.0

@emilhe
Copy link
Author

emilhe commented Mar 12, 2025

Thanks for the heads up! And I agree with the decision - there is still some work left here. I expect to be able to work more on it Friday, or alternatively next week.

@AnnMarieW
Copy link
Owner

One more heads up - I'm going to release dmc 1.0 with Mantine 7.17.0 because of this bug in 7.17.1 mantinedev/mantine#7539

This has been fixed but not released, but we should be sure to skip 7.17.1 🩹

@emilhe
Copy link
Author

emilhe commented Mar 13, 2025

PR moved to the main repo as the async branch has been merged.

@emilhe emilhe closed this Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants