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

Restore horizontal scrolling of outputs for Firefox #15171

Merged
merged 14 commits into from
Oct 11, 2023
Merged
2 changes: 2 additions & 0 deletions galata/test/jupyterlab/notebook-mobile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@

test('Execute code cell', async ({ page }) => {
await page.sidebar.close('left');
// TODO: calling `setCell` just once leads to very flaky test
fcollonval marked this conversation as resolved.
Show resolved Hide resolved
await page.notebook.setCell(0, 'code', 'print("hello")');
await page.notebook.setCell(0, 'code', 'print("hello")');
await page.notebook.addCell('code', '2 * 3');
await page.notebook.runCellByCell();
const nbPanel = await page.notebook.getNotebookInPanel();
const imageName = 'mobile-layout.png';
expect(await nbPanel.screenshot()).toMatchSnapshot(imageName);

Check failure on line 24 in galata/test/jupyterlab/notebook-mobile.test.ts

View workflow job for this annotation

GitHub Actions / Visual Regression Tests

[jupyterlab] › test/jupyterlab/notebook-mobile.test.ts:15:7 › Notebook Layout on Mobile › Execute code cell

1) [jupyterlab] › test/jupyterlab/notebook-mobile.test.ts:15:7 › Notebook Layout on Mobile › Execute code cell Error: Screenshot comparison failed: 193 pixels (ratio 0.01 of all image pixels) are different. Expected: /home/runner/work/jupyterlab/jupyterlab/galata/test-results/test-jupyterlab-notebook-mobile-Notebook-Layout-on-Mobile-Execute-code-cell-jupyterlab/mobile-layout-expected.png Received: /home/runner/work/jupyterlab/jupyterlab/galata/test-results/test-jupyterlab-notebook-mobile-Notebook-Layout-on-Mobile-Execute-code-cell-jupyterlab/mobile-layout-actual.png Diff: /home/runner/work/jupyterlab/jupyterlab/galata/test-results/test-jupyterlab-notebook-mobile-Notebook-Layout-on-Mobile-Execute-code-cell-jupyterlab/mobile-layout-diff.png 22 | const nbPanel = await page.notebook.getNotebookInPanel(); 23 | const imageName = 'mobile-layout.png'; > 24 | expect(await nbPanel.screenshot()).toMatchSnapshot(imageName); | ^ 25 | }); 26 | }); 27 | at /home/runner/work/jupyterlab/jupyterlab/galata/test/jupyterlab/notebook-mobile.test.ts:24:40

Check failure on line 24 in galata/test/jupyterlab/notebook-mobile.test.ts

View workflow job for this annotation

GitHub Actions / Visual Regression Tests

[jupyterlab] › test/jupyterlab/notebook-mobile.test.ts:15:7 › Notebook Layout on Mobile › Execute code cell

1) [jupyterlab] › test/jupyterlab/notebook-mobile.test.ts:15:7 › Notebook Layout on Mobile › Execute code cell Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Error: Screenshot comparison failed: 193 pixels (ratio 0.01 of all image pixels) are different. Expected: /home/runner/work/jupyterlab/jupyterlab/galata/test-results/test-jupyterlab-notebook-mobile-Notebook-Layout-on-Mobile-Execute-code-cell-jupyterlab-retry1/mobile-layout-expected.png Received: /home/runner/work/jupyterlab/jupyterlab/galata/test-results/test-jupyterlab-notebook-mobile-Notebook-Layout-on-Mobile-Execute-code-cell-jupyterlab-retry1/mobile-layout-actual.png Diff: /home/runner/work/jupyterlab/jupyterlab/galata/test-results/test-jupyterlab-notebook-mobile-Notebook-Layout-on-Mobile-Execute-code-cell-jupyterlab-retry1/mobile-layout-diff.png 22 | const nbPanel = await page.notebook.getNotebookInPanel(); 23 | const imageName = 'mobile-layout.png'; > 24 | expect(await nbPanel.screenshot()).toMatchSnapshot(imageName); | ^ 25 | }); 26 | }); 27 | at /home/runner/work/jupyterlab/jupyterlab/galata/test/jupyterlab/notebook-mobile.test.ts:24:40
});
});
Copy link
Member

