Skip to content
This repository has been archived by the owner on Feb 16, 2023. It is now read-only.

WIP Adds left and right widgets area #275

Closed

Conversation

JasonWeill
Copy link
Contributor

@JasonWeill JasonWeill commented Nov 10, 2021

Related issue: #272

This change adds left and right widgets areas to RetroLab and enables the toc extension which should be available in the left area.

It is WIP for a few reasons:

  1. There is a show/hide menu item for the left and right areas, but it does not yet include a cascading submenu for each widget in these areas.
  2. The primary tree view in RetroLab is not presently centered as it was before this change. (fixed)
  3. The toc-extension is not on the same 3.2.x branch as the other extensions are (blocked on Backport PR #11420 on branch 3.2.x (Makes ILabShell optional in toc extension) jupyterlab#11421) (fixed)

@github-actions
Copy link
Contributor

Binder 👈 Launch RetroLab on Binder

@jtpio
Copy link
Member

jtpio commented Nov 10, 2021

Thanks @jweill-aws for starting this!

app/webpack.config.js Outdated Show resolved Hide resolved
@jtpio
Copy link
Member

jtpio commented Nov 11, 2021

JupyterLab 3.2.3 was released recently: https://github.com/jupyterlab/jupyterlab/releases/tag/v3.2.3

So we can update this PR to the new packages with:

jlpm run update:dependency --regex @jupyterlab/ latest

@jtpio jtpio added the enhancement New feature or request label Nov 11, 2021
@JasonWeill
Copy link
Contributor Author

I should be able to modify the toc-extension to accept an IShell object, but despite application/src/frontend.ts exporting the IShell interface, the compiled frontend.d.ts file doesn't declare this interface as exported.

The only additional method I need is currentChanged, which is not part of the IShell interface, but which is implemented (with different signatures) in both RetroShell and LabShell:

RetroShell:

  get currentChanged(): ISignal<RetroShell, void>;

LabShell:

  export type IChangedArgs = FocusTracker.IChangedArgs<Widget>;
  ...
  get currentChanged(): ISignal<this, ILabShell.IChangedArgs>;

It seems like if we can declare currentChanged in a consistent way, then move this definition up to IShell, and ensure that this interface is exported properly, we can use it in the toc extension.

@jtpio
Copy link
Member

jtpio commented Dec 3, 2021

For sure extensions should just work the same in both lab and retro. In fact most of them already do.

I think the point here was to be able to make some progress on that particular TOC extension, and lay the foundations for the left and right areas so other extensions can also be used. So we can merge this PR and iterate.

TOC is still a bit special for now because of the way it currently works. See jupyterlab/jupyterlab#11530 for more info. Making it compatible with RetroLab probably requires some breaking changes, and since it is part of the jupyterlab repo this adds significant delays before being able to consume the changes it here in RetroLab.

Here is an example of jupyterlab-github as another third-party extension added to the left area in RetroLab, using a local build of this PR locally and a minor change upstream to make the extension compatible (jupyterlab/jupyterlab-github#128):

jupyterlab-github.mp4

@JasonWeill
Copy link
Contributor Author

I'm still seeing the command palette appear in the context menu as an extension, but not the TOC extension. When I choose this option, though, the left pane appears with the TOC extension in it.

image

@jtpio
Copy link
Member

jtpio commented Dec 6, 2021

I'm still seeing the command palette appear in the context menu as an extension

Probably this is because the palette is added to the left area here:

https://github.com/jupyterlab/jupyterlab/blob/5829e22634c8ee7704e805b7bbb5cb15ca7ca8b6/packages/apputils-extension/src/palette.ts#L103

Which seems to be the case even though by default the palette is displayed as a modal now (before the default was to add it to the left area).

@JasonWeill
Copy link
Contributor Author

@jtpio I noticed that and was confused. Is it safe to remove the palette from the left area? Does it ever display there in JupyterLab?

@bollwyvl
Copy link
Contributor

bollwyvl commented Dec 6, 2021 via email

@JasonWeill
Copy link
Contributor Author

Thanks @bollwyvl — didn't realize that there was a config option to make the palette visible. This raises two questions:

  1. Is this option enabled by default in RetroLab? If not, why is the menu item appearing?
  2. Why does the contextual menu only show one option for the left sidebar? Why isn't the TOC extension also appearing?

@jtpio
Copy link
Member

jtpio commented Dec 7, 2021

Is this option enabled by default in RetroLab? If not, why is the menu item appearing?

The palette widget is still added to the left area in RetroLab, but until now it was just not displayed. I think it now shows up with the new changes in this PR:

https://github.com/jupyterlab/retrolab/pull/275/files#diff-f36bed968c6b03b7651feb852096ecf68500f97b2f1e42ed1a6350fd5e592bb0R675-R685

Why does the contextual menu only show one option for the left sidebar? Why isn't the TOC extension also appearing?

Normally I would expect the TOC to now be picked up based on the recent upstream changes. One way to know would be to set a breakpoint in the plugin that calls retroShell.widgetsList('left') to create the menu entries. It could also be that the menu is being created before the toc extension is added to the left area.

@JasonWeill
Copy link
Contributor Author

@jtpio I noticed when I was looking into the menu item issue that the menu gets generated once, after the command palette plugin has been activated, but it doesn't get regenerated with two items after the TOC plugin has been activated. I'm concerned that the lack of a notebook tracker or a widget tracker in RetroLab's left pane may be related to this: there is no widgetAdded event that the menu plugin can listen to. Should we be using a widget tracker with the left sidebar? Is there another pattern that would result in the menu getting updated?

@jtpio
Copy link
Member

jtpio commented Dec 9, 2021

One way to handle this for now could be to wait for app.restored before creating the menu entries.

I'm concerned that the lack of a notebook tracker or a widget tracker in RetroLab's left pane

RetroLab supports notebook trackers and other widget trackers. For example the notebook tracker plugin is added here:

'@jupyterlab/notebook-extension:tracker',

Although it's true there is no way to know a widget has been added to the left / right areas. But isn't that the case for the LabShell too?

@JasonWeill
Copy link
Contributor Author

One way to handle this for now could be to wait for app.restored before creating the menu entries.

@jtpio Thanks! I've updated this PR in this way. Still not seeing the toc extension load, but at least it appears as its own item in the menu now. Progress!

@JasonWeill
Copy link
Contributor Author

At the February 2 JupyterLab meeting, someone mentioned using a "live inspector" extension in lieu of the toc-extension to try out the left and right areas. Which should I use? In JupyterLab, I found property-inspector which requires ILabShell (not compatible with the RetroShell used by RetroLab) and inspector which doesn't seem to be added to any shell area.

@jtpio
Copy link
Member

jtpio commented Feb 3, 2022

I found property-inspector which requires ILabShell (not compatible with the RetroShell used by RetroLab)

Right for reference IPropertyInspectorProvider is provided here and the plugin does depends on ILabShell:

https://github.com/jupyterlab/jupyterlab/blob/2c2d83fc4b39f292ed12dc9d90c32a2cdb606943/packages/application-extension/src/index.tsx#L994-L1032

Maybe it can be made optional so ILabShell is only used for listening to the currentChanged signal.

@bollwyvl
Copy link
Contributor

bollwyvl commented Feb 3, 2022

So many inspectors!

I was actually referring to the thing called @jupyterlab/inspector:IInspector that now appears as a main area widget under Contextual Help.

This is an implementation of a top-level jupyter message and is a great interactive computing "passive" learning tool, as it doesn't require any keystrokes, etc. to see a stream of new information

This was originally a (right) sidebar, and was moved to the main area to be more flexible.

In nb6, it appears in the "pager" area at the bottom of the page... i personally never liked that position, and would not be sad at all if it appeared next to the code-of-interest, and consumed some of the whitespace.

It also looks to only require ITranslator which is good for business.

@jtpio
Copy link
Member

jtpio commented Mar 8, 2022

@jweill-aws FYI the RetroLab code base has now been integrated in the Notebook repo: https://github.com/jupyter/notebook/

Would you like to resume continue this PR over there?

Thanks!

@JasonWeill
Copy link
Contributor Author

Closing in favor of jupyter/notebook#6327.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants