Left right panel in Notebook view

[brichet]

trafficstars

This PR follows https://github.com/jupyter/notebook/pull/6327 to add left and right panel to Notebook.

The left and right panels can be opened from the view menu.

Closes https://github.com/jupyter/notebook/pull/6327

[github-actions[bot]]

:point_left: Launch a Binder on branch brichet/notebook/left-right-widgets

[jtpio]

Nice thanks @brichet for picking this up :+1:

[brichet]

Is there a way to retrieve the yarn.lock file generated during the check_release action ? It seems that it changes, but I can't reproduce it locally.

[jtpio]

Is there a way to retrieve the yarn.lock file generated during the check_release action ? It seems that it changes, but I can't reproduce it locally.

hmm strange, if it changes during the check release then it should also change locally after running jlpm.

[jtpio]

Just tried running jlpm in a fresh GitHub Codespace and it does modify the yarn.lock:

So these changes to the yarn.lock should be committed and pushed as well.

[brichet]

So these changes to the yarn.lock should be committed and pushed as well.

Right, sorry for that, indeed after cleaning everything it changes yarn.lock.

[brichet]

Maybe we could increase the test time. This PR adds some little ui-test, but I think 10 minutest is too short.

[jtpio]

Maybe we could increase the test time. This PR adds some little ui-test, but I think 10 minutest is too short.

Hmm normally 10 minutes should be more than enough? Or do the new tests require that much more time?

Here is a screenshot of the cancelled check: https://github.com/jupyter/notebook/runs/7686750791?check_suite_focus=true

But on main they take 5 minutes to complete so still plenty of room: https://github.com/jupyter/notebook/runs/7686282650?check_suite_focus=true

[brichet]

Hmm normally 10 minutes should be more than enough? Or do the new tests require that much more time?

Not that much, they should be really quick, only opening left (toc) and right (notebooktools) panels.

[jtpio]

Maybe there is something that makes them time out on CI (normally locally as well) with the new changes?

[brichet]

The timeout is quite long, 4 minutes : https://github.com/jupyter/notebook/blob/5f1980e8456bfa1dda87e23789c0d626f85646e2/ui-tests/playwright.config.ts#L5 When a test fail 2 times (it is retried 1 time) while awaiting a specific screenshot or element, it almost timeout the whole tests. Is there a reason to have a 4 minutes timeout ? Galata is set to 1 minute by default.

That will not fix the tests but may allow us to get the errors instead of cancelled test.

[jtpio]

Is there a reason to have a 4 minutes timeout ?

This was added in https://github.com/jupyterlab/retrolab/pull/273, but I don't see any reason for that particular value. So we can change it and see.

[jtpio]

Looks like the -darwin snapshot should be removed?

[brichet]

Looks like the -darwin snapshot should be removed?

Good catch

[brichet]

The errors on tests seems not related (save status, kernel status...), maybe we can try running them again.

[jtpio]

The errors on tests seems not related (save status, kernel status...), maybe we can try running them again.

Yes these are a bit flaky sometimes.

[jtpio]

Just tried on Binder and it looks good.

https://user-images.githubusercontent.com/591645/183838825-7babc575-e43b-445e-b1cb-ab132aa07553.mp4

At first it might feel a bit odd that the notebook is shifted to the side when the left or right area is opened. @hbcarlos had some good feedback about this on the original PR: https://github.com/jupyterlab/retrolab/pull/275#issuecomment-974317305

Posting it here for reference:

I think by default they should be hidden and only show up when the user triggers an action that requires something on the side panel. Also, regarding the design, we could divide the shell into three parts to keep the notebook always in the middle and then create the box panel for the side panels, the shell would be divided like in the following image:

Probably we could try to change how the side panels are positioned in the NotebookShell. Maybe this could be tried out in a follow-up PR.

For reference Google Docs does shift the main page a bit when opening an add-on on the right side:

https://user-images.githubusercontent.com/591645/183842255-c965372f-f2cd-4a6b-b062-8a845e0b284f.mp4

Although their top area takes the full width, which reduces this feel of being "off" since the page is still centered with the top area.

[brichet]

Nice, thanks for sharing, it would be better that way. And as in Google Docs, we could use that space to display a button which close the panel.

[brichet]

The debugger panel is also missing, but I think it can be added in a follow up PR as well. I'm not sure it will be as easy to include than the other panels, as it depend on current opened widget (notebook, console...).

[jtpio]

The debugger panel is also missing, but I think it can be added in a follow up PR as well. I'm not sure it will be as easy to include than the other panels

Agree this can be added in a different PR :+1: Hopefully the extension should already handle the active widget properly, otherwise we can submit fixes upstream in JupyterLab.

[jtpio]

FYI @brichet I'm planning to do a pass on the changes, especially the ones on the shell.

Overall it looks good. I would like to experiment with the alternative side-panel approach mentioned above. Will report here when I have more updates.

[brichet]

Thanks @jtpio. Let me know if you want me to give this alternative side panel a try.

[jtpio]

Here is what the alternative approach looks like with some local changes (to be committed and pushed):

https://user-images.githubusercontent.com/591645/185116801-0573fad4-125d-40b1-b7eb-2d7400223025.mp4

This feels a bit more similar to what Google Docs does, keeping the notebook centered with the top area.

If that looks good to you @brichet I'll push the commit to the branch directly. There are a couple of things we might want to discuss in the implementation of the shell, I'll leave some comments right after.

[brichet]

Here is what the alternative approach looks like with some local changes (to be committed and pushed)

Look perfect, thanks @jtpio. Maybe we could add a X on panel to easily close it, but it can be done in a follow-up PR. Fell free to push the commit to the branch. ~~Do I have to give you permissions on my fork ?~~

[jtpio]

Maybe we could add a X on panel to easily close it, but it can be done in a follow-up PR.

Agree this would be quite useful :+1:

[jtpio]

Looking into adding the debugger extension as well by default. This seems to be handled nicely after adding the dependencies to app/package.json and the plugins to app/index.js:

However the menu entry seems to be missing the title which is likely to be an issue in the debugger extension in JupyterLab.

EDIT: opened https://github.com/jupyterlab/jupyterlab/pull/12987

[brichet]

The tests are failing because there is no error thrown when adding a widget in side panel on tree view : https://github.com/jupyter/notebook/runs/8039561039?check_suite_focus=true#step:5:140

Now the side panels are available in any page, these tests should be changed, right ?

[jtpio]

Now the side panels are available in any page, these tests should be changed, right ?

yes that sounds right.

[jtpio]

Not sure what's happening with the UI tests currently failing on this PR.

This seems to be reproducible locally as well, stuck on "Shut Down All":

This could be related to the new DOM structure. Looks like some extra #main-panel >> selectors have been added in this PR which seems related.

[brichet]

Not sure what's happening with the UI tests currently failing on this PR.

It seems that the selector does not select the correct button anymore. Two more buttons with same text are added in left panel (but not displayed) from the running-extension package in jupyter : https://github.com/jupyterlab/jupyterlab/blob/3b83ceee09ce39ed3bf7de4e250135e5749cf3e4/packages/running-extension/src/index.ts#L78

This raises 2 questions :

• Do we need the running-extension package ? It seems that we need the running package to build the main panel widget, but not sure about the extension.

• Why isn't the left panel added to the View menu when the extension widget is added to the left panel ? EDIT : because the side panels are still narrowed to notebook view https://github.com/jupyter/notebook/blob/a26486ae3be0d2c8eb94b372164db99214168f3f/packages/application-extension/src/index.ts#L120-L122