Workspaces #3490
Workspaces #3490
Conversation
… available to clients.
There was a problem hiding this comment.
I have tested this locally and the user experience is magical! I really like how it turned out and the 2s debounce time is very reasonable. It is really easy and intuitive to create workspaces, return to them, branch of existing ones, etc. Super excited about this!
| dev_mode/static | ||
| dev_mode/themes |
There was a problem hiding this comment.
Thanks for adding this :)
|
|
||
| commands.addCommand(CommandIDs.tree, { | ||
| execute: (args: IRouter.ICommandArgs) => { | ||
| const path = (args.path as string).replace('/tree', ''); | ||
| return app.restored.then(() => { | ||
| const path = decodeURIComponent((args.path.match(pattern)[1])); |
There was a problem hiding this comment.
Does this now handle filenames and paths with spaces?
There was a problem hiding this comment.
Encode/decode URI component fixes spaces.
| /** | ||
| * The command string that will be invoked upon matching. | ||
| */ | ||
| command: string; |
There was a problem hiding this comment.
Would it also make sense to provide command args for the router to use?
There was a problem hiding this comment.
Yes, it would. I just wanted to keep the surface area of the router exactly as small as we currently need and then expand as we expand. But if you think it should go in now, I'm happy to add it.
There was a problem hiding this comment.
fine to leave it as is for the beta
| } | ||
| }); | ||
|
|
||
| // Order the matching rules by rank and execute them. | ||
| matches.sort((a, b) => a.rank - b.rank).forEach(rule => { |
There was a problem hiding this comment.
Does this trigger only the first matching rule, or all of them? Server side routing definitely only triggers one, so it would be a bit surprising if multiple can get triggered. If the promises are ignored, that also makes it difficult to ensure ordering between them.
There was a problem hiding this comment.
This triggers all of them. This is a decision all routers need to make. One idea we've been considering is having the resolution of the promise contain a value that can stop the further execution of commands. This requires some thought.
There was a problem hiding this comment.
ok make sense that we still have to figure that out. For the beta I would probably feel better if we only ran the first one.
| .catch(reason => { | ||
| console.warn(`Saving workspace (${id}) failed.`, reason); | ||
| }); | ||
| }, 2000); |
There was a problem hiding this comment.
Let's use a module const for this and add documentation so our future selves can understand the meaning of 2000.
| resolved = true; | ||
|
|
||
| // Populate the workspace placeholder. | ||
| workspace = decodeURIComponent((args.path.match(pattern)[1])); |
There was a problem hiding this comment.
I have verified that workspaces can have spaces!
|
@afshin after using it a bit more I think we should change the default state of the application to have the left panel expanded. I think that in most layout restorations, the left panel will be open so starting with it open will make the state restoration less "everything moving around" |
|
The splash screen behavior is much improved! Awesome! |
|
This can be merged once the tests pass. Fantastic! |
|
Presumably we also need to merge jupyterlab/jupyterlab_server#34 and release a new jupyterlab_launcher too, right? |
lock
bot
added
the
status:resolved-locked
This PR adds the concept of named sessions that are saved on the server, these are called workspaces. This is a new feature and is not prominently exposed while we consider what the best user interface and experience should be.
To use this feature:
/lab/workspaces/<workspace_name><workspace_name>exists on the back-end, the workspace data will load the saved layout and open files in the JupyterLab interface. If it does not exist, it will be created.Depends on jupyterlab/jupyterlab_server#34