-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[lexical-yjs][lexical-react] Feature: Allow custom Yjs XmlText #6483
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
size-limit report 📦
|
This changes a public API in a backwards incompatible way, so it might be tricky to accept as-is |
Thanks for reviewing @etrepum! I thought about adding another param but I think a object param is probably better for the amount of parameters this function takes. |
I don't disagree that an options object probably makes more sense, but this is a module that gets a fair bit of use and I think it's unlikely to get approved without a migration plan that does not immediately break compatibility. I think the other concerns will be around what the use case is (a more concrete description would probably make sense, since there are existing examples that use multiple editors in the same collab without this feature, e.g. sticky notes and comments in the playground), as well as the lack of tests and documentation around the new feature. |
Thanks for the feedback @etrepum. I've updated the API now to receive an additional parameter. |
Description