Default to the `full` windowing mode

[jtpio]

trafficstars

Fixes #7231 Fixes https://github.com/jupyter/notebook/issues/7318 Closes https://github.com/jupyter/notebook/pull/7301

notebook-windowing-mode-full.webm

[github-actions[bot]]

:point_left: Launch a Binder on branch jtpio/notebook/fix-full-css

[jtpio]

cc @afshin for review as the author of the upstream PR: https://github.com/jupyterlab/jupyterlab/pull/15533

Not sure why but there seems to be some logic setting a width and padding-right on the notebook widget, which conflict with the styles applied here in the Notebook CSS.

I'm not super happy with these !important to get around it, so if you have any suggestion (or if there is a way to fix this upstream) that would be appreciated, thanks!

[jtpio]

I'm not super happy with these !important to get around it, so if you have any suggestion (or if there is a way to fix this upstream) that would be appreciated, thanks!

Maybe we could live with it for now, as we are still defaulting to defer in Notebook for now (https://github.com/jupyter/notebook/issues/7318).

[krassowski]

How to test this on Binder? Each time I try to switch setting to "full" when I reload the notebook it switches back to defer. It looks like setting switching code will make it hard for users to toggle the mode, so I am now less convinced that this is a good idea (with the current implementation).

[jtpio]

It looks like setting switching code will make it hard for users to toggle the mode, so I am now less convinced that this is a good idea (with the current implementation).

Yes indeed. Ideally we would be able to temporarily default the windowing mode to defer as suggested in https://github.com/jupyter/notebook/issues/7318. But it looks like using settings transform to do this might not be possible at the moment, so it's not clear what the best path forward is.

Of note there is a similar issue with the defaults used in the file browser (checkboxes on, notebook sorted first), which affect JupyterLab.

[krassowski]

Another idea, can notebook server extension inject something into overrides.json?

[jtpio]

Probably. But it might be a bit odd to go down that way, because there is already a way for setting the windowing mode (via the settings). So that would still be a workaround.

Otherwise, maybe there should be a programmatic way to more dynamically set the windowing mode, similar to this request for more easily customizing the file browser: https://github.com/jupyterlab/jupyterlab/issues/14623.

Providing a custom notebook factory in Notebook might also work, but it sounds a bit overkill and would lead to duplicating upstream code from JupyterLab, just to be able to change one default.

[krassowski]

Not sure why but there seems to be some logic setting a width and padding-right on the notebook widget, which conflict with the styles applied here in the Notebook CSS.

I'm not super happy with these !important to get around it, so if you have any suggestion (or if there is a way to fix this upstream) that would be appreciated, thanks!

While I understand that the !important directive is not pretty I am afraid there is no easy fix for this on JupyterLab side. We spent a lot of time looking at a solution which would work across browsers with @afshin and it was hard to get it right and came to the conclusions that the width and padding have to be computed programmatically.

Now, there is always a way to create a new <style> tag and update it programatically, and just flick a class. This would add a bit more complexity on the lab side though. Is this a solution that would help you, or do you think that sticking to two !important directives is ok?

[jtpio]

bot please update playwright snapshots

[jtpio]

Also not sure where we got it from, but it looks like the Notebook now shows a scrollbar by default even when there is a single cell only. This seems to be the case in Notebook 7.1.x too, so not related to this PR in particular:

notebook-default-scrollbar.webm

[jtpio]

Also not sure where we got it from, but it looks like the Notebook now shows a scrollbar by default even when there is a single cell only. This seems to be the case in Notebook 7.1.x too, so not related to this PR in particular:

Should be fixed by https://github.com/jupyter/notebook/pull/7327

[jtpio]

Looks like there is a some growing padding at the bottom of the last cell when adding new cells:

full-last-cell-extra.webm

[jtpio]

Looks like there is a some growing padding at the bottom of the last cell when adding new cells:

This seems to be the same issue as originally reported in https://github.com/jupyter/notebook/pull/6855#issue-1688053001.

@afshin @krassowski or anyone who has worked on the notebook windowing mode, would there be a way to fix or work around that?

[jtpio]

This seems to be the same issue as originally reported in #6855 (comment).

It looks like this might be caused by the fixed height set on jp-WindowedPanel-inner:

Also cc @fcollonval who may know of a fix or workaround.

[jtpio]

OK gonna work around that for now in https://github.com/jupyter/notebook/pull/7335, by sticking to defer in Notebook until the issue above is resolved.

[jtpio]

It looks like this might be caused by the fixed height set on jp-WindowedPanel-inner:

Looking at the implementation in JupyterLab, this seems to be coming from this place:

https://github.com/jupyterlab/jupyterlab/blob/393d80f2553ad8d1debd62c14817378d5f22ff9c/packages/ui-components/src/components/windowedlist.ts#L1484-L1485

Which leads to getEstimatedTotalSize implemented here:

https://github.com/jupyterlab/jupyterlab/blob/393d80f2553ad8d1debd62c14817378d5f22ff9c/packages/ui-components/src/components/windowedlist.ts#L279-L297

Wondering if there could a miscalculation happening in this function?

[jtpio]

Stepping into the code, testing with a notebook of 3 cells and creating a 4th one, it looks like _lastMeasuredIndex is already set to 3:

But the measured field for the item at index 3 seems to be undefined:

[jtpio]

The undefined might be a false lead, as it seem to be set later.

However comparing with JupyterLab, the empty cells seem to have a smaller size there (39 instead of 54), which could explain why the total estimated height would then be higher than in JupyterLab:

It's not clear yet why the size of the empty cell is estimated to 54 in Notebook 7 and 39 in JupyterLab.

[jtpio]

It's not clear yet why the size of the empty cell is estimated to 54 in Notebook 7 and 39 in JupyterLab.

So it could be because Notebook 7 sets some custom padding to the first and last cells, here for the notebook to look nicer:

https://github.com/jupyter/notebook/blob/b18084867c8c11ad887d8b619545400c6cfb7999/packages/notebook-extension/style/base.css#L73-L83

Disabling it seems to indeed be fixing the issue (cells are estimated at 39px):

notebook-cell-padding-issue.webm

So not sure there is something to do in the full windowing logic since it's using the a ResizeObserver to get the values. Maybe we should look into removing these cell padding and tweak some other elements to give the nicer look instead.

[jtpio]

One consequence of this CSS change is the blue border not encompassing the whole cell anymore:

Before	After

[jtpio]

bot please update playwright snapshots

[jtpio]

bot please update playwright snapshots

[jtpio]

bot please update playwright snapshots

[jtpio]

bot please update playwright snapshots

[jtpio]

OK looks like we can now get this one in, and align with the JupyterLab default.