Attempt removing use of flexbox in the notebook DOM

[SylvainCorlay]

This will also make it possible to make use of break-inside: avoid CSS media properties for nbconvert webpdf exports.

Besides, it may also improce rendering performances of notebooks.

[jupyterlab-probot]

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

[fcollonval]

please run benchmark

[github-actions]

Benchmark report

The execution time (in milliseconds) are grouped by test file, test type and browser.
For each case, the following values are computed: min <- [1st quartile - median - 3rd quartile] -> max.

The mean relative comparison is computed with 95% confidence.

Results table

Test file	large_code_notebook	large_md_notebook
open
chromium
actual	5654 <- [7060 - 7247 - 7365] -> 8282	3422 <- [3535 - 3580 - 3622] -> 4046
expected	5821 <- [7118 - 7202 - 7275] -> 8187	3372 <- [3494 - 3541 - 3620] -> 4041
Mean relative change	0.4% ± 1.4%	0.3% ± 1.1%
switch-from
chromium
actual	682 <- [724 - 783 - 935] -> 1155	542 <- [568 - 596 - 787] -> 914
expected	674 <- [727 - 763 - 917] -> 1192	527 <- [572 - 611 - 774] -> 906
Mean relative change	1.5% ± 4.0%	-0.4% ± 4.8%
switch-to
chromium
actual	869 <- [907 - 927 - 950] -> 1105	686 <- [727 - 763 - 839] -> 923
expected	822 <- [886 - 910 - 936] -> 1082	674 <- [710 - 737 - 819] -> 977
Mean relative change	1.9% ± 1.3%	2.4% ± 2.3%
close
chromium
actual	781 <- [881 - 916 - 1053] -> 1387	618 <- [648 - 660 - 677] -> 773
expected	763 <- [881 - 928 - 1259] -> 1444	611 <- [648 - 666 - 685] -> 798
Mean relative change	-2.4% ± 5.0%	-1.2% ± 1.4%

Changes are computed with expected as reference.

[jtpio]

Thanks @SylvainCorlay for opening the PR.

Some of the failing UI tests look relevant (caret not vertically centered):

[fcollonval]

This impact the mobile styles. It would be good to add UI tests for that to check we are doing ok.

[jtpio]

This impact the mobile styles. It would be good to add UI tests for that to check we are doing ok.

Yes, it seems there is already an issue with 3.2.4 and the mobile styling: #11516

[SylvainCorlay]

The test failure is due to the fact that I use the "inline-block" display to mimick the horizontal flex box instead of an horizontal table, and then a zero font size.

I think this is not the right approach. Using inline-block is really a hack and we should use a table.

[SylvainCorlay]

I made the following diagrams of the DOM structure (happy to contribute them to the docs).

Currently, horizontal flexboxes are use for

• inputWrapper
• outputWrapper
• inputArea
• outputArea-child

However, in the nbconvert case, there is not collapser, so the only flexboxes are inputArea and outputArea-child, so we could start there.

Here with the flexboxes in yellow:

[SylvainCorlay]

please run benchmark

[SylvainCorlay]

please run benchmark

[SylvainCorlay]

please run benchmark

[SylvainCorlay]

Visual regression tests are passing.

[fcollonval]

Thanks @SylvainCorlay

[jtpio]

I made the following diagrams of the DOM structure (happy to contribute them to the docs).

It could be interesting to have that diagram in this section of the docs for example: https://jupyterlab.readthedocs.io/en/stable/extension/notebook.html

Although it might become outdated at some point. In that case we could mention "as of JupyterLab 3.2.x" or "4.0.0" depending on where this lands.

[gabalafou]

Hi, hope you don't mind me dropping in on this thread out of the blue.

I leave the following remarks only in a spirit of spreading knowledge, raising awareness, and asking questions; I am not requesting any changes or making recommendations.

I heard about this PR in today's JupyterLab team meeting, and it raised a red flag in my mind, because as a general rule of thumb, the CSS display property affects HTML semantics, which in turn is important for accessibility. However, I did a little digging and luckily, in the specific case of display: table, I don't think the general rule of thumb applies (in most cases... there are always caveats in this line of work). More on the topic at Adrian Roselli's blog for anyone who wants the nitty-gritty on tables, CSS display properties, and ARIA.

tl;dr-- display:table is probably not an accessibility issue in this PR but please be careful when applying display properties in a non-standard way.

That said, I can't help but wonder if display:table is an anti-pattern, though perhaps a desirable and/or necessary one for the break-inside: avoid declaration. I also wonder if an @media rule would be an appropriate solution here, though I'm not sure I would prefer it, given that I think it is extremely desirable to reduce markup and style variants as much as possible, but I thought I should at least mention it.

:)

[SylvainCorlay]

Hi, hope you don't mind me dropping in on this thread out of the blue.

Quite the contrary. Many thanks for your feedback.

That said, I can't help but wonder if display:table is an anti-pattern, though perhaps a desirable and/or necessary one for the break-inside: avoid declaration. I also wonder if an @media rule would be an appropriate solution here, though I'm not sure I would prefer it, given that I think it is extremely desirable to reduce markup and style variants as much as possible, but I thought I should at least mention it.

I have the exact same thought. There is a companion PR in nbconvert jupyter/nbconvert#1679 that overrides the CSS but I would rather have as little as possible in the @media rule. The reason is that I fear it may not be as thoroughly tested as the general case and will be more likely to break. More generally, maintaining two versions will be more difficult.

[krassowski]

This PR has introduced two significant regressions:

• "Create new view for output" in jupyter lab 4.0.0 not working as before #14564
• Horizontal scrolling unavailable in Firefox #14625

[SylvainCorlay]

I am working on a fix for #14564.
I opened PR #14911 to address #14564.