Skip to content

Commit

Permalink
Improve completer rendering performance (jupyterlab#13663)
Browse files Browse the repository at this point in the history
* Improve completer rendering performance

* Fix existing tests

* Preferentially use `insertText` for population

* Robustify insertText test

* Fix minor styling problems seen in visual tests/in self-review

* Remove unused `lineHeight` and `charWidth`

* Harmonize renderer methods use, invalidate size cache on filtering if needed

* Add a galata test for completer with documentation panel

* Add test for lazy rendering, fix two logic issues

* Use node cloning as it is more reliable than measuring size in contained nodes.

If we try to measure size of a child element of parent with `contain: strict`
we may get 0s. Therefore it is more reliable to clone node and attach it to
the body instead.

* (Re)set hoverbox styles earlier to correctly position during

the transition from predefined to dynamic sizing.

* Update Playwright Snapshots

* Update migration guide

* Fix formatting in RST

* Add counters to async methods, remove old `scoreCmp()`, escape in `resolve()`

* Reset screenshot updated by bot

* Fix typos in documentation

* Fix more typos

* Restore old method of copying attributes to avoid pollution

* Better cache invalidation, cache `completionItems()`, fix resets

* Add unit tests for `queryChanged` signal

* Apply suggestions from code review

Co-authored-by: Duc Trung Le <[email protected]>

* Apply suggestions from code review

Co-authored-by: Duc Trung Le <[email protected]>

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Duc Trung Le <[email protected]>
  • Loading branch information
3 people authored Jan 11, 2023
1 parent bb0595d commit d614e29
Show file tree
Hide file tree
Showing 14 changed files with 1,118 additions and 218 deletions.
37 changes: 33 additions & 4 deletions docs/source/extension/extension_migration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,39 @@ bumped their major version (following semver convention). We want to point out p
If you instantiate standalone cells outside of a notebook, you will probably need to set the constructor option
``placeholder`` to ``false`` to ensure direct rendering of the cell.
- ``@jupyterlab/completer`` from 3.x to 4.x
Major version bumped following the removal of ``ICompletionManager`` token and the replacement
of ``ICompletableAttributes`` interface by ``ICompletionProvider``. To create a completer provider
for JupyterLab, users need to implement the interface ``ICompletionProvider`` and then register
this provider with ``ICompletionProviderManager`` token.
Major version was bumped following major refactor aimed at performance improvements and enabling easier third-party integration.

* Adding custom completion suggestions (items):
- In 3.x and earlier adding custom completion items required re-registering the completer connector for each file/cell
using ``register`` method of old manager provided by ``ICompletionManager`` token; in 4.x this token and associated
``ICompletableAttributes`` interface was removed and a proper method of registering a custom source of completions
(a provider of completions) was added. To create a completer provider for JupyterLab, users need to implement the
``ICompletionProvider`` interface and then register this provider with ``ICompletionProviderManager`` token.
- In 3.x merging completions from different sources had to be performed by creating a connector internally merging
results from other connectors. in 4.x ``IProviderReconciliator`` is used to merge completions from multiple providers,
and can be customised in constructor for custom completion handlers (``CompletionHandler``); customizing reconciliator
in JupyterLab-managed completers is not yet possible.
* Rendering with ``Completer.IRenderer``:
- In 3.x it was not possible to easily swap the renderer of JupyterLab-managed completers.
In 4.x the renderer from the completion provider with highest rank is now used for all
JupyterLab-managed completers. This behaviour is subject to revision in the future (please leave feedback).
- Completer box is now using delayed rendering for off-screen content to improve time to first paint
for top suggestions. To position the completer without rendering all items we search for the widest
item using heuristic which can be adjusted in custom renderers (``itemWidthHeuristic``).
- The documentation panel now implements a loading indicator (a progress bar) customizable via
optional ``createLoadingDocsIndicator`` renderer method.
- ``createItemNode`` was removed in favour of ``createCompletionItemNode`` which is now required.
- ``createCompletionItemNode`` is no longer responsible for sanitization of labels which is now a
responsibility of the model (see below).
* Model:
- In 3.x it was not possible to easily swap the model of JupyterLab-managed completers.
In 4.x the model factory from the completion provider with highest rank is now used for
JupyterLab-managed completers. This behaviour is subject to revision in the future (please leave feedback).
- Old methods for updating and accessing the completion items: ``setOptions``, ``options``, and ``items`` were removed
in favour of ``completionItems`` and ``setCompletionItems`` which are now required members of ``Completer.IModel``.
- New signal ``queryChanged`` was added and has to be emitted by models.
- Model is now responsible for sanitization of labels and preserving original label on ``insertText`` attribute
(if not already defined); this change was required to properly handle escaping of HTML tags.
- ``@jupyterlab/codeeditor`` from 3.x to 4.x
* Remove ``ISelectionStyle`` (and therefore ``defaultSelectionStyle`` and ``IEditor.selectionStyle``). This was envisaged
for real-time collaboration. But this is not used in the final implementation.
Expand Down
49 changes: 46 additions & 3 deletions galata/test/jupyterlab/completer.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// Copyright (c) Jupyter Development Team.
// Distributed under the terms of the Modified BSD License.

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

const fileName = 'notebook.ipynb';
const COMPLETER_SELECTOR = '.jp-Completer';
Expand Down Expand Up @@ -34,7 +35,50 @@ test.describe('Completer', () => {
completer = page.locator(COMPLETER_SELECTOR);
await completer.waitFor();
const imageName = 'completer.png';
// TODO: on first trigger types are not properly displayed, reference image will need updating
expect(await completer.screenshot()).toMatchSnapshot(imageName);
});

test('Show documentation panel', async ({ page, tmpPath }) => {
const scriptName = 'completer_panel.py';
await page.contents.uploadFile(
path.resolve(__dirname, `./notebooks/${scriptName}`),
`${tmpPath}/${scriptName}`
);
await galata.Mock.mockSettings(page, [], {
...galata.DEFAULT_SETTINGS,
'@jupyterlab/completer-extension:tracker': {
showDocumentationPanel: true
}
});
await page.notebook.save();
await page.goto();
await page.notebook.openByPath(fileName);

await page.notebook.setCell(
0,
'code',
'from completer_panel import option_1, option_2'
);
await page.notebook.runCell(0, true);
await page.notebook.addCell('code', 'option');
await page.notebook.enterCellEditingMode(1);

// we need to wait until the completer gets bound to the cell after entering it
await page.waitForTimeout(50);
await page.keyboard.press('Tab');
let completer = page.locator(COMPLETER_SELECTOR);
await completer.waitFor();
await page.keyboard.press('Escape');
await page.waitForTimeout(50);
await expect(completer).toBeHidden();
await page.keyboard.press('Tab');
completer = page.locator(COMPLETER_SELECTOR);
await completer.waitFor();
await page.waitForSelector('.jp-Completer-loading-bar');
await page.waitForSelector('.jp-Completer-loading-bar', {
state: 'detached'
});
const imageName = 'completer-with-doc-panel.png';
expect(await completer.screenshot()).toMatchSnapshot(imageName);
});

Expand Down Expand Up @@ -119,7 +163,6 @@ test.describe('Completer', () => {
await completer.waitFor();

const imageName = 'completer-console.png';
// TODO: on first trigger types are not properly displayed, reference image will need updating
expect(await completer.screenshot()).toMatchSnapshot(imageName);
});

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
15 changes: 15 additions & 0 deletions galata/test/jupyterlab/notebooks/completer_panel.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
def option_1():
"""Documentation of 1st option.
This docstring is intentionally long to fill the documentation panel box.
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur vel
auctor felis. Phasellus non risus maximus tortor molestie consectetur.
Suspendisse in mauris eget nunc imperdiet elementum. Morbi consequat velit
at elit suscipit, quis placerat est vulputate. Ut semper leo et fermentum
gravida. Cras dui nisl, varius et lectus quis, maximus facilisis justo.
"""


def option_2():
"""Documentation of 2nd option."""
2 changes: 1 addition & 1 deletion packages/completer-extension/schema/tracker.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
},
"autoCompletion": {
"title": "Enable autocompletion.",
"description": "Autocompletion setting.",
"description": "Autocompletion setting.",
"type": "boolean",
"default": false
}
Expand Down
5 changes: 3 additions & 2 deletions packages/completer/src/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,6 @@ export class CompletionHandler implements IDisposable {
): Completer.ITextState {
return {
text: editor.model.sharedModel.getSource(),
lineHeight: editor.lineHeight,
charWidth: editor.charWidth,
line: position.line,
column: position.column
};
Expand Down Expand Up @@ -467,6 +465,9 @@ export namespace CompletionHandler {
*/
deprecated?: boolean;

/**
* Method allowing to update fields asynchronously.
*/
resolve?: (
patch?: Completer.IPatch
) => Promise<CompletionHandler.ICompletionItem>;
Expand Down
Loading

0 comments on commit d614e29

Please sign in to comment.