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

Notebook: Escaping code completion pop-up disables cell edit mode #14328

Merged
merged 2 commits into from
Oct 22, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/playwright.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,4 @@ jobs:
path: |
examples/playwright/test-results/
examples/playwright/playwright-report/
retention-days: 2
retention-days: 7
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we increasing this by so much?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sometimes (sickness/holiday) it takes some time to go back to the failed PR and check the results. In my case 5 days would be probably enough thought.

115 changes: 114 additions & 1 deletion examples/playwright/src/tests/theia-notebook-editor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
// *****************************************************************************

import { PlaywrightWorkerArgs, expect, test } from '@playwright/test';
import { Locator, PlaywrightWorkerArgs, expect, test } from '@playwright/test';
import { TheiaApp } from '../theia-app';
import { TheiaAppLoader, TheiaPlaywrightTestConfig } from '../theia-app-loader';
import { TheiaNotebookCell } from '../theia-notebook-cell';
Expand Down Expand Up @@ -188,8 +188,121 @@ test.describe('Theia Notebook Cell interaction', () => {
expect(await cell.executionCount()).toBe('3');
});

test('Check arrow up and down works', async () => {
const cell = await firstCell(editor);
await editor.addCodeCell();
const secondCell = (await editor.cells())[1];
// second cell is selected after creation
expect(await secondCell.isSelected()).toBe(true);
// select cell above
await editor.page.keyboard.type('second cell');
await secondCell.editor.page.keyboard.press('ArrowUp');
expect(await cell.isSelected()).toBe(true);

// select cell below
await cell.app.page.keyboard.press('ArrowDown');
expect(await secondCell.isSelected()).toBe(true);
});

test('Check k(up)/j(down) selection works', async () => {
const cell = await firstCell(editor);
await editor.addCodeCell();
const secondCell = (await editor.cells())[1];
// second cell is selected after creation
expect(await secondCell.isSelected()).toBe(true);
await secondCell.selectCell(); // deselect editor focus

// select cell above
await secondCell.editor.page.keyboard.press('k');
expect(await cell.isSelected()).toBe(true);

// select cell below
await cell.app.page.keyboard.press('j');
expect(await secondCell.isSelected()).toBe(true);
});

test('Check x/c/v works', async () => {
const cell = await firstCell(editor);
await cell.addEditorText('print("First cell")');
await editor.addCodeCell();
const secondCell = (await editor.cells())[1];
await secondCell.addEditorText('print("Second cell")');
await secondCell.selectCell(); // deselect editor focus

// cut second cell
await secondCell.page.keyboard.press('x');
await editor.waitForCellCountChanged(2);
expect((await editor.cells()).length).toBe(1);

// paste second cell
await cell.selectCell();
await cell.page.keyboard.press('v');
await editor.waitForCellCountChanged(1);
expect((await editor.cells()).length).toBe(2);
const pastedCell = (await editor.cells())[1];
expect(await pastedCell.isSelected()).toBe(true);

// copy first cell
await cell.selectCell(); // deselect editor focus
await cell.page.keyboard.press('c');
// paste copied cell
await cell.page.keyboard.press('v');
await editor.waitForCellCountChanged(2);
expect((await editor.cells()).length).toBe(3);
expect(await (await editor.cells())[0].editorText()).toBe('print("First cell")');
expect(await (await editor.cells())[1].editorText()).toBe('print("First cell")');
expect(await (await editor.cells())[2].editorText()).toBe('print("Second cell")');
});

test('Check LineNumber switch `l` works', async () => {
const cell = await firstCell(editor);
await cell.addEditorText('print("First cell")');
await cell.selectCell();
await cell.page.keyboard.press('l');
// NOTE: div.line-numbers is not visible
await cell.editor.locator.locator('.overflow-guard > div.line-numbers').waitFor({ state: 'attached' });
});

test('Check Collapse output switch `o` works', async () => {
const cell = await firstCell(editor);
await cell.addEditorText('print("Check output collapse")');
await cell.selectCell();
await cell.execute(); // produce output
expect(await cell.outputText()).toBe('Check output collapse');

await cell.page.keyboard.press('o');
await (await cell.outputContainer()).waitFor({ state: 'hidden' });
await cell.page.keyboard.press('o');
await (await cell.outputContainer()).waitFor({ state: 'visible' });

expect(await cell.outputText()).toBe('Check output collapse');
});

