-
Notifications
You must be signed in to change notification settings - Fork 907
Implement use node path plugin #4547
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
base: main
Are you sure you want to change the base?
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
🦋 Changeset detectedLatest commit: e4291ad The changes in this PR will be included in the next version bump. This PR includes changesets to release 43 packages
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 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Can't we use |
If i understand correctly, we could only select props from the editor object, and trigger re-renders, whenever any node value or the editors selection changed, when using My version is more fine grained, it will only react to node operations, not to text operations, nor to selection operations. Also it will react faster, immediately after and operation was applied, it doesn't wait for operations to be flushed to the But after this has been said, i am not too familiar yet with the |
What could be quite nice, would actually be something like that: import { useEffect, useMemo, useState } from 'react';
import { Editor, type Node } from 'slate';
import { ReactEditor, useSlateStatic } from 'slate-react';
export const useNodePath = (node: Node) => {
const editor = useSlateStatic();
const [, setCacheKey] = useState(0);
const pathRef = useMemo(() => {
const path = ReactEditor.findPath(editor, node);
if (path) {
return Editor.pathRef(editor, path, {
affinity: 'backward',
onChange: () => {
setCacheKey((prev) => prev + 1);
},
});
}
return {
current: null,
unref: () => {},
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [node]);
useEffect(
() => () => {
pathRef.unref();
},
[pathRef],
);
return pathRef.current;
}; Of course the change handler on pathRefs would have to be implemented by slate-react. Actually the hook could go into that package then. Or we could even think about a hook called What do you think about that @zbeyens ? |
Something like that: ianstormtaylor/slate#5930 |
Yes that's better. I'm working on the onNodeChange prop right now. |
I get the feeling, that using the following might be sufficient 😅 const key = editor.api.findKey(element);
const { path, selection } = useEditorSelector(
(editor) => ({
path: editor.api.findPath(element),
selection: editor.selection,
}),
[key],
{
equalityFn: (a, b) => {
const isPathEqual =
a.path && b.path ? PathApi.equals(a.path, b.path) : a.path === b.path;
const isSelectionEqual =
a.selection && b.selection
? RangeApi.equals(a.selection, b.selection)
: a.selection === b.selection;
return isPathEqual && isSelectionEqual;
},
},
); |
Perhaps we could add more parameters like |
Couldn't we just use |
Yes if that's synced |
Checklist
yarn typecheck
yarn lint:fix
yarn test
yarn brl
yarn changeset
This implements a useNodePath hook, which will trigger a re-render, whenever a path of a node has been changed.