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/equation #953

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

Feat/equation #953

wants to merge 5 commits into from

Conversation

jkcs
Copy link
Contributor

@jkcs jkcs commented Jul 19, 2024

close #741
/claim #741

Copy link

vercel bot commented Jul 19, 2024

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

Name Status Preview Updated (UTC)
blocknote ✅ Ready (Inspect) Visit Preview Jul 19, 2024 7:20am

Copy link

vercel bot commented Jul 19, 2024

@jkcs is attempting to deploy a commit to the TypeCell Team on Vercel.

A member of the Team first needs to authorize it.

@jkcs
Copy link
Contributor Author

jkcs commented Jul 19, 2024

Please confirm the following:

  1. Is the direction correct?
  2. Is it necessary to migrate to packages/core or any other package?

@matthewlipski
Copy link
Collaborator

Really nicely done! I have a few questions r.e. the implementation:

I see that the equations are implemented as TipTap nodes first and then converted into BlockNote inline content. Which functionality are we gaining from this vs implementing as a React custom block without content and storing the Latex as a prop instead? Keyboard navigation & storing the Latex in content instead of props are obvious ones, is there more?

I ask this because imo it would be preferable to keep things within the BlockNote API rather than diving into TipTap's. So if the difference in functionality is not too bad then I think that might make more sense + we could keep the current implementation as an advanced/pro example. I appreciate that the current UX is basically 1:1 with Notions (great job on that btw), it's just that we wanna minimize code complexity for these examples.

@YousefED wdyt? Should we just have an example using the TipTap API, have a simplifed one using the BlockNote API, or both?

@jkcs
Copy link
Contributor Author

jkcs commented Jul 24, 2024

@matthewlipski As you already know, the reason I'm using TipTap nodes here is because BlockNote doesn't have a method similar to UpdateBlock to update inline nodes, and it also does not provide nodeSize and pos props for implementing Keyboard navigation.

I am in favor of implementing everything using the BlockNote API exclusively.Should we extend these APIs?

Perhaps we could provide an example like this for users to refer to a method of combining TipTap and BlockNote API for use?

@matthewlipski
Copy link
Collaborator

I think the suggestion of having this as an example of using a TipTap node to create inline content is a good one👍

Do you want to give implementing it strictly using the BlockNote API a shot? You can simplify things quite a bit imo. Regarding keyboard navigation, I think it's best to just set content: "none" in the inline content config, which should make the arrow keys just ignore it. Also recommend saving the Latex to an equation prop instead of the block's content, since I think you'll otherwise be limited by the BlockNote API.

I don't know if we want to extend the API in this PR tbh, it should be possible to implement the simplified version in the current state and API design tends to take quite a while.

@YousefED YousefED self-assigned this Jul 29, 2024
@jkcs
Copy link
Contributor Author

jkcs commented Jul 30, 2024

I will try to implement it using the complete Blocknote API, but keyboard access to it may be different from the current PR

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

Successfully merging this pull request may close these issues.

Custom Block for Katex
3 participants