Skip to content

Commit

Permalink
Backport PR #12368 on branch 3.3.x (Toolbar items may not act on the …
Browse files Browse the repository at this point in the history
…proper target) (#12409)

* Backport PR #12368: Toolbar items may not act on the proper target

* Use a valid 3.3.x selector

Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>
Co-authored-by: Frédéric Collonval <fcollonval@gmail.com>
  • Loading branch information
3 people committed Apr 14, 2022
1 parent 161614c commit 1856413
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 4 deletions.
40 changes: 39 additions & 1 deletion galata/test/jupyterlab/notebook-toolbar.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ async function populateNotebook(page: IJupyterLabPageFixture) {
'## This is **bold** and *italic* [link to jupyter.org!](http://jupyter.org)'
);
await page.notebook.addCell('code', '2 ** 3');
// await page.notebook.runCell(2, true);
}

test.describe('Notebook Toolbar', () => {
Expand Down Expand Up @@ -100,3 +99,42 @@ test.describe('Notebook Toolbar', () => {
expect(await nbPanel.screenshot()).toMatchSnapshot(imageName);
});
});

test('Toolbar items act on owner widget', async ({ page }) => {
// Given two side-by-side notebooks and the second being active
const file1 = 'notebook1.ipynb';
await page.notebook.createNew(file1);
const panel1 = await page.activity.getPanel(file1);
const tab1 = await page.activity.getTab(file1);

// FIXME Calling a second time `page.notebook.createNew` is not robust
await page.menu.clickMenuItem('File>New>Notebook');
try {
await page.waitForSelector('.jp-Dialog', { timeout: 5000 });
await page.click('.jp-Dialog .jp-mod-accept');
} catch (reason) {
// no-op
}

const tab2 = await page.activity.getTab();

const tab2BBox = await tab2.boundingBox();
await page.mouse.move(
tab2BBox.x + 0.5 * tab2BBox.width,
tab2BBox.y + 0.5 * tab2BBox.height
);
await page.mouse.down();
await page.mouse.move(900, tab2BBox.y + tab2BBox.height + 200);
await page.mouse.up();

const classlist = await tab1.getAttribute('class');
expect(classlist.split(' ')).not.toContain('jp-mod-current');

// When clicking on toolbar item of the first file
await (await panel1.$('button[title="Insert a cell below (B)"]')).click();

// Then the first file is activated and the action is performed
const classlistEnd = await tab1.getAttribute('class');
expect(classlistEnd.split(' ')).toContain('jp-mod-current');
expect(await page.notebook.getCellCount()).toEqual(2);
});
34 changes: 31 additions & 3 deletions packages/apputils/src/mainareawidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ import { Printing } from './printing';
import { Spinner } from './spinner';
import { ReactiveToolbar, Toolbar } from './toolbar';

/**
* A flag to indicate that event handlers are caught in the capture phase.
*/
const USE_CAPTURE = true;

/**
* A widget meant to be contained in the JupyterLab main area.
*
Expand Down Expand Up @@ -164,14 +169,32 @@ export class MainAreaWidget<T extends Widget = Widget>
*/
protected onActivateRequest(msg: Message): void {
if (this._isRevealed) {
if (this._content) {
this._focusContent();
}
this._focusContent();
} else {
this._spinner.node.focus();
}
}

/**
* Handle `after-attach` messages for the widget.
*/
protected onAfterAttach(msg: Message): void {
super.onAfterAttach(msg);
// Focus content in capture phase to ensure relevant commands operate on the
// current main area widget.
// Add the event listener directly instead of using `handleEvent` in order
// to save sub-classes from needing to reason about calling it as well.
this.node.addEventListener('mousedown', this._evtMouseDown, USE_CAPTURE);
}

/**
* Handle `before-detach` messages for the widget.
*/
protected onBeforeDetach(msg: Message): void {
this.node.removeEventListener('mousedown', this._evtMouseDown, USE_CAPTURE);
super.onBeforeDetach(msg);
}

/**
* Handle `'close-request'` messages.
*/
Expand Down Expand Up @@ -267,6 +290,11 @@ export class MainAreaWidget<T extends Widget = Widget>

private _isRevealed = false;
private _revealed: Promise<void>;
private _evtMouseDown = () => {
if (!this.node.contains(document.activeElement)) {
this._focusContent();
}
};
}

/**
Expand Down

0 comments on commit 1856413

Please sign in to comment.