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.

Sign up for GitHub

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

Jump to bottom

Management of widgets in tree view #6478

Closed
wants to merge 1 commit into from
Closed

Management of widgets in tree view #6478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 31 additions & 4 deletions packages/application/src/shell.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { Message, MessageLoop, IMessageHandler } from '@lumino/messaging';

import { ISignal, Signal } from '@lumino/signaling';

import { Panel, Widget, BoxLayout } from '@lumino/widgets';
import { Panel, Widget, BoxLayout, TabPanel } from '@lumino/widgets';

/**
* The Jupyter Notebook application shell token.
Expand Down Expand Up @@ -73,6 +73,7 @@ export class NotebookShell extends Widget implements JupyterFrontEnd.IShell {
rootLayout.addWidget(this._main);

this.layout = rootLayout;
this._mainIsTabPanel = false;
}

/**
Expand Down Expand Up @@ -103,12 +104,29 @@ export class NotebookShell extends Widget implements JupyterFrontEnd.IShell {
return this._menuWrapper;
}

/**
* Get the status of main area.
*/
get mainIsTabPanel(): Boolean {
Copy link
Member

Choose a reason for hiding this comment

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

Need to think about this, but at first glance trying to know what a page or a set of extensions is trying to add sounds a bit odd.

The shell should be generic enough, for example we could imaging other custom application reusing NotebookShell but without the tree view (or a different one not using a TabPanel).

Copy link
Member

Choose a reason for hiding this comment

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

Also for more context the NotebookShell was initially thought as only allowing 1 widget in the main area on purpose. As a way to have document / widget centric pages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that this solution is not generic enough.

Also for more context the NotebookShell was initially thought as only allowing 1 widget in the main area on purpose.

Is that still relevant as there is already the tree view that would allow several widgets in main area ?

Indeed if the tree view will be the only one needing several widgets, the better way might be to expose its tab panel as said in #6410. Would the idea be to use token ?

Otherwise, another way could be to use a dock panel like in JupyterLab, but limited to one widget by default. That would keep the initial context but allows several widgets in main area if necessary.

Copy link
Member

Choose a reason for hiding this comment

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

Is that still relevant as there is already the tree view that would allow several widgets in main area ?

Good point. For the tree page it would make sense for widgets to be added to the tab panel. And would be natural to extension authors using app.shell.add(widget, 'main').

Would the idea be to use token ?

Yes the idea would have been to require the token to be able to add new widgets to the tab panel. Thinking more about it this might be simpler to implement, although maybe less straighforward to extension authors who would somehow need to create a dedicated plugin for this.

return this._mainIsTabPanel;
}

/**
* Set the status of main area.
*/
set mainIsTabPanel(value: Boolean) {
this._mainIsTabPanel = value;
}

/**
* Activate a widget in its area.
*/
activateById(id: string): void {
const widget = find(this.widgets('main'), w => w.id === id);
if (widget) {
if (this._mainIsTabPanel) {
(this._main.widgets[0] as TabPanel).tabBar.currentTitle = widget.title;
}
widget.activate();
}
}
Expand Down Expand Up @@ -137,11 +155,16 @@ export class NotebookShell extends Widget implements JupyterFrontEnd.IShell {
return this._menuHandler.addWidget(widget, rank);
}
if (area === 'main' || area === undefined) {
if (this._main.widgets.length > 0) {
let main = this._main;

// If main area is a TabPanel, then the widget is added to it.
if (this._mainIsTabPanel) main = main.widgets[0] as TabPanel;
else if (this._main.widgets.length > 0) {
// do not add the widget if there is already one
return;
}
this._main.addWidget(widget);

main.addWidget(widget);
this._main.update();
this._currentChanged.emit(void 0);
}
Expand Down Expand Up @@ -175,7 +198,10 @@ export class NotebookShell extends Widget implements JupyterFrontEnd.IShell {
case 'menu':
return iter(this._menuHandler.panel.widgets);
case 'main':
return iter(this._main.widgets);
// If the main area is a TabPanel returns the list of its Widgets.
if (this._mainIsTabPanel)
return iter((this._main.widgets[0] as TabPanel).widgets);
else return iter(this._main.widgets);
default:
throw new Error(`Invalid area: ${area}`);
}
Expand All @@ -188,6 +214,7 @@ export class NotebookShell extends Widget implements JupyterFrontEnd.IShell {
private _spacer: Widget;
private _main: Panel;
private _currentChanged = new Signal<this, void>(this);
private _mainIsTabPanel: Boolean;
}

/**
Expand Down
14 changes: 8 additions & 6 deletions packages/tree-extension/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
} from '@jupyterlab/ui-components';

import { Menu, MenuBar, TabPanel } from '@lumino/widgets';
import { NotebookShell } from '@jupyter-notebook/application';

/**
* The file browser factory.
Expand Down Expand Up @@ -116,6 +117,11 @@ const browserWidget: JupyterFrontEndPlugin<void> = {
const tabPanel = new TabPanel({ tabPlacement: 'top', tabsMovable: true });
tabPanel.addClass('jp-TreePanel');

// Add the TabPanel to the main area.
app.shell.add(tabPanel, 'main', { rank: 100 });
// Set the main area as a TabPanel.
(app.shell as NotebookShell).mainIsTabPanel = true;

const trans = translator.load('notebook');

const { defaultBrowser: browser } = factory;
Expand All @@ -124,8 +130,7 @@ const browserWidget: JupyterFrontEndPlugin<void> = {
browser.node.setAttribute('aria-label', trans.__('File Browser Section'));
browser.title.icon = folderIcon;

tabPanel.addWidget(browser);
tabPanel.tabBar.addTab(browser.title);
app.shell.add(browser, 'main');

// Toolbar
toolbarRegistry.addFactory(
Expand Down Expand Up @@ -155,8 +160,7 @@ const browserWidget: JupyterFrontEndPlugin<void> = {
running.id = 'jp-running-sessions';
running.title.label = trans.__('Running');
running.title.icon = runningIcon;
tabPanel.addWidget(running);
tabPanel.tabBar.addTab(running.title);
app.shell.add(running, 'main');
}

// show checkboxes by default if there is no user setting override
Expand All @@ -171,8 +175,6 @@ const browserWidget: JupyterFrontEndPlugin<void> = {
.catch((reason: Error) => {
console.error(reason.message);
});

app.shell.add(tabPanel, 'main', { rank: 100 });
}
};

Expand Down