Implementing Collaborative Timeline Slider with Undo/Redo Functionality #338

Meriem-BenIsmail · 2024-08-19T16:32:41Z

Description:

This pull request introduces a comprehensive timeline and undo/redo functionality to the Jupyter Collaboration project. The primary goal of this feature is to enhance the collaborative experience by allowing users to track changes, restore previous versions of documents, and seamlessly undo or redo actions.

Key Features:

Timeline Slider Component:
- Added a component (TimelineSliderComponent) that provides an interactive slider to navigate through the history of document changes.
- The slider fetches and displays timestamps of changes, allowing users to select specific versions of the document and to restore them if needed.
Document Forking and Restoration:
- Implemented functions to fork the current state of the document and create new versions.
- Users can restore the document to any previous state using the restore mode.
Undo/Redo Management:
- Integrated UndoManager to handle undo and redo operations for document edits.
- Users can perform multiple undo/redo actions to revert or reapply changes.
API Enhancements:
- Extended the existing API to support timeline fetching and document restoration.
- Introduced new endpoints for creating document forks and managing undo/redo operations.
Notifications:
- Implemented user notifications to provide feedback on actions such as successful document restoration or errors.

Usage:

Accessing the Timeline:
- Click on the history icon in the status bar to open the timeline slider.
- Use the slider to navigate through different versions of the document.
Restoring a Version:
- Select a desired timestamp from the slider and click the "Restore version" button to revert the document to the selected state.
- A notification will confirm the successful restoration or display an error if the operation fails.
Undo/Redo Actions:
- Use the timeline slider to perform undo/redo operations by moving back and forth between timestamps.

Note: The timeline is tested with various document types, including notebooks and files in different directory structures and also YJCad files of JupyterCAD.

Here is a demo of the functionality

github-actions · 2024-08-19T16:32:54Z

👈 Launch a Binder on branch Meriem-BenIsmail/jupyter-collaboration/collaborative-timeline-feature

davidbrochart · 2024-08-20T12:58:04Z

Thanks for working on this @Meriem-BenIsmail !
I just tried your branch, and I'm not able to go back in time:

Peek.2024-08-20.14-53.mp4

Is there something I'm doing wrong?

Meriem-BenIsmail · 2024-08-20T13:38:16Z

Thanks for working on this @Meriem-BenIsmail ! I just tried your branch, and I'm not able to go back in time:

Peek.2024-08-20.14-53.mp4
Is there something I'm doing wrong?

That's weird I tried to recreate your vid and it works for me

Thanks for the feedback though, I will have a look at what might be causing the problem.
Note: when the timeline is opened and we start using the slider, the document displayed is a fork so any changes made after using the slider aren't saved. This is done to prevent any unwanted modifications when going through the history of the document.

krassowski

This would be super exciting to have, it would be a revolution compared to the existing .ipynb_checkpoints mechanism!

I've tried to review that as if this PR was ready for review, I hope it is not too early to give fairly detailed feedback on the implementation.

krassowski · 2024-08-23T21:21:56Z

packages/docprovider-extension/src/filebrowser.ts

 import {
  FileBrowser,
  IDefaultFileBrowser,
  IFileBrowserFactory
 } from '@jupyterlab/filebrowser';
+import { IStatusBar } from '@jupyterlab/statusbar';


I am worried about the widget taking too much space in the statusbar; I think this is fine it as-is in this PR but I would consider moving it to a sidebar.

krassowski · 2024-08-23T21:23:52Z

packages/docprovider-extension/src/filebrowser.ts

