feat: API design feedback #1756
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
| * Each level of the path is a child of the previous level. | ||
| * The entire document can be described by the path []. | ||
| */ | ||
| export type Path = BlockIdentifier[]; |
There was a problem hiding this comment.
Because a block has a unique id, I'm not sure whether Path is useful? For example, in Point below, we could just use a blockIdentfier instead of path
There was a problem hiding this comment.
Fair.
a Path can be useful for being able to compare depths of things based on their content alone.
For example:
const a = ['abc', '123']
const b = ['abc', '345']Based on this alone, you know they share parents, and therefore siblings of some kind, but I guess you would always have to check with the actual document to see that they really exist or not, so you are probably right
There was a problem hiding this comment.
Nice! Lots of great stuff in here. Besides discussing the open comments, how do we move forward from here?
My suggestion would be to extract isolated work items and prioritize them. Maybe first identify which other substantial core improvements are missing, the following come to mind:
- UI elements cleanup
- Streamline Rendered content, "external HTML", markdown, exporters (i.e.: topics discussed in Berin)
- Zod props
- unified style / theming API
- ...?
|
|
||
| ```ts | ||
| editor.transform.insertBlocks(ctx:{ | ||
| at: Location, |
There was a problem hiding this comment.
Is it useful to insertblocks at a Location vs the current API (before / after / inside an existing block)?
There was a problem hiding this comment.
Toss-up for insertBlocks, but for insertContent(ctx: {at: Location; content: string })
| }) | ||
| ``` | ||
|
|
||
| ## References |
There was a problem hiding this comment.
Nice, currently things like comments can't be represented in the BlockNote API, and indeed, this would enable us to separate them from the document data definitely feels like the "right approach"
There was a problem hiding this comment.
Yeah I like this idea a lot, feels more general than ProseMirror's decorations. In theory it would also make it much nicer for our UI architecture, since instead of writing plugins you could just use references. E.g. for the formatting toolbar, you could listen to selection changes and create a reference pointing to the selection when you want to show it.
| */ | ||
| export type Path = BlockIdentifier[]; | ||
|
|
||
| /** |
There was a problem hiding this comment.
How would this work for things like table cells?
There was a problem hiding this comment.
Likely would use a Point to refer to table-cells as it is right now.
If a table-cell was implemented as nested block, then it would just be a Path
|
|
||
| We've done pretty well with our existing API, but there are a few things that could be improved. | ||
|
|
||
| Referencing content within the document is either awkward or non-existent. Right now we essentially really only have an API for referencing blocks by their id with no further level of granularity. |
There was a problem hiding this comment.
Overall, I think the concept of a Location is useful to have - but if we start implementing this I think we should have more sight on practical use-cases. i.e. which use-cases / APIs do we want to "unlock" with this?
There was a problem hiding this comment.
insertContentAt(at: Location, content: Block | PartialBlock | string)
getSelection(): {range: Range}
A number of the low-level Tiptap API uses for things like deleting some chars
This will also be very useful for server-side editing ops like rewriting a paragraph
|
|
||
| ## Transforms separation of concerns | ||
|
|
||
| Right now all transformation functions are defined directly on the `Editor` instance, this is not ideal because it only further muddles the API. |
There was a problem hiding this comment.
Cool. I think maybe this should be a separate work-item ("cleaning up BlockNoteEditor"). Doing this hand-in-hand with updating documentation probably gives us a lot of insights into what we think would be a better organization of functions
There was a problem hiding this comment.
Working on that 😉
| - Relationships between blocks/inline-content/styles can be defined (e.g. allow for a todo block to only have todo item children) | ||
| - Properties of blocks/inline-content/styles can be defined (e.g. adding `heading` to the `toggleable` group) | ||
|
|
||
| This may or may not be useful, but it is a thought. |
There was a problem hiding this comment.
cool, let's keep in mind 👍
|
|
||
| Right now it is overly burdensome to have to pass around 3 different types to the editor, and it is also not very type-safe (when you just end up with `any` everywhere). | ||
|
|
||
| The idea is to have a single type that is a union of the 3 types, and then make type predicates available to check if accessed properties are valid (and likely just assertions too). |
There was a problem hiding this comment.
would be great if we can get this to work!
|
|
||
| These forms of configuration are not mutually exclusive, and can be combined in different ways. For example, knowing that the editor has collaboration enabled, might change the what the keybindings do for undo/redo. | ||
|
|
||
| In an ideal world, these configurations would be made at the "lowest possible level", like configuring the number of levels of headings would be configured when composing the schema for the editor, rather than at the editor level. |
| This is a proposal for how to support nested blocks. | ||
|
|
||
| ```json | ||
| { |
There was a problem hiding this comment.
this is technically what we already support to some level, right?
There was a problem hiding this comment.
Yep, modeled on the multi-column, but generic for others to implement
| "type": "alert", | ||
| "content": null, | ||
| "children": [ | ||
| { |
There was a problem hiding this comment.
I'm not sure if this will work out. now suddenly, children can not only contain blocks anymore, but also inline content. I'm afraid this will break quite a bit (both technically and in terms of explainability)
There was a problem hiding this comment.
or wait, alert-title and alert-content are considered blocks here as well?
There was a problem hiding this comment.
Yep, considered blocks. Would need to introduce the concept of a "nested block" which is not valid on it's own but only in the context of being part of another block.
This is also shown with the table example below
Agreed, I feel like this is a bit larger than I'm willing to take on at the moment though.
Agreed, I will add this to the list.
Will add to the list
I think this is the same as the ui elements cleanup
I feel like these are the most pressing at the moment, but let me know what you think! |
|
Forgot this one:
|
| @@ -0,0 +1,55 @@ | |||
| # BlockNote Formats | |||
|
|
|||
| Right now, there are several formats supported by BlockNote: | |||
There was a problem hiding this comment.
I would even consider the "rendered" HTML another format (i.e.: what you see in the editor)
There was a problem hiding this comment.
Yep, I did miss that one
| /** | ||
| * Items that are available to the slash menu | ||
| */ | ||
| slashMenuItems?: ( |
There was a problem hiding this comment.
Let's cross this bridge when we get there, but I think we can come up with something more generic / scalable
There was a problem hiding this comment.
100% was just a concept of cross-cutting concerns
| public get document(): Block[]; | ||
| public set document(document: Block[]); | ||
| public get selection(): Location; // likely more complex than this | ||
| public set selection(selection: Location); |
There was a problem hiding this comment.
Thinking about naming, my association with a "transform" is that it would change something. I wouldn't expect things like selection, getBlock etc. to be "transforms". Should these be something else, or do we need a better name?
There was a problem hiding this comment.
I toyed with calling this editor.state but it felt overly generic, but maybe that is the right way to go.
My way for explaining transforms was that it is all about transitions of state & what you might need to go from A -> B
|
|
||
| [Looking at Slate](https://docs.slatejs.org/concepts/03-locations) (highly recommend reading the docs), they have the concept of a `Location` which is a way to reference a specific point in the document, but it does not have to be so specific as positions, it has levels of granularity. | ||
|
|
||
| This gives us a unified way to reference content within the document, allowing for much more granular editing. Take a look at the `Location.ts` file for more details around this. |
There was a problem hiding this comment.
Do we want to necessarily use the Location concept? I like the granularity but it feels like we could achieve something similar by just using block/inline content ID references. I.e.:
- Low granularity: Point to the start/end of a block by its ID.
- Mid granularity: Point to the start/end of inline content by its ID.
- High granularity: Point to the start of inline content with an offset by its ID.
I feel like Slate's Location is necessary due to the fact that you can only reference nodes by their positions. Since we set IDs, I think we should make use of that since imo it'll result in a simpler API. So our Location could look more like:
type Location = {
id: string;
side?: "start" | "end";
offset?: number
}|
|
||
| - It gives a single API for all state management, which is more convenient/consistent for consumers | ||
|
|
||
| As for which library to use, I think we should use @tanstack/store. |
This is to open some decisions for discussion around some core parts of the blocknote API.
There are a few proposals in here that can be seen in the ADR directory as markdown files. With some Typescript files to support how they might actually look like in an implementation.