mdx-editor / editor

A rich text editor React component for markdown
https://mdxeditor.dev
MIT License
1.78k stars 138 forks source link

Support span-level tags #92

Open dzikoysk opened 11 months ago

dzikoysk commented 11 months ago

I'm not sure if you have any plans to support non-standard markdown elements, but a color picker/palette would be a decent enhancement towards a user-friendly WYSIWYG editor :)

petyosi commented 11 months ago

That's not in my plans. I am trying to stay pretty true to the markdown restrictions and philosophy; I have no intent on making the editor a general purpose WYSIWYG editor - there are other tools out there that do that job.

dzikoysk commented 11 months ago

Can we somehow achieve that with inlined HTML that is a part of Markdown, and which should be able to support this kind of features?

petyosi commented 11 months ago

Of course, you can do that. You can write a plugin that parses/serializes a markup of your choice, and then do the respective Lexical node that will handle the editing experience. Notice that you will need some working knowledge of the Lexical API.

dzikoysk commented 11 months ago

So there is no plan to support inlined html at all in this library, I'd need to implement that part of the editor on my own. Ideally, we could bind toolbar button with an HTML tag via the API, so it'd be quite generic way to implement any supported tag (like div, table, pre, p) & span-level HTML tags (span, cite, del)

Well, that's kinda crucial information for me, thanks for clarifying this out. For now I need to find a different Markdown editor that covers that part of the spec, but I hope it'll be available someday as the rest looks pretty cool! :)

dzikoysk commented 11 months ago

I didn't find a better markdown-based visual editor, so I decided to take a look at sources to check the current state of the API. It looks like that the current implementation of bold/italic/underline formatting is basically hard-coded as a core plugin and handled via the LexicalTextVisitor.

I think that the current implementation could be slightly more generic, and it'd handle e.g. span-level tags out of the box:

https://github.com/mdx-editor/editor/blob/20420a80f9f7c836eebeb82bf4f46c6e6ce6a8e1/src/plugins/core/LexicalTextVisitor.ts#L11-L13

https://github.com/mdx-editor/editor/blob/20420a80f9f7c836eebeb82bf4f46c6e6ce6a8e1/src/plugins/core/LexicalTextVisitor.ts#L46-L88

If this part of the library could be somehow customized (e.g. via CorePluginParams), it'd be probably just enough to insert any tag with custom attributes via the toolbar, right?

Initially, I tried to write a plugin that behaves the same as the CorePlugin, but I'm kinda stuck copying parts of the core implementation that basically does the same thing - just with an additional html tag (span atm). Sadly, I think that these plugins might be incompatible together, as both of them try to access the same part of the source.

I don't have that much time to dive deeper into this API, so I wonder if you have any thoughts on the idea of making the current implementation of LexicalTextVisitor a little bit more generic to support this use case. If not, I'll probably try to achieve that on a hard fork, but it's something I'd like to truly avoid.

petyosi commented 11 months ago

That's a good research and the points you make are valid. Indeed, if you want to further process the LexicalText, you would be hard-pressed to compete with the core implementation which cannot be overwritten (nor somehow given lower priority). I will think about a way to make that possible - hang in there.

dzikoysk commented 11 months ago

Great and no rush! This feature is my only missing must-have requirement so far, but because it's a part of a new project, the willingness to develop the API in this direction is all I need for now :) Thanks

petyosi commented 10 months ago

Leaving comment here for the progress:

In this branch, I've included parsing and rendering of known HTML tags in a "catch-all" mode. The user has no control over their attributes, though. This means that markdown which includes HTML can be edited without errors.

However, I feel like this is very open ended, because the user has no widgets to control the tag attributes. This can be done of course, for example, there can be state endpoints which let the developer access the current HTML node and show toolbar (or even inline) controls for the purpose. @dzikoysk - can you give me some input here?

dzikoysk commented 10 months ago

This is great that you've decided to address this! Maybe I'll describe it via the real-world example of what I'm trying to achieve, so we can take a look at the expected result from 2 perspectives.

