Set undoManager in constructor for cells in notebook
#321
Conversation
|
|
||
| /** | ||
| * Cell notebook awareness or null if the cell is standalone. | ||
| * Cell notebook awareness or null. |
There was a problem hiding this comment.
Unrelated, but I spotted that if the cell is standalone then this._awareness is defined in line 184 (old line 180) so it definitely is not null.
| * Defer setting the undo manager as it requires the | ||
| * cell to be attached to the notebook Y document. | ||
| */ | ||
| setUndoManager(): void { |
There was a problem hiding this comment.
Should we remove it as it is not part of the public API defined in api.ts and was only used internally in ynotebook.ts, or should we keep it and deprecate, making it a no-op?
There was a problem hiding this comment.
If it's not part of the public API, I'd say we should remove it.
| * Defer setting the undo manager as it requires the | ||
| * cell to be attached to the notebook Y document. | ||
| */ | ||
| setUndoManager(): void { |
There was a problem hiding this comment.
If it's not part of the public API, I'd say we should remove it.
|
Thank you for the review @davidbrochart! Would you be able to release a new version with this patch? I do not have release rights on this repo. |
|
I released v3.0.4, sorry for the delay. |
2 participants
A fix for jupyterlab/jupyterlab#17247.
Previously an assumption was made that the undo manager can be only instantiated after the cell's
ymodelgets inserted into the notebook (more precisely, after it gets added to the_ycellsarray). This is because doing so before that resulted in an error being raised due tocell.ymodel.docbeingnull(as the docs is set onymodelonly when it gets inserted into anYArrraywith a document).However, the
docset during insertion is just the notebook'sdocand we can access it earlier in the cell's constructor underthis._notebook.ydoc; further, theY.UndoManagerhas a way to pass a doc explicitly if one cannot rely on extraction fromymodel(which is exactly the case we face).That assumption dates back to jupyterlab/jupyterlab#13168 (around the same time as the
docparameter was added to yjs in yjs/yjs@6fa8778).