diff --git a/docs/source/extension/extension_migration.rst b/docs/source/extension/extension_migration.rst index cc6bb5216a96..94926cd4bf33 100644 --- a/docs/source/extension/extension_migration.rst +++ b/docs/source/extension/extension_migration.rst @@ -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. diff --git a/galata/test/jupyterlab/completer.test.ts b/galata/test/jupyterlab/completer.test.ts index 850f84c5bf9d..7853aaba552f 100644 --- a/galata/test/jupyterlab/completer.test.ts +++ b/galata/test/jupyterlab/completer.test.ts @@ -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'; @@ -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); }); @@ -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); }); diff --git a/galata/test/jupyterlab/completer.test.ts-snapshots/completer-console-jupyterlab-linux.png b/galata/test/jupyterlab/completer.test.ts-snapshots/completer-console-jupyterlab-linux.png index a5811ac1ddbe..840d72eceea9 100644 Binary files a/galata/test/jupyterlab/completer.test.ts-snapshots/completer-console-jupyterlab-linux.png and b/galata/test/jupyterlab/completer.test.ts-snapshots/completer-console-jupyterlab-linux.png differ diff --git a/galata/test/jupyterlab/completer.test.ts-snapshots/completer-with-doc-panel-jupyterlab-linux.png b/galata/test/jupyterlab/completer.test.ts-snapshots/completer-with-doc-panel-jupyterlab-linux.png new file mode 100644 index 000000000000..2ca648bb04d5 Binary files /dev/null and b/galata/test/jupyterlab/completer.test.ts-snapshots/completer-with-doc-panel-jupyterlab-linux.png differ diff --git a/galata/test/jupyterlab/notebooks/completer_panel.py b/galata/test/jupyterlab/notebooks/completer_panel.py new file mode 100644 index 000000000000..13208232c538 --- /dev/null +++ b/galata/test/jupyterlab/notebooks/completer_panel.py @@ -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.""" diff --git a/packages/completer-extension/schema/tracker.json b/packages/completer-extension/schema/tracker.json index 337fbadba95d..8fc0491ce9aa 100644 --- a/packages/completer-extension/schema/tracker.json +++ b/packages/completer-extension/schema/tracker.json @@ -33,7 +33,7 @@ }, "autoCompletion": { "title": "Enable autocompletion.", - "description": "Autocompletion setting.", + "description": "Autocompletion setting.", "type": "boolean", "default": false } diff --git a/packages/completer/src/handler.ts b/packages/completer/src/handler.ts index 955cb84ed699..dc29a920b24c 100644 --- a/packages/completer/src/handler.ts +++ b/packages/completer/src/handler.ts @@ -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 }; @@ -467,6 +465,9 @@ export namespace CompletionHandler { */ deprecated?: boolean; + /** + * Method allowing to update fields asynchronously. + */ resolve?: ( patch?: Completer.IPatch ) => Promise; diff --git a/packages/completer/src/model.ts b/packages/completer/src/model.ts index b14201da8658..930f54e04395 100644 --- a/packages/completer/src/model.ts +++ b/packages/completer/src/model.ts @@ -7,6 +7,15 @@ import { ISignal, Signal } from '@lumino/signaling'; import { CompletionHandler } from './handler'; import { Completer } from './widget'; +/** + * Escape HTML by native means of the browser. + */ +function escapeHTML(text: string) { + const node = document.createElement('span'); + node.textContent = text; + return node.innerHTML; +} + /** * An implementation of a completer model. */ @@ -18,6 +27,13 @@ export class CompleterModel implements Completer.IModel { return this._stateChanged; } + /** + * A signal emitted when query string changes (at invocation, or as user types). + */ + get queryChanged(): ISignal { + return this._queryChanged; + } + /** * The original completion request details. */ @@ -98,6 +114,8 @@ export class CompleterModel implements Completer.IModel { const ending = original.text.substring(end); query = query.substring(0, query.lastIndexOf(ending)); this._query = query; + this._processedItemsCache = null; + this._queryChanged.emit({ newValue: this._query, origin: 'editorUpdate' }); this._stateChanged.emit(undefined); } @@ -124,6 +142,8 @@ export class CompleterModel implements Completer.IModel { } set query(newValue: string) { this._query = newValue; + this._processedItemsCache = null; + this._queryChanged.emit({ newValue: this._query, origin: 'setter' }); } /** @@ -162,11 +182,17 @@ export class CompleterModel implements Completer.IModel { * This is a read-only property. */ completionItems(): CompletionHandler.ICompletionItems { - let query = this._query; - if (query) { - return this._markup(query); + if (!this._processedItemsCache) { + let query = this._query; + if (query) { + this._processedItemsCache = this._markup(query); + } else { + this._processedItemsCache = this._completionItems.map(item => { + return this._escapeItemLabel(item); + }); + } } - return this._completionItems; + return this._processedItemsCache; } /** @@ -186,6 +212,7 @@ export class CompleterModel implements Completer.IModel { this._orderedTypes = Private.findOrderedCompletionItemTypes( this._completionItems ); + this._processedItemsCache = null; this._stateChanged.emit(undefined); } @@ -345,44 +372,40 @@ export class CompleterModel implements Completer.IModel { // e.g. Given label `foo(b, a, r)` and query `bar`, // don't count parameters, `b`, `a`, and `r` as matches. const index = item.label.indexOf('('); - const prefix = index > -1 ? item.label.substring(0, index) : item.label; + let prefix = index > -1 ? item.label.substring(0, index) : item.label; + prefix = escapeHTML(prefix); let match = StringExt.matchSumOfSquares(prefix, query); // Filter non-matching items. if (match) { // Highlight label text if there's a match let marked = StringExt.highlight( - item.label, + escapeHTML(item.label), match.indices, Private.mark ); + // Use `Object.assign` to evaluate getters. + const highlightedItem = Object.assign({}, item); + highlightedItem.label = marked.join(''); + highlightedItem.insertText = item.insertText ?? item.label; results.push({ - ...item, - // Allow for lazily retrieved documentation (with a getter) - documentation: item.documentation, - label: marked.join(''), - // If no insertText is present, preserve original label value - // by setting it as the insertText. - insertText: item.insertText ? item.insertText : item.label, + item: highlightedItem, score: match.score }); } } - results.sort(Private.scoreCmp2); - - // Delete the extra score attribute to not leak implementation details - // to JavaScript callers. - results.forEach(x => { - delete (x as any).score; - }); + results.sort(Private.scoreCmp); - return results; + // Extract only the item (dropping the extra score attribute to not leak + // implementation details to JavaScript callers. + return results.map(match => match.item); } /** * Lazy load missing data of item at `activeIndex`. * @param {number} activeIndex - index of item * @return Return `undefined` if the completion item with `activeIndex` index can not be found. - * Return a promise of `null` if another `resolveItem` is called. Otherwise return the + * Return a promise of `null` if another `resolveItem` is called (but still updates the + * underlying completion item with resolved data). Otherwise return the * promise of resolved completion item. */ resolveItem( @@ -394,7 +417,7 @@ export class CompleterModel implements Completer.IModel { return undefined; } - let completionItems = this.completionItems(); + let completionItems = this._completionItems; if (!completionItems || !completionItems[activeIndex]) { return undefined; } @@ -410,6 +433,8 @@ export class CompleterModel implements Completer.IModel { } return resolvedItem .then(activeItem => { + // Escape the label it in place + this._escapeItemLabel(activeItem, true); ( Object.keys(activeItem) as Array< keyof CompletionHandler.ICompletionItem @@ -432,30 +457,57 @@ export class CompleterModel implements Completer.IModel { }); } + /** + * Escape item label, storing the original label and adding `insertText` if needed. + * If escaping changes label creates a new item unless `inplace` is true. + */ + private _escapeItemLabel( + item: CompletionHandler.ICompletionItem, + inplace: boolean = false + ): CompletionHandler.ICompletionItem { + const escapedLabel = escapeHTML(item.label); + // If there was no insert text, use the original (unescaped) label. + if (escapedLabel !== item.label) { + const newItem = inplace ? item : Object.assign({}, item); + newItem.insertText = item.insertText ?? item.label; + newItem.label = escapedLabel; + return newItem; + } + return item; + } + /** * Reset the state of the model. */ private _reset(): void { + const hadQuery = this._query; this._current = null; this._cursor = null; this._completionItems = []; this._original = null; this._query = ''; + this._processedItemsCache = null; this._subsetMatch = false; this._typeMap = {}; this._orderedTypes = []; + if (hadQuery) { + this._queryChanged.emit({ newValue: this._query, origin: 'reset' }); + } } private _current: Completer.ITextState | null = null; private _cursor: Completer.ICursorSpan | null = null; private _isDisposed = false; private _completionItems: CompletionHandler.ICompletionItems = []; + private _processedItemsCache: CompletionHandler.ICompletionItems | null = + null; private _original: Completer.ITextState | null = null; private _query = ''; private _subsetMatch = false; private _typeMap: Completer.TypeMap = {}; private _orderedTypes: string[] = []; private _stateChanged = new Signal(this); + private _queryChanged = new Signal(this); /** * A counter to cancel ongoing `resolveItem` call. @@ -510,24 +562,13 @@ namespace Private { } /** - * A sort comparison function for item match scores. - * - * #### Notes - * This orders the items first based on score (lower is better), then - * by locale order of the item text. + * A filtered completion matching result. */ - export function scoreCmp(a: IMatch, b: IMatch): number { - const delta = a.score - b.score; - if (delta !== 0) { - return delta; - } - return a.raw.localeCompare(b.raw); - } - - /** - * A filtered completion menu matching result. - */ - export interface ICompletionMatch extends CompletionHandler.ICompletionItem { + export interface ICompletionMatch { + /** + * The completion item data. + */ + item: CompletionHandler.ICompletionItem; /** * A score which indicates the strength of the match. * @@ -543,12 +584,12 @@ namespace Private { * This orders the items first based on score (lower is better), then * by locale order of the item text. */ - export function scoreCmp2(a: ICompletionMatch, b: ICompletionMatch): number { + export function scoreCmp(a: ICompletionMatch, b: ICompletionMatch): number { const delta = a.score - b.score; if (delta !== 0) { return delta; } - return a.insertText?.localeCompare(b.insertText ?? '') ?? 0; + return a.item.insertText?.localeCompare(b.item.insertText ?? '') ?? 0; } /** diff --git a/packages/completer/src/widget.ts b/packages/completer/src/widget.ts index 8c5a3d7530e6..19bddc9b494e 100644 --- a/packages/completer/src/widget.ts +++ b/packages/completer/src/widget.ts @@ -25,14 +25,14 @@ const ITEM_CLASS = 'jp-Completer-item'; const ACTIVE_CLASS = 'jp-mod-active'; /** - * The minimum height of a completer widget. + * The class used by item listing which determines the height of the completer. */ -const MIN_HEIGHT = 20; +const LIST_CLASS = 'jp-Completer-list'; /** - * The maximum height of a completer widget. + * Class of the documentation panel. */ -const MAX_HEIGHT = 300; +const DOC_PANEL_CLASS = 'jp-Completer-docpanel'; /** * A flag to indicate that event handlers are caught in the capture phase. @@ -61,15 +61,38 @@ export class Completer extends Widget { constructor(options: Completer.IOptions) { super({ node: document.createElement('div') }); this.sanitizer = options.sanitizer ?? new Sanitizer(); - this._renderer = - options.renderer ?? Completer.getDefaultRenderer(this.sanitizer); + this._defaultRenderer = Completer.getDefaultRenderer(this.sanitizer); + this._renderer = options.renderer ?? this._defaultRenderer; this.model = options.model ?? null; this.editor = options.editor ?? null; this.addClass('jp-Completer'); + this._updateConstraints(); } /** - * The sanitizer used to sanitize untrusted html inputs. + * Cache style constraints from CSS. + */ + _updateConstraints() { + const tempNode = document.createElement('div'); + tempNode.classList.add(LIST_CLASS); + tempNode.style.visibility = 'hidden'; + tempNode.style.overflowY = 'scroll'; + document.body.appendChild(tempNode); + const computedStyle = window.getComputedStyle(tempNode); + this._maxHeight = parseInt(computedStyle.maxHeight, 10); + this._minHeight = parseInt(computedStyle.minHeight, 10); + this._scrollbarWidth = tempNode.offsetWidth - tempNode.clientWidth; + document.body.removeChild(tempNode); + const tempDocPanel = document.createElement('div'); + tempDocPanel.classList.add(DOC_PANEL_CLASS); + this._docPanelWidth = Private.measureSize( + tempDocPanel, + 'inline-block' + ).width; + } + + /** + * The sanitizer used to sanitize untrusted HTML inputs. */ readonly sanitizer: ISanitizer; @@ -127,10 +150,12 @@ export class Completer extends Widget { } if (this._model) { this._model.stateChanged.disconnect(this.onModelStateChanged, this); + this._model.queryChanged.disconnect(this.onModelQueryChanged, this); } this._model = model; if (this._model) { this._model.stateChanged.connect(this.onModelStateChanged, this); + this._model.queryChanged.connect(this.onModelQueryChanged, this); } } @@ -149,6 +174,7 @@ export class Completer extends Widget { * Dispose of the resources held by the completer widget. */ dispose(): void { + this._sizeCache = undefined; this._model = null; super.dispose(); } @@ -191,6 +217,9 @@ export class Completer extends Widget { if (this._model) { this._model.reset(true); } + // Clear size cache. + this._sizeCache = undefined; + this.node.scrollTop = 0; } /** @@ -235,6 +264,32 @@ export class Completer extends Widget { } } + /** + * Handle model query changes. + */ + protected onModelQueryChanged( + model: Completer.IModel, + queryChange: Completer.IQueryChange + ): void { + // If query was changed by the user typing, the filtered down items + // may no longer reach/exceed the maxHeight of the completer widget, + // hence size needs to be recalculated. + if (this._sizeCache && queryChange.origin === 'editorUpdate') { + const newItems = model.completionItems(); + const oldItems = this._sizeCache.items; + // Only reset size if the number of items changed, or the longest item changed. + const oldWidest = oldItems[this._findWidestItemIndex(oldItems)]; + const newWidest = newItems[this._findWidestItemIndex(newItems)]; + const heuristic = this._getPreferredItemWidthHeuristic(); + if ( + newItems.length !== this._sizeCache.items.length || + heuristic(oldWidest) !== heuristic(newWidest) + ) { + this._sizeCache = undefined; + } + } + } + /** * Handle `update-request` messages. */ @@ -244,22 +299,32 @@ export class Completer extends Widget { return; } - if (this._resetFlag) { - this._resetFlag = false; + // If this is the first time the current completer session has loaded, + // populate any initial subset match. This is being done before node + // gets rendered to avoid rendering it twice. + if (!model.query) { + this._populateSubset(); + } + + let items = model.completionItems(); + + // If there are no items, reset and bail. + if (!items.length) { if (!this.isHidden) { + this.reset(); this.hide(); this._visibilityChanged.emit(undefined); } return; } - let node: HTMLElement | null = null; - let completionItemList = model.completionItems(); - node = this._createCompletionItemNode(model, completionItemList); + // Update constraints before any DOM modifications + this._updateConstraints(); - if (!node) { - return; - } + // Do not trigger any geometry updates from async code when in lock. + this._geometryLock = true; + + const node = this._createCompleterNode(model, items); let active = node.querySelectorAll(`.${ITEM_CLASS}`)[this._activeIndex]; active.classList.add(ACTIVE_CLASS); @@ -267,22 +332,14 @@ export class Completer extends Widget { // Add the documentation panel if (this._showDoc) { let docPanel = document.createElement('div'); - docPanel.className = 'jp-Completer-docpanel'; + docPanel.className = DOC_PANEL_CLASS; + this._docPanel = docPanel; node.appendChild(docPanel); + this._docPanelExpanded = false; } const resolvedItem = this.model?.resolveItem(this._activeIndex); this._updateDocPanel(resolvedItem); - // If this is the first time the current completer session has loaded, - // populate any initial subset match. - if (!model.query) { - const populated = this._populateSubset(); - if (populated) { - this.update(); - return; - } - } - if (this.isHidden) { this.show(); this._setGeometry(); @@ -290,22 +347,27 @@ export class Completer extends Widget { } else { this._setGeometry(); } + this._geometryLock = false; + } + + /** + * Get cached dimensions of the completer box. + */ + protected get sizeCache(): Completer.IDimensions | undefined { + if (!this._sizeCache) { + return; + } + return { + width: this._sizeCache.width, + height: this._sizeCache.height + }; } - private _createCompletionItemNode( + private _createCompleterNode( model: Completer.IModel, items: CompletionHandler.ICompletionItems - ): HTMLElement | null { - // If there are no items, reset and bail. - if (!items.length) { - this._resetFlag = true; - this.reset(); - if (!this.isHidden) { - this.hide(); - this._visibilityChanged.emit(undefined); - } - return null; - } + ): HTMLElement { + const current = ++this._renderCounter; // Clear the node. let node = this.node; @@ -317,15 +379,138 @@ export class Completer extends Widget { // Populate the completer items. let ul = document.createElement('ul'); - ul.className = 'jp-Completer-list'; - for (let item of items) { - let li = this._renderer.createCompletionItemNode(item, orderedTypes); + ul.className = LIST_CLASS; + + // Add first N items to fill the first "page" assuming that the completer + // would reach its maximum allowed height. + const first = this._renderer.createCompletionItemNode( + items[0], + orderedTypes + ); + const renderedItems = [first]; + + const firstItemSize = Private.measureSize(first, 'inline-grid'); + const pageSize = Math.max( + Math.ceil(this._maxHeight / firstItemSize.height), + 5 + ); + // We add one item in case if height heuristic is inacurate. + const toRenderImmediately = Math.min(pageSize + 1, items.length); + + const start = performance.now(); + for (let i = 1; i < toRenderImmediately; i++) { + const li = this._renderer.createCompletionItemNode( + items[i], + orderedTypes + ); + renderedItems.push(li); + } + + for (const li of renderedItems) { ul.appendChild(li); } + + if (pageSize < items.length) { + // If the first "page" is completely filled, we can pre-calculate size: + // - height will equal maximum allowed height, + // - width will be estimated from the widest item. + // If the page size is larger than the number of items, then there are + // few items and the benefit from pre-computing the size is negligible. + const widestItemIndex = this._findWidestItemIndex(items); + const widestItem = + widestItemIndex < renderedItems.length + ? renderedItems[widestItemIndex] + : this._renderer.createCompletionItemNode( + items[widestItemIndex], + orderedTypes + ); + + // The node needs to be cloned to avoid side-effect of detaching it. + const widestItemSize = Private.measureSize( + widestItem.cloneNode(true) as HTMLElement, + 'inline-grid' + ); + + this._sizeCache = { + height: this._maxHeight, + width: widestItemSize.width + this._scrollbarWidth, + items: items + }; + } + + if (toRenderImmediately < items.length) { + // Render remaining items on idle in subsequent animation frames, + // in chunks of size such that each frame would take about 16ms + // allowing for 4ms of overhead, but keep the chunks no smaller + // than 5 items at a time. + const timePerItem = (performance.now() - start) / toRenderImmediately; + + const chunkSize = Math.max(5, Math.floor(12 / timePerItem)); + + let alreadyRendered = toRenderImmediately; + let previousChunkFinal = renderedItems[renderedItems.length - 1]; + + const renderChunk = () => { + if (alreadyRendered >= items.length) { + return; + } + // Add a filler so that the list with partially rendered items has the total + // height equal to the (predicted) final height to avoid scrollbar jitter. + const predictedMissingHeight = + firstItemSize.height * (items.length - alreadyRendered); + previousChunkFinal.style.marginBottom = `${predictedMissingHeight}px`; + + requestAnimationFrame(() => { + if (current != this._renderCounter) { + // Bail if rendering afresh was requested in the meantime. + return; + } + previousChunkFinal.style.marginBottom = ''; + const limit = Math.min(items.length, alreadyRendered + chunkSize); + for (let i = alreadyRendered; i < limit; i++) { + const li = this._renderer.createCompletionItemNode( + items[i], + orderedTypes + ); + ul.appendChild(li); + previousChunkFinal = li; + } + alreadyRendered = limit; + + renderChunk(); + }); + }; + renderChunk(); + } + node.appendChild(ul); return node; } + /** + * Use preferred heuristic to find the index of the widest item. + */ + private _findWidestItemIndex( + items: CompletionHandler.ICompletionItems + ): number { + const widthHeuristic = this._getPreferredItemWidthHeuristic(); + + const widthHeuristics = items.map(widthHeuristic); + return widthHeuristics.indexOf(Math.max(...widthHeuristics)); + } + + /** + * Get item width heuristic function from renderer if available, + * or the default one otherwise. + */ + private _getPreferredItemWidthHeuristic(): ( + item: CompletionHandler.ICompletionItem + ) => number { + return this._renderer.itemWidthHeuristic + ? this._renderer.itemWidthHeuristic.bind(this._renderer) + : this._defaultRenderer.itemWidthHeuristic.bind(this._defaultRenderer); + } + /** * Cycle through the available completer items. * @@ -365,9 +550,7 @@ export class Completer extends Widget { active = items[this._activeIndex] as HTMLElement; active.classList.add(ACTIVE_CLASS); - let completionList = this.node.querySelector( - '.jp-Completer-list' - ) as Element; + let completionList = this.node.querySelector(`.${LIST_CLASS}`) as Element; ElementExt.scrollIntoViewIfNeeded(completionList, active); this._indexChanged.emit(this._activeIndex); @@ -519,8 +702,10 @@ export class Completer extends Widget { return false; } - const items = this.node.querySelectorAll(`.${ITEM_CLASS}`); - const subset = Private.commonSubset(Private.itemValues(items)); + const items = model.completionItems(); + const subset = Private.commonSubset( + items.map(item => item.insertText || item.label) + ); const { query } = model; // If a common subset exists and it is not the current query, highlight it. @@ -551,6 +736,7 @@ export class Completer extends Widget { const start = model.cursor.start; const position = editor.getPositionAt(start) as CodeEditor.IPosition; const anchor = editor.getCoordinateForPosition(position) as DOMRect; + const style = window.getComputedStyle(node); const borderLeft = parseInt(style.borderLeftWidth!, 10) || 0; const paddingLeft = parseInt(style.paddingLeft!, 10) || 0; @@ -564,85 +750,154 @@ export class Completer extends Widget { (editor.host.closest('.jp-MainAreaWidget > .lm-Widget') as HTMLElement) || editor.host; + const items = model.completionItems(); + + // Fast cache invalidation (only checks for length rather than length + width) + if (this._sizeCache && this._sizeCache.items.length !== items.length) { + this._sizeCache = undefined; + } + // Calculate the geometry of the completer. HoverBox.setGeometry({ anchor, host: host, - maxHeight: MAX_HEIGHT, - minHeight: MIN_HEIGHT, + maxHeight: this._maxHeight, + minHeight: this._minHeight, node: node, + size: this._sizeCache, offset: { horizontal: borderLeft + paddingLeft }, privilege: 'below', style: style, outOfViewDisplay: { - top: 'hidden-inside', - bottom: 'hidden-inside', + top: 'stick-inside', + bottom: 'stick-inside', left: 'stick-inside', right: 'stick-outside' } }); + const current = ++this._geometryCounter; + if (!this._sizeCache) { + // If size was not pre-calculated using heuristics, save the actual + // size into cache once rendered. + requestAnimationFrame(() => { + if (current != this._geometryCounter) { + // Do not set size to cache if it may already be outdated. + return; + } + let rect = node.getBoundingClientRect(); + this._sizeCache = { + width: rect.width, + height: rect.height, + items: items + }; + }); + } } - /** - * Create a loading bar element for document panel. - */ - private _createLoadingBar(): HTMLElement { - const loadingContainer = document.createElement('div'); - loadingContainer.classList.add('jp-Completer-loading-bar-container'); - const loadingBar = document.createElement('div'); - loadingBar.classList.add('jp-Completer-loading-bar'); - loadingContainer.append(loadingBar); - return loadingContainer; - } /** * Update the display-state and contents of the documentation panel */ private _updateDocPanel( resolvedItem: Promise | undefined ): void { - let docPanel = this.node.querySelector('.jp-Completer-docpanel'); + let docPanel = this._docPanel; if (!docPanel) { return; } + this._toggleDocPanel(true); if (!resolvedItem) { - docPanel.setAttribute('style', 'display:none'); + this._toggleDocPanel(false); return; } docPanel.textContent = ''; - docPanel.appendChild(this._createLoadingBar()); + const loadingIndicator = + this._renderer.createLoadingDocsIndicator?.() ?? + this._defaultRenderer.createLoadingDocsIndicator(); + docPanel.appendChild(loadingIndicator); + resolvedItem .then(activeItem => { if (!activeItem) { return; } + if (!docPanel) { + return; + } if (activeItem.documentation) { - let node: HTMLElement; - const nodeRenderer = - this._renderer.createDocumentationNode ?? - Completer.getDefaultRenderer(this.sanitizer) - .createDocumentationNode; - node = nodeRenderer(activeItem); - docPanel!.textContent = ''; - docPanel!.appendChild(node); - docPanel!.setAttribute('style', ''); + const node = + this._renderer.createDocumentationNode?.(activeItem) ?? + this._defaultRenderer.createDocumentationNode(activeItem); + docPanel.textContent = ''; + docPanel.appendChild(node); } else { - docPanel!.setAttribute('style', 'display:none'); + this._toggleDocPanel(false); } }) .catch(e => console.error(e)); } + private _toggleDocPanel(show: boolean): void { + let docPanel = this._docPanel; + if (!docPanel) { + return; + } + if (show) { + if (this._docPanelExpanded) { + return; + } + docPanel.style.display = ''; + this._docPanelExpanded = true; + } else { + if (!this._docPanelExpanded) { + return; + } + docPanel.style.display = 'none'; + this._docPanelExpanded = false; + } + const sizeCache = this._sizeCache; + if (sizeCache) { + sizeCache.width += this._docPanelWidth * (show ? +1 : -1); + if (!this._geometryLock) { + this._setGeometry(); + } + } + } + private _activeIndex = 0; private _editor: CodeEditor.IEditor | null | undefined = null; private _model: Completer.IModel | null = null; private _renderer: Completer.IRenderer; - private _resetFlag = false; + private _defaultRenderer: Completer.Renderer; private _selected = new Signal(this); private _visibilityChanged = new Signal(this); private _indexChanged = new Signal(this); private _lastSubsetMatch: string = ''; private _showDoc: boolean; + private _sizeCache: Private.IDimensionsCache | undefined; + + /** + * The maximum height of a completer widget. + */ + private _maxHeight: number; + + /** + * The minimum height of a completer widget. + */ + private _minHeight: number; + + private _scrollbarWidth: number; + private _docPanelWidth: number; + private _docPanel: HTMLElement | undefined; + private _geometryLock = false; + + /** + * Increasing this counter invalidates previous request to save geometry cache in animation callback. + */ + private _geometryCounter: number = 0; + + private _docPanelExpanded = false; + private _renderCounter: number = 0; } export namespace Completer { @@ -691,24 +946,31 @@ export namespace Completer { readonly text: string; /** - * The height of a character in the editor. + * The line number of the editor cursor. */ - readonly lineHeight: number; + readonly line: number; /** - * The width of a character in the editor. + * The character number of the editor cursor within a line. */ - readonly charWidth: number; + readonly column: number; + } + /** + * Information about the query string change. + */ + export interface IQueryChange { /** - * The line number of the editor cursor. + * The new value of the query. */ - readonly line: number; - + newValue: string; /** - * The character number of the editor cursor within a line. + * The event which caused the query to change, one of: + * - `editorUpdate`: as a result of editor change, e.g. user typing code, + * - `setter`: programatically, e.g. by the logic in the widget, + * - `reset`: due to completer model being reset. */ - readonly column: number; + origin: 'setter' | 'editorUpdate' | 'reset'; } /** @@ -716,10 +978,15 @@ export namespace Completer { */ export interface IModel extends IDisposable { /** - * A signal emitted when state of the completer menu changes. + * A signal emitted when state of the completer model changes. */ readonly stateChanged: ISignal; + /** + * A signal emitted when query string changes (at invocation, or as user types). + */ + readonly queryChanged: ISignal; + /** * The current text state details. */ @@ -844,6 +1111,14 @@ export namespace Completer { /** * Create an item node (an `li` element) from a ICompletionItem * for a text completer menu. + * + * #### Notes + * The item provided to renderer is already pre-processed by the model: + * - the `label` is escaped to ensure that no user-generated HTML is included; + * if `insertText` was not originally provided, it is set to raw `label` + * (prior to escaping) if needed, + * - if there were any matches against the query the `label` has them + * highlighted with ``s. */ createCompletionItemNode(item: T, orderedTypes: string[]): HTMLLIElement; @@ -852,13 +1127,48 @@ export namespace Completer { * documentation panel. */ createDocumentationNode?(activeItem: T): HTMLElement; + + /** + * Create a loading indicator element for document panel. + */ + createLoadingDocsIndicator?(): HTMLElement; + + /** + * Get a heuristic for the width of an item. + * + * As a performance optimization completer will infer the hover box width + * from the widest item node which will be rendered before all other nodes. + * By default the widest item is selected based on label length heuristic; + * renderers which customize item rendering can use this method to provide + * a custom heuristic. + */ + itemWidthHeuristic?(a: T): number; + } + + /** + * A namespace for the default renderer. + */ + export namespace Renderer { + export interface IOptions { + /** + * The sanitizer used to sanitize untrusted HTML inputs. + */ + sanitizer?: ISanitizer; + } } /** * The default implementation of an `IRenderer`. */ export class Renderer implements IRenderer { - constructor(readonly sanitizer: ISanitizer = new Sanitizer()) {} + constructor(options?: Renderer.IOptions) { + this.sanitizer = options?.sanitizer || new Sanitizer(); + } + + /** + * The sanitizer used to sanitize untrusted HTML inputs. + */ + readonly sanitizer: ISanitizer; /** * Create an item node from an ICompletionItem for a text completer menu. @@ -867,13 +1177,13 @@ export namespace Completer { item: CompletionHandler.ICompletionItem, orderedTypes: string[] ): HTMLLIElement { - let baseNode = this._createBaseNode(item.insertText || item.label); + let wrapperNode = this._createWrapperNode(item.insertText || item.label); if (item.deprecated) { - baseNode.classList.add('jp-Completer-deprecated'); + wrapperNode.classList.add('jp-Completer-deprecated'); } return this._constructNode( - baseNode, - this._createMatchNode(item.label), + wrapperNode, + this._createLabelNode(item.label), !!item.type, item.type, orderedTypes, @@ -889,7 +1199,7 @@ export namespace Completer { ): HTMLElement { const host = document.createElement('div'); host.classList.add('jp-RenderedText'); - const sanitizer = { sanitize: (dirty: string) => dirty }; + const sanitizer = { sanitize: this.sanitizer.sanitize }; const source = activeItem.documentation || ''; renderText({ host, sanitizer, source }).catch(console.error); @@ -897,9 +1207,30 @@ export namespace Completer { } /** - * Create base node with the value to be inserted + * Get a heuristic for the width of an item. */ - private _createBaseNode(value: string): HTMLLIElement { + itemWidthHeuristic(item: CompletionHandler.ICompletionItem): number { + return ( + item.label.replace(/<\?mark>/g, '').length + (item.type?.length || 0) + ); + } + + /** + * Create a loading bar for the documentation panel. + */ + createLoadingDocsIndicator(): HTMLElement { + const loadingContainer = document.createElement('div'); + loadingContainer.classList.add('jp-Completer-loading-bar-container'); + const loadingBar = document.createElement('div'); + loadingBar.classList.add('jp-Completer-loading-bar'); + loadingContainer.append(loadingBar); + return loadingContainer; + } + + /** + * Create base node with the value to be inserted. + */ + private _createWrapperNode(value: string): HTMLLIElement { const li = document.createElement('li'); li.className = ITEM_CLASS; // Set the raw, un-marked up value as a data attribute. @@ -910,13 +1241,11 @@ export namespace Completer { /** * Create match node to highlight potential prefix match within result. */ - private _createMatchNode(result: string): HTMLElement { + private _createLabelNode(result: string): HTMLElement { const matchNode = document.createElement('code'); matchNode.className = 'jp-Completer-match'; // Use innerHTML because search results include tags. - matchNode.innerHTML = this.sanitizer.sanitize(result, { - allowedTags: ['mark'] - }); + matchNode.innerHTML = result; return matchNode; } @@ -988,10 +1317,24 @@ export namespace Completer { !_defaultRenderer || (sanitizer && _defaultRenderer.sanitizer !== sanitizer) ) { - _defaultRenderer = new Renderer(sanitizer); + _defaultRenderer = new Renderer({ sanitizer: sanitizer }); } return _defaultRenderer; } + + /** + * Pre-calculated dimensions of the completer widget box. + */ + export interface IDimensions { + /** + * The total width including the documentation panel if visible. + */ + width: number; + /** + * The total height of the visible part of the completer. + */ + height: number; + } } /** @@ -1035,20 +1378,6 @@ namespace Private { return subset; } - /** - * Returns the list of raw item values currently in the DOM. - */ - export function itemValues(items: NodeList): string[] { - const values: string[] = []; - for (let i = 0, len = items.length; i < len; i++) { - const attr = (items[i] as HTMLElement).getAttribute('data-value'); - if (attr) { - values.push(attr); - } - } - return values; - } - /** * Returns true for any modified click event (i.e., not a left-click). */ @@ -1061,4 +1390,33 @@ namespace Private { event.metaKey ); } + + /** + * Measure size of provided HTML element without painting it. + * + * #### Notes + * The provided element has to be detached (not connected to DOM), + * or a side-effect of detaching it will occur. + */ + export function measureSize(element: HTMLElement, display: string): DOMRect { + if (element.isConnected) { + console.warn( + 'Measuring connected elements with `measureSize` has side-effects' + ); + } + element.style.visibility = 'hidden'; + element.style.display = display; + document.body.appendChild(element); + const size = element.getBoundingClientRect(); + document.body.removeChild(element); + element.removeAttribute('style'); + return size; + } + + export interface IDimensionsCache extends Completer.IDimensions { + /** + * The items for which the cache was most originally computed. + */ + items: CompletionHandler.ICompletionItems; + } } diff --git a/packages/completer/style/base.css b/packages/completer/style/base.css index 294a5e9752c1..e5af865f81ed 100644 --- a/packages/completer/style/base.css +++ b/packages/completer/style/base.css @@ -20,6 +20,9 @@ display: flex; flex-direction: row; + /* Needed to avoid scrollbar issues when using cached width. */ + box-sizing: content-box; + /* Position the completer relative to the text editor, align the '.' */ margin: 4px 0 0 -30px; z-index: 10001; @@ -28,7 +31,8 @@ .jp-Completer-docpanel { border-left: var(--jp-border-width) solid var(--jp-border-color1); width: 400px; - overflow: scroll; + overflow-y: scroll; + overflow-x: auto; padding: 8px; max-height: calc((10 * var(--jp-private-completer-item-height)) - 16px); } @@ -48,19 +52,20 @@ overflow-x: auto; max-height: calc((10 * var(--jp-private-completer-item-height))); min-height: calc(var(--jp-private-completer-item-height)); + width: 100%; } .jp-Completer-item { - display: table-row; box-sizing: border-box; margin: 0; padding: 0; height: var(--jp-private-completer-item-height); min-width: 150px; + display: grid; + grid-template-columns: min-content 1fr min-content; } .jp-Completer-item .jp-Completer-match { - display: table-cell; box-sizing: border-box; margin: 0; padding: 0 8px 0 6px; @@ -68,7 +73,6 @@ font-family: var(--jp-code-font-family); font-size: var(--jp-code-font-size); line-height: var(--jp-private-completer-item-height); - vertical-align: middle; } .jp-Completer-deprecated .jp-Completer-match { @@ -77,7 +81,6 @@ } .jp-Completer-item .jp-Completer-type { - display: table-cell; box-sizing: border-box; height: var(--jp-private-completer-item-height); background: transparent; @@ -85,8 +88,6 @@ } .jp-Completer-item .jp-Completer-icon { - vertical-align: middle; - /* Normal element size from LabIconStyle.ISheetOptions */ height: 16px; width: 16px; @@ -96,7 +97,6 @@ text-align: center; color: white; width: var(--jp-private-completer-item-height); - min-width: var(--jp-private-completer-item-height); font-family: var(--jp-ui-font-family); font-size: var(--jp-ui-font-size1); line-height: calc( @@ -107,7 +107,6 @@ } .jp-Completer-item .jp-Completer-typeExtended { - display: table-cell; box-sizing: border-box; height: var(--jp-private-completer-item-height); text-align: right; @@ -117,7 +116,6 @@ font-size: var(--jp-code-font-size); line-height: var(--jp-private-completer-item-height); padding-right: 8px; - vertical-align: middle; } .jp-Completer-item:hover { diff --git a/packages/completer/test/model.spec.ts b/packages/completer/test/model.spec.ts index 1b2115225401..5e110d3c38e3 100644 --- a/packages/completer/test/model.spec.ts +++ b/packages/completer/test/model.spec.ts @@ -61,6 +61,17 @@ describe('completer/model', () => { model.setCompletionItems([]); model.setCompletionItems([]); expect(called).toBe(3); + const itemsWithResolve = [ + { + label: 'foo', + resolve: async () => { + return { label: 'foo', documentation: 'Foo docs' }; + } + } + ]; + model.setCompletionItems(itemsWithResolve); + model.setCompletionItems(itemsWithResolve); + expect(called).toBe(4); }); it('should signal when original request changes', () => { @@ -140,6 +151,95 @@ describe('completer/model', () => { }); }); + describe('#queryChanged', () => { + it('should signal when query is set via public setter', () => { + const model = new CompleterModel(); + let called: Record<'setter' | 'editorUpdate' | 'reset', number> = { + setter: 0, + editorUpdate: 0, + reset: 0 + }; + const listener = (sender: any, args: Completer.IQueryChange) => { + called[args.origin]++; + }; + model.queryChanged.connect(listener); + expect(called.setter).toBe(0); + model.query = 'foo'; + expect(called.setter).toBe(1); + model.query = 'bar'; + expect(called.setter).toBe(2); + expect(called.editorUpdate).toBe(0); + }); + + it('should signal when query gets reset', () => { + const model = new CompleterModel(); + let called: Record<'setter' | 'editorUpdate' | 'reset', number> = { + setter: 0, + editorUpdate: 0, + reset: 0 + }; + const listener = (sender: any, args: Completer.IQueryChange) => { + called[args.origin]++; + }; + model.queryChanged.connect(listener); + expect(called.reset).toBe(0); + model.query = 'foo'; + expect(called.reset).toBe(0); + model.reset(); + expect(called.reset).toBe(1); + // Should not call again (query does not change with second reset) + model.reset(); + expect(called.reset).toBe(1); + }); + + it('should signal when current text changes', () => { + const model = new CompleterModel(); + let called: Record<'setter' | 'editorUpdate' | 'reset', number> = { + setter: 0, + editorUpdate: 0, + reset: 0 + }; + const currentValue = 'foo'; + const newValue = 'foob'; + const cursor: Completer.ICursorSpan = { start: 0, end: 0 }; + const request = makeState(currentValue); + const change = makeState(newValue); + const listener = (sender: any, args: Completer.IQueryChange) => { + called[args.origin]++; + }; + model.queryChanged.connect(listener); + expect(called.editorUpdate).toBe(0); + model.original = request; + model.cursor = cursor; + model.current = change; + expect(called.editorUpdate).toBe(1); + }); + + it('should not signal when current text is unchanged', () => { + const model = new CompleterModel(); + let called: Record<'setter' | 'editorUpdate' | 'reset', number> = { + setter: 0, + editorUpdate: 0, + reset: 0 + }; + const currentValue = 'foo'; + const newValue = 'foob'; + const cursor: Completer.ICursorSpan = { start: 0, end: 0 }; + const request = makeState(currentValue); + const change = makeState(newValue); + const listener = (sender: any, args: Completer.IQueryChange) => { + called[args.origin]++; + }; + model.queryChanged.connect(listener); + expect(called.editorUpdate).toBe(0); + model.original = request; + model.cursor = cursor; + model.current = change; + model.current = change; + expect(called.editorUpdate).toBe(1); + }); + }); + describe('#completionItems()', () => { it('should default to { items: [] }', () => { let model = new CompleterModel(); @@ -220,6 +320,31 @@ describe('completer/model', () => { model.reset(); expect(model.completionItems()).toEqual(want); }); + + it('should escape HTML markup', () => { + let model = new CompleterModel(); + let want: CompletionHandler.ICompletionItems = [ + { + label: '<foo></foo>', + insertText: '' + } + ]; + model.setCompletionItems([{ label: '' }]); + expect(model.completionItems()).toEqual(want); + }); + + it('should escape HTML with matches markup', () => { + let model = new CompleterModel(); + let want: CompletionHandler.ICompletionItems = [ + { + label: '<foo>smile</foo>', + insertText: 'smile' + } + ]; + model.setCompletionItems([{ label: 'smile' }]); + model.query = 'smi'; + expect(model.completionItems()).toEqual(want); + }); }); describe('#original', () => { @@ -437,6 +562,22 @@ describe('completer/model', () => { resolve: undefined }); }); + it('should escape HTML markup', async () => { + let model = new CompleterModel(); + const item = { + label: '', + resolve: () => + Promise.resolve({ label: '', documentation: 'Foo docs' }) + }; + model.setCompletionItems([item]); + const resolved = await model.resolveItem(0); + expect(resolved).toEqual({ + label: '<foo></foo>', + insertText: '', + documentation: 'Foo docs', + resolve: undefined + }); + }); }); }); }); diff --git a/packages/metapackage/test/completer/widget.spec.ts b/packages/metapackage/test/completer/widget.spec.ts index b022b6f20b1d..40da9359d68c 100644 --- a/packages/metapackage/test/completer/widget.spec.ts +++ b/packages/metapackage/test/completer/widget.spec.ts @@ -32,12 +32,37 @@ function createEditorWidget(): CodeEditorWrapper { return new CodeEditorWrapper({ factory, model }); } +/** + * jsdom mock for getBoundingClientRect returns zeros for all fields, + * see https://github.com/jsdom/jsdom/issues/653. We can do better, + * and need to do better to get meaningful tests for rendering. + */ +function betterGetBoundingClientRectMock() { + const style = window.getComputedStyle(this); + const top = parseFloat(style.top) || 0; + const left = parseFloat(style.left) || 0; + const dimensions = { + width: parseFloat(style.width) || parseFloat(style.minWidth) || 0, + height: parseFloat(style.height) || parseFloat(style.minHeight) || 0, + top, + left, + x: left, + y: top, + bottom: 0, + right: 0 + }; + return { + ...dimensions, + toJSON: () => dimensions + }; +} + class CustomRenderer extends Completer.Renderer { createCompletionItemNode( item: CompletionHandler.ICompletionItem, orderedTypes: string[] ): HTMLLIElement { - let li = super.createCompletionItemNode!(item, orderedTypes); + let li = super.createCompletionItemNode(item, orderedTypes); li.classList.add(TEST_ITEM_CLASS); return li; } @@ -45,7 +70,7 @@ class CustomRenderer extends Completer.Renderer { createDocumentationNode( item: CompletionHandler.ICompletionItem ): HTMLElement { - const element = super.createDocumentationNode!(item); + const element = super.createDocumentationNode(item); element.classList.add(TEST_DOC_CLASS); return element; } @@ -70,10 +95,47 @@ class LogWidget extends Completer { super.onUpdateRequest(msg); this.methods.push('onUpdateRequest'); } + + protected onModelQueryChanged( + model: Completer.IModel, + queryChange: Completer.IQueryChange + ): void { + super.onModelQueryChanged(model, queryChange); + this.methods.push(`onModelQueryChanged:${queryChange.origin}`); + } + + public get sizeCache() { + return super.sizeCache; + } } describe('completer/widget', () => { describe('Completer', () => { + beforeAll(() => { + const style = document.createElement('style'); + style.innerHTML = ` + .jp-Completer-list { + overflow-y: scroll; + overflow-x: auto; + max-height: 300px; + min-height: 20px; + width: 100%; + } + .jp-Completer-docpanel { + width: 400px; + overflow: scroll; + } + .jp-Completer-item { + box-sizing: border-box; + height: 20px; + min-width: 150px; + display: grid; + grid-template-columns: min-content 1fr min-content; + } + `; + document.head.appendChild(style); + }); + describe('#constructor()', () => { it('should create a completer widget', () => { const widget = new Completer({ editor: null }); @@ -177,8 +239,8 @@ describe('completer/widget', () => { value = selected; }; options.model!.setCompletionItems([ - { label: 'foo', insertText: 'bar' }, - { label: 'baz' } + { label: 'bar label', insertText: 'bar' }, + { label: 'foo label' } ]); Widget.attach(anchor, document.body); @@ -245,8 +307,6 @@ describe('completer/widget', () => { let request: Completer.ITextState = { column: position.column, - lineHeight: editor.lineHeight, - charWidth: editor.charWidth, line: position.line, text: 'a' }; @@ -947,8 +1007,6 @@ describe('completer/widget', () => { const request: Completer.ITextState = { column: position.column, - lineHeight: editor.lineHeight, - charWidth: editor.charWidth, line: position.line, text: 'a' }; @@ -992,21 +1050,12 @@ describe('completer/widget', () => { it('should un-hide widget if multiple items are available', () => { let anchor = createEditorWidget(); let model = new CompleterModel(); - let coords = { left: 0, right: 0, top: 100, bottom: 120 }; let request: Completer.ITextState = { column: 0, - lineHeight: 0, - charWidth: 0, line: 0, - coords, text: 'f' }; - let options: Completer.IOptions = { - editor: anchor.editor, - model - }; - Widget.attach(anchor, document.body); model.original = request; model.setCompletionItems([ @@ -1015,7 +1064,10 @@ describe('completer/widget', () => { { label: 'baz' } ]); - let widget = new Completer(options); + let widget = new Completer({ + editor: anchor.editor, + model + }); widget.hide(); expect(widget.isHidden).toBe(true); Widget.attach(widget, document.body); @@ -1024,6 +1076,203 @@ describe('completer/widget', () => { widget.dispose(); anchor.dispose(); }); + + it('should pre-compute and cache dimensions when items are many', () => { + window.HTMLElement.prototype.getBoundingClientRect = + betterGetBoundingClientRectMock; + let anchor = createEditorWidget(); + let model = new CompleterModel(); + + Widget.attach(anchor, document.body); + model.setCompletionItems( + Array.from({ length: 20 }, (_, i) => { + return { label: `candidate ${i}` }; + }) + ); + + let widget = new LogWidget({ + editor: anchor.editor, + model + }); + Widget.attach(widget, document.body); + expect(widget.sizeCache).toBe(undefined); + MessageLoop.sendMessage(widget, Widget.Msg.UpdateRequest); + expect(widget.sizeCache).toEqual({ + width: 150, + height: 300 + }); + widget.dispose(); + expect(widget.sizeCache).toBe(undefined); + anchor.dispose(); + }); + + it('should not fix nor cache dimensions when items are few', () => { + window.HTMLElement.prototype.getBoundingClientRect = + betterGetBoundingClientRectMock; + let anchor = createEditorWidget(); + let model = new CompleterModel(); + + Widget.attach(anchor, document.body); + model.setCompletionItems( + Array.from({ length: 3 }, (_, i) => { + return { label: `candidate ${i}` }; + }) + ); + + let widget = new LogWidget({ + editor: anchor.editor, + model + }); + Widget.attach(widget, document.body); + expect(widget.sizeCache).toBe(undefined); + MessageLoop.sendMessage(widget, Widget.Msg.UpdateRequest); + expect(widget.sizeCache).toEqual(undefined); + widget.dispose(); + anchor.dispose(); + }); + + it('should render completions lazily in chunks', async () => { + window.HTMLElement.prototype.getBoundingClientRect = + betterGetBoundingClientRectMock; + let anchor = createEditorWidget(); + let model = new CompleterModel(); + + Widget.attach(anchor, document.body); + model.setCompletionItems( + Array.from({ length: 30 }, (_, i) => { + return { label: `candidate ${i}` }; + }) + ); + + let widget = new Completer({ + editor: anchor.editor, + model + }); + Widget.attach(widget, document.body); + MessageLoop.sendMessage(widget, Widget.Msg.UpdateRequest); + + // First "page": + // - 300px/20px = 15 items at each "page", + // - we add one item in case if height heuristic is inacurate. + let items = widget.node.querySelectorAll(`.${ITEM_CLASS}`); + expect(items).toHaveLength(15 + 1); + + // Second and consecutive pages will be rendered in chunks of size + // depending on performance, but not smaller than 5 at a time. This + // means we should get 30 items in no more than 3 frames. + let previousCount = 15 + 1; + for (let i = 0; i < 3; i++) { + await framePromise(); + items = widget.node.querySelectorAll(`.${ITEM_CLASS}`); + expect(items.length).toBeGreaterThanOrEqual( + Math.min(previousCount + 5, 30) + ); + previousCount = items.length; + } + expect(items).toHaveLength(30); + }); + }); + + describe('#onModelQueryChanged()', () => { + let position: CodeEditor.IPosition; + let widget: LogWidget; + let model: CompleterModel; + let wrapper: CodeEditorWrapper; + const expectedSize = { width: 150, height: 300 }; + + beforeEach(() => { + window.HTMLElement.prototype.getBoundingClientRect = + betterGetBoundingClientRectMock; + wrapper = createEditorWidget(); + model = new CompleterModel(); + const editor = wrapper.editor; + + editor.model.sharedModel.setSource('c'); + position = editor.getPositionAt(1)!; + + editor.setCursorPosition(position); + + const original: Completer.ITextState = { + ...position, + text: 'c' + }; + Widget.attach(wrapper, document.body); + + model.original = original; + model.cursor = { start: 0, end: 1 }; + + model.setCompletionItems([ + ...Array.from({ length: 20 }, (_, i) => { + return { label: `candidate ${i}` }; + }), + ...Array.from({ length: 20 }, (_, i) => { + return { label: `candx ${i}` }; + }) + ]); + + widget = new LogWidget({ editor, model }); + Widget.attach(widget, document.body); + MessageLoop.sendMessage(widget, Widget.Msg.UpdateRequest); + }); + + afterEach(() => { + widget.dispose(); + wrapper.dispose(); + }); + + it('should not invalidate size cache if query change keeps situation the same', () => { + expect(widget.sizeCache).toEqual(expectedSize); + // Filtering which does not change the matching items. + const current: Completer.ITextState = { + ...position, + text: 'cand' + }; + expect(widget.methods).not.toContain( + 'onModelQueryChanged:editorUpdate' + ); + model.handleTextChange(current); + expect(widget.methods).toContain('onModelQueryChanged:editorUpdate'); + expect(widget.sizeCache).toEqual(expectedSize); + }); + + it('should invalidate size cache if query change leads to change in number of items', () => { + expect(widget.sizeCache).toEqual(expectedSize); + // Filtering which reduces the matching items to single `candidate 3` + const current: Completer.ITextState = { + ...position, + text: 'c3' + }; + expect(widget.methods).not.toContain( + 'onModelQueryChanged:editorUpdate' + ); + model.handleTextChange(current); + expect(widget.methods).toContain('onModelQueryChanged:editorUpdate'); + expect(widget.sizeCache).toEqual(undefined); + }); + + it('should invalidate size cache if query change leads to change in the widest item', () => { + expect(widget.sizeCache).toEqual(expectedSize); + // First filter to 20 items with `candidate` prefix, this should invalidate + let current: Completer.ITextState = { + ...position, + text: 'candi' + }; + + model.handleTextChange(current); + expect(widget.sizeCache).toEqual(undefined); + + // Establish new cache + MessageLoop.sendMessage(widget, Widget.Msg.UpdateRequest); + expect(widget.sizeCache).toEqual(expectedSize); + + // Then filter to 20 items with `candx` prefix + current = { + ...position, + text: 'candx' + }; + model.handleTextChange(current); + expect(widget.sizeCache).toEqual(undefined); + }); }); }); }); diff --git a/packages/ui-components/src/hoverbox.ts b/packages/ui-components/src/hoverbox.ts index 9679826fe4e9..6debf8c8f65b 100644 --- a/packages/ui-components/src/hoverbox.ts +++ b/packages/ui-components/src/hoverbox.ts @@ -7,9 +7,9 @@ const HOVERBOX_CLASS = 'jp-HoverBox'; /** - * The class name added to a hovering node that is scrolled out of view. + * The z-index used to hide hovering node that is scrolled out of view. */ -const OUTOFVIEW_CLASS = 'jp-mod-outofview'; +const OUTOFVIEW_Z_INDEX = '-1000'; type OutOfViewDisplay = | 'hidden-inside' @@ -112,6 +112,15 @@ export namespace HoverBox { left?: OutOfViewDisplay; right?: OutOfViewDisplay; }; + + /** + * Exact size of the hover box. Pass it for faster rendering (allowing the + * positioning algorithm to to place it immediately at requested position). + */ + size?: { + width: number; + height: number; + }; } /** @@ -122,11 +131,20 @@ export namespace HoverBox { export function setGeometry(options: IOptions): void { const { anchor, host, node, privilege, outOfViewDisplay } = options; + const hostRect = host.getBoundingClientRect(); + // Add hover box class if it does not exist. - node.classList.add(HOVERBOX_CLASS); + if (!node.classList.contains(HOVERBOX_CLASS)) { + node.classList.add(HOVERBOX_CLASS); + } - // Make sure the node is visible so that its dimensions can be queried. - node.classList.remove(OUTOFVIEW_CLASS); + // Start with the node displayed as if it was in view. + if (node.style.visibility) { + node.style.visibility = ''; + } + if (node.style.zIndex === '') { + node.style.zIndex = ''; + } // Clear any previously set max-height. node.style.maxHeight = ''; @@ -135,9 +153,9 @@ export namespace HoverBox { node.style.marginTop = ''; const style = options.style || window.getComputedStyle(node); - const innerHeight = window.innerHeight; - const spaceAbove = anchor.top; - const spaceBelow = innerHeight - anchor.bottom; + const spaceAbove = anchor.top - hostRect.top; + const spaceBelow = hostRect.bottom - anchor.bottom; + const marginTop = parseInt(style.marginTop!, 10) || 0; const marginLeft = parseInt(style.marginLeft!, 10) || 0; const minHeight = parseInt(style.minHeight!, 10) || options.minHeight; @@ -169,11 +187,25 @@ export namespace HoverBox { (spaceBelow >= minHeight || spaceAbove >= minHeight); if (!withinBounds) { - node.classList.add(OUTOFVIEW_CLASS); + node.style.zIndex = OUTOFVIEW_Z_INDEX; + node.style.visibility = 'hidden'; return; } + if (options.size) { + node.style.width = `${options.size.width}px`; + node.style.height = `${options.size.height}px`; + node.style.contain = 'strict'; + } else { + node.style.contain = ''; + node.style.width = 'auto'; + node.style.height = ''; + } + // Position the box vertically. + const initialHeight = options.size + ? options.size.height + : node.getBoundingClientRect().height; const offsetAbove = (options.offset && options.offset.vertical && @@ -185,8 +217,8 @@ export namespace HoverBox { options.offset.vertical.below) || 0; let top = renderBelow - ? innerHeight - spaceBelow + offsetBelow - : spaceAbove - node.getBoundingClientRect().height + offsetAbove; + ? hostRect.bottom - spaceBelow + offsetBelow + : hostRect.top + spaceAbove - initialHeight + offsetAbove; node.style.top = `${Math.floor(top)}px`; // Position the box horizontally. @@ -194,17 +226,10 @@ export namespace HoverBox { let left = anchor.left + offsetHorizontal; node.style.left = `${Math.ceil(left)}px`; - node.style.width = 'auto'; - // Expand the menu width by the scrollbar size, if present. - if (node.scrollHeight >= maxHeight) { - node.style.width = `${2 * node.offsetWidth - node.clientWidth}`; - node.scrollTop = 0; - } + let rect = node.getBoundingClientRect(); // Move left to fit in the window. - const rect = node.getBoundingClientRect(); - const hostRect = host.getBoundingClientRect(); // Query together to avoid extra layout recalculation const right = rect.right; if (right > window.innerWidth) { left -= right - window.innerWidth; @@ -218,7 +243,8 @@ export namespace HoverBox { } // Hide the hover box before querying the DOM for the anchor coordinates. - node.classList.add(OUTOFVIEW_CLASS); + // Using z-index set directly for performance. + node.style.zIndex = '-1000'; const bottom = rect.bottom; @@ -233,6 +259,8 @@ export namespace HoverBox { document.elementFromPoint(left, bottom) ); + node.style.zIndex = ''; + const topEdgeInside = includesLeftTop || includesRightTop; const bottomEdgeInside = includesLeftBottom || includesRightBottom; const leftEdgeInside = includesLeftTop || includesLeftBottom; @@ -358,8 +386,9 @@ export namespace HoverBox { } } - if (!hide) { - node.classList.remove(OUTOFVIEW_CLASS); + if (hide) { + node.style.zIndex = OUTOFVIEW_Z_INDEX; + node.style.visibility = 'hidden'; } if (leftChanged) { diff --git a/packages/ui-components/style/hoverbox.css b/packages/ui-components/style/hoverbox.css index a9a6f64748a8..d0bf57500c70 100644 --- a/packages/ui-components/style/hoverbox.css +++ b/packages/ui-components/style/hoverbox.css @@ -7,7 +7,3 @@ .jp-HoverBox { position: fixed; } - -.jp-HoverBox.jp-mod-outofview { - display: none; -}