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

Backport PR #15479 on branch 4.0.x (Workaround/input box focus) #15563

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
55 changes: 50 additions & 5 deletions galata/test/jupyterlab/outputarea-stdin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ test.use({
locale: 'en-US'
});

const loopedInput = `\
from time import sleep
input()
print('before sleep')
sleep(0.1)
print('after sleep')`;

const ACTIVE_INPUT =
'.jp-OutputArea-stdin-item:not(.jp-OutputArea-stdin-hiding) .jp-Stdin-input';

test.describe('Stdin for ipdb', () => {
test.beforeEach(async ({ page }) => {
await page.notebook.createNew();
Expand All @@ -24,20 +34,20 @@ test.describe('Stdin for ipdb', () => {
await page.keyboard.press('Control+Enter');

// enter a bunch of nonsense commands into the stdin attached to ipdb
await page.waitForSelector('.jp-Stdin-input');
await page.waitForSelector(ACTIVE_INPUT);
await page.keyboard.insertText('foofoo');
await page.keyboard.press('Enter');

await page.waitForSelector('.jp-Stdin-input');
await page.waitForSelector(ACTIVE_INPUT);
await page.keyboard.insertText('barbar');
await page.keyboard.press('Enter');

await page.waitForSelector('.jp-Stdin-input');
await page.waitForSelector(ACTIVE_INPUT);
await page.keyboard.insertText('bazbaz');
await page.keyboard.press('Enter');

// search for the first nonsense command
await page.waitForSelector('.jp-Stdin-input');
await page.waitForSelector(ACTIVE_INPUT);
await page.keyboard.insertText('foo');
await page.keyboard.press('Control+ArrowUp');

Expand All @@ -54,7 +64,7 @@ test.describe('Stdin for ipdb', () => {

// Check that the input remains focused and cursor is at the end.
await page.keyboard.insertText('x');
await expect(page.locator('.jp-Stdin-input')).toHaveValue('foofoox');
await expect(page.locator(ACTIVE_INPUT)).toHaveValue('foofoox');
});

test('Typing in stdin box', async ({ page }) => {
Expand Down Expand Up @@ -82,4 +92,39 @@ test.describe('Stdin for ipdb', () => {
alphabet + alphabet.toUpperCase() + digits
);
});

test('Subsequent execution in short succession', async ({ page }) => {
await page.notebook.setCell(0, 'code', loopedInput);
// Run the selected (only) cell without proceeding and without waiting
// for it to complete (as it should stay waiting for input).
await page.keyboard.press('Control+Enter');

// Wait for first input
await page.waitForSelector('.jp-Stdin-input');

// Note: this test does not wait for subsequent inputs on purpose

await page.getByText('before sleep').waitFor();

// Press enter five times (should do nothing)
for (let j = 0; j < 5; j++) {
await page.keyboard.press('Enter');
}
// Press a key which should go to the input
await page.keyboard.press('x');

await page.getByText('after sleep').waitFor();

// Press enter five times (should submit and then do nothing)
for (let j = 0; j < 5; j++) {
await page.keyboard.press('Enter');
}

const cellInput = await page.notebook.getCellInput(0);
const editor = await cellInput.$('.cm-content');
const contentAfter = await editor.evaluate((e: any) =>
e.cmView.view.state.doc.toString()
);
expect(contentAfter).toBe(loopedInput);
});
});
38 changes: 36 additions & 2 deletions packages/outputarea/src/widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ const OUTPUT_AREA_OUTPUT_CLASS = 'jp-OutputArea-output';
*/
const OUTPUT_AREA_PROMPT_CLASS = 'jp-OutputArea-prompt';

const OUTPUT_AREA_STDIN_HIDING_CLASS = 'jp-OutputArea-stdin-hiding';

/**
* The class name added to OutputPrompt.
*/
Expand Down Expand Up @@ -450,10 +452,12 @@ export class OutputArea extends Widget {
if (this.model.length >= this.maxNumberOutputs) {
this.maxNumberOutputs = this.model.length;
}
this.layout.addWidget(panel);

this._inputRequested.emit();

// Get the input node to ensure focus after updating the model upon user reply.
const inputNode = input.node.getElementsByTagName('input')[0];

/**
* Wait for the stdin to complete, add it to the model (so it persists)
* and remove the stdin widget.
Expand All @@ -463,14 +467,37 @@ export class OutputArea extends Widget {
if (this.model.length >= this.maxNumberOutputs) {
this.maxNumberOutputs = this.model.length + 1;
}
panel.addClass(OUTPUT_AREA_STDIN_HIDING_CLASS);
// Use stdin as the stream so it does not get combined with stdout.
// Note: because it modifies DOM it may (will) shift focus away from the input node.
this.model.add({
output_type: 'stream',
name: 'stdin',
text: value + '\n'
});
panel.dispose();
// Refocus the input node after it lost focus due to update of the model.
inputNode.focus();

// Keep the input in view for a little while; this (along refocusing)
// ensures that we can avoid the cell editor stealing the focus, and
// leading to user inadvertently modifying editor content when executing
// consecutive commands in short succession.
window.setTimeout(() => {
// Tack currently focused element to ensure that it remains on it
// after disposal of the panel with the old input
// (which modifies DOM and can lead to focus jump).
const focusedElement = document.activeElement;
// Dispose the old panel with no longer needed input box.
panel.dispose();
// Refocus the element that was focused before.
if (focusedElement && focusedElement instanceof HTMLElement) {
focusedElement.focus();
}
}, 500);
});

// Note: the `input.value` promise must be listened to before we attach the panel
this.layout.addWidget(panel);
}

/**
Expand Down Expand Up @@ -1085,6 +1112,11 @@ export class Stdin extends Widget implements IStdin {
* not be called directly by user code.
*/
handleEvent(event: KeyboardEvent): void {
if (this._resolved) {
// Do not handle any more key events if the promise was resolved.
event.preventDefault();
return;
}
const input = this._input;

if (event.type === 'keydown') {
Expand All @@ -1104,6 +1136,7 @@ export class Stdin extends Widget implements IStdin {
this._value += input.value;
Stdin._historyPush(this._historyKey, input.value);
}
this._resolved = true;
this._promise.resolve(void 0);
} else if (event.key === 'Escape') {
// currently this gets clobbered by the documentsearch:end command at the notebook level
Expand Down Expand Up @@ -1219,6 +1252,7 @@ export class Stdin extends Widget implements IStdin {
private _trans: TranslationBundle;
private _value: string;
private _valueCache: string;
private _resolved: boolean = false;
}

export namespace Stdin {
Expand Down
6 changes: 6 additions & 0 deletions packages/outputarea/style/base.css
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,12 @@ body.lm-mod-override-cursor .jp-OutputArea-output.jp-mod-isolated::before {
opacity: 1;
}

.jp-OutputArea-stdin-hiding {
/* soft-hide the output, preserving focus */
opacity: 0;
height: 0;
}

/*-----------------------------------------------------------------------------
| Output Area View
|----------------------------------------------------------------------------*/
Expand Down