feat!: automatically use node view components#158
Conversation
|
This change would constitute a breaking change. I'm considering resetting |
|
There's also part of me that's worried that @smoores-dev and I have talked through doing this before and decided not to for some reason I can't remember. If so, please help me remember! |
|
This PR also fixes #157 because it makes it so that the second, forced render after mounting the editor view includes the initial node view portals. |
tilgovi
force-pushed
the
feat-automatic-react-node-views
branch
from
afd0833 to
0cf126c
Compare
|
I think that this was mostly a desire to avoid coupling things while we were still solidifying the API. That said, does this change as is still work with non-React nodeViews? It seems that we would want to expose two node view props, or distinguish between types of node views by inspecting their props (i.e., looking for a |
This change does exactly what you describe regarding distinguishing between node view types. It wraps all the node view constructors, but ends up wrapping it with the identity function if the node view constructor returns a node view without a component property. |
|
Oh I see! I had missed the new branch in createReactNodeViewConstructor. Awesome :) |
Remove the need to call `useNodeView` and render the portals manually. Make it happen automatically it `<ProseMirror>`. Remove `useNodeView` from the public API.
tilgovi
force-pushed
the
feat-automatic-react-node-views
branch
from
3788ee3 to
6f1a805
Compare
|
|
||
| let componentRef: NodeViewWrapperRef | null = null; | ||
| const { component: NodeView } = nodeView; | ||
| if (!NodeView) { |
There was a problem hiding this comment.
worth adding a comment here to note that this is the branch which continues to allow specifying non-React nodeviews?
saranrapjs
left a comment
saranrapjs
left a comment
There was a problem hiding this comment.
This looks good to me! I am assuming (but have not run myself to confirm) that this passes the failing widget decoration test — is that worth adding here, too?
I checked that it passed, but I'll commit it separately. |
Remove the need to call `useNodeView` and render the portals manually. Make it happen automatically it `<ProseMirror>`. Remove `useNodeView` from the public API.
3 participants
Remove the need to call
useNodeViewand render the portals manually. Make it happen automatically it<ProseMirror>.Remove
useNodeViewfrom the public API.