Skip to content

Remove tab #117

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

Merged
merged 3 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion glue_jupyterlab/glue_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def render_viewer(self) -> None:
"""Fill the place holder output with glu-jupyter widgets"""

all_tabs = self._document.get_tab_names()
for frontend_tab in self._viewers:
for frontend_tab in list(self._viewers):
if frontend_tab not in all_tabs:
# Tab removed from the frontend
self.remove_tab(frontend_tab)
Expand Down
12 changes: 4 additions & 8 deletions glue_jupyterlab/glue_ydoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,12 @@ def get(self) -> str:

contents["__main__"].setdefault("viewers", [])

while len(contents["__main__"]["viewers"]) != len(tab_names):
contents["__main__"]["viewers"].append([])

viewer_names = []
for idx, tab in enumerate(tab_names):
viewers = tabs[tab]
contents["__main__"]["viewers"] = []
for tab in tab_names:
viewers = tabs.get(tab, {})
viewer_names = sorted(list(viewers.keys()))

contents["__main__"]["viewers"][idx] = viewer_names
contents["__main__"]["viewers"].append(viewer_names)
for viewer in viewer_names:
contents[viewer] = viewers[viewer]

Expand All @@ -92,7 +89,6 @@ def get(self) -> str:
contents[data_name] = dataset[data_name]

self.add_links_to_contents(links, contents)

return json.dumps(contents, indent=2, sort_keys=True)

def set(self, value: str) -> None:
Expand Down
12 changes: 12 additions & 0 deletions glue_jupyterlab/tests/test_ydoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,18 @@ def test_get(session_path, yglue_doc):
assert "NewScatter" in updated_content["__main__"]["viewers"][2]


def test_get_remove_tab(yglue_doc):
## Fake editing of the y structure
with yglue_doc._ydoc.begin_transaction() as t:
# Remove a tab
yglue_doc._ytabs.pop(t, "Tab 1", None)

updated_content = json.loads(yglue_doc.get())

assert "Tab 1" not in updated_content["__main__"]["tab_names"]
assert len(updated_content["__main__"]["viewers"]) == 1


def test_links(yglue_doc_links, identity_link):
yglue_doc_links.get()
links = yglue_doc_links.links
Expand Down
8 changes: 8 additions & 0 deletions src/document/sharedModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,14 @@ export class GlueSessionSharedModel
}, false);
}

removeTab(name: string): void {
Copy link
Member

Choose a reason for hiding this comment

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

Probably in a follow up PR, but the viewers of the tab could also be deleted to clean the session file.
Maybe we can open an issue on it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed I wanted to do it in this PR, but it needs more changes in the YDoc structure since we need to keep track of the viewers in a YMap

if (this._tabs.has(name)) {
this.transact(() => {
this._tabs.delete(name);
}, false);
}
}

getTabNames(): string[] {
return [...this._tabs.keys()];
}
Expand Down
1 change: 1 addition & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export interface IGlueSessionSharedModel
tabsChanged: ISignal<IGlueSessionSharedModel, IDict>;
localStateChanged: ISignal<IGlueSessionSharedModel, { keys: string[] }>;
addTab(): void;
removeTab(tabName: string): void;
getTabNames(): string[];
getTabData(tabName: string): IDict<IGlueSessionViewerTypes> | undefined;

Expand Down
43 changes: 29 additions & 14 deletions src/viewPanel/sessionWidget.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,21 @@
import { PromiseDelegate } from '@lumino/coreutils';
import { TabBar, BoxPanel, Widget } from '@lumino/widgets';

import { Dialog, InputDialog, showDialog } from '@jupyterlab/apputils';
import { IRenderMimeRegistry } from '@jupyterlab/rendermime';
import { DocumentRegistry } from '@jupyterlab/docregistry';
import { INotebookTracker } from '@jupyterlab/notebook';
import { IRenderMimeRegistry } from '@jupyterlab/rendermime';
import { IKernelConnection } from '@jupyterlab/services/lib/kernel/kernel';
import { CommandRegistry } from '@lumino/commands';
import { PromiseDelegate } from '@lumino/coreutils';
import { Message } from '@lumino/messaging';
import { BoxPanel, TabBar, Widget } from '@lumino/widgets';
import { IJupyterYWidgetManager } from 'yjs-widgets';

import { CommandIDs } from '../commands';
import { HTabPanel } from '../common/tabPanel';
import { DATASET_MIME, IDict, IGlueSessionSharedModel } from '../types';
import { GlueSessionModel } from '../document/docModel';
import { LinkEditor } from '../linkPanel/linkEditor';
import { mockNotebook } from '../tools';
import { DATASET_MIME, IDict, IGlueSessionSharedModel } from '../types';
import { TabView } from './tabView';
import { LinkEditor } from '../linkPanel/linkEditor';

import { Message } from '@lumino/messaging';
import { CommandRegistry } from '@lumino/commands';
import { CommandIDs } from '../commands';
import { IJupyterYWidgetManager } from 'yjs-widgets';
import { IKernelConnection } from '@jupyterlab/services/lib/kernel/kernel';

export class SessionWidget extends BoxPanel {
constructor(options: SessionWidget.IOptions) {
Expand Down Expand Up @@ -50,6 +48,18 @@ export class SessionWidget extends BoxPanel {
this._tabPanel.topBar.addRequested.connect(() => {
this._model.addTab();
});
this._tabPanel.topBar.tabCloseRequested.connect(async (tab, arg) => {
const confirm = await showDialog({
title: 'Delete Tab',
body: 'Are you sure you want to delete this tab?',
buttons: [Dialog.cancelButton(), Dialog.okButton({ label: 'Delete' })]
});
if (confirm.button.accept) {
arg.title.owner.close();
this._tabPanel.topBar.removeTabAt(arg.index);
this._model.removeTab(arg.title.label);
}
});
if (this._model) {
this._linkWidget = new LinkEditor({ sharedModel: this._model });
this._tabPanel.addTab(this._linkWidget, 0);
Expand Down Expand Up @@ -166,7 +176,12 @@ export class SessionWidget extends BoxPanel {
let newTabIndex: number | undefined = undefined;
const currentIndex = this._tabPanel.topBar.currentIndex;
const tabNames = this._model.getTabNames();

Object.keys(this._tabViews).forEach(k => {
if (!tabNames.includes(k)) {
this._tabViews[k].dispose();
delete this._tabViews[k];
}
});
tabNames.forEach((tabName, idx) => {
// Tab already exists, we don't do anything
if (tabName in this._tabViews) {
Expand All @@ -182,7 +197,7 @@ export class SessionWidget extends BoxPanel {
notebookTracker: this._notebookTracker,
commands: this._commands
}));

tabWidget.title.closable = true;
this._tabPanel.addTab(tabWidget, idx + 1);
});

Expand Down
2 changes: 1 addition & 1 deletion src/viewPanel/tabLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ export class TabLayout extends Layout {
private _grid: GridStack;
private _gridItems: Map<string, GridStackItem>;
private _gridItemChanged = new Signal<this, TabLayout.IChange>(this);
private _resizeTimeout = 0;
private _resizeTimeout: any = 0;
private _commands: CommandRegistry;
}

Expand Down
4 changes: 4 additions & 0 deletions src/viewPanel/tabView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ export class TabView extends Widget {
super.dispose();
}

protected onCloseRequest(msg: Message): void {
this.dispose();
}

/**
* The tab name
*/
Expand Down
Loading