I'm developing an application that handles various resources. I decided to try to enhance this with a concept of a notebook, where non-technical users can note various staff. Because I want to keep that simple, I wanted to avoid custom formats/and complex impls like BBCode (known from e.g. forums in the past), so Markdown is definitely my go to. This is a list of (core) features I'd like to eventually support here:

My main goals:

I don't really have any specific requirements other than achieving the goal, so I guess I can adjust to the API. To handle something like that in any form: icon on toolbar that opens available color palette/inlined color icons on toolbar/toolbar above a selection (that'd be probably superior for long documents):

obraz

I'd describe my hierarchy of needs as follows:

  1. Finished component for a color picker, like colorPlugin(["#fff", "#000"]), or
  2. Some targeted API for HTML attributes, like (pseudocode):
    ColorPalletePlugin {
    redColorIcon.onClick((event) => { // register an action for the icon in the toolbar
      if (event.editor.selectedText== null) { // make sure user selected something in the editor
        // Attributes to add. Some notes:
        // - if text is already in span tag, it should only update/merge its attributes
        // - if text is already in other tag, it should wrap it as well (e.g. <span [...]><abbr>abc</abbr></span>)
        event.editor.selectedText.wrap('span', [style: 'color: #FF0000;'])
      }
    }) 
    }

    Ideally, HTML tags should be simply rendered by the browser, or

  3. Without any specialized API, I'd simply appreciate some guidelines about the new API that allows to achieve that at some point as I tried before :)

I hope it clarifies a bit my use-case, let me know if it helps you at some point, or should try to be more precise.

mgalgs commented 9 months ago

I've included parsing and rendering of known HTML tags in a "catch-all" mode. The user has no control over their attributes, though. This means that markdown which includes HTML can be edited without errors.

FWIW, I personally would be perfectly happy with this (html "pass-through", no widgets). I understand that others, like @dzikoysk, have more in-depth use cases where more full-featured html editing capabilities would be required, but html pass-through is much lower hanging fruit that would still be useful to a wide variety of use cases. The tolerant-html branch looks really promising!

petyosi commented 9 months ago

Thanks for the chime-in. I will certainly merge this in a state similar to this, just want to ensure that there's some rudimentary API for people to build similar things like the use case of @dzikoysk.

github-actions[bot] commented 9 months ago

:tada: This issue has been resolved in version 1.11.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

petyosi commented 9 months ago

@mgalgs @dzikoysk Merged the branch and published a release today. You can check some of the things I managed to implement with the Lexical API in this example.

This is far from universal, bulet-proof HTML support of all tags, but I think it's a step in the direction of more tolerant HTML support, so that people can edit documents without exceptions being thrown.

For the record: I'm still not sure how to use Lexical to wrap the selection in an inline HTML for example (asked about that in Lexical's discord). This, however, is a problem that should be solved in userland, not in the editor itself.

dzikoysk commented 9 months ago

This is awesome! I'll try to take a look at it within this week and see how it works. I'll definitely share some feedback on that, I hope I'll be able to do as much as possible on my side, so I don't really have to bother you with it anymore 😅

dzikoysk commented 8 months ago

So I finally got some time to play with it. Indeed, the read-only part of the editor seems to respect tags, so that's great! ❤️ Speaking about the API preview, I've found some issues with current implementation:

  1. Removing the HTML node works only after a full page reload:

https://github.com/mdx-editor/editor/assets/4235722/b9c85d4a-213f-4c14-b45e-c7c924d6a8f5

I tried to find out the cause, and it looks like the $getNearestNodeOfType(node, GenericHTMLNode) does not return any value for changes applied to the current session. After the reload, the state is probably reinitialized, so it works properly.

  1. Overlapping tags crashes editor:

https://github.com/mdx-editor/editor/assets/4235722/8877372e-6c62-43fc-b56d-04fea9d1657f

Full error:

Uncaught TypeError: "key" is read-only
    set Lexical.dev.js:5336
    $patchStyleText LexicalSelection.dev.js:512
    onClick ColorToolbarComponent.tsx:33
    beginUpdate Lexical.dev.js:8208
    updateEditor Lexical.dev.js:8284
    update Lexical.dev.js:9800
    onClick ColorToolbarComponent.tsx:32
