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

Use block factory to create elements in exit break #3457

Merged
merged 8 commits into from
Aug 22, 2024

Conversation

nklhtv
Copy link
Contributor

@nklhtv nklhtv commented Aug 22, 2024

Checklist

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

Copy link

changeset-bot bot commented Aug 22, 2024

🦋 Changeset detected

Latest commit: 68a8abf

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

This PR includes changesets to release 5 packages
Name Type
@udecode/plate-autoformat Patch
@udecode/plate-code-block Patch
@udecode/plate-break Patch
@udecode/plate Patch
@udecode/plate-basic-elements Patch

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 Aug 22, 2024

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 Aug 22, 2024 6:00pm

zbeyens
zbeyens previously approved these changes Aug 22, 2024
@nklhtv
Copy link
Contributor Author

nklhtv commented Aug 22, 2024

@zbeyens There are many more instances where the same patch could be applied. Do you want me to create separate PR for each package or fix them all in one PR?

@zbeyens
Copy link
Member

zbeyens commented Aug 22, 2024

You can do it here

Copy link
Member

@zbeyens zbeyens left a comment

Choose a reason for hiding this comment

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

It looks like there is overlap between editor.blockFactory and editor.insertNodes. One could instead override the latter. I think editor.blockFactory should be used as the default block (e.g. paragraph) insertion, not for other types. I'm planning to rename it to insertDefaultBlock. What do you think?

@nklhtv
Copy link
Contributor Author

nklhtv commented Aug 22, 2024

It looks like there is overlap between editor.blockFactory and editor.insertNodes. One could instead override the latter. I think editor.blockFactory should be used as the default block (e.g. paragraph) insertion, not for other types. I'm planning to rename it to insertDefaultBlock. What do you think?

I do not use any of the default element plugins, but if i had to, i would never try to extend its TElement with additional props, instead i would create my own plugin that works with my own types.

Thats not the case with non-element plugins like exitBreak, normalizeTypes, trailingBlock or nodeFactory. Ideally those would work with any type through the blockFactory as its not that easy to embed a custom version of those into an app (except for nodeFactory which i already did)

I will remove the references to blockFactory from places that do not create default elements in my next commit.

zbeyens
zbeyens previously approved these changes Aug 22, 2024
auto-merge was automatically disabled August 22, 2024 17:51

Head branch was pushed to by a user without write access

@nklhtv
Copy link
Contributor Author

nklhtv commented Aug 22, 2024

@zbeyens I need help with the failed tests. I belive what i did to fix the test was not the correct way to fix them.

@zbeyens zbeyens merged commit b290c8d into udecode:main Aug 22, 2024
10 checks passed
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.

2 participants