Skip to content
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

Implementing Collaborative Timeline Slider with Undo/Redo Functionality #338

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 100 additions & 3 deletions packages/docprovider-extension/src/filebrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@ import {
JupyterFrontEndPlugin
} from '@jupyterlab/application';
import { Dialog, showDialog } from '@jupyterlab/apputils';
import { IDocumentWidget } from '@jupyterlab/docregistry';
import { DocumentWidget, IDocumentWidget } from '@jupyterlab/docregistry';
import { Widget } from '@lumino/widgets';
import {
FileBrowser,
IDefaultFileBrowser,
IFileBrowserFactory
} from '@jupyterlab/filebrowser';
import { IStatusBar } from '@jupyterlab/statusbar';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


import { IEditorTracker } from '@jupyterlab/fileeditor';
import { ILogger, ILoggerRegistry } from '@jupyterlab/logconsole';
import { INotebookTracker } from '@jupyterlab/notebook';
Expand All @@ -28,17 +31,21 @@ import { YFile, YNotebook } from '@jupyter/ydoc';

import {
ICollaborativeDrive,
IForkProvider,
IGlobalAwareness,
TimelineWidget,
YDrive
} from '@jupyter/docprovider';
import { Awareness } from 'y-protocols/awareness';
import { URLExt } from '@jupyterlab/coreutils';

/**
* The command IDs used by the file browser plugin.
*/
namespace CommandIDs {
export const openPath = 'filebrowser:open-path';
}
const DOCUMENT_TIMELINE_URL = 'api/collaboration/timeline';

/**
* The default collaborative drive provider.
Expand Down Expand Up @@ -91,7 +98,7 @@ export const ynotebook: JupyterFrontEndPlugin<void> = {
optional: [ISettingRegistry],
activate: (
app: JupyterFrontEnd,
drive: ICollaborativeDrive,
drive: YDrive,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

settingRegistry: ISettingRegistry | null
): void => {
let disableDocumentWideUndoRedo = true;
Expand Down Expand Up @@ -127,6 +134,95 @@ export const ynotebook: JupyterFrontEndPlugin<void> = {
);
}
};
/**
* A plugin to add a timeline slider status item to the status bar.
*/
export const statusBarTimeline: JupyterFrontEndPlugin<void> = {
id: '@jupyter/docprovider-extension:statusBarTimeline',
description: 'Plugin to add a timeline slider to the status bar',
autoStart: true,
requires: [IStatusBar, ICollaborativeDrive],
activate: async (
app: JupyterFrontEnd,
statusBar: IStatusBar,
drive: ICollaborativeDrive
): Promise<void> => {
function isYDrive(drive: YDrive | ICollaborativeDrive): drive is YDrive {
return 'getProviderForPath' in drive;
}
try {
let sliderItem: Widget | null = null;
let timelineWidget: TimelineWidget | null = null;

const updateTimelineForDocument = async (documentPath: string) => {
if (drive && isYDrive(drive)) {
// Dispose of the previous timelineWidget if it exists
if (timelineWidget) {
timelineWidget.dispose();
timelineWidget = null;
}

const provider = (await drive.getProviderForPath(
documentPath
)) as IForkProvider;
const fullPath = URLExt.join(
app.serviceManager.serverSettings.baseUrl,
DOCUMENT_TIMELINE_URL,
documentPath.split(':')[1]
);

timelineWidget = new TimelineWidget(
fullPath,
provider,
provider.contentType,
provider.format
);

const elt = document.getElementById('slider-status-bar');
if (elt && !timelineWidget.isAttached) {
Widget.attach(timelineWidget, elt);
} else if (!timelineWidget.isAttached) {
Widget.attach(timelineWidget, document.body);
}
}
};

if (app.shell.currentChanged) {
app.shell.currentChanged.connect(async (_, args) => {
const currentWidget = args.newValue as DocumentWidget;
if (timelineWidget) {
// Dispose of the timelineWidget when the document is closed
timelineWidget.dispose();
timelineWidget = null;
}
if (currentWidget && 'context' in currentWidget) {
await updateTimelineForDocument(currentWidget.context.path);
}
});
}

if (statusBar) {
if (!sliderItem) {
sliderItem = new Widget();
sliderItem.addClass('jp-StatusBar-GroupItem');
sliderItem.addClass('jp-mod-highlighted');
sliderItem.id = 'slider-status-bar';
statusBar.registerStatusItem('slider-status-bar', {
item: sliderItem,
align: 'left',
rank: 4,
isActive: () => {
const currentWidget = app.shell.currentWidget;
return !!currentWidget && 'context' in currentWidget;
}
});
}
}
} catch (error) {
console.error('Failed to activate statusBarTimeline plugin:', error);
}
}
};