@@ -91,7 +94,7 @@ export const ynotebook: JupyterFrontEndPlugin<void> = {
  optional: [ISettingRegistry],
  activate: (
    app: JupyterFrontEnd,
-    drive: ICollaborativeDrive,
+    drive: YDrive,


Is this change required? If not, I would suggest keeping the more general interface here to keep specific plugins compatible with different implementation of collaborative drive.

Of course a change in typing would not make them incompatible, but it would make it easy to later accidentally access properties/method which only occur in YDrive but are absent in ICollaborativeDrive (whereas the only guaranteed that the plugin system gives us is that we will receive an implementation of ICollaborativeDrive)

Suggested change

drive: YDrive,

drive: ICollaborativeDrive,

@Meriem-BenIsmail What do you think about @krassowski's comment?

Yes I agree I changed it back to ICollaborativeDrive and then added a type guard as he suggested.

krassowski · 2024-08-23T21:32:33Z

packages/docprovider-extension/src/filebrowser.ts

+  requires: [IStatusBar],
+  optional: [ICollaborativeDrive],
+  activate: async (
+    app: JupyterFrontEnd,
+    statusBar: IStatusBar,
+    drive: YDrive | null
+  ): Promise<void> => {
+    try {
+      if (!drive) {
+        console.warn('Collaborative drive not available');
+        return;
+      }


I think ICollaborativeDrive should be required here, as otherwise this plugin does nothing. This way users disabling the ICollaborativeDrive plugin in Plugin Manager will be notified that this plugin will also be disabled.

We cannot assume that we will always get an YDrive class instance. It can be a different ICollaborativeDrive implementation. Hence, where you do require YDrive I would suggest the following typing and checks:

Suggested change

requires: [IStatusBar],

optional: [ICollaborativeDrive],

activate: async (

app: JupyterFrontEnd,

statusBar: IStatusBar,

drive: YDrive | null

): Promise<void> => {

try {

if (!drive) {

console.warn('Collaborative drive not available');

return;

}

requires: [IStatusBar, ICollaborativeDrive],

activate: async (

app: JupyterFrontEnd,

statusBar: IStatusBar,

drive: YDrive | ICollaborativeDrive

): Promise<void> => {

try {

if (!isYDrive(drive)) {

console.warn('Collaborative drive not available');

return;

}

Where isYDrive could be defined as a type guard like:

function isYDrive(drive: YDrive | ICollaborativeDrive): drive is YDrive { return 'updateTimelineForNotebook' in drive; // or something smarter }

krassowski · 2024-08-23T21:33:14Z

packages/docprovider-extension/src/filebrowser.ts

+      }
+
+      let sliderItem: Widget | null = null;
+      const updateTimelineForDocument = async (document: any) => {


What is any here? It should be replaced with a proper type :)

krassowski · 2024-08-23T21:34:10Z

packages/docprovider-extension/src/filebrowser.ts

@@ -144,7 +210,7 @@ export const defaultFileBrowser: JupyterFrontEndPlugin<IDefaultFileBrowser> = {
  ],
  activate: async (
    app: JupyterFrontEnd,
-    drive: ICollaborativeDrive,
+    drive: YDrive,


Same as before, I would suggest keeping it as ICollaborativeDrive if this change is not strictly required:

Suggested change

drive: YDrive,

drive: ICollaborativeDrive,

krassowski · 2024-08-23T22:03:34Z

packages/docprovider/src/ydrive.ts

+    return this._timelineWidget || null;
+  }
+
+  async updateTimelineForNotebook(notebookPath: string): Promise<void> {


It looks like it supports not only notebook but any documents, right? Later below I propose to rewrite this procedure with side effects into a more generic function, let's call it getProviderFor(path: string): Promise<WebSocketProvider>

krassowski · 2024-08-23T22:21:51Z

packages/docprovider/src/ydrive.ts

+        this._timelineWidget = new TimelineWidget(
+          notebookPath,
+          provider,
+          options.contentType,
+          options.format
+        );
+      } else {
+        this.updateTimelineForNotebook(options.path);
+        this._timelineWidget.update();
+      }
+      const elt = document.getElementById('slider-status-bar');
+      if (elt && !this._timelineWidget.isAttached) {
+        Widget.attach(this._timelineWidget, elt);


Conceptually, it is not a responsibility of the drive to know anything about widgets. Usually we would create a new "model" (like timelineModel) to interface between one object knowing about multiple things and the widgets, but here a simpler solution might be possible.

Practically, querying by ID is a code smell.

Here is a proposal:

let's move instationation and attachement of TimelineWidget to the @jupyter/docprovider-extension:statusBarTimeline plugin (where it will be a breeze because you have the status bar element there widget reference)

replace updateTimelineForNotebook method on YDrive with getProviderForPath(path: string): Promise<IForkProvider> where:
interface IForkProvider { connectToFork: (arguments) => Promise<someType>; contentType: string; format: string; }
and mark WebSocketProvider as implementing IForkProvider interface

if absolutely needed add a signal on YDrive such as modelCreated: Signal<YDrive, IModelCreatedArgs> so that you can call getProviderForPath() immediately once this _onCreate() is called, where:
interface IModelCreatedArgs { options: Contents.ISharedFactoryOptions; sharedModel: YDocument<DocumentChange>; }
but maybe you could just use document tracker instead.

This should enable you to update the widget without having to built it into the drive (as you only need to get hold of IForkProvider)

Maybe getProviderForPath → getForkProvider?

krassowski · 2024-08-23T22:24:18Z

packages/docprovider/src/yprovider.ts

+  get sharedModel(): YDocument<DocumentChange> {
+    return this._sharedModel;
+  }
+  setPath(path: string) {
+    this._path = path;
+  }
+  setSharedModel(sharedModel: YDocument<DocumentChange>) {
+    this._sharedModel = sharedModel;
+  }


Suggested change

get sharedModel(): YDocument<DocumentChange> {

return this._sharedModel;

}

setPath(path: string) {

this._path = path;

}

setSharedModel(sharedModel: YDocument<DocumentChange>) {

this._sharedModel = sharedModel;

}

Is this used anywhere? It would be a significant expansion to the public API, let's keep it small to make it managable to maintain.

krassowski · 2024-08-23T22:29:56Z

projects/jupyter-server-ydoc/jupyter_server_ydoc/handlers.py

+    async def get(self, path: str) -> None:
+
+        file_id_manager = self.settings["file_id_manager"]
+        file_id = file_id_manager.get_id("/".join(self.request.path.split("/")[4:]))


This looks odd. The code removes four parts of the path - why? I think that elsewhere we just access the path as-is. I would guess that it was meant to remove four letters RTC: if present but this is just a guess.

krassowski · 2024-08-23T22:32:11Z

packages/docprovider/style/slider.css

+  gap: 6px;
+}
+
+.restore-btn button {


Bare elements (without e.g. a class name) on the right-most side are bad for performance because browsers match CSS selectors from right to left. A browser would need to check rest of the rule against all buttons on the page which might be many.

dependency conflicts fixed timestamp order fork handler added connect fork read only notebook connected (fork) fork document in frontend fork in the frontend + apply updates added fork indication to the page added fork indication to the page distinction between fork and original document added to slider adjustement in fork handler file add css slider context menu option for opening file (fork) + timeline added notebook condition added file menu option to open timeline for notebook alternative: add file menu option to open right widget added timeline slider to status bar slider added to status bar status bar timeline slider with logic to switch between notebooks added error handling in the backend handler clean console add isActive to the registerStatusItem fix: internal server error fixed in updateTimelineforNotebook function fork cnd added to timeline status bar plugin undo manager refactoring fork logic for 1 notebook. fork logic implemented for multiple notebooks opened at the same time ydoc.share fork works with multiple notebooks + freezing notebook when sliding through timeline notebook id testing cad undo manager tests clearDoc method remove is timelineFork open rebase build error resolved using undo manager in the backend undo/redo steps added freeze document updated to the new version of pycrdt, fixed problem with undostack rebase restore version btn undo manager agnostic for different type of documents restoring version restoring version restore btn : fix style delete unused files delete unused files jupytercad jupyter cad jupytercad integration + rebase main conflicts conflicts fixed console error: empty response. icon visible only when data is not null moving fetch timeline in slider component: on click of the history button get format & content type from query params get format & content type from query params: fix updating contenttype and format when switching between documents remove sharedmodel from update content support for documents inside folders/subfolders. clean drive.ts move test files in folder delete unused dependency return comments deleted by accident fixes in jupyter-server-ydoc delete test documents add test-folder to gitignore styling restore button styling restore button pre commit hooks fixed pre commit issues fixed pre commit issues add license header to new files pre commit hooks python test: added jupytercad_core to CI/CD workflow python test debug python test debug collaborative timeline slider added in the status bar dependency conflicts fixed timestamp order fork handler added connect fork read only notebook connected (fork) fork document in frontend fork in the frontend + apply updates added fork indication to the page added fork indication to the page distinction between fork and original document added to slider adjustement in fork handler file add css slider context menu option for opening file (fork) + timeline added notebook condition added file menu option to open timeline for notebook alternative: add file menu option to open right widget added timeline slider to status bar slider added to status bar status bar timeline slider with logic to switch between notebooks added error handling in the backend handler clean console add isActive to the registerStatusItem fix: internal server error fixed in updateTimelineforNotebook function fork cnd added to timeline status bar plugin undo manager refactoring fork logic for 1 notebook. fork logic implemented for multiple notebooks opened at the same time ydoc.share fork works with multiple notebooks + freezing notebook when sliding through timeline notebook id testing cad undo manager tests clearDoc method remove is timelineFork open rebase build error resolved using undo manager in the backend undo/redo steps added freeze document updated to the new version of pycrdt, fixed problem with undostack rebase restore version btn undo manager agnostic for different type of documents restoring version restoring version restore btn : fix style delete unused files delete unused files jupytercad jupyter cad jupytercad integration + rebase main conflicts conflicts fixed console error: empty response. icon visible only when data is not null moving fetch timeline in slider component: on click of the history button get format & content type from query params get format & content type from query params: fix updating contenttype and format when switching between documents remove sharedmodel from update content support for documents inside folders/subfolders. clean drive.ts move test files in folder delete unused dependency return comments deleted by accident fixes in jupyter-server-ydoc delete test documents add test-folder to gitignore styling restore button styling restore button pre commit hooks fixed pre commit issues python test debug: test.yaml python test debug: test.yaml python test debug: test.yaml changed order of dependencies in test.yml removed jupytercad to test dependencies version removed jupytercad to test dependencies version pre commit changed the way document types are imported in the backend fixed yarn.lock after rebase

davidbrochart · 2024-08-27T08:02:04Z

I am still unable to get this PR working. Here is what I did:

micromamba create -n jupyter-collaboration
micromamba activate jupyter-collaboration
micromamba install pip nodejs
pip install -e ".[dev]"
pip install -e projects/jupyter-collaboration-ui -e projects/jupyter-docprovider -e projects/jupyter-server-ydoc
pip install jupyterlab
jupyter labextension develop --overwrite projects/jupyter-collaboration-ui
jupyter labextension develop --overwrite projects/jupyter-docprovider

@Meriem-BenIsmail can you confirm that it works on your side in a fresh environment?
@krassowski did it work on your side?

Meriem-BenIsmail · 2024-08-27T08:40:34Z

I am still unable to get this PR working. Here is what I did:

micromamba create -n jupyter-collaboration
micromamba activate jupyter-collaboration
micromamba install pip nodejs
pip install -e ".[dev]"
pip install -e projects/jupyter-collaboration-ui -e projects/jupyter-docprovider -e projects/jupyter-server-ydoc
pip install jupyterlab
jupyter labextension develop --overwrite projects/jupyter-collaboration-ui
jupyter labextension develop --overwrite projects/jupyter-docprovider

@Meriem-BenIsmail can you confirm that it works on your side in a fresh environment? @krassowski did it work on your side?

I just tested it in a fresh environment following the same commands as you did here. It seems to work from me.
Are you having any error (console or at runtime) that may help me figure out what exactly is going wrong ?

davidbrochart · 2024-08-27T09:14:11Z

Ah it seems that it only works the first time a notebook is opened, but not after closing and reopening it:

Peek.2024-08-27.11-12.mp4

Meriem-BenIsmail · 2024-08-27T09:26:28Z

Ah it seems that it only works the first time a notebook is opened, but not after closing and reopening it:

Peek.2024-08-27.11-12.mp4

Oh I see I haven't noticed this. It works also when you reload the page btw.
I will look into this.

projects/jupyter-server-ydoc/jupyter_server_ydoc/handlers.py

…pe in handler

davidbrochart · 2024-08-27T14:45:14Z

It seems that the changes don't appear at the same place on the slider depending on whether you're undoing or redoing:

Peek.2024-08-27.16-40.mp4

projects/jupyter-server-ydoc/jupyter_server_ydoc/handlers.py

Meriem-BenIsmail · 2024-08-28T08:11:56Z

It seems that the changes don't appear at the same place on the slider depending on whether you're undoing or redoing:

Peek.2024-08-27.16-40.mp4

I think that has to do with the fact that the undo manager doesn't track empty updates, it only tracks relevant modifications to the document. But when fetching the updates/timestamps of a document from the ystore, I see that some of the updates aren't modifications to the file itself I'm not sure about the reason.

davidbrochart · 2024-08-28T08:36:10Z

What about setting the slider's step number to the initial undo stack's length?

Meriem-BenIsmail · 2024-08-28T08:46:54Z

What about setting the slider's step number to the initial undo stack's length?

Will I also get the timestamps to display them in the frontend from the undostack that way ?

davidbrochart · 2024-08-28T08:57:22Z

I think so yes, for instance when you apply all the updates to the fork, you can check if the document's undo stack grows after each applied update, and build a timestamp list that corresponds to each stack item this way.

projects/jupyter-server-ydoc/jupyter_server_ydoc/handlers.py

davidbrochart · 2024-08-28T14:30:51Z

There is something weird happening when after a document was restored to a previous state:

when restored, the dirty indicator should be cleared, meaning that the file has been saved.
after reopening the file, the timeline seems to be broken:

Peek.2024-08-28.15-46.mp4

Meriem-BenIsmail · 2024-08-28T14:50:31Z

There is something weird happening when after a document was restored to a previous state:

when restored, the dirty indicator should be cleared, meaning that the file has been saved.

after reopening the file, the timeline seems to be broken:

Peek.2024-08-28.15-46.mp4

I think the dirty indicator is not cleared because even if we could restore the document state, the displayed version is a still a fork so maybe that's the cause.
Also I tried to recreate your case here but I don't get the same behavior, could you walk me through the problem exactly ?

davidbrochart · 2024-08-28T15:08:57Z

I think the dirty indicator is not cleared because even if we could restore the document state, the displayed version is a still a fork so maybe that's the cause.

You don't reconnect to the root document after restoring?

Meriem-BenIsmail · 2024-08-28T15:17:11Z

I think the dirty indicator is not cleared because even if we could restore the document state, the displayed version is a still a fork so maybe that's the cause.

You don't reconnect to the root document after restoring?

I could do that but I would still need to reload the document if I want to use the timeline again.

davidbrochart · 2024-08-28T15:20:06Z

The document would reload by itself, just by reconnecting to the root.

Meriem-BenIsmail · 2024-08-28T16:10:10Z

The document would reload by itself, just by reconnecting to the root.

but i still need to refetch the new timeline so i would need to reload the browser and i don't think that happens automatically if i reconnect.

davidbrochart · 2024-08-28T16:43:51Z

Refreshing the browser is not an option but you can surely update the timeline.

davidbrochart · 2024-08-29T07:26:56Z

Thanks for keeping up the great work @Meriem-BenIsmail, this is starting to look really nice! I like the user experience.
One thing though is that I see you don't allow to undo the very first change here:

if action == "undo" and len(undo_manager.undo_stack) > 1:
    undo_manager.undo()

I suppose this has something to do with the fact that JupyterLab doesn't like empty notebooks (see this issue)? For text files however, this means that we cannot go back to to an empty file:

Peek.2024-08-29.09-16.mp4

projects/jupyter-server-ydoc/jupyter_server_ydoc/handlers.py

Meriem-BenIsmail · 2024-08-29T07:53:32Z

Thanks for keeping up the great work @Meriem-BenIsmail, this is starting to look really nice! I like the user experience. One thing though is that I see you don't allow to undo the very first change here:
if action == "undo" and len(undo_manager.undo_stack) > 1:
    undo_manager.undo()
I suppose this has something to do with the fact that JupyterLab doesn't like empty notebooks (see this issue)? For text files however, this means that we cannot go back to to an empty file:

Peek.2024-08-29.09-16.mp4

that would require a special handling of notebooks. Do you have an idea if this issue only appears in notebooks ?

davidbrochart · 2024-08-29T08:05:30Z

Do you have an idea if this issue only appears in notebooks ?

Potentially yes, but it can be considered as a bug if a document doesn't allow an "empty state", so let's keep it like that for now, and revisit the issue if/when jupyterlab/jupyterlab#16718 is fixed.

projects/jupyter-server-ydoc/jupyter_server_ydoc/handlers.py

davidbrochart · 2024-08-29T09:59:54Z

You need to require pycrdt-websocket>=0.14.2,<0.15.0 here.

davidbrochart · 2024-08-29T13:56:03Z

I think this is looking good. I'll give more time for review, and merge tomorrow otherwise.

davidbrochart · 2024-08-30T14:34:12Z

@krassowski were all your requested changes applied?

davidbrochart · 2024-09-04T14:53:10Z

Thanks again @Meriem-BenIsmail!

krassowski · 2024-09-04T15:18:06Z

Sorry for not getting back here earlier, got sick and did not remember to comment back - most of the things were addressed so good call to merge!

davidbrochart added the enhancement New feature or request label Aug 20, 2024

Meriem-BenIsmail marked this pull request as ready for review August 20, 2024 08:01

Meriem-BenIsmail force-pushed the collaborative-timeline-feature branch from add8143 to 05df889 Compare August 23, 2024 08:01

Meriem-BenIsmail changed the title ~~Implementing Collaborative Timeline Slider with Undo/Redo Functionality for Jupyter Collaboration~~ Implementing Collaborative Timeline Slider with Undo/Redo Functionality Aug 23, 2024

krassowski self-requested a review August 23, 2024 20:33

krassowski requested changes Aug 23, 2024

View reviewed changes

Meriem-BenIsmail added 2 commits August 26, 2024 13:24

added code changes as requested.

169cc00

Meriem-BenIsmail force-pushed the collaborative-timeline-feature branch from 4fbab40 to 169cc00 Compare August 26, 2024 11:26

davidbrochart reviewed Aug 27, 2024

View reviewed changes