Skip to content

Commit

Permalink
Toolbar items may not act on the proper target (jupyterlab#12368)
Browse files Browse the repository at this point in the history
* Notebook toolbar buttons do not respect context
Fixes jupyterlab#12177

* Make sure main area widget focuses its content in bubble phase of mousedown (#4)

Co-authored-by: Afshin Taylor Darian <[email protected]>
  • Loading branch information
fcollonval and afshin authored Apr 14, 2022
1 parent c82fb00 commit b01083c
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 4 deletions.
42 changes: 41 additions & 1 deletion galata/test/jupyterlab/notebook-toolbar.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,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 @@ -99,3 +98,44 @@ 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[data-command="notebook:insert-cell-below"]')
).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 @@ -8,6 +8,11 @@ import { BoxLayout, BoxPanel, Widget } from '@lumino/widgets';
import { DOMUtils } from './domutils';
import { Printing } from './printing';

/**
* 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 @@ -163,14 +168,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 @@ -266,6 +289,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 b01083c

Please sign in to comment.