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

feat: insert markdown value in the current editor selection point. #294

Merged
merged 6 commits into from
Jan 13, 2024

Conversation

forhappy
Copy link
Contributor

This is to support inserting markdown content in the current selection(where the cursor is) point, we can programmatically insert new markdown content to the existing markdown editor now.

NOTE: the Signal insertMarkdown$ can be only used in plugins, there is no way to use this utility API from outside of the editor.

Screen.Recording.2024-01-10.at.3.53.28.PM.mov

New idea to discuss:

We can even expose a MDXEditor API(insertMarkdown) so that the user can insert markdown content from outside of the editor.

@forhappy
Copy link
Contributor Author

This is a new version of markdown insert feature based on MDXEditor V2, happy to discuss the implementation ideas ;)

Hey @petyosi would you help take a look into this PR?

@petyosi
Copy link
Contributor

petyosi commented Jan 11, 2024

Hey @forhappy ,

I mentioned that this should be a built-in feature; here's what I have in mind as a possible implementation: ad5a516.

This eliminates the need of an additional mdast import point node, which I see as a potential problem down the road.

The commit needs some clean up of course, as I'm just butchering the root type, but you will get the idea. Does this work for you? Thank you very much for your help!

@forhappy
Copy link
Contributor Author

Hey @petyosi Thanks for your comments and sample code, your solution is simpler and easy to understand, let me verify if it would work in some edge cases I have observed, will let you know if everything works well.

A side 🗒️: while I am happy to rewrite my PR based on your idea, I don't meant to take your credit:) so it's totally ok for you to refine your idea and polish your change then I will cancel my PR. But if you don't mind I can raise a second revision and finalize this feature with you.

@petyosi
Copy link
Contributor

petyosi commented Jan 11, 2024

That would be amazing! Please go ahead and take that approach in your PR. It is great that you have some edge cases to test it with.

Cheers,

@braincore
Copy link
Contributor

Will insertMarkdown be the recommended way to add something like a user-mentions dialog?

@forhappy
Copy link
Contributor Author

Will insertMarkdown be the recommended way to add something like a user-mentions dialog?

It can be used to insert markdown content(in the future we can even insert HTML content) to the current cursor point, one potential use case is the user can now input markdown content from somewhere(not in the editor) and then insert the markdown to the editor. This will be very helpful if you build something like Notion AI where the content is generated by AI and the user would like to insert the AI generated content into the editor.

@forhappy
Copy link
Contributor Author

Hey @petyosi , I've updated the implementations and examples based on your suggestions/ideas, I've done very minor changes on the original fakeRoot(added comments and removed the type conversion by new RootNode directly), besides I added one example where NestedEditor is used via JSX component.

I also tested a few edge cases and it works(e.g. insert markdown content in a table cell), thank you so much for the guidance ;)

Copy link
Contributor

@petyosi petyosi left a comment

Choose a reason for hiding this comment

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

Constructing a new root node seemed attractive as an idea, but, unfortunately, it does not seem to work :(. See my comments.

// during which process we need a root lexical node to host all children nodes
// while we're visiting the Mdast tree nodes.
const shadowRoot: RootNode = new RootNode()
shadowRoot.append(...rootNodes)
Copy link
Contributor

Choose a reason for hiding this comment

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

I might be missing something but this line should not do anything? rootNodes is an empty array. It also does not bind the variable to the nodes of the shadowRoot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, new a RootNode won't work, I could have missed some steps to verify it, now I changed back to use your code and it can work.

const shadowRoot: RootNode = new RootNode()
shadowRoot.append(...rootNodes)
tryImportingMarkdown(r, shadowRoot, markdownToInsert)
$insertNodes(rootNodes)
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, rootNodes is still an empty array, nothing has changed its value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I oversight this issue, I have changed back to use the original code and it now works. Thank you for your help.

// while we're visiting the Mdast tree nodes.
const shadowRoot: RootNode = new RootNode()
shadowRoot.append(...rootNodes)
tryImportingMarkdown(r, shadowRoot, markdownToInsert)
Copy link
Contributor

Choose a reason for hiding this comment

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

From what I tested, this indeed inserts the markdown - but always at the bottom and not at the cursor. As far as I can tell, constructing a new root node and appending to it just append nodes to the root of the active editor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After I use the following code now it works:

         const shadowRoot = {
          append(node: LexicalNode) {
            rootNodes.push(node)
          },
          getType() {
            return selection?.getNodes()[0].getType()
          }
        }

        tryImportingMarkdown(r, shadowRoot as RootNode, markdownToInsert)
        $insertNodes(rootNodes)

Copy link
Contributor Author

@forhappy forhappy left a comment

Choose a reason for hiding this comment

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

Updated the insert logic and now it works

const shadowRoot: RootNode = new RootNode()
shadowRoot.append(...rootNodes)
tryImportingMarkdown(r, shadowRoot, markdownToInsert)
$insertNodes(rootNodes)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I oversight this issue, I have changed back to use the original code and it now works. Thank you for your help.

@petyosi petyosi merged commit c3b8847 into mdx-editor:main Jan 13, 2024
Copy link

🎉 This PR is included in version 2.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants