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

Reconcile tree routing handler with workspaces. #4708

Merged
merged 11 commits into from Jun 12, 2018
108 changes: 60 additions & 48 deletions packages/application-extension/src/index.tsx
Expand Up @@ -6,7 +6,7 @@ import {
} from '@jupyterlab/application';

import {
Dialog, ICommandPalette, showDialog
Dialog, ICommandPalette, IWindowResolver, showDialog, showErrorMessage
} from '@jupyterlab/apputils';

import {
Expand Down Expand Up @@ -54,7 +54,7 @@ namespace CommandIDs {
*/
namespace Patterns {
export
const tree = /^\/tree\/(.+)/;
const tree = /[^?]*(\/tree\/([^?]+))/;
}


Expand All @@ -63,22 +63,24 @@ namespace Patterns {
*/
const main: JupyterLabPlugin<void> = {
id: '@jupyterlab/application-extension:main',
requires: [ICommandPalette, IRouter],
activate: (app: JupyterLab, palette: ICommandPalette, router: IRouter) => {
requires: [ICommandPalette, IRouter, IWindowResolver],
activate: (app: JupyterLab, palette: ICommandPalette, router: IRouter, resolver: IWindowResolver) => {
// Requiring the window resolver guarantees that the application extension
// only loads if there is a viable window name. Otherwise, the application
// will short-circuit and ask the user to navigate away.
const workspace = resolver.name ? `"${resolver.name}"` : '[default: /lab]';

console.log(`Starting application in workspace: ${workspace}`);

// If there were errors registering plugins, tell the user.
if (app.registerPluginErrors.length !== 0) {
const body = (
<pre>
{app.registerPluginErrors.map(e => e.message).join('\n')}
</pre>
);
let options = {
title: 'Error Registering Plugins',
body,
buttons: [Dialog.okButton()],
okText: 'DISMISS'
};
showDialog(options).then(() => { /* no-op */ });

showErrorMessage('Error Registering Plugins', { message: body });
}

addCommands(app, palette);
Expand All @@ -89,9 +91,8 @@ const main: JupyterLabPlugin<void> = {
app.commands.notifyCommandChanged();
});

let builder = app.serviceManager.builder;

let doBuild = () => {
const builder = app.serviceManager.builder;
const build = () => {
return builder.build().then(() => {
return showDialog({
title: 'Build Complete',
Expand All @@ -104,37 +105,33 @@ const main: JupyterLabPlugin<void> = {
router.reload();
}
}).catch(err => {
showDialog({
title: 'Build Failed',
body: (<pre>{err.message}</pre>)
});
showErrorMessage('Build Failed', { message: <pre>{err.message}</pre> });
});
};

if (builder.isAvailable && builder.shouldCheck) {
builder.getStatus().then(response => {
if (response.status === 'building') {
return doBuild();
return build();
}

if (response.status !== 'needed') {
return;
}
let body = (<div>

const body = (<div>
<p>
JupyterLab build is suggested:
<br />
<pre>{response.message}</pre>
</p>
</div>);

showDialog({
title: 'Build Recommended',
body,
buttons: [Dialog.cancelButton(), Dialog.okButton({ label: 'BUILD' })]
}).then(result => {
if (result.button.accept) {
return doBuild();
}
});
}).then(result => result.button.accept ? build() : undefined);
});
}

Expand Down Expand Up @@ -191,23 +188,6 @@ const router: JupyterLabPlugin<IRouter> = {
const base = PageConfig.getOption('pageUrl');
const router = new Router({ base, commands });

commands.addCommand(CommandIDs.tree, {
execute: (args: IRouter.ILocation) => {
const path = decodeURIComponent((args.path.match(Patterns.tree)[1]));

// File browser navigation waits for the application to be restored.
// As a result, this command cannot return a promise because it would
// create a circular dependency on the restored promise that would
// cause the application to never restore.
const opened = commands.execute('filebrowser:navigate-main', { path });

// Change the URL back to the base application URL without adding the
// URL change to the browser history.
opened.then(() => { router.navigate('', { silent: true }); });
}
});

router.register({ command: CommandIDs.tree, pattern: Patterns.tree });
app.started.then(() => {
// Route the very first request on load.
router.route();
Expand All @@ -223,6 +203,37 @@ const router: JupyterLabPlugin<IRouter> = {
};


/**
* The tree route handler provider.
*/
const tree: JupyterLabPlugin<void> = {
id: '@jupyterlab/application-extension:tree',
autoStart: true,
requires: [IRouter],
activate: (app: JupyterLab, router: IRouter) => {
const { commands } = app;

commands.addCommand(CommandIDs.tree, {
execute: (args: IRouter.ILocation) => {
const { request } = args;
const path = decodeURIComponent((args.path.match(Patterns.tree)[2]));
const url = request.replace(request.match(Patterns.tree)[1], '');
const immediate = true;

// Silently remove the tree portion of the URL leaving the rest intact.
router.navigate(url, { silent: true });

return commands.execute('filebrowser:navigate-main', { path })
.then(() => commands.execute('apputils:save-statedb', { immediate }))
.catch(reason => { console.warn(`Tree routing failed:`, reason); });
}
});

router.register({ command: CommandIDs.tree, pattern: Patterns.tree });
}
};


/**
* The default URL not found extension.
*/
Expand All @@ -231,6 +242,9 @@ const notfound: JupyterLabPlugin<void> = {
activate: (app: JupyterLab, router: IRouter) => {
const bad = PageConfig.getOption('notFoundUrl');
const base = router.base;
const message = `
The path: ${bad} was not found. JupyterLab redirected to: ${base}
`;

if (!bad) {
return;
Expand All @@ -240,11 +254,7 @@ const notfound: JupyterLabPlugin<void> = {
// URL change to the browser history.
router.navigate('', { silent: true });

showDialog({
title: 'Path Not Found',
body: `The path: ${bad} was not found. JupyterLab redirected to: ${base}`,
buttons: [Dialog.okButton()]
});
showErrorMessage('Path Not Found', { message });
},
requires: [IRouter],
autoStart: true
Expand Down Expand Up @@ -375,6 +385,8 @@ function addCommands(app: JupyterLab, palette: ICommandPalette): void {
/**
* Export the plugins as default.
*/
const plugins: JupyterLabPlugin<any>[] = [main, layout, router, notfound, busy];
const plugins: JupyterLabPlugin<any>[] = [
main, layout, router, tree, notfound, busy
];

export default plugins;
48 changes: 36 additions & 12 deletions packages/apputils-extension/src/index.ts
Expand Up @@ -13,7 +13,8 @@ import {
} from '@jupyterlab/apputils';

import {
DataConnector, ISettingRegistry, IStateDB, SettingRegistry, StateDB, URLExt
DataConnector, ISettingRegistry, IStateDB, PageConfig, SettingRegistry,
StateDB, URLExt
} from '@jupyterlab/coreutils';

import {
Expand Down Expand Up @@ -93,10 +94,10 @@ namespace CommandIDs {
*/
namespace Patterns {
export
const cloneState = /(\?clone\=|\&clone\=)([^&]+)($|&)/;
const cloneState = /[?&]clone([=&]|$)/;

export
const loadState = /^\/workspaces\/([^?]+)/;
const loadState = /^\/workspaces\/([^?\/]+)/;

export
const resetOnLoad = /(\?reset|\&reset)($|&)/;
Expand Down Expand Up @@ -273,7 +274,11 @@ const resolver: JupyterLabPlugin<IWindowResolver> = {

return Private.redirect(router);
})
.then(() => resolver);
.then(() => {
PageConfig.setOption('workspace', resolver.name);

return resolver;
});
}
};

Expand Down Expand Up @@ -304,8 +309,8 @@ const state: JupyterLabPlugin<IStateDB> = {
id: '@jupyterlab/apputils-extension:state',
autoStart: true,
provides: IStateDB,
requires: [IRouter, IWindowResolver],
activate: (app: JupyterLab, router: IRouter, resolver: IWindowResolver) => {
requires: [IRouter, IWindowResolver, ISplashScreen],
activate: (app: JupyterLab, router: IRouter, resolver: IWindowResolver, splash: ISplashScreen) => {
let debouncer: number;
let resolved = false;

Expand Down Expand Up @@ -447,9 +452,13 @@ const state: JupyterLabPlugin<IStateDB> = {
delete query['clone'];

const url = path + URLExt.objectToQueryString(query) + hash;
const silent = true;
const cloned = commands.execute(CommandIDs.saveState, { immediate })
.then(() => router.stop);

router.navigate(url, { silent });
// After the state has been cloned, navigate to the URL.
cloned.then(() => { router.navigate(url, { silent: true }); });

return cloned;
}

// After the state database has finished loading, save it.
Expand All @@ -458,12 +467,15 @@ const state: JupyterLabPlugin<IStateDB> = {
}
});
// Both the load state and clone state patterns should trigger the load
// state command if the URL matches one of them.
// state command if the URL matches one of them, but cloning a workspace
// outranks loading it because it is an explicit user action.
router.register({
command: CommandIDs.loadState, pattern: Patterns.loadState
command: CommandIDs.loadState, pattern: Patterns.cloneState,
rank: 20 // Set loading rank at a higher priority than the default 100.
});
router.register({
command: CommandIDs.loadState, pattern: Patterns.cloneState
command: CommandIDs.loadState, pattern: Patterns.loadState,
rank: 30 // Set loading rank at a higher priority than the default 100.
});

commands.addCommand(CommandIDs.reset, {
Expand All @@ -480,11 +492,14 @@ const state: JupyterLabPlugin<IStateDB> = {
const { hash, path, search } = args;
const query = URLExt.queryStringToObject(search || '');
const reset = 'reset' in query;
const clone = 'clone' in query;

if (!reset) {
return;
}

const loading = splash.show();

// If the state database has already been resolved, resetting is
// impossible without reloading.
if (resolved) {
Expand All @@ -498,12 +513,21 @@ const state: JupyterLabPlugin<IStateDB> = {
// Maintain the query string parameters but remove `reset`.
delete query['reset'];

const silent = true;
const hard = true;
const url = path + URLExt.objectToQueryString(query) + hash;
const cleared = commands.execute(CommandIDs.recoverState)
.then(() => router.stop); // Stop routing before new route navigation.

// After the state has been reset, navigate to the URL.
cleared.then(() => { router.navigate(url, { silent: true }); });
if (clone) {
cleared.then(() => { router.navigate(url, { silent, hard }); });
Copy link
Member

Choose a reason for hiding this comment

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

In this branch is the splash not disposed?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's right. It's not disposed because the hard navigation means a reload.

Copy link
Member Author

Choose a reason for hiding this comment

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

The visual effect I was going for was the splash page still existing as the page unloads. If you look closely, you'll see the splash start over after the refresh, but I think it's still less jarring than the splash disappearing and then reappearing.

} else {
cleared.then(() => {
router.navigate(url, { silent });
loading.dispose();
});
}

return cleared;
}
Expand Down
11 changes: 5 additions & 6 deletions packages/apputils/src/dialog.ts
Expand Up @@ -51,14 +51,13 @@ function showDialog<T>(options: Partial<Dialog.IOptions<T>>={}): Promise<Dialog.
*/
export
function showErrorMessage(title: string, error: any): Promise<void> {
console.error(error);
let options = {
console.warn('Showing error:', error);

return showDialog({
title: title,
body: error.message || title,
buttons: [Dialog.okButton()],
okText: 'DISMISS'
};
return showDialog(options).then(() => { /* no-op */ });
buttons: [Dialog.okButton({ label: 'DISMISS' })]
}).then(() => { /* no-op */ });
}

/**
Expand Down
27 changes: 25 additions & 2 deletions packages/coreutils/src/pageconfig.ts
Expand Up @@ -24,6 +24,17 @@ declare var require: any;
*/
export
namespace PageConfig {
/**
* The tree URL construction options.
*/
export
interface ITreeOptions {
/**
* If `true`, the tree URL will include the current workspace, if any.
*/
workspace?: boolean;
}

/**
* Get global configuration data for the Jupyter application.
*
Expand Down Expand Up @@ -121,10 +132,22 @@ namespace PageConfig {

/**
* Get the tree url for a JupyterLab application.
*
* @param options - The tree URL construction options.
*/
export
function getTreeUrl(): string {
return URLExt.join(getBaseUrl(), getOption('pageUrl'), 'tree');
function getTreeUrl(options: ITreeOptions = { }): string {
const base = getBaseUrl();
const page = getOption('pageUrl');
const workspaces = getOption('workspacesUrl');
const workspace = getOption('workspace');
const includeWorkspace = !!options.workspace;

if (includeWorkspace && workspace) {
return URLExt.join(base, workspaces, workspace, 'tree');
} else {
return URLExt.join(base, page, 'tree');
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/filebrowser-extension/src/index.ts
Expand Up @@ -400,7 +400,7 @@ function addCommands(app: JupyterLab, tracker: InstanceTracker<FileBrowser>, bro
commands.addCommand(CommandIDs.share, {
execute: () => {
const path = encodeURIComponent(browser.selectedItems().next().path);
const tree = PageConfig.getTreeUrl();
const tree = PageConfig.getTreeUrl({ workspace: true });

Clipboard.copyToSystem(URLExt.join(tree, path));
},
Expand Down