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

Enable full notebook windowing mode by default #15964

Merged
merged 17 commits into from
Apr 2, 2024

Conversation

krassowski
Copy link
Member

@krassowski krassowski commented Mar 11, 2024

References

Code changes

Enables full notebook windowing mode by default to gather wider user feedback during JupyterLab 4.2.0 pre-release testing. If we do not get far enough with tackling issues from #15258 before 4.2.0 we can revert this PR.

User-facing changes

JupyterLab works faster when long notebooks are open.

Backwards-incompatible changes

None

@krassowski krassowski added this to the 4.2.0 milestone Mar 11, 2024
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@jtpio
Copy link
Member

jtpio commented Mar 12, 2024

Linking to jupyter/notebook#7231 as something we will likely want to fix if this becomes the default.

Unless Notebook still defaults to defer to avoid issues originally spotted in jupyter/notebook#6855 (not sure these still apply though)

@krassowski
Copy link
Member Author

krassowski commented Mar 12, 2024

Thanks, I added jupyter/notebook#7231 to the list in #15258.

Looking at visual regression CI failures, something to consider is whether we want to pin some tests to use defer mode, or whether we want more tests to run in both defer and full mode. I think both solutions may be desirable, depending on specific test:

Notice:   32 failed
    [galata] › test/galata/notebook.spec.ts:22:7 › Notebook Tests › Create Markdown cell ───────────
    [galata] › test/galata/notebook.spec.ts:40:7 › Notebook Tests › Create Raw cell ────────────────
    [galata] › test/galata/notebook.spec.ts:59:7 › Notebook Tests › Create Code cell ───────────────
    [galata] › test/galata/notebook.spec.ts:82:7 › Notebook Tests › Run Cells ──────────────────────
    [galata] › test/galata/notebook.spec.ts:99:7 › Notebook Tests › Open and run cell by cell ──────
    [jupyterlab] › test/jupyterlab/collapsible-headings.test.ts:155:7 › Collapsible Headings; keyboard navigation › ReExpand Headers 01 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams default › Mermaid Diagram 4 er in default theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams default › Mermaid Diagram 5 journey in default theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams default › Mermaid Diagram 6 gantt in default theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams default › Mermaid Diagram 7 pie in default theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams default › Mermaid Diagram 8 quadrant in default theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams default › Mermaid Diagram 9 requirement in default theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams default › Mermaid Diagram 10 c4 in default theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams default › Mermaid Diagram 11 mindmap in default theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams default › Mermaid Diagram 12 timeline in default theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams default › Mermaid Diagram 13 sankey in default theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams default › Mermaid Diagram 14 xy in default theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams dark › Mermaid Diagram 4 er in dark theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams dark › Mermaid Diagram 5 journey in dark theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams dark › Mermaid Diagram 6 gantt in dark theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams dark › Mermaid Diagram 7 pie in dark theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams dark › Mermaid Diagram 8 quadrant in dark theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams dark › Mermaid Diagram 9 requirement in dark theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams dark › Mermaid Diagram 10 c4 in dark theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams dark › Mermaid Diagram 11 mindmap in dark theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams dark › Mermaid Diagram 12 timeline in dark theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams dark › Mermaid Diagram 13 sankey in dark theme 
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:76:11 › Notebook Mermaid Diagrams dark › Mermaid Diagram 14 xy in dark theme 
    [jupyterlab] › test/jupyterlab/notebook-run.test.ts:33:7 › Notebook Run 

@krassowski
Copy link
Member Author

krassowski commented Mar 28, 2024

[galata] › test/galata/notebook.spec.ts:22:7 › Notebook Tests › Create Markdown cell ───────────
Before After
image image

This one was taking snapshots of the entire main area; I think we should not do that. Especially because it is supposed to test if galata is working, not jupyterlab UI itself.

it does not make much sense, especially for cells; instead,
capture the notebook itself. Add a new test for saving,
because that was implicitly tested by `Run Cells` test
(and is not anymore). Of note, one of the tests,
`Open and run cell by cell` is still taking a snapshot
of the entire panel; this is intentional - we do want
one snapshot to be more encompassing (but not all of them!).
@krassowski
Copy link
Member Author

bot please update snapshots

as the toolbar is irrelevant for the snapshot of headings!
The rest of the UI does not really matter on these snapshots
and it only makes the content small and hard to read in the docs.
Copy link
Contributor

Documentation snapshots updated.

Copy link
Contributor

Galata snapshots updated.

@krassowski krassowski marked this pull request as ready for review March 28, 2024 17:52
@krassowski
Copy link
Member Author

@gabalafou I hope the trimming of snapshots in this PR will make you happy :)

@krassowski krassowski mentioned this pull request Mar 29, 2024
9 tasks
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @krassowski

@krassowski krassowski merged commit 3d9a2f1 into jupyterlab:main Apr 2, 2024
83 checks passed
gderocher pushed a commit to gderocher/jupyterlab that referenced this pull request Apr 26, 2024
* Enable `full` windowing mode by default

* Use `none` mode for mermaid snapshots tests

* Account for the windowed scrollbar button

* Do not capture the full main area in galata notebook tests

it does not make much sense, especially for cells; instead,
capture the notebook itself. Add a new test for saving,
because that was implicitly tested by `Run Cells` test
(and is not anymore). Of note, one of the tests,
`Open and run cell by cell` is still taking a snapshot
of the entire panel; this is intentional - we do want
one snapshot to be more encompassing (but not all of them!).

* Update the debugger button sizing

* Only capture notebook content in custom CSS test of headings

as the toolbar is irrelevant for the snapshot of headings!

* Only capture the main area for altair and html docs snapshots

The rest of the UI does not really matter on these snapshots
and it only makes the content small and hard to read in the docs.

* Update Playwright Snapshots

* Update Playwright Snapshots

* Fix toolbar test (account for the scrollbar button)

* Revert spurious snapshot updates

* Fix missing dot in selector

* Use `none` for collapsible headings test

* Add notebook-tab-saved snapshot

* Fix misplaced config

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Full windowed mode for notebooks enabled by default
3 participants