/**
* The default file browser factory provider.
Expand All @@ -144,7 +240,7 @@ export const defaultFileBrowser: JupyterFrontEndPlugin<IDefaultFileBrowser> = {
],
activate: async (
app: JupyterFrontEnd,
drive: ICollaborativeDrive,
drive: YDrive,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Suggested change
drive: YDrive,
drive: ICollaborativeDrive,

fileBrowserFactory: IFileBrowserFactory,
router: IRouter | null,
tree: JupyterFrontEnd.ITreeResolver | null,
Expand Down Expand Up @@ -292,6 +388,7 @@ namespace Private {
router.routed.disconnect(listener);

const paths = await tree?.paths;

if (paths?.file || paths?.browser) {
// Restore the model without populating it.
await browser.model.restore(browser.id, false);
Expand Down
6 changes: 4 additions & 2 deletions packages/docprovider-extension/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import {
yfile,
ynotebook,
defaultFileBrowser,
logger
logger,
statusBarTimeline
} from './filebrowser';
import { notebookCellExecutor } from './executor';

Expand All @@ -25,7 +26,8 @@ const plugins: JupyterFrontEndPlugin<any>[] = [
ynotebook,
defaultFileBrowser,
logger,
notebookCellExecutor
notebookCellExecutor,
statusBarTimeline
];

export default plugins;
61 changes: 61 additions & 0 deletions packages/docprovider/src/TimelineSlider.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/* -----------------------------------------------------------------------------
| Copyright (c) Jupyter Development Team.
| Distributed under the terms of the Modified BSD License.
|----------------------------------------------------------------------------*/

import { ReactWidget } from '@jupyterlab/apputils';
import { TimelineSliderComponent } from './component';
import * as React from 'react';
import { IForkProvider } from './ydrive';

export class TimelineWidget extends ReactWidget {
private apiURL: string;
private provider: IForkProvider;
private contentType: string;
private format: string;

constructor(
apiURL: string,
provider: IForkProvider,
contentType: string,
format: string
) {
super();
this.apiURL = apiURL;
this.provider = provider;
this.contentType = contentType;
this.format = format;
this.addClass('timeline-slider-wrapper');
}

render(): JSX.Element {
return (
<TimelineSliderComponent
key={this.apiURL}
apiURL={this.apiURL}
provider={this.provider}
contentType={this.contentType}
format={this.format}
/>
);
}
updateContent(apiURL: string, provider: IForkProvider): void {
this.apiURL = apiURL;
this.provider = provider;
this.contentType = this.provider.contentType;
this.format = this.provider.format;

this.update();
}
extractFilenameFromURL(url: string): string {
try {
const parsedURL = new URL(url);
const pathname = parsedURL.pathname;
const segments = pathname.split('/');
return segments[segments.length - 1];
} catch (error) {
console.error('Invalid URL:', error);
return '';
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
extractFilenameFromURL(url: string): string {
try {
const parsedURL = new URL(url);
const pathname = parsedURL.pathname;
const segments = pathname.split('/');
return segments[segments.length - 1];
} catch (error) {
console.error('Invalid URL:', error);
return '';
}
}

This appears to be unused. Instead in the component.tsx there is a standalone extractFilenameFromURL function (the same name as this method) which does a similar thing but is actually used.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
Loading
Loading