Choose a reason for hiding this comment

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

@jtpio is this good enough or should we shrink top/bottom margins back to closer match the previous snapshot?

A part of me things that maybe the larger spacing is good for mobile (and possibly even required by accessibility) because the tap targets are easier to separate. I do not have a strong opinion though.

Copy link
Member

Choose a reason for hiding this comment

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

Looks good on the new screenshot. Actually a bit more spacing makes it a bit more pleasant to read it seems.

Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure there was a specific reason for having the margins very small on mobile in the first place, maybe it was just a side-effect of the table layout.

krassowski marked this conversation as resolved.
Show resolved Hide resolved
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
58 changes: 58 additions & 0 deletions galata/test/jupyterlab/print.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright (c) Jupyter Development Team.
// Distributed under the terms of the Modified BSD License.

import { expect, test } from '@jupyterlab/galata';
import * as path from 'path';

test.use({ autoGoto: false });

const fileName = 'simple_notebook.ipynb';

test.describe('Print layout', () => {
test('Notebook', async ({ page, tmpPath }) => {
await page.emulateMedia({ media: 'print' });
await page.contents.uploadFile(
path.resolve(__dirname, `./notebooks/${fileName}`),
`${tmpPath}/${fileName}`
);
await page.contents.uploadFile(
path.resolve(__dirname, './notebooks/WidgetArch.png'),
`${tmpPath}/WidgetArch.png`
);

await page.goto();

await page.notebook.openByPath(`${tmpPath}/${fileName}`);

await page.getByText('Python 3 (ipykernel) | Idle').waitFor();

await page.notebook.run();

let printedNotebookURL = '';
await Promise.all([
page.waitForRequest(
async request => {
const url = request.url();
if (url.match(/\/nbconvert\//) !== null) {
printedNotebookURL = url;
return true;
}
return false;
},
{ timeout: 1000 }
),
page.keyboard.press('Control+P')
]);

const newPage = await page.context().newPage();

await newPage.goto(printedNotebookURL, { waitUntil: 'networkidle' });

// Wait until MathJax loading message disappears
const mathJaxMessage = newPage.locator('#MathJax_Message');
await expect(mathJaxMessage).toHaveCount(1);
await mathJaxMessage.waitFor({ state: 'hidden' });

expect(await newPage.screenshot()).toMatchSnapshot('printed-notebook.png');
});
});
krassowski marked this conversation as resolved.
Show resolved Hide resolved
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
39 changes: 30 additions & 9 deletions packages/cells/style/inputarea.css
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,15 @@

/* All input areas */
.jp-InputArea {
display: table;
table-layout: fixed;
display: flex;
flex-direction: row;
width: 100%;
overflow: hidden;
}

.jp-InputArea-editor {
display: table-cell;
flex: 1 1 auto;
overflow: hidden;
vertical-align: top;

/* This is the non-active, default styling */
border: var(--jp-border-width) solid var(--jp-cell-editor-border-color);
Expand All @@ -27,8 +26,7 @@
}

.jp-InputPrompt {
display: table-cell;
vertical-align: top;
flex: 0 0 var(--jp-cell-prompt-width);
width: var(--jp-cell-prompt-width);
color: var(--jp-cell-inprompt-font-color);
font-family: var(--jp-cell-prompt-font-family);
Expand All @@ -52,17 +50,40 @@
user-select: none;
}

/*-----------------------------------------------------------------------------
| Print
|----------------------------------------------------------------------------*/
@media print {
.jp-InputArea {
display: table;
table-layout: fixed;
}

.jp-InputArea-editor {
display: table-cell;
vertical-align: top;
}

.jp-InputPrompt {
display: table-cell;
vertical-align: top;
}
}

/*-----------------------------------------------------------------------------
| Mobile
|----------------------------------------------------------------------------*/
@media only screen and (width <= 760px) {
.jp-InputArea {
flex-direction: column;
}

.jp-InputArea-editor {
display: table-row;
margin-left: var(--jp-notebook-padding);
margin-left: var(--jp-code-padding);
}

.jp-InputPrompt {
display: table-row;
flex: 0 0 auto;
text-align: left;
}
}
11 changes: 10 additions & 1 deletion packages/cells/style/widget.css
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@
width: calc(
var(--jp-cell-prompt-width) - var(--jp-private-cell-scrolling-output-offset)
);
flex: 0 0
calc(
var(--jp-cell-prompt-width) -
var(--jp-private-cell-scrolling-output-offset)
);
}

.jp-CodeCell.jp-mod-outputsScrolled .jp-OutputArea-promptOverlay {
Expand All @@ -117,7 +122,7 @@
|----------------------------------------------------------------------------*/

.jp-MarkdownOutput {
display: table-cell;
flex: 1 1 auto;
width: 100%;
margin-top: 0;
margin-bottom: 0;
Expand Down Expand Up @@ -235,4 +240,8 @@ cell outputs.
.jp-Cell-outputWrapper {
display: block;
}

.jp-MarkdownOutput {
display: table-cell;
}
}
10 changes: 10 additions & 0 deletions packages/notebook/style/base.css
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@

.jp-Notebook .jp-Cell .jp-InputPrompt {
cursor: move;
float: left;
}

/*-----------------------------------------------------------------------------
Expand Down Expand Up @@ -458,3 +459,12 @@ div.jp-Cell-Placeholder-content-2 {
div.jp-Cell-Placeholder-content-3 {
top: 140px;
}

/*-----------------------------------------------------------------------------
| Printing
|----------------------------------------------------------------------------*/
@media print {
.jp-Notebook .jp-Cell .jp-InputPrompt {
float: none;
}
}
39 changes: 27 additions & 12 deletions packages/outputarea/style/base.css
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@
}

.jp-OutputArea-child {
display: table;
table-layout: fixed;
display: flex;
flex-direction: row;
width: 100%;
overflow: hidden;
}

.jp-OutputPrompt {
width: var(--jp-cell-prompt-width);
flex: 0 0 var(--jp-cell-prompt-width);
color: var(--jp-cell-outprompt-font-color);
font-family: var(--jp-cell-prompt-font-family);
padding: var(--jp-code-padding);
Expand All @@ -43,13 +44,7 @@
user-select: none;
}

.jp-OutputArea-prompt {
display: table-cell;
vertical-align: top;
}

.jp-OutputArea-output {
display: table-cell;
width: 100%;
height: auto;
overflow: auto;
Expand Down Expand Up @@ -91,6 +86,11 @@
box-shadow: inset 0 0 1px var(--jp-inverse-layout-color0);
}

.jp-OutputArea-child .jp-OutputArea-output {
flex-grow: 1;
flex-shrink: 1;
}

/**
* Isolated output.
*/
Expand Down Expand Up @@ -198,6 +198,7 @@ body.lm-mod-override-cursor .jp-OutputArea-output.jp-mod-isolated::before {
.jp-OutputArea-output.jp-OutputArea-executeResult {
margin-left: 0;
width: 100%;
flex: 1 1 auto;
}

/* Text output with the Out[] prompt needs a top padding to match the
Expand Down Expand Up @@ -267,22 +268,36 @@ body.lm-mod-override-cursor .jp-OutputArea-output.jp-mod-isolated::before {

@media print {
.jp-OutputArea-child {
display: table;
table-layout: fixed;
break-inside: avoid-page;
}

.jp-OutputArea-prompt {
display: table-cell;
vertical-align: top;
}

.jp-OutputArea-output {
display: table-cell;
}
}

/*-----------------------------------------------------------------------------
| Mobile
|----------------------------------------------------------------------------*/
@media only screen and (width <= 760px) {
.jp-OutputArea-child {
flex-direction: column;
}

.jp-OutputPrompt {
display: table-row;
flex: 0 0 auto;
text-align: left;
}

.jp-OutputArea-child .jp-OutputArea-output {
display: table-row;
margin-left: var(--jp-notebook-padding);
.jp-OutputArea-promptOverlay {
display: none;
}
}

Expand Down