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: Full Markdown AI integration #4069

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

supersimple33
Copy link
Contributor

@supersimple33 supersimple33 commented Feb 10, 2025

Addresses #4040 now with ui movement.

Checklist

  • yarn typecheck
  • yarn lint:fix
  • yarn test
  • yarn brl
  • yarn changeset
  • ui changelog

Copy link

codesandbox bot commented Feb 10, 2025

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link

changeset-bot bot commented Feb 10, 2025

🦋 Changeset detected

Latest commit: 834b0b9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@udecode/plate-ai Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Feb 10, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
plate ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 15, 2025 10:19pm

@supersimple33
Copy link
Contributor Author

@zbeyens @felixfeng33 Okay I saw what you guys meant earlier with the ai-menu not properly moving itself as needed and blinding the ai content. I drafted up this PR which includes my previous fix but would in theory update the useRef of the anchorNode once that content has been set. Unfortunately I seem unable to get any father as I would expect my code to work but my toDOMNode call inside of ai-menu.tsx is failing unexpectedly. It seems that I can get a key for the WeakMap fine but that key must be stored at a new memory address and thus not the real one. I was wondering if you guys had any ideas as to why this is happening?

@supersimple33
Copy link
Contributor Author

As solution could be to use the id supplying plugin and then use a standard call to find the node with the correct id but I was trying to avoid that.

@supersimple33
Copy link
Contributor Author

I also found a one line thing where some braces were missing in a call that should have expanded an object and made #4070 since this is still in development.

@zbeyens zbeyens requested a review from felixfeng33 February 11, 2025 07:35
@supersimple33
Copy link
Contributor Author

Ok turns out I was being a little bit silly and didn't realize I wasn't giving the DOM enough time to update so the WeakMap literally couldn't contain values which hadn't been created yet. Anyways the latest commit has a fix for that however I am not super confident the way I made updateAnchorElement follows best practices so please let me know there. @zbeyens @felixfeng33

@supersimple33
Copy link
Contributor Author

Also heres a video of the changes.

Screen.Recording.2025-02-11.at.11.18.37.AM.mov

@zbeyens
Copy link
Member

zbeyens commented Feb 12, 2025

Can you undo/redo the AI changes?

@felixfeng33
Copy link
Collaborator

felixfeng33 commented Feb 14, 2025

I see the menu sometimes appears in the top left corner. And in the ideal circumstances we shouldn't see the md marks after this line is done
image
image

@supersimple33
Copy link
Contributor Author

Can you undo/redo the AI changes?

@zbeyens Yes all ai changes are undone or redone with a single hit of the button.

@supersimple33
Copy link
Contributor Author

@felixfeng33 Ok looks like I left in a debugging thing but I took it out and that should stop the bar from going to the top left. Also, I was trying to minimize the number of calls to the markdown deserializer and was just keeping it to the last call in the end. But I could switch it to run after every chunk I just figured that may be expensive for little benefit.

@felixfeng33
Copy link
Collaborator

felixfeng33 commented Feb 16, 2025

@supersimple33 Yes call deserializer in every chunk will make all things very tricky, but I think we should take a try.

@supersimple33
Copy link
Contributor Author

@felixfeng33 Ok so I tried a quick change to clear the aiNodes and reformat them every time but the trouble there is because of the speed at which chunks are loaded most of the time you fail to actually see what the editor is writing except for brief moments until the end when no more chunks are coming. I'm willing to investigate further but could we first get this merged and then I work on having it be updated continuously?

@zbeyens
Copy link
Member

zbeyens commented Feb 18, 2025

This is a nice fix but it introduces a regression so we can't merge this right now. In the meantime you could use configure(() => ({ useHooks: ... }) in your plugin

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.

3 participants