-
-
Notifications
You must be signed in to change notification settings - Fork 625
refactor(extensions): rewrite to use new extension system #2143
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
2e2cb06 to
f449a1c
Compare
f449a1c to
b94e7ee
Compare
| }); | ||
| const L = 0.2126 * c[0] + 0.7152 * c[1] + 0.0722 * c[2]; | ||
| return L <= 0.179; | ||
| export const CursorPlugin = createExtension((_editor, options) => { |
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.
TODO, there were some defaults here that were being set that may no longer be working
|
|
||
| return { | ||
| key: "yForkDoc", | ||
| store, |
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.
I'm not entirely certain on this pattern, perhaps we can also allow derived values to be the store?
The point here was to hide the internal state, which should not need to be accessed by anyone other than the plugin.
| * Whether a comment is currently being composed | ||
| */ | ||
| private pendingComment = false; | ||
| export const CommentsPlugin = createExtension( |
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.
Ideally, this would be part of a separate package now. But will leave for now
|
|
||
| export const FilePanelPlugin = createExtension((editor) => { | ||
| const store = createStore({ | ||
| blockId: undefined as string | undefined, |
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.
I think that the minimal state that a file panel really needs, is the block which it is currently open to. This being a string implies that it is open, and being undefined means that it is closed. To get the position, you can read it from the DOM.
I think we can do scroll updates & mouse events from floating UI, if not let's discuss.
| const store = createStore({ | ||
| show: false, | ||
| referencePos: null as DOMRect | null, | ||
| }); |
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.
This can likely be simplified further. The only state that this plugin really needs is whether it is being shown or not (for programmatic dismissal). Even the reference position should be derive-able from the current editor state. We will likely want a hook which makes it easy to have a menu at wherever the current selection is, and this should store the majority of the state.
| // (this.options.editor.formattingToolbar?.shown || | ||
| // this.options.editor.linkToolbar?.shown || | ||
| // this.options.editor.filePanel?.shown) |
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.
Ideally this would not have to be aware of every menu that could be open. I'm not sure if this means that there should be a single "menu" extension which stores the current state of whatever menu is currently active? Or, if these menus should be setting keyboard handlers to capture the Tab key or otherwise not giving this event to the editor.
| // TODO defaults? | ||
| const placeholders = options.placeholders; |
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.
There were some defaults here that we should also apply
| { enabled: false }, | ||
| { | ||
| onUpdate() { | ||
| editor.transact((tr) => tr.setMeta(PLUGIN_KEY, {})); |
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.
TODO check this
| this.view = new SideMenuView(editor, editorView, (state) => { | ||
| this.emit("update", state); | ||
| view = new SideMenuView(editor, editorView, (state) => { | ||
| store.setState(state); |
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.
TODO, I did not refactor this extension & maybe should have. It was doing a lot so I didn't take the time for it.
Honestly the gist of the state would be that it knows what the currently hovered block is & whether it is open. There is a lot of handling for drag & drop, which could use some refactoring, but maybe later.
For now, if this integration is enough, then maybe we can stick with it, if not I'll help with the refactor here.
|
|
||
| private view: SuggestionMenuView<BSchema, I, S> | undefined; | ||
| private triggerCharacters: string[] = []; | ||
| export const SuggestionMenuPlugin = createExtension((editor) => { |
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.
Same as the SideMenuPlugin, did not refactor, but hopefully the shim works.
| > implements PluginView | ||
| { | ||
| public state?: TableHandlesState<I, S>; | ||
| export class TableHandlesView implements PluginView { |
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.
Same as the SideMenuPlugin, shimmed, but unsure if it works
| export * from "./extensions-shared/UiElementPosition.js"; | ||
| export * from "./extensions/FilePanel/FilePanelPlugin.js"; | ||
| export * from "./extensions/FormattingToolbar/FormattingToolbarPlugin.js"; | ||
| export * from "./extensions/LinkToolbar/LinkToolbarPlugin.js"; | ||
| export * from "./extensions/LinkToolbar/LinkToolbar.js"; | ||
| export * from "./extensions/LinkToolbar/protocols.js"; | ||
| export * from "./extensions/SideMenu/SideMenuPlugin.js"; | ||
| export * from "./extensions/SuggestionMenu/DefaultGridSuggestionItem.js"; |
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.
Ideally just one export from extensions
| }, | ||
| "dependencies": { | ||
| "@emoji-mart/data": "^1.2.1", | ||
| "@handlewithcare/prosemirror-inputrules": "0.1.3", |
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.
Pretty nice to be able to add this, allows us to undo an input rule
Summary
This sets up a few things:
initlifecycle method, an extension can be initialized with the current view & be called back when unmounted if it returns a callback function, or listens to it's abortController.useEditorStatehook which allows for efficient selection of editor state in a convenient Redux-like APIRationale
The BlockNote extension API was somewhat haphazardly created out of the need to extend the API for BlockNote for things that an editor naturally needs, such as keyboard shortcuts, input rules, and exposing the ProseMirror plugin API for mroe advanced use cases. We migrated this in piecemeal, supporting ProseMirror Plugins, Tiptap extensions & a stripped down BlockNote extension API, but we need more!
So we've introduced what we feel to be a pretty scalable API, where in an extension can have a lifecycle, added & removed from the editor, expose methods which other parts of the app/UI can call. This gives everything that an extension can need, state to expose, access to the editor instance to register callbacks on, etc.
Changes
This majorly refactors all of the existing built-in extensions to use the new
createExtensionAPI, along the way, trying to simplify them to use the minimal state possible & leaving the rest to the UI to actually derive.Impact
Testing
Screenshots/Video
Checklist
Additional Notes
Follow ups:
useMenuAt.tsfor a starting point, it implements the virtual elements to place things within the editor at the correct position).useMenuAtPosition( whichPos: (ctx: {editor: BlockNoteEditor }) => { from: number, to?: number }, floatingUIOptions: Object)useMenuAtBlock( whichBlock: (ctx: {editor: BlockNoteEditor }) => BlockID, floatingUIOptions: Object)built on previous? Should this just be based on the location API?