My current ColorToolbarComponent sources ```ts import { corePluginHooks, $isGenericHTMLNode, GenericHTMLNode } from '@mdxeditor/editor' import { $patchStyleText } from '@lexical/selection' import { $getSelection, LexicalNode } from 'lexical' import { MdxJsxAttribute } from 'mdast-util-mdx' import {useMemo} from "react"; import {$getNearestNodeOfType} from "@lexical/utils"; export const HTMLToolbarComponent = () => { const [currentSelection, activeEditor] = corePluginHooks.useEmitterValues('currentSelection', 'activeEditor') const currentHTMLNode = useMemo(() => { return ( activeEditor?.getEditorState().read(() => { const selectedNodes = currentSelection?.getNodes() || [] if (selectedNodes.length === 1) { const node: LexicalNode = selectedNodes[0] console.log(node, node.getParent(), $isGenericHTMLNode(node), $getNearestNodeOfType(node, GenericHTMLNode)) return $getNearestNodeOfType(selectedNodes[0], GenericHTMLNode) } else { console.log('none') return null } }) || null ) }, [currentSelection, activeEditor]) return ( <> { activeEditor?.update( () => { const attributesWithoutClass = currentHTMLNode?.getAttributes().filter((attr) => attr.name !== 'class') || [] const newClassAttr: MdxJsxAttribute = { type: 'mdxJsxAttribute', name: 'class', value: e.target.value } currentHTMLNode?.updateAttributes([...attributesWithoutClass, newClassAttr]) }, { discrete: true } ) e.target.focus() }} /> ) } function getCssClass(node: GenericHTMLNode | null) { return (node?.getAttributes().find((attr) => attr.name === 'class')?.value as string) ?? '' } ```

For now, I'm trying to make these 2 operations work, but overall I think that's a great start.

EssamWisam commented 8 months ago

@dzikoysk remove HTML node works without refreshing for me. However, it only operates on a single element. I was able to generalize it to work on all text elements in a selection like so:

  const currentHTMLNodes = React.useMemo(() => {
    return (
      activeEditor?.getEditorState().read(() => {
        const selectedNodes = currentSelection?.getNodes() || []
        const nearestNodes = []
        for (const node of selectedNodes) {
          if (!$isGenericHTMLNode(node))
          nearestNodes.push($getNearestNodeOfType(node, GenericHTMLNode))
        }
        return nearestNodes;
      }) || null
    )
  }, [currentSelection, activeEditor])

and


      <button
        disabled={currentHTMLNodes.length === 0}
        onClick={() => {
          if (activeEditor !== null && currentSelection !== null) {
            activeEditor.update(() => {
              for (const currentHTMLNode of currentHTMLNodes) {
                let selection = currentHTMLNode?.select()
                currentHTMLNode?.remove()
                selection?.insertNodes(currentHTMLNode?.getChildren() || [])
              }

            })
          }
        }}
      >
        remove HTML nodes
      </button>

This may help with an imperfect solution to the other problem by clearing the existing nodes then applying the new style. For this purpose, it would be more ideal to readjust the existing nodes which are partially captured in the selection by moving the single opening/closing tag after/before the selection but I am not sure of how to do that.

dzikoysk commented 8 months ago

It's interesting that $getNearestNodeOfType(node, GenericHTMLNode) returns valid value for modified nodes in your case. As far as I see, this method is basically looking through parents for closest GenericHTMLNode. When I'm loading the page with colored text, the parent of selected colored text is indeed a generic-html, but then, after removing and adding the color one more time within the same session, it's actually not updated - selected text is still linked to the paragraph:

const currentHTMLNode = // [...] {
  if (selectedNodes.length === 1) {
    console.log(selectedNodes[0], selectedNodes[0].getParent())
    return $getNearestNodeOfType(selectedNodes[0], GenericHTMLNode)
  }
}

obraz

Because the structure seems to be already invalid in the removal function, it might be an issue with $patchStyleText(currentSelection, { color: 'orange' }) that is not properly updating current state 🤔