-
-
Notifications
You must be signed in to change notification settings - Fork 525
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: change
event allows getting a list of the changes made
#1585
base: feat/blocknote-transactions
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
4ce1f42
to
4a3d364
Compare
24382b7
to
96b242b
Compare
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/server-util
@blocknote/react
@blocknote/shadcn
@blocknote/xl-docx-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
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 is so awesome, and implementation actually cleaner / clearer than expected! really cool
} else if (transaction.getMeta("uiEvent") === "drop") { | ||
source = { type: "drop" }; | ||
} else if (transaction.getMeta("history$")) { | ||
source = { |
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.
does this work for collaboration as well (yjs history)?
const changes: BlocksChanged<BSchema, ISchema, SSchema> = []; | ||
// TODO when we upgrade to Tiptap v3, we can get the appendedTransactions which would give us things like the actual inserted Block IDs. | ||
// since they are appended to the transaction via the unique-id plugin | ||
const combinedTransaction = combineTransactionSteps(transaction.before, [ |
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 don't completely understand this. How do we get the block ids now then, if we don't yet support the uniqueid transactions?
let originalDispatch: typeof editor.dispatch; | ||
|
||
beforeEach(() => { | ||
transaction = undefined as unknown as Transaction; |
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.
not easier to test the onchange event directly instead of patching like this?
|
||
const getEditor = setupTestEnv(); | ||
|
||
describe("Test getBlocksChangedByTransaction", () => { |
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 add test-cases for child blocks (add / update / delete child blocks). We can debate whether this should trigger a change on the parent or not, but at least it should be tested / documented imo
This implements a
getChanges
API which will derive the changes made within a transaction as a list of: inserts, updates and deletions of blocknote blocksTo test this you can update an example like so:
Things left:
Resolves #1541 #482 #1566