-
-
Notifications
You must be signed in to change notification settings - Fork 518
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
feat: position storage #1529
base: main
Are you sure you want to change the base?
feat: position storage #1529
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
c7b80a9
to
2c3c9cf
Compare
2c3c9cf
to
0ae54a2
Compare
/** | ||
* Internal properties that are not part of the public API and may change in the future. | ||
* | ||
* @internal | ||
*/ | ||
public readonly '~internal': { | ||
/** | ||
* Stores positions of elements in the editor. | ||
*/ | ||
positionStorage: PositionStorage<BSchema, ISchema, SSchema>; | ||
}; |
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.
Let's have a discussion on this.
In general, here is what I'm thinking:
- The editor instance is the main thing passed around and is essentially a god object
- configuration is primarily done through the editor instance, making it the most convenient place to store and retrieve references to the current config
I think what we need to be aiming for here instead is a de-coupled approach like prosekit, where the editor doesn't "have" these sorts of values, it can execute commands to move between states, but the values of these are stored elsewhere (like context).
Given that we don't have a good solution for state management at the moment, I propose here to just sweep things under the rug for a little while, and put them under an ~internal
property. Using ~
makes it both cumbersome (read intentional) and shows the value last in TS autocomplete idea from standard-schema.
For configuration, I think that if the schema was defined separately, it would be much easier to configure things like tables and codeBlocks. For example:
import { defaultSchema, tableBlock } from '@blocknote/core/schema';|
import { BlockNoteEditor } from '@blocknote/core';
const editor = BlockNoteEditor.create({
schema: defaultSchema.replace('table', tableBlock.configure({ option: true })
})
// editor.schema.table.config.option === true
Obviously, there are a lot of different ways to do it, but the point is that the editor doesn't really have to know about this.
What about all of the prosemirror plugins? It could maybe be something like:
import { BlockNoteEditor, BlockNoteExtension, getDefaultExtensions } from '@blocknote/core';
const editor = BlockNoteEditor.create({
extensions: getDefaultExtensions.remove(['tables']).add({ myExt: BlockNoteExtension.create({ name: 'myExt', plugin: new Plugin() }) })
})
// editor.extensions.myExt.plugin
Again, different approaches here too
constructor( | ||
editor: BlockNoteEditor<BSchema, ISchema, SSchema>, | ||
{ shouldMount = true }: { shouldMount?: boolean } = {} | ||
) { | ||
this.editor = editor; | ||
this.onTransactionHandler = this.onTransactionHandler.bind(this); | ||
|
||
if (!shouldMount) { | ||
return; | ||
} | ||
|
||
if (!this.editor._tiptapEditor) { | ||
throw new Error("Editor not mounted"); | ||
} | ||
this.editor._tiptapEditor.on("transaction", this.onTransactionHandler); | ||
} |
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.
An alternative to this, would be to have a prosemirror plugin that gets these transactions between state updates. Perhaps that is preferable?
eead3ec
to
c6fd9ac
Compare
This is still in progress, the idea here is to make a class that can be used for keeping track of positions in the editor across collaborative or single user transactions.
This would be useful not only for the suggestions plugin, but for anything that would need to hold onto a position across transactions.
I still am working through setting up the tests to prove that it is able to keep track of those positions, but I had it preliminarily working for the suggestion plugin at one point, but then decided to solve this in a more generic way.