-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Change <Slate>
to a controlled component
#3216
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
import React, { useMemo } from 'react' | ||
import { Editor, Node, Operation, Range } from 'slate' | ||
|
||
import { ReactEditor } from '../plugin/react-editor' | ||
import { FocusedContext } from '../hooks/use-focused' | ||
import { EditorContext } from '../hooks/use-editor' | ||
import { SlateContext } from '../hooks/use-slate' | ||
import { EDITOR_TO_ON_CHANGE } from '../utils/weak-maps' | ||
|
||
/** | ||
* A wrapper around the provider to handle `onChange` events, because the editor | ||
* is a mutable singleton so it won't ever register as "changed" otherwise. | ||
*/ | ||
|
||
export const Slate = (props: { | ||
editor: Editor | ||
value: Node[] | ||
selection: Range | null | ||
children: JSX.Element | JSX.Element[] | ||
onChange: (children: Node[], selection: Range | null) => void | ||
[key: string]: any | ||
}) => { | ||
const { editor, children, onChange, value, selection, ...rest } = props | ||
const context: [Editor] = useMemo(() => { | ||
editor.children = value | ||
editor.selection = selection | ||
return [editor] | ||
}, [value, selection, ...Object.values(rest)]) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i wonder if there will be some gotchas around memoizing around the extra props. for the most part this is probably what a caller is going to need but this seems like a subtle trap for users who are not paying attention There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, although by including them it's less restrictive (not more) so I think it's alright? Otherwise implementing your own custom editor properties becomes impossible I think? |
||
EDITOR_TO_ON_CHANGE.set(editor, onChange) | ||
|
||
return ( | ||
<SlateContext.Provider value={context}> | ||
<EditorContext.Provider value={editor}> | ||
<FocusedContext.Provider value={ReactEditor.isFocused(editor)}> | ||
{children} | ||
</FocusedContext.Provider> | ||
</EditorContext.Provider> | ||
</SlateContext.Provider> | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,22 @@ | ||
export * from './components/editable' | ||
// Components | ||
export { | ||
RenderDecorationProps, | ||
RenderElementProps, | ||
RenderMarkProps, | ||
Editable, | ||
} from './components/editable' | ||
export { DefaultElement } from './components/element' | ||
export { DefaultMark, DefaultDecoration } from './components/leaf' | ||
export * from './hooks/use-editor' | ||
export * from './hooks/use-focused' | ||
export * from './hooks/use-read-only' | ||
export * from './hooks/use-selected' | ||
export * from './hooks/use-slate' | ||
export * from './react-command' | ||
export * from './react-editor' | ||
export * from './with-react' | ||
export { Slate } from './components/slate' | ||
|
||
// Hooks | ||
export { useEditor } from './hooks/use-editor' | ||
export { useFocused } from './hooks/use-focused' | ||
export { useReadOnly } from './hooks/use-read-only' | ||
export { useSelected } from './hooks/use-selected' | ||
export { useSlate } from './hooks/use-slate' | ||
|
||
// Plugin | ||
export { InsertDataCommand, ReactCommand } from './plugin/react-command' | ||
export { ReactEditor } from './plugin/react-editor' | ||
export { withReact } from './plugin/with-react' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moving this to a different module was a good decision i think, it took a little bit to find it just diving through github.