test('Check arrow-up/arrow-down/escape with code completion', async () => {
await editor.addMarkdownCell();
const mdCell = (await editor.cells())[1];
await mdCell.addEditorText('h');

await editor.page.keyboard.press('Control+Space'); // call CC (suggestWidgetVisible=true)
await ensureCodeCompletionVisible(mdCell.editor.locator);
await editor.page.keyboard.press('Escape'); // close CC
// check the same cell still selected and not lose the edit mode
expect(await mdCell.editor.isFocused()).toBe(true);

await editor.page.keyboard.press('Control+Space'); // call CC (suggestWidgetVisible=true)
await ensureCodeCompletionVisible(mdCell.editor.locator);
await editor.page.keyboard.press('ArrowUp'); // select next entry in CC list
await editor.page.keyboard.press('Enter'); // apply completion
// check the same cell still selected and not the second one due to 'ArrowDown' being pressed
expect(await mdCell.isSelected()).toBe(true);

});
});

async function ensureCodeCompletionVisible(parent: Locator): Promise<void> {
await parent.locator('div.monaco-editor div.suggest-widget').waitFor({ timeout: 5000 });
}

async function firstCell(editor: TheiaNotebookEditor): Promise<TheiaNotebookCell> {
return (await editor.cells())[0];
}
Expand Down
13 changes: 13 additions & 0 deletions examples/playwright/src/theia-monaco-editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,19 @@ export class TheiaMonacoEditor extends TheiaPageObject {
await this.page.keyboard.type(text);
}

/**
* @returns `true` if the editor is focused, `false` otherwise.
*/
async isFocused(): Promise<boolean> {
const viewElement = await this.viewElement();
const monacoEditor = await viewElement?.$('div.monaco-editor');
if (!monacoEditor) {
throw new Error('Couldn\'t retrieve monaco editor element.');
}
const editorClass = await monacoEditor.getAttribute('class');
return editorClass?.includes('focused') ?? false;
}

protected replaceEditorSymbolsWithSpace(content: string): string | Promise<string | undefined> {
// [ ] &nbsp; => \u00a0 -- NO-BREAK SPACE
// [·] &middot; => \u00b7 -- MIDDLE DOT
Expand Down
17 changes: 16 additions & 1 deletion examples/playwright/src/theia-notebook-cell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,14 @@ export class TheiaNotebookCell extends TheiaPageObject {
return text?.substring(1, text.length - 1);
}

/**
* @returns `true` if the cell is selected (blue vertical line), `false` otherwise.
*/
async isSelected(): Promise<boolean> {
const markerClass = await this.locator.locator('div.theia-notebook-cell-marker').getAttribute('class');
return markerClass?.includes('theia-notebook-cell-marker-selected') ?? false;
}

/**
* @returns The output text of the cell.
*/
Expand All @@ -190,7 +198,14 @@ export class TheiaNotebookCell extends TheiaPageObject {
return spanTexts.join('');
}

protected async outputContainer(): Promise<Locator> {
/**
* Selects the cell itself not it's editor. Important for shortcut usage like copy-, cut-, paste-cell.
*/
async selectCell(): Promise<void> {
await this.sidebar().click();
}

async outputContainer(): Promise<Locator> {
const outFrame = await this.outputFrame();
// each cell has it's own output div with a unique id = cellHandle<handle>
const cellOutput = outFrame.locator(`div#cellHandle${await this.cellHandle()}`);
Expand Down
2 changes: 1 addition & 1 deletion examples/playwright/src/theia-notebook-editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export class TheiaNotebookEditor extends TheiaEditor {
await this.waitForCellCountChanged(currentCellsCount);
}

protected async waitForCellCountChanged(prevCount: number): Promise<void> {
async waitForCellCountChanged(prevCount: number): Promise<void> {
await this.viewLocator().locator('li.theia-notebook-cell').evaluateAll(
(elements, currentCount) => elements.length !== currentCount, prevCount
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ export class NotebookCellActionContribution implements MenuContribution, Command
{
command: NotebookCellCommands.STOP_EDIT_COMMAND.id,
keybinding: 'esc',
when: `editorTextFocus && ${NOTEBOOK_EDITOR_FOCUSED}`,
when: `editorTextFocus && ${NOTEBOOK_EDITOR_FOCUSED} && !suggestWidgetVisible`,
},
{
command: NotebookCellCommands.EXECUTE_SINGLE_CELL_COMMAND.id,
Expand Down
Loading