From 39e4644f15f51a418914ce42ddf7cebabb29c786 Mon Sep 17 00:00:00 2001 From: krassowski Date: Mon, 14 Sep 2020 18:12:46 +0100 Subject: [PATCH 01/13] Fix and start testing hover tooltips --- atest/05_Features/Hover.robot | 36 +++++++++++++++++++ packages/jupyterlab-lsp/schema/hover.json | 6 ++++ .../src/components/free_tooltip.ts | 2 ++ packages/jupyterlab-lsp/src/features/hover.ts | 7 ++-- .../jupyterlab-lsp/src/features/signature.ts | 6 ++-- packages/jupyterlab-lsp/style/hover.css | 9 +++++ packages/jupyterlab-lsp/style/index.css | 1 + 7 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 atest/05_Features/Hover.robot create mode 100644 packages/jupyterlab-lsp/style/hover.css diff --git a/atest/05_Features/Hover.robot b/atest/05_Features/Hover.robot new file mode 100644 index 000000000..5e09c7d04 --- /dev/null +++ b/atest/05_Features/Hover.robot @@ -0,0 +1,36 @@ +*** Settings *** +Suite Setup Setup Suite For Screenshots hover +Test Setup Setup Hover Test +Test Teardown Clean Up After Working With File Hover.ipynb +Force Tags feature:hover +Resource ../Keywords.robot + +*** Variables *** +${HOVER_BOX} css:.lsp-hover + +*** Test Cases *** +Hover works in notebooks + Hover Over python_add + Capture Page Screenshot 02-hover-shown.png + Element Should Contain ${HOVER_BOX} python_add(a: int, b: int) + Page Should Contain Element ${HOVER_BOX} code.language-python + +Hover works in foreign code (javascript) + Hover Over js_add + Capture Page Screenshot 02-hover-shown.png + Element Should Contain ${HOVER_BOX} function js_add(a: any, b: any): any + Page Should Contain Element ${HOVER_BOX} code.language-typescript + # also for multiple cells of the same document + Hover Over Math + Element Should Contain ${HOVER_BOX} const Math: Math + +*** Keywords *** +Hover Over + [Arguments] ${symbol} + ${sel} = Set Variable If "${symbol}".startswith(("xpath", "css")) ${symbol} xpath:(//span[@role="presentation"][contains(., "${symbol}")])[last()] + Wait Until Keyword Succeeds 10 x 0.1 s Mouse Over ${sel} + Press Keys ${sel} CTRL + Wait Until Keyword Succeeds 20x 0.5s Page Should Contain Element ${HOVER_BOX} + +Setup Hover Test + Setup Notebook Python Hover.ipynb diff --git a/packages/jupyterlab-lsp/schema/hover.json b/packages/jupyterlab-lsp/schema/hover.json index 13bcd4697..2195f6ea5 100644 --- a/packages/jupyterlab-lsp/schema/hover.json +++ b/packages/jupyterlab-lsp/schema/hover.json @@ -11,6 +11,12 @@ "enum": ["Alt", "Control", "Shift", "Meta", "AltGraph"], "default": "Control", "description": "Keyboard key which activates the tooltip on hover." + }, + "debouncerDelay": { + "title": "Debouncer delay", + "type": "number", + "default": 50, + "description": "Number of milliseconds to delay sending out the request hover request to the language server; you can get better responsiveness adjusting this value, but setting it to zero can actually slow it down as the server might get overwhelmed when moving the mose over the code." } }, "jupyter.lab.shortcuts": [] diff --git a/packages/jupyterlab-lsp/src/components/free_tooltip.ts b/packages/jupyterlab-lsp/src/components/free_tooltip.ts index eec6fab62..6085cd845 100644 --- a/packages/jupyterlab-lsp/src/components/free_tooltip.ts +++ b/packages/jupyterlab-lsp/src/components/free_tooltip.ts @@ -95,6 +95,7 @@ export namespace EditorTooltip { markup: lsProtocol.MarkupContent; ce_editor: CodeEditor.IEditor; position: IEditorPosition; + className?: string; } } @@ -123,6 +124,7 @@ export class EditorTooltipManager { position: PositionConverter.cm_to_ce(position), moveToLineEnd: false }); + tooltip.addClass(options.className); Widget.attach(tooltip, document.body); this.currentTooltip = tooltip; return tooltip; diff --git a/packages/jupyterlab-lsp/src/features/hover.ts b/packages/jupyterlab-lsp/src/features/hover.ts index f046b162c..7fdae8a4a 100644 --- a/packages/jupyterlab-lsp/src/features/hover.ts +++ b/packages/jupyterlab-lsp/src/features/hover.ts @@ -66,7 +66,7 @@ export class HoverCM extends CodeMirrorIntegration { // TODO: make the debounce rate configurable this.debounced_get_hover = new Debouncer>( this.on_hover, - 50 + this.settings.composite.debouncerDelay ); super.register(); } @@ -108,7 +108,7 @@ export class HoverCM extends CodeMirrorIntegration { } else { return { kind: 'markdown', - value: '```' + content.language + '\n' + content.value + '```' + value: '```' + content.language + '\n' + content.value + '\n```' }; } } @@ -151,7 +151,8 @@ export class HoverCM extends CodeMirrorIntegration { this.lab_integration.tooltip.create({ markup, position: editor_position, - ce_editor: this.virtual_editor.find_ce_editor(cm_editor) + ce_editor: this.virtual_editor.find_ce_editor(cm_editor), + className: 'lsp-hover' }); }; diff --git a/packages/jupyterlab-lsp/src/features/signature.ts b/packages/jupyterlab-lsp/src/features/signature.ts index b3bb9efea..0adc5de3a 100644 --- a/packages/jupyterlab-lsp/src/features/signature.ts +++ b/packages/jupyterlab-lsp/src/features/signature.ts @@ -112,12 +112,12 @@ export class SignatureCM extends CodeMirrorIntegration { root_position ); - let tooltip = this.lab_integration.tooltip.create({ + this.lab_integration.tooltip.create({ markup, position: editor_position, - ce_editor: this.virtual_editor.find_ce_editor(cm_editor) + ce_editor: this.virtual_editor.find_ce_editor(cm_editor), + className: 'lsp-signature-help' }); - tooltip.addClass('lsp-signature-help'); } get signatureCharacters() { diff --git a/packages/jupyterlab-lsp/style/hover.css b/packages/jupyterlab-lsp/style/hover.css new file mode 100644 index 000000000..0c5e8171f --- /dev/null +++ b/packages/jupyterlab-lsp/style/hover.css @@ -0,0 +1,9 @@ +.lsp-hover .jp-RenderedHTMLCommon { + /* Reduce the default padding from 20px to 0px */ + padding-right: 0px; +} + +.lsp-hover .jp-RenderedHTMLCommon > *:last-child { + /* Reduce the default margin */ + margin-bottom: 0; +} diff --git a/packages/jupyterlab-lsp/style/index.css b/packages/jupyterlab-lsp/style/index.css index f2fef7c3d..f42615e7d 100644 --- a/packages/jupyterlab-lsp/style/index.css +++ b/packages/jupyterlab-lsp/style/index.css @@ -1,4 +1,5 @@ @import url('./highlight.css'); +@import url('./hover.css'); .lsp-document-locator:hover { cursor: pointer; From a4d8625915c21ba397cda75e989ab1994116a2bd Mon Sep 17 00:00:00 2001 From: krassowski Date: Mon, 14 Sep 2020 18:26:25 +0100 Subject: [PATCH 02/13] Lint the tests --- atest/05_Features/Hover.robot | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/atest/05_Features/Hover.robot b/atest/05_Features/Hover.robot index 5e09c7d04..134d8c2ca 100644 --- a/atest/05_Features/Hover.robot +++ b/atest/05_Features/Hover.robot @@ -6,30 +6,30 @@ Force Tags feature:hover Resource ../Keywords.robot *** Variables *** -${HOVER_BOX} css:.lsp-hover +${HOVER_BOX} css:.lsp-hover *** Test Cases *** Hover works in notebooks - Hover Over python_add + Hover Over python_add Capture Page Screenshot 02-hover-shown.png Element Should Contain ${HOVER_BOX} python_add(a: int, b: int) Page Should Contain Element ${HOVER_BOX} code.language-python Hover works in foreign code (javascript) - Hover Over js_add + Hover Over js_add Capture Page Screenshot 02-hover-shown.png Element Should Contain ${HOVER_BOX} function js_add(a: any, b: any): any Page Should Contain Element ${HOVER_BOX} code.language-typescript # also for multiple cells of the same document - Hover Over Math + Hover Over Math Element Should Contain ${HOVER_BOX} const Math: Math *** Keywords *** Hover Over - [Arguments] ${symbol} + [Arguments] ${symbol} ${sel} = Set Variable If "${symbol}".startswith(("xpath", "css")) ${symbol} xpath:(//span[@role="presentation"][contains(., "${symbol}")])[last()] Wait Until Keyword Succeeds 10 x 0.1 s Mouse Over ${sel} - Press Keys ${sel} CTRL + Press Keys ${sel} CTRL Wait Until Keyword Succeeds 20x 0.5s Page Should Contain Element ${HOVER_BOX} Setup Hover Test From 2b1f1fa34d3058b2f005093bc91f034af6629431 Mon Sep 17 00:00:00 2001 From: krassowski Date: Mon, 14 Sep 2020 18:29:45 +0100 Subject: [PATCH 03/13] Add changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2eed0b54..f45085d0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,10 @@ - bug fixes - fix syntax highlighting of %%language cells slowing down editing in notebooks ([#361]) + - fix syntax highlighting in hover tooltips and reduce unnecessary padding and margin ([#363]) [#361]: https://github.com/krassowski/jupyterlab-lsp/issues/361 +[#363]: https://github.com/krassowski/jupyterlab-lsp/issues/363 ### `@krassowski/jupyterlab-lsp 2.0.5` (2020-09-11) From acdaffe6271cc1dc90ed818f37448563dd325026 Mon Sep 17 00:00:00 2001 From: krassowski Date: Mon, 14 Sep 2020 19:19:12 +0100 Subject: [PATCH 04/13] Add the test notebook --- atest/examples/Hover.ipynb | 63 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 atest/examples/Hover.ipynb diff --git a/atest/examples/Hover.ipynb b/atest/examples/Hover.ipynb new file mode 100644 index 000000000..8b93c44b2 --- /dev/null +++ b/atest/examples/Hover.ipynb @@ -0,0 +1,63 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "def python_add(a: int, b: int):\n", + " \"\"\"documentation\"\"\"\n", + " return\n", + "\n", + "\n", + "python_add(1, 2)" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "%%javascript\n", + "function js_add(a, b) {\n", + " return a + b\n", + "}\n", + "\n", + "js_add(1, 2)" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "%%javascript\n", + "Math" + ] + } + ], + "metadata": { + "kernelspec": { + "display_name": "Python 3", + "language": "python", + "name": "python3" + }, + "language_info": { + "codemirror_mode": { + "name": "ipython", + "version": 3 + }, + "file_extension": ".py", + "mimetype": "text/x-python", + "name": "python", + "nbconvert_exporter": "python", + "pygments_lexer": "ipython3", + "version": "3.7.5" + } + }, + "nbformat": 4, + "nbformat_minor": 4 +} From add3e270f01deebc5dbcdd4d3a9f1338b85af189 Mon Sep 17 00:00:00 2001 From: krassowski Date: Mon, 14 Sep 2020 21:52:17 +0100 Subject: [PATCH 05/13] Fix missing CSS rules for hover availability signaling --- packages/jupyterlab-lsp/style/highlight.css | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/jupyterlab-lsp/style/highlight.css b/packages/jupyterlab-lsp/style/highlight.css index 7832ea194..87b0051a8 100644 --- a/packages/jupyterlab-lsp/style/highlight.css +++ b/packages/jupyterlab-lsp/style/highlight.css @@ -56,6 +56,8 @@ .cm-lsp-hover-available { text-decoration: var(--jp-editor-mirror-lsp-hover-decoration-style); text-decoration-color: var(--jp-editor-mirror-lsp-hover-decoration-color); + text-decoration-line: underline; + text-decoration-skip-ink: none; } /* highlight */ From 35e0530ae06e2f70df8f5d7d407cedf2d1639e77 Mon Sep 17 00:00:00 2001 From: krassowski Date: Tue, 15 Sep 2020 02:45:47 +0100 Subject: [PATCH 06/13] Hover tooltips: add cache, better deprecated API support --- CHANGELOG.md | 3 + atest/05_Features/Hover.robot | 12 +- atest/examples/Hover.ipynb | 2 +- packages/jupyterlab-lsp/schema/hover.json | 10 +- .../jupyterlab-lsp/src/adapter_manager.ts | 1 + packages/jupyterlab-lsp/src/converter.ts | 4 + packages/jupyterlab-lsp/src/features/hover.ts | 373 ++++++++++++------ 7 files changed, 277 insertions(+), 128 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f45085d0a..bef873673 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ - fix syntax highlighting of %%language cells slowing down editing in notebooks ([#361]) - fix syntax highlighting in hover tooltips and reduce unnecessary padding and margin ([#363]) + - greatly improve performance of hover action ([#363]) + - improve support for expanded hovers tooltips using deprecated API ([#363]) + - do not hide hover tooltips too eagerly (allowing selecting text/easy scrolling of longer tooltips) ([#363]) [#361]: https://github.com/krassowski/jupyterlab-lsp/issues/361 [#363]: https://github.com/krassowski/jupyterlab-lsp/issues/363 diff --git a/atest/05_Features/Hover.robot b/atest/05_Features/Hover.robot index 134d8c2ca..7c79b8647 100644 --- a/atest/05_Features/Hover.robot +++ b/atest/05_Features/Hover.robot @@ -7,15 +7,20 @@ Resource ../Keywords.robot *** Variables *** ${HOVER_BOX} css:.lsp-hover +${HOVER_SIGNAL} css:.cm-lsp-hover-available *** Test Cases *** Hover works in notebooks + Enter Cell Editor 1 Hover Over python_add + Element Text Should Be ${HOVER_SIGNAL} python_add Capture Page Screenshot 02-hover-shown.png Element Should Contain ${HOVER_BOX} python_add(a: int, b: int) Page Should Contain Element ${HOVER_BOX} code.language-python + Element Should Contain ${HOVER_BOX} Add documentation Hover works in foreign code (javascript) + Enter Cell Editor 2 Hover Over js_add Capture Page Screenshot 02-hover-shown.png Element Should Contain ${HOVER_BOX} function js_add(a: any, b: any): any @@ -28,8 +33,13 @@ Hover works in foreign code (javascript) Hover Over [Arguments] ${symbol} ${sel} = Set Variable If "${symbol}".startswith(("xpath", "css")) ${symbol} xpath:(//span[@role="presentation"][contains(., "${symbol}")])[last()] + Wait Until Keyword Succeeds 10 x 0.1 s Trigger Tooltip ${sel} + +Trigger Tooltip + [Arguments] ${sel} Wait Until Keyword Succeeds 10 x 0.1 s Mouse Over ${sel} - Press Keys ${sel} CTRL + Wait Until Page Contains Element ${HOVER_SIGNAL} + Press Keys None CTRL Wait Until Keyword Succeeds 20x 0.5s Page Should Contain Element ${HOVER_BOX} Setup Hover Test diff --git a/atest/examples/Hover.ipynb b/atest/examples/Hover.ipynb index 8b93c44b2..909d6333a 100644 --- a/atest/examples/Hover.ipynb +++ b/atest/examples/Hover.ipynb @@ -7,7 +7,7 @@ "outputs": [], "source": [ "def python_add(a: int, b: int):\n", - " \"\"\"documentation\"\"\"\n", + " \"\"\"Add documentation\"\"\"\n", " return\n", "\n", "\n", diff --git a/packages/jupyterlab-lsp/schema/hover.json b/packages/jupyterlab-lsp/schema/hover.json index 2195f6ea5..9bf588b63 100644 --- a/packages/jupyterlab-lsp/schema/hover.json +++ b/packages/jupyterlab-lsp/schema/hover.json @@ -12,11 +12,17 @@ "default": "Control", "description": "Keyboard key which activates the tooltip on hover." }, - "debouncerDelay": { - "title": "Debouncer delay", + "throttlerDelay": { + "title": "Throttler delay", "type": "number", "default": 50, "description": "Number of milliseconds to delay sending out the request hover request to the language server; you can get better responsiveness adjusting this value, but setting it to zero can actually slow it down as the server might get overwhelmed when moving the mose over the code." + }, + "cacheSize": { + "title": "Cache size", + "type": "number", + "default": 25, + "description": "Up to how many hover responses should be cached at any given time. The cache being is invalidated after any change in the editor." } }, "jupyter.lab.shortcuts": [] diff --git a/packages/jupyterlab-lsp/src/adapter_manager.ts b/packages/jupyterlab-lsp/src/adapter_manager.ts index d0ba71da3..cc103b47f 100644 --- a/packages/jupyterlab-lsp/src/adapter_manager.ts +++ b/packages/jupyterlab-lsp/src/adapter_manager.ts @@ -125,6 +125,7 @@ export class WidgetAdapterManager implements ILSPAdapterManager { widget.context.pathChanged.connect(reconnect); // TODO: maybe emit adapterCreated. Should it be handled by statusbar? + this.refreshAdapterFromCurrentWidget(); } isAnyActive() { diff --git a/packages/jupyterlab-lsp/src/converter.ts b/packages/jupyterlab-lsp/src/converter.ts index 7b4e5a648..537fecf9b 100644 --- a/packages/jupyterlab-lsp/src/converter.ts +++ b/packages/jupyterlab-lsp/src/converter.ts @@ -7,6 +7,10 @@ export class PositionConverter { return { line: position.line, ch: position.character }; } + static cm_to_lsp(position: CodeMirror.Position): lsProtocol.Position { + return { line: position.line, character: position.ch }; + } + static lsp_to_ce(position: lsProtocol.Position): CodeEditor.IPosition { return { line: position.line, column: position.character }; } diff --git a/packages/jupyterlab-lsp/src/features/hover.ts b/packages/jupyterlab-lsp/src/features/hover.ts index 7fdae8a4a..b6ad374a3 100644 --- a/packages/jupyterlab-lsp/src/features/hover.ts +++ b/packages/jupyterlab-lsp/src/features/hover.ts @@ -2,7 +2,7 @@ import { getModifierState, uris_equal } from '../utils'; import { IRootPosition, is_equal, IVirtualPosition } from '../positioning'; import * as lsProtocol from 'vscode-languageserver-protocol'; import * as CodeMirror from 'codemirror'; -import { Debouncer } from '@lumino/polling'; +import { Throttler } from '@lumino/polling'; import { CodeMirrorIntegration, IEditorRange @@ -20,58 +20,141 @@ import { CodeHover as LSPHoverSettings, ModifierKey } from '../_hover'; import { LabIcon } from '@jupyterlab/ui-components'; import hoverSvg from '../../style/icons/hover.svg'; +import { IEditorChange } from '../virtual/editor'; +import { CodeEditor } from '@jupyterlab/codeeditor'; +import { PositionConverter } from '../converter'; export const hoverIcon = new LabIcon({ name: 'lsp:hover', svgstr: hoverSvg }); +interface IResponseData { + response: lsProtocol.Hover; + uri: string; + editor_range: IEditorRange; + ce_editor: CodeEditor.IEditor; +} + +class ResponseCache { + protected _data: Array; + get data() { + return this._data; + } + + constructor(public maxSize: number) { + this._data = []; + } + + store(item: IResponseData) { + if (this._data.length >= this.maxSize) { + this._data.shift(); + } + this._data.push(item); + } + + clean() { + this._data = []; + } +} + +function to_markup( + content: string | lsProtocol.MarkedString +): lsProtocol.MarkupContent { + if (typeof content === 'string') { + // coerce to MarkedString object + return { + kind: 'plaintext', + value: content + }; + } else { + return { + kind: 'markdown', + value: '```' + content.language + '\n' + content.value + '\n```' + }; + } +} + export class HoverCM extends CodeMirrorIntegration { - protected hover_character: IRootPosition; + protected last_hover_character: IRootPosition; private last_hover_response: lsProtocol.Hover; - private show_next_tooltip: boolean; - private last_hover_character: CodeMirror.Position; protected hover_marker: CodeMirror.TextMarker; private virtual_position: IVirtualPosition; lab_integration: HoverLabIntegration; settings: FeatureSettings; + protected cache: ResponseCache; - private debounced_get_hover: Debouncer>; + private debounced_get_hover: Throttler>; get modifierKey(): ModifierKey { return this.settings.composite.modifierKey; } + protected restore_from_cache( + root_position: IRootPosition + ): IResponseData | null { + const virtual_position = this.virtual_editor.root_position_to_virtual_position( + root_position + ); + const document_uri = this.virtual_document.document_info.uri; + const matching_items = this.cache.data.filter(cache_item => { + if (!uris_equal(cache_item.uri, document_uri)) { + return false; + } + let range = cache_item.response.range; + let { line, ch } = virtual_position; + return ( + line >= range.start.line && + line <= range.end.line && + (line != range.start.line || ch >= range.start.character) && + (line != range.end.line || ch <= range.end.character) + ); + }); + return matching_items.length != 0 ? matching_items[0] : null; + } + register(): void { + this.cache = new ResponseCache(this.settings.composite.cacheSize); this.wrapper_handlers.set('mousemove', this.handleMouseOver); - this.wrapper_handlers.set( - 'mouseleave', - // TODO: remove_tooltip() but allow the mouse to leave if it enters the tooltip - // (a bit tricky: normally we would just place the tooltip within, but it was designed to be attached to body) - this.remove_range_highlight - ); + this.wrapper_handlers.set('mouseleave', this.remove_range_highlight); // show hover after pressing the modifier key this.wrapper_handlers.set('keydown', (event: KeyboardEvent) => { if ( - (!this.modifierKey || getModifierState(event, this.modifierKey)) && - this.hover_character === this.last_hover_character + getModifierState(event, this.modifierKey) && + typeof this.last_hover_character !== 'undefined' ) { - this.show_next_tooltip = true; - this.handleHover( - this.last_hover_response, - this.virtual_document.document_info.uri - ); + let response_data = this.restore_from_cache(this.last_hover_character); + if (response_data == null) { + return; + } + this.handleResponse(response_data, this.last_hover_character, true); } }); - // TODO: make the debounce rate configurable - this.debounced_get_hover = new Debouncer>( - this.on_hover, - this.settings.composite.debouncerDelay - ); + this.debounced_get_hover = this.create_throttler(); + + this.settings.changed.connect(() => { + this.cache.maxSize = this.settings.composite.cacheSize; + this.debounced_get_hover = this.create_throttler(); + }); super.register(); } - on_hover = async () => { + create_throttler() { + return new Throttler>(this.on_hover, { + limit: this.settings.composite.throttlerDelay, + edge: 'trailing' + }); + } + + afterChange(change: IEditorChange, root_position: IRootPosition) { + super.afterChange(change, root_position); + // reset cache on any change in the document + this.cache.clean(); + this.last_hover_character = undefined; + this.remove_range_highlight(); + } + + protected on_hover = async () => { return await this.connection.getHoverTooltip( this.virtual_position, this.virtual_document.document_info, @@ -96,64 +179,52 @@ export class HoverCM extends CodeMirrorIntegration { return contents as lsProtocol.MarkupContent; } - // now we have MarkedString - let content = contents[0]; - - if (typeof content === 'string') { - // coerce to MarkedString object + let markups = contents.map(to_markup); + if (markups.every(markup => markup.kind == 'plaintext')) { return { kind: 'plaintext', - value: content + value: markups.map(markup => markup.value).join('\n') }; } else { return { kind: 'markdown', - value: '```' + content.language + '\n' + content.value + '\n```' + value: markups.map(markup => markup.value).join('\n\n') }; } } - public handleHover = (response: lsProtocol.Hover, documentUri: string) => { - if (!uris_equal(documentUri, this.virtual_document.document_info.uri)) { - return; + public handleResponse = ( + response_data: IResponseData, + root_position: IRootPosition, + show_tooltip: boolean + ) => { + let response = response_data.response; + + // testing for object equality because the response will likely be reused from cache + if (this.last_hover_response != response) { + this.remove_range_highlight(); + this.hover_marker = this.highlight_range( + response_data.editor_range, + 'cm-lsp-hover-available' + ); } - this.hide_hover(); - this.last_hover_character = null; - this.last_hover_response = null; - if ( - !this.hover_character || - !response || - !response.contents || - (Array.isArray(response.contents) && response.contents.length === 0) - ) { - return; - } + this.last_hover_response = response; - this.hover_marker = this.highlight_range( - this.editor_range_for_hover(response.range), - 'cm-lsp-hover-available' - ); + if (show_tooltip) { + this.lab_integration.tooltip.remove(); + const markup = HoverCM.get_markup_for_hover(response); + let editor_position = this.virtual_editor.root_position_to_editor( + root_position + ); - if (!this.show_next_tooltip) { - this.last_hover_response = response; - this.last_hover_character = this.hover_character; - return; + this.lab_integration.tooltip.create({ + markup, + position: editor_position, + ce_editor: response_data.ce_editor, + className: 'lsp-hover' + }); } - - const markup = HoverCM.get_markup_for_hover(response); - let root_position = this.hover_character; - let cm_editor = this.get_cm_editor(root_position); - let editor_position = this.virtual_editor.root_position_to_editor( - root_position - ); - - this.lab_integration.tooltip.create({ - markup, - position: editor_position, - ce_editor: this.virtual_editor.find_ce_editor(cm_editor), - className: 'lsp-hover' - }); }; protected is_token_empty(token: CodeMirror.Token) { @@ -166,7 +237,17 @@ export class HoverCM extends CodeMirrorIntegration { return target.closest('.CodeMirror-sizer') != null; } - public async _handleMouseOver(event: MouseEvent) { + protected is_useful_response(response: lsProtocol.Hover) { + return ( + response && + response.contents && + !(Array.isArray(response.contents) && response.contents.length === 0) + ); + } + + protected async _handleMouseOver(event: MouseEvent) { + const show_tooltip = getModifierState(event, this.modifierKey); + // currently the events are coming from notebook panel; ideally these would be connected to individual cells, // (only cells with code) instead, but this is more complex to implement right. In any case filtering // is needed to determine in hovered character belongs to this virtual document @@ -176,8 +257,7 @@ export class HoverCM extends CodeMirrorIntegration { // happens because mousemove is attached to panel, not individual code cells, // and because some regions of the editor (between lines) have no characters if (root_position == null) { - // this.remove_range_highlight(); - this.hover_character = null; + this.remove_range_highlight(); return; } @@ -193,24 +273,56 @@ export class HoverCM extends CodeMirrorIntegration { document !== this.virtual_document || !this.is_event_inside_visible(event) ) { - // this.remove_range_highlight(); - this.hover_character = null; + this.remove_range_highlight(); return; } - if (!is_equal(root_position, this.hover_character)) { - this.hover_character = root_position; + if (!is_equal(root_position, this.last_hover_character)) { this.virtual_position = virtual_position; - const hover = await this.debounced_get_hover.invoke(); - this.handleHover(hover, this.virtual_document.document_info.uri); + this.last_hover_character = root_position; + + let response_data = this.restore_from_cache(root_position); + + if (response_data == null) { + let response = await this.debounced_get_hover.invoke(); + if (this.is_useful_response(response)) { + let ce_editor = this.virtual_editor.get_editor_at_root_position( + root_position + ); + let cm_editor = this.virtual_editor.ce_editor_to_cm_editor.get( + ce_editor + ); + + let editor_range = this.get_editor_range( + response, + root_position, + token, + cm_editor + ); + + response_data = { + response: this.add_range_if_needed( + response, + editor_range, + ce_editor + ), + uri: this.virtual_document.document_info.uri, + editor_range: editor_range, + ce_editor: ce_editor + }; + + this.cache.store(response_data); + } else { + this.remove_range_highlight(); + return; + } + } + + this.handleResponse(response_data, root_position, show_tooltip); } } - public handleMouseOver = (event: MouseEvent) => { - // proceed when no hover modifier or hover modifier pressed - this.show_next_tooltip = - !this.modifierKey || getModifierState(event, this.modifierKey); - + protected handleMouseOver = (event: MouseEvent) => { try { return this._handleMouseOver(event); } catch (e) { @@ -225,56 +337,15 @@ export class HoverCM extends CodeMirrorIntegration { } }; - protected editor_range_for_hover(range: lsProtocol.Range): IEditorRange { - let character = this.hover_character; - // NOTE: foreign document ranges are checked before the request is sent, - // no need to to this again here. - - if (range) { - let ce_editor = this.virtual_editor.get_editor_at_root_position( - character - ); - let cm_editor = this.virtual_editor.ce_editor_to_cm_editor.get(ce_editor); - return this.range_to_editor_range(range, cm_editor); - } else { - // construct range manually using the token information - let ce_editor = this.virtual_document.root.get_editor_at_source_line( - character - ); - let cm_editor = this.virtual_editor.ce_editor_to_cm_editor.get(ce_editor); - let token = this.virtual_editor.getTokenAt(character); - - let start_in_root = { - line: character.line, - ch: token.start - } as IRootPosition; - let end_in_root = { - line: character.line, - ch: token.end - } as IRootPosition; - - return { - start: this.virtual_editor.root_position_to_editor(start_in_root), - end: this.virtual_editor.root_position_to_editor(end_in_root), - editor: cm_editor - }; - } - } - - hide_hover() { - this.lab_integration.tooltip.remove(); - this.remove_range_highlight(); - } - protected remove_range_highlight = () => { if (this.hover_marker) { this.hover_marker.clear(); this.hover_marker = null; } - this.last_hover_character = null; }; remove(): void { + this.cache.clean(); this.remove_range_highlight(); this.debounced_get_hover.dispose(); super.remove(); @@ -282,9 +353,63 @@ export class HoverCM extends CodeMirrorIntegration { // just to be sure this.debounced_get_hover = null; this.remove_range_highlight = null; - this.handleHover = null; + this.handleResponse = null; this.on_hover = null; } + + private get_editor_range( + response: lsProtocol.Hover, + position: IRootPosition, + token: CodeMirror.Token, + cm_editor: CodeMirror.Editor + ): IEditorRange { + if (typeof response.range !== 'undefined') { + return this.range_to_editor_range(response.range, cm_editor); + } + + // construct the range manually using the token information + let start_in_root = { + line: position.line, + ch: token.start + } as IRootPosition; + let end_in_root = { + line: position.line, + ch: token.end + } as IRootPosition; + + return { + start: this.virtual_editor.root_position_to_editor(start_in_root), + end: this.virtual_editor.root_position_to_editor(end_in_root), + editor: cm_editor + }; + } + + private add_range_if_needed( + response: lsProtocol.Hover, + editor_range: IEditorRange, + ce_editor: CodeEditor.IEditor + ): lsProtocol.Hover { + if (typeof response.range !== 'undefined') { + return response; + } + return { + ...response, + range: { + start: PositionConverter.cm_to_lsp( + this.virtual_editor.transform_from_editor_to_root( + ce_editor, + editor_range.start + ) + ), + end: PositionConverter.cm_to_lsp( + this.virtual_editor.transform_from_editor_to_root( + ce_editor, + editor_range.end + ) + ) + } + }; + } } class HoverLabIntegration implements IFeatureLabIntegration { From 8ae254bea170a7c8da549efa9ba160898378d6d7 Mon Sep 17 00:00:00 2001 From: krassowski Date: Tue, 15 Sep 2020 04:14:18 +0100 Subject: [PATCH 07/13] Solve cache collisions, clean up state when removing range highlight --- atest/05_Features/Hover.robot | 6 ++- packages/jupyterlab-lsp/src/features/hover.ts | 38 +++++++++++++------ 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/atest/05_Features/Hover.robot b/atest/05_Features/Hover.robot index 7c79b8647..ca747972a 100644 --- a/atest/05_Features/Hover.robot +++ b/atest/05_Features/Hover.robot @@ -26,6 +26,7 @@ Hover works in foreign code (javascript) Element Should Contain ${HOVER_BOX} function js_add(a: any, b: any): any Page Should Contain Element ${HOVER_BOX} code.language-typescript # also for multiple cells of the same document + Enter Cell Editor 3 Hover Over Math Element Should Contain ${HOVER_BOX} const Math: Math @@ -38,9 +39,10 @@ Hover Over Trigger Tooltip [Arguments] ${sel} Wait Until Keyword Succeeds 10 x 0.1 s Mouse Over ${sel} + Click Element ${sel} Wait Until Page Contains Element ${HOVER_SIGNAL} - Press Keys None CTRL - Wait Until Keyword Succeeds 20x 0.5s Page Should Contain Element ${HOVER_BOX} + Press Keys ${sel} CTRL + Wait Until Keyword Succeeds 10x 0.1s Page Should Contain Element ${HOVER_BOX} Setup Hover Test Setup Notebook Python Hover.ipynb diff --git a/packages/jupyterlab-lsp/src/features/hover.ts b/packages/jupyterlab-lsp/src/features/hover.ts index b6ad374a3..09e6cf679 100644 --- a/packages/jupyterlab-lsp/src/features/hover.ts +++ b/packages/jupyterlab-lsp/src/features/hover.ts @@ -1,4 +1,4 @@ -import { getModifierState, uris_equal } from '../utils'; +import { getModifierState } from '../utils'; import { IRootPosition, is_equal, IVirtualPosition } from '../positioning'; import * as lsProtocol from 'vscode-languageserver-protocol'; import * as CodeMirror from 'codemirror'; @@ -23,6 +23,7 @@ import hoverSvg from '../../style/icons/hover.svg'; import { IEditorChange } from '../virtual/editor'; import { CodeEditor } from '@jupyterlab/codeeditor'; import { PositionConverter } from '../converter'; +import { VirtualDocument } from '../virtual/document'; export const hoverIcon = new LabIcon({ name: 'lsp:hover', @@ -31,7 +32,7 @@ export const hoverIcon = new LabIcon({ interface IResponseData { response: lsProtocol.Hover; - uri: string; + document: VirtualDocument; editor_range: IEditorRange; ce_editor: CodeEditor.IEditor; } @@ -91,14 +92,12 @@ export class HoverCM extends CodeMirrorIntegration { } protected restore_from_cache( - root_position: IRootPosition + root_position: IRootPosition, + document: VirtualDocument, + virtual_position: IVirtualPosition ): IResponseData | null { - const virtual_position = this.virtual_editor.root_position_to_virtual_position( - root_position - ); - const document_uri = this.virtual_document.document_info.uri; const matching_items = this.cache.data.filter(cache_item => { - if (!uris_equal(cache_item.uri, document_uri)) { + if (cache_item.document !== document) { return false; } let range = cache_item.response.range; @@ -123,7 +122,17 @@ export class HoverCM extends CodeMirrorIntegration { getModifierState(event, this.modifierKey) && typeof this.last_hover_character !== 'undefined' ) { - let response_data = this.restore_from_cache(this.last_hover_character); + const document = this.virtual_editor.document_at_root_position( + this.last_hover_character + ); + const virtual_position = this.virtual_editor.root_position_to_virtual_position( + this.last_hover_character + ); + let response_data = this.restore_from_cache( + this.last_hover_character, + document, + virtual_position + ); if (response_data == null) { return; } @@ -157,6 +166,7 @@ export class HoverCM extends CodeMirrorIntegration { protected on_hover = async () => { return await this.connection.getHoverTooltip( this.virtual_position, + // this might be wrong - should not it be using the specific virtual document? this.virtual_document.document_info, false ); @@ -281,7 +291,11 @@ export class HoverCM extends CodeMirrorIntegration { this.virtual_position = virtual_position; this.last_hover_character = root_position; - let response_data = this.restore_from_cache(root_position); + let response_data = this.restore_from_cache( + root_position, + document, + virtual_position + ); if (response_data == null) { let response = await this.debounced_get_hover.invoke(); @@ -306,7 +320,7 @@ export class HoverCM extends CodeMirrorIntegration { editor_range, ce_editor ), - uri: this.virtual_document.document_info.uri, + document: document, editor_range: editor_range, ce_editor: ce_editor }; @@ -341,6 +355,8 @@ export class HoverCM extends CodeMirrorIntegration { if (this.hover_marker) { this.hover_marker.clear(); this.hover_marker = null; + this.last_hover_response = undefined; + this.last_hover_character = undefined; } }; From ff513be738566e7358159798d87092c409696906 Mon Sep 17 00:00:00 2001 From: krassowski Date: Wed, 16 Sep 2020 02:33:23 +0100 Subject: [PATCH 08/13] Improve tooltip trigger when editor active, hide on mouseleave, constrain feature events wrappers types --- atest/05_Features/Hover.robot | 16 +- .../src/components/free_tooltip.ts | 12 +- .../src/editor_integration/codemirror.ts | 42 ++++- packages/jupyterlab-lsp/src/features/hover.ts | 144 ++++++++++++------ .../jupyterlab-lsp/src/features/signature.ts | 21 +-- packages/jupyterlab-lsp/src/utils.ts | 29 ++-- .../src/virtual/codemirror_editor.ts | 12 +- 7 files changed, 187 insertions(+), 89 deletions(-) diff --git a/atest/05_Features/Hover.robot b/atest/05_Features/Hover.robot index ca747972a..2529da477 100644 --- a/atest/05_Features/Hover.robot +++ b/atest/05_Features/Hover.robot @@ -16,8 +16,12 @@ Hover works in notebooks Element Text Should Be ${HOVER_SIGNAL} python_add Capture Page Screenshot 02-hover-shown.png Element Should Contain ${HOVER_BOX} python_add(a: int, b: int) + # syntax highlight should work Page Should Contain Element ${HOVER_BOX} code.language-python + # testing multi-element responses Element Should Contain ${HOVER_BOX} Add documentation + # it should be possible to move the mouse over the tooltip in order to copy/scroll + Mouse Over ${HOVER_BOX} Hover works in foreign code (javascript) Enter Cell Editor 2 @@ -25,6 +29,10 @@ Hover works in foreign code (javascript) Capture Page Screenshot 02-hover-shown.png Element Should Contain ${HOVER_BOX} function js_add(a: any, b: any): any Page Should Contain Element ${HOVER_BOX} code.language-typescript + # should be hidden once moving the mouse away + Mouse Over ${STATUSBAR} + Page Should Not Contain Element ${HOVER_BOX} + Page Should Not Contain Element ${HOVER_SIGNAL} # also for multiple cells of the same document Enter Cell Editor 3 Hover Over Math @@ -34,15 +42,15 @@ Hover works in foreign code (javascript) Hover Over [Arguments] ${symbol} ${sel} = Set Variable If "${symbol}".startswith(("xpath", "css")) ${symbol} xpath:(//span[@role="presentation"][contains(., "${symbol}")])[last()] - Wait Until Keyword Succeeds 10 x 0.1 s Trigger Tooltip ${sel} + Wait Until Keyword Succeeds 2x 0.1 s Trigger Tooltip ${sel} Trigger Tooltip [Arguments] ${sel} - Wait Until Keyword Succeeds 10 x 0.1 s Mouse Over ${sel} - Click Element ${sel} + Mouse Over ${sel} Wait Until Page Contains Element ${HOVER_SIGNAL} + Click Element ${sel} Press Keys ${sel} CTRL - Wait Until Keyword Succeeds 10x 0.1s Page Should Contain Element ${HOVER_BOX} + Wait Until Keyword Succeeds 2x 0.1s Page Should Contain Element ${HOVER_BOX} Setup Hover Test Setup Notebook Python Hover.ipynb diff --git a/packages/jupyterlab-lsp/src/components/free_tooltip.ts b/packages/jupyterlab-lsp/src/components/free_tooltip.ts index 6085cd845..e0dff522e 100644 --- a/packages/jupyterlab-lsp/src/components/free_tooltip.ts +++ b/packages/jupyterlab-lsp/src/components/free_tooltip.ts @@ -9,7 +9,8 @@ import { IEditorPosition } from '../positioning'; import { PositionConverter } from '../converter'; import { Widget } from '@lumino/widgets'; import { IRenderMimeRegistry } from '@jupyterlab/rendermime'; -import { ILSPAdapterManager } from '../tokens'; +import { WidgetAdapter } from '../adapters/adapter'; +import { IDocumentWidget } from '@jupyterlab/docregistry'; const MIN_HEIGHT = 20; const MAX_HEIGHT = 250; @@ -95,6 +96,7 @@ export namespace EditorTooltip { markup: lsProtocol.MarkupContent; ce_editor: CodeEditor.IEditor; position: IEditorPosition; + adapter: WidgetAdapter; className?: string; } } @@ -102,15 +104,11 @@ export namespace EditorTooltip { export class EditorTooltipManager { private currentTooltip: FreeTooltip = null; - constructor( - private rendermime_registry: IRenderMimeRegistry, - private adapterManager: ILSPAdapterManager - ) {} + constructor(private rendermime_registry: IRenderMimeRegistry) {} create(options: EditorTooltip.IOptions): FreeTooltip { this.remove(); - let { markup, position } = options; - let adapter = this.adapterManager.currentAdapter; + let { markup, position, adapter } = options; let widget = adapter.widget; const bundle = markup.kind === 'plaintext' diff --git a/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts b/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts index 70f6f59b2..e4f3a4f7d 100644 --- a/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts +++ b/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts @@ -55,6 +55,35 @@ export interface IEditOutcome { errors: string[]; } +/** + * Interface for storage of HTMLElement event specifications (event name + handler). + */ +interface IHTMLEventMap< + T extends keyof HTMLElementEventMap = keyof HTMLElementEventMap +> extends Map void> { + set( + k: E, + handler: (event: HTMLElementEventMap[E]) => void + ): this; + get(k: E): (event: HTMLElementEventMap[E]) => void; +} + +type CodeMirrorEventName = + | CodeMirror.DOMEvent + | 'change' + | 'changes' + | 'beforeChange' + | 'cursorActivity' + | 'beforeSelectionChange' + | 'viewportChange' + | 'gutterClick' + | 'focus' + | 'blur' + | 'scroll' + | 'update' + | 'renderLine' + | 'overwriteToggle'; + /** * One feature of each type exists per VirtualDocument * (the initialization is performed by the adapter). @@ -64,9 +93,16 @@ export abstract class CodeMirrorIntegration is_registered: boolean; feature: IFeature; - protected readonly editor_handlers: Map; - protected readonly connection_handlers: Map; - protected readonly wrapper_handlers: Map; + protected readonly editor_handlers: Map< + CodeMirrorEventName, + CodeMirrorHandler + >; + // TODO use better type constraints for connection event names and for responses + protected readonly connection_handlers: Map< + string, + (response: object) => void + >; + protected readonly wrapper_handlers: IHTMLEventMap; protected wrapper: HTMLElement; protected virtual_editor: CodeMirrorVirtualEditor; diff --git a/packages/jupyterlab-lsp/src/features/hover.ts b/packages/jupyterlab-lsp/src/features/hover.ts index 09e6cf679..36d85afc5 100644 --- a/packages/jupyterlab-lsp/src/features/hover.ts +++ b/packages/jupyterlab-lsp/src/features/hover.ts @@ -8,13 +8,13 @@ import { IEditorRange } from '../editor_integration/codemirror'; import { FeatureSettings, IFeatureLabIntegration } from '../feature'; -import { EditorTooltipManager } from '../components/free_tooltip'; +import { EditorTooltipManager, FreeTooltip } from '../components/free_tooltip'; import { JupyterFrontEnd, JupyterFrontEndPlugin } from '@jupyterlab/application'; import { IRenderMimeRegistry } from '@jupyterlab/rendermime'; -import { ILSPAdapterManager, ILSPFeatureManager, PLUGIN_ID } from '../tokens'; +import { ILSPFeatureManager, PLUGIN_ID } from '../tokens'; import { ISettingRegistry } from '@jupyterlab/settingregistry'; import { CodeHover as LSPHoverSettings, ModifierKey } from '../_hover'; import { LabIcon } from '@jupyterlab/ui-components'; @@ -86,8 +86,9 @@ export class HoverCM extends CodeMirrorIntegration { protected cache: ResponseCache; private debounced_get_hover: Throttler>; + private tooltip: FreeTooltip; - get modifierKey(): ModifierKey { + protected get modifierKey(): ModifierKey { return this.settings.composite.modifierKey; } @@ -114,31 +115,34 @@ export class HoverCM extends CodeMirrorIntegration { register(): void { this.cache = new ResponseCache(this.settings.composite.cacheSize); - this.wrapper_handlers.set('mousemove', this.handleMouseOver); - this.wrapper_handlers.set('mouseleave', this.remove_range_highlight); - // show hover after pressing the modifier key - this.wrapper_handlers.set('keydown', (event: KeyboardEvent) => { - if ( - getModifierState(event, this.modifierKey) && - typeof this.last_hover_character !== 'undefined' - ) { - const document = this.virtual_editor.document_at_root_position( - this.last_hover_character - ); - const virtual_position = this.virtual_editor.root_position_to_virtual_position( - this.last_hover_character - ); - let response_data = this.restore_from_cache( - this.last_hover_character, - document, - virtual_position - ); - if (response_data == null) { - return; - } - this.handleResponse(response_data, this.last_hover_character, true); - } + + this.wrapper_handlers.set('mousemove', event => { + // as CodeMirror.Editor does not support mouseleave nor mousemove, + // we simulate the mouseleave for the editor in wrapper's mousemove; + // this is used to hide the tooltip on leaving cells in notebook + this.updateUnderlineAndTooltip(event) + .then(keep_tooltip => { + if (!keep_tooltip) { + this.maybeHideTooltip(event.target); + } + }) + .catch(console.warn); }); + this.wrapper_handlers.set('mouseleave', this.onMouseLeave); + + // show hover after pressing the modifier key + // TODO: when the editor (notebook or file editor) is not focused, the keydown event is not getting to us + // (probably getting captured by lab); this gives subpar experience when using hover in two editors open + // side-by-side, BUT this does not happen for mousemove which properly reads keyModifier from the event + // (so this is no too bad as most of the time the user will get the desired outcome - they just need to + // budge the mice when holding ctrl if looking at a document which is not active). + // whether the editor is focused + this.wrapper_handlers.set('keydown', this.onKeyDown); + // or just the wrapper (e.g. the notebook but no cell active) + this.editor_handlers.set('keydown', (instance, event: KeyboardEvent) => + this.onKeyDown(event) + ); + this.debounced_get_hover = this.create_throttler(); this.settings.changed.connect(() => { @@ -148,7 +152,45 @@ export class HoverCM extends CodeMirrorIntegration { super.register(); } - create_throttler() { + protected onKeyDown = (event: KeyboardEvent) => { + if ( + getModifierState(event, this.modifierKey) && + typeof this.last_hover_character !== 'undefined' + ) { + const document = this.virtual_editor.document_at_root_position( + this.last_hover_character + ); + const virtual_position = this.virtual_editor.root_position_to_virtual_position( + this.last_hover_character + ); + let response_data = this.restore_from_cache( + this.last_hover_character, + document, + virtual_position + ); + if (response_data == null) { + return; + } + event.stopPropagation(); + this.handleResponse(response_data, this.last_hover_character, true); + } + }; + + protected onMouseLeave = (event: MouseEvent) => { + this.remove_range_highlight(); + this.maybeHideTooltip(event.relatedTarget); + }; + + protected maybeHideTooltip(mouse_target: EventTarget) { + if ( + typeof this.tooltip !== 'undefined' && + mouse_target !== this.tooltip.node + ) { + this.tooltip.dispose(); + } + } + + protected create_throttler() { return new Throttler>(this.on_hover, { limit: this.settings.composite.throttlerDelay, edge: 'trailing' @@ -203,11 +245,17 @@ export class HoverCM extends CodeMirrorIntegration { } } + /** + * Underlines the word if a tooltip is available. + * Displays tooltip if asked to do so. + * + * Returns true is the tooltip was shown. + */ public handleResponse = ( response_data: IResponseData, root_position: IRootPosition, show_tooltip: boolean - ) => { + ): boolean => { let response = response_data.response; // testing for object equality because the response will likely be reused from cache @@ -228,13 +276,16 @@ export class HoverCM extends CodeMirrorIntegration { root_position ); - this.lab_integration.tooltip.create({ + this.tooltip = this.lab_integration.tooltip.create({ markup, position: editor_position, ce_editor: response_data.ce_editor, + adapter: this.adapter, className: 'lsp-hover' }); + return true; } + return false; }; protected is_token_empty(token: CodeMirror.Token) { @@ -255,7 +306,12 @@ export class HoverCM extends CodeMirrorIntegration { ); } - protected async _handleMouseOver(event: MouseEvent) { + /** + * Returns true if the tooltip should stay. + */ + protected async _updateUnderlineAndTooltip( + event: MouseEvent + ): Promise { const show_tooltip = getModifierState(event, this.modifierKey); // currently the events are coming from notebook panel; ideally these would be connected to individual cells, @@ -332,13 +388,15 @@ export class HoverCM extends CodeMirrorIntegration { } } - this.handleResponse(response_data, root_position, show_tooltip); + return this.handleResponse(response_data, root_position, show_tooltip); + } else { + return true; } } - protected handleMouseOver = (event: MouseEvent) => { + protected updateUnderlineAndTooltip = (event: MouseEvent) => { try { - return this._handleMouseOver(event); + return this._updateUnderlineAndTooltip(event); } catch (e) { if ( !( @@ -435,10 +493,9 @@ class HoverLabIntegration implements IFeatureLabIntegration { constructor( app: JupyterFrontEnd, settings: FeatureSettings, - renderMimeRegistry: IRenderMimeRegistry, - adapterManager: ILSPAdapterManager + renderMimeRegistry: IRenderMimeRegistry ) { - this.tooltip = new EditorTooltipManager(renderMimeRegistry, adapterManager); + this.tooltip = new EditorTooltipManager(renderMimeRegistry); } } @@ -446,26 +503,19 @@ const FEATURE_ID = PLUGIN_ID + ':hover'; export const HOVER_PLUGIN: JupyterFrontEndPlugin = { id: FEATURE_ID, - requires: [ - ILSPFeatureManager, - ISettingRegistry, - IRenderMimeRegistry, - ILSPAdapterManager - ], + requires: [ILSPFeatureManager, ISettingRegistry, IRenderMimeRegistry], autoStart: true, activate: ( app: JupyterFrontEnd, featureManager: ILSPFeatureManager, settingRegistry: ISettingRegistry, - renderMimeRegistry: IRenderMimeRegistry, - adapterManager: ILSPAdapterManager + renderMimeRegistry: IRenderMimeRegistry ) => { const settings = new FeatureSettings(settingRegistry, FEATURE_ID); const labIntegration = new HoverLabIntegration( app, settings, - renderMimeRegistry, - adapterManager + renderMimeRegistry ); featureManager.register({ diff --git a/packages/jupyterlab-lsp/src/features/signature.ts b/packages/jupyterlab-lsp/src/features/signature.ts index 0adc5de3a..3c54008f4 100644 --- a/packages/jupyterlab-lsp/src/features/signature.ts +++ b/packages/jupyterlab-lsp/src/features/signature.ts @@ -6,7 +6,7 @@ import { JupyterFrontEnd, JupyterFrontEndPlugin } from '@jupyterlab/application'; -import { ILSPAdapterManager, ILSPFeatureManager, PLUGIN_ID } from '../tokens'; +import { ILSPFeatureManager, PLUGIN_ID } from '../tokens'; import { ISettingRegistry } from '@jupyterlab/settingregistry'; import { IRenderMimeRegistry } from '@jupyterlab/rendermime'; import { EditorTooltipManager } from '../components/free_tooltip'; @@ -116,6 +116,7 @@ export class SignatureCM extends CodeMirrorIntegration { markup, position: editor_position, ce_editor: this.virtual_editor.find_ce_editor(cm_editor), + adapter: this.adapter, className: 'lsp-signature-help' }); } @@ -163,10 +164,9 @@ class SignatureLabIntegration implements IFeatureLabIntegration { constructor( app: JupyterFrontEnd, settings: FeatureSettings, - renderMimeRegistry: IRenderMimeRegistry, - adapterManager: ILSPAdapterManager + renderMimeRegistry: IRenderMimeRegistry ) { - this.tooltip = new EditorTooltipManager(renderMimeRegistry, adapterManager); + this.tooltip = new EditorTooltipManager(renderMimeRegistry); } } @@ -174,26 +174,19 @@ const FEATURE_ID = PLUGIN_ID + ':signature'; export const SIGNATURE_PLUGIN: JupyterFrontEndPlugin = { id: FEATURE_ID, - requires: [ - ILSPFeatureManager, - ISettingRegistry, - IRenderMimeRegistry, - ILSPAdapterManager - ], + requires: [ILSPFeatureManager, ISettingRegistry, IRenderMimeRegistry], autoStart: true, activate: ( app: JupyterFrontEnd, featureManager: ILSPFeatureManager, settingRegistry: ISettingRegistry, - renderMimeRegistry: IRenderMimeRegistry, - adapterManager: ILSPAdapterManager + renderMimeRegistry: IRenderMimeRegistry ) => { const settings = new FeatureSettings(settingRegistry, FEATURE_ID); const labIntegration = new SignatureLabIntegration( app, settings, - renderMimeRegistry, - adapterManager + renderMimeRegistry ); featureManager.register({ diff --git a/packages/jupyterlab-lsp/src/utils.ts b/packages/jupyterlab-lsp/src/utils.ts index ad6e2f361..9ae4ffd73 100644 --- a/packages/jupyterlab-lsp/src/utils.ts +++ b/packages/jupyterlab-lsp/src/utils.ts @@ -47,24 +47,31 @@ export function getModifierState( // Full list of modifier keys and mappings to physical keys on different OSes: // https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/getModifierState - if (event.getModifierState !== undefined) { - return event.getModifierState(modifierKey); - } + // the key approach is needed for CodeMirror events which do not set + // *key (e.g. ctrlKey) correctly + const key = (event as KeyboardEvent).key || null; + let value = false; switch (modifierKey) { case 'Shift': - return event.shiftKey; + value = event.shiftKey || key == 'Shift'; + break; case 'Alt': - return event.altKey; + value = event.altKey || key == 'Alt'; + break; case 'Control': - return event.ctrlKey; + value = event.ctrlKey || key == 'Control'; + break; case 'Meta': - return event.metaKey; - default: - console.warn( - `State of the modifier key "${modifierKey}" could not be determined.` - ); + value = event.metaKey || key == 'Meta'; + break; + } + + if (event.getModifierState !== undefined) { + return value || event.getModifierState(modifierKey); } + + return value; } export class DefaultMap extends Map { diff --git a/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts b/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts index 05cc32478..1d0f73194 100644 --- a/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts +++ b/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts @@ -23,7 +23,10 @@ import { IDocumentWidget } from '@jupyterlab/docregistry'; import { JupyterFrontEndPlugin } from '@jupyterlab/application'; import { ILSPVirtualEditorManager, PLUGIN_ID } from '../tokens'; -export type CodeMirrorHandler = (instance: any, ...args: any[]) => void; +export type CodeMirrorHandler = ( + instance: CodeMirrorVirtualEditor, + ...args: any[] +) => void; type WrappedHandler = (instance: CodeMirror.Editor, ...args: any[]) => void; // eslint-disable-next-line @typescript-eslint/ban-ts-ignore @@ -111,7 +114,7 @@ export class CodeMirrorVirtualEditor ) { if (!(prop in target)) { console.warn( - `Unimplemented method ${prop.toString()} for VirtualCodeMirrorEditor` + `Unimplemented method ${prop.toString()} for CodeMirrorVirtualEditor` ); return; } else { @@ -202,7 +205,10 @@ export class CodeMirrorVirtualEditor this.on('change', this.emit_change.bind(this)); } - private emit_change(doc: CodeMirror.Doc, change: CodeMirror.EditorChange) { + private emit_change( + instance: CodeMirrorVirtualEditor, + change: CodeMirror.EditorChange + ) { this.change.emit(change as IEditorChange); } From 01aceff9a5409e59d209d33d5bb25323b882eb6b Mon Sep 17 00:00:00 2001 From: krassowski Date: Thu, 17 Sep 2020 23:17:39 +0100 Subject: [PATCH 09/13] Fix tests using action chains in custom SeleniumLibrary extension --- atest/05_Features/Hover.robot | 10 ++++++---- atest/mouse_over_with_modifier.py | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 atest/mouse_over_with_modifier.py diff --git a/atest/05_Features/Hover.robot b/atest/05_Features/Hover.robot index 2529da477..397a7592c 100644 --- a/atest/05_Features/Hover.robot +++ b/atest/05_Features/Hover.robot @@ -4,6 +4,7 @@ Test Setup Setup Hover Test Test Teardown Clean Up After Working With File Hover.ipynb Force Tags feature:hover Resource ../Keywords.robot +Library ../mouse_over_with_modifier.py *** Variables *** ${HOVER_BOX} css:.lsp-hover @@ -42,15 +43,16 @@ Hover works in foreign code (javascript) Hover Over [Arguments] ${symbol} ${sel} = Set Variable If "${symbol}".startswith(("xpath", "css")) ${symbol} xpath:(//span[@role="presentation"][contains(., "${symbol}")])[last()] - Wait Until Keyword Succeeds 2x 0.1 s Trigger Tooltip ${sel} + Wait Until Keyword Succeeds 3x 0.1 s Trigger Tooltip ${sel} Trigger Tooltip [Arguments] ${sel} + # bring the cursor to the element Mouse Over ${sel} + # move it back and forth (wiggle) while hodling the ctrl modifier + Mouse Over With Control ${sel} x_wiggle=5 Wait Until Page Contains Element ${HOVER_SIGNAL} - Click Element ${sel} - Press Keys ${sel} CTRL - Wait Until Keyword Succeeds 2x 0.1s Page Should Contain Element ${HOVER_BOX} + Wait Until Keyword Succeeds 3x 0.1s Page Should Contain Element ${HOVER_BOX} Setup Hover Test Setup Notebook Python Hover.ipynb diff --git a/atest/mouse_over_with_modifier.py b/atest/mouse_over_with_modifier.py new file mode 100644 index 000000000..bcf37f833 --- /dev/null +++ b/atest/mouse_over_with_modifier.py @@ -0,0 +1,18 @@ +from robot.api import logger +from robot.libraries.BuiltIn import BuiltIn +from selenium.webdriver import ActionChains +from selenium.webdriver.common.keys import Keys +from SeleniumLibrary import SeleniumLibrary + + +def mouse_over_with_control(locator, x_wiggle=0): + logger.info("Getting currently open browser desired capabilities") + sl: SeleniumLibrary = BuiltIn().get_library_instance("SeleniumLibrary") + action_chains = ActionChains(sl.driver) + action_chains.key_down(Keys.CONTROL) + action_chains.move_to_element(sl.find_element(locator)) + if x_wiggle: + action_chains.move_by_offset(xoffset=x_wiggle, yoffset=0) + action_chains.move_by_offset(xoffset=-x_wiggle, yoffset=0) + action_chains.key_up(Keys.CONTROL) + return action_chains.perform() From 752149c84ed3edf2bc23279ed3946c0fb82cc6b8 Mon Sep 17 00:00:00 2001 From: krassowski Date: Fri, 18 Sep 2020 01:36:13 +0100 Subject: [PATCH 10/13] Fix cache, do not trigger: from key if already shown, line background, add test for key-based trigger, removed unused variables, return early if possible, move unpacking before loop --- atest/05_Features/Hover.robot | 36 +++++++++--- ...th_modifier.py => mouse_over_extension.py} | 20 +++++-- packages/jupyterlab-lsp/src/features/hover.ts | 55 ++++++++++++------- 3 files changed, 77 insertions(+), 34 deletions(-) rename atest/{mouse_over_with_modifier.py => mouse_over_extension.py} (64%) diff --git a/atest/05_Features/Hover.robot b/atest/05_Features/Hover.robot index 397a7592c..557657615 100644 --- a/atest/05_Features/Hover.robot +++ b/atest/05_Features/Hover.robot @@ -4,7 +4,7 @@ Test Setup Setup Hover Test Test Teardown Clean Up After Working With File Hover.ipynb Force Tags feature:hover Resource ../Keywords.robot -Library ../mouse_over_with_modifier.py +Library ../mouse_over_extension.py *** Variables *** ${HOVER_BOX} css:.lsp-hover @@ -13,7 +13,7 @@ ${HOVER_SIGNAL} css:.cm-lsp-hover-available *** Test Cases *** Hover works in notebooks Enter Cell Editor 1 - Hover Over python_add + Trigger Tooltip python_add Element Text Should Be ${HOVER_SIGNAL} python_add Capture Page Screenshot 02-hover-shown.png Element Should Contain ${HOVER_BOX} python_add(a: int, b: int) @@ -24,9 +24,14 @@ Hover works in notebooks # it should be possible to move the mouse over the tooltip in order to copy/scroll Mouse Over ${HOVER_BOX} +Hover can be triggered via modifier key once cursor stopped moving + Enter Cell Editor 1 + ${element} = Last Occurrence python_add + Wait Until Keyword Succeeds 4x 0.1 s Trigger Via Modifier Key Press ${element} + Hover works in foreign code (javascript) Enter Cell Editor 2 - Hover Over js_add + Trigger Tooltip js_add Capture Page Screenshot 02-hover-shown.png Element Should Contain ${HOVER_BOX} function js_add(a: any, b: any): any Page Should Contain Element ${HOVER_BOX} code.language-typescript @@ -36,23 +41,38 @@ Hover works in foreign code (javascript) Page Should Not Contain Element ${HOVER_SIGNAL} # also for multiple cells of the same document Enter Cell Editor 3 - Hover Over Math + Trigger Tooltip Math Element Should Contain ${HOVER_BOX} const Math: Math *** Keywords *** -Hover Over +Last Occurrence [Arguments] ${symbol} ${sel} = Set Variable If "${symbol}".startswith(("xpath", "css")) ${symbol} xpath:(//span[@role="presentation"][contains(., "${symbol}")])[last()] - Wait Until Keyword Succeeds 3x 0.1 s Trigger Tooltip ${sel} + [Return] ${sel} -Trigger Tooltip +Trigger Via Hover With Modifier [Arguments] ${sel} # bring the cursor to the element Mouse Over ${sel} # move it back and forth (wiggle) while hodling the ctrl modifier Mouse Over With Control ${sel} x_wiggle=5 Wait Until Page Contains Element ${HOVER_SIGNAL} - Wait Until Keyword Succeeds 3x 0.1s Page Should Contain Element ${HOVER_BOX} + Wait Until Keyword Succeeds 4x 0.1s Page Should Contain Element ${HOVER_BOX} + +Trigger Via Modifier Key Press + [Arguments] ${sel} + # bring the cursor to the element + Mouse Over ${sel} + Wait Until Page Contains Element ${HOVER_SIGNAL} + Mouse Over And Wiggle ${sel} 5 + Press Keys ${sel} CTRL + Wait Until Keyword Succeeds 4x 0.1s Page Should Contain Element ${HOVER_BOX} + +Trigger Tooltip + [Arguments] ${symbol} + [Documentation] The default way to trigger the hover tooltip + ${sel} = Last Occurrence ${symbol} + Wait Until Keyword Succeeds 4x 0.1 s Trigger Via Hover With Modifier ${sel} Setup Hover Test Setup Notebook Python Hover.ipynb diff --git a/atest/mouse_over_with_modifier.py b/atest/mouse_over_extension.py similarity index 64% rename from atest/mouse_over_with_modifier.py rename to atest/mouse_over_extension.py index bcf37f833..cfb867454 100644 --- a/atest/mouse_over_with_modifier.py +++ b/atest/mouse_over_extension.py @@ -1,18 +1,28 @@ -from robot.api import logger from robot.libraries.BuiltIn import BuiltIn from selenium.webdriver import ActionChains from selenium.webdriver.common.keys import Keys from SeleniumLibrary import SeleniumLibrary +def wiggle(action_chains, x_wiggle): + if x_wiggle: + action_chains.move_by_offset(xoffset=x_wiggle, yoffset=0) + action_chains.move_by_offset(xoffset=-x_wiggle, yoffset=0) + + def mouse_over_with_control(locator, x_wiggle=0): - logger.info("Getting currently open browser desired capabilities") sl: SeleniumLibrary = BuiltIn().get_library_instance("SeleniumLibrary") action_chains = ActionChains(sl.driver) action_chains.key_down(Keys.CONTROL) action_chains.move_to_element(sl.find_element(locator)) - if x_wiggle: - action_chains.move_by_offset(xoffset=x_wiggle, yoffset=0) - action_chains.move_by_offset(xoffset=-x_wiggle, yoffset=0) + wiggle(action_chains, x_wiggle) action_chains.key_up(Keys.CONTROL) return action_chains.perform() + + +def mouse_over_and_wiggle(locator, x_wiggle=5): + sl: SeleniumLibrary = BuiltIn().get_library_instance("SeleniumLibrary") + action_chains = ActionChains(sl.driver) + action_chains.move_to_element(sl.find_element(locator)) + wiggle(action_chains, x_wiggle) + return action_chains.perform() diff --git a/packages/jupyterlab-lsp/src/features/hover.ts b/packages/jupyterlab-lsp/src/features/hover.ts index 36d85afc5..4f80952f7 100644 --- a/packages/jupyterlab-lsp/src/features/hover.ts +++ b/packages/jupyterlab-lsp/src/features/hover.ts @@ -93,16 +93,15 @@ export class HoverCM extends CodeMirrorIntegration { } protected restore_from_cache( - root_position: IRootPosition, document: VirtualDocument, virtual_position: IVirtualPosition ): IResponseData | null { + const { line, ch } = virtual_position; const matching_items = this.cache.data.filter(cache_item => { if (cache_item.document !== document) { return false; } let range = cache_item.response.range; - let { line, ch } = virtual_position; return ( line >= range.start.line && line <= range.end.line && @@ -110,6 +109,13 @@ export class HoverCM extends CodeMirrorIntegration { (line != range.end.line || ch <= range.end.character) ); }); + if (matching_items.length > 1) { + console.warn( + 'LSP: Potential hover cache malfunction: ', + virtual_position, + matching_items + ); + } return matching_items.length != 0 ? matching_items[0] : null; } @@ -157,16 +163,16 @@ export class HoverCM extends CodeMirrorIntegration { getModifierState(event, this.modifierKey) && typeof this.last_hover_character !== 'undefined' ) { + // does not need to be shown if it is already visible (otherwise we would be creating an identical tooltip again!) + if (this.tooltip && this.tooltip.isVisible && !this.tooltip.isDisposed) { + return; + } const document = this.virtual_editor.document_at_root_position( this.last_hover_character ); - const virtual_position = this.virtual_editor.root_position_to_virtual_position( - this.last_hover_character - ); let response_data = this.restore_from_cache( - this.last_hover_character, document, - virtual_position + this.virtual_position ); if (response_data == null) { return; @@ -312,6 +318,13 @@ export class HoverCM extends CodeMirrorIntegration { protected async _updateUnderlineAndTooltip( event: MouseEvent ): Promise { + const target = event.target as HTMLElement; + + // if over an empty space in a line (and not over a token) then not worth checking + if (target.classList.contains('CodeMirror-line')) { + return; + } + const show_tooltip = getModifierState(event, this.modifierKey); // currently the events are coming from notebook panel; ideally these would be connected to individual cells, @@ -330,9 +343,6 @@ export class HoverCM extends CodeMirrorIntegration { let token = this.virtual_editor.getTokenAt(root_position); let document = this.virtual_editor.document_at_root_position(root_position); - let virtual_position = this.virtual_editor.root_position_to_virtual_position( - root_position - ); if ( this.is_token_empty(token) || @@ -344,14 +354,13 @@ export class HoverCM extends CodeMirrorIntegration { } if (!is_equal(root_position, this.last_hover_character)) { + let virtual_position = this.virtual_editor.root_position_to_virtual_position( + root_position + ); this.virtual_position = virtual_position; this.last_hover_character = root_position; - let response_data = this.restore_from_cache( - root_position, - document, - virtual_position - ); + let response_data = this.restore_from_cache(document, virtual_position); if (response_data == null) { let response = await this.debounced_get_hover.invoke(); @@ -470,15 +479,19 @@ export class HoverCM extends CodeMirrorIntegration { ...response, range: { start: PositionConverter.cm_to_lsp( - this.virtual_editor.transform_from_editor_to_root( - ce_editor, - editor_range.start + this.virtual_editor.root_position_to_virtual_position( + this.virtual_editor.transform_from_editor_to_root( + ce_editor, + editor_range.start + ) ) ), end: PositionConverter.cm_to_lsp( - this.virtual_editor.transform_from_editor_to_root( - ce_editor, - editor_range.end + this.virtual_editor.root_position_to_virtual_position( + this.virtual_editor.transform_from_editor_to_root( + ce_editor, + editor_range.end + ) ) ) } From 67abb1ee8e3a4932f13492cc4b830246a85c6029 Mon Sep 17 00:00:00 2001 From: krassowski Date: Fri, 18 Sep 2020 01:43:33 +0100 Subject: [PATCH 11/13] Update changelog --- CHANGELOG.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09463dda9..60ce251fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,18 +1,24 @@ ## CHANGELOG -### `@krassowski/jupyterlab-lsp 2.0.6` (2020-09-15) +### `@krassowski/jupyterlab-lsp 2.0.7` (unreleased) - bug fixes - - fix syntax highlighting of %%language cells slowing down editing in notebooks ([#361]) - fix syntax highlighting in hover tooltips and reduce unnecessary padding and margin ([#363]) - greatly improve performance of hover action ([#363]) - improve support for expanded hovers tooltips using deprecated API ([#363]) - do not hide hover tooltips too eagerly (allowing selecting text/easy scrolling of longer tooltips) ([#363]) -[#361]: https://github.com/krassowski/jupyterlab-lsp/issues/361 [#363]: https://github.com/krassowski/jupyterlab-lsp/issues/363 +### `@krassowski/jupyterlab-lsp 2.0.6` (2020-09-15) + +- bug fixes + + - fix syntax highlighting of %%language cells slowing down editing in notebooks ([#361]) + +[#361]: https://github.com/krassowski/jupyterlab-lsp/issues/361 + ### `@krassowski/jupyterlab-lsp 2.0.5` (2020-09-11) - bug fixes From 411f91b6b38cfc83a97f48e36c2c5318bd081979 Mon Sep 17 00:00:00 2001 From: krassowski Date: Fri, 18 Sep 2020 09:00:02 +0100 Subject: [PATCH 12/13] Remove the highlight when hovering over empty space --- packages/jupyterlab-lsp/src/features/hover.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/jupyterlab-lsp/src/features/hover.ts b/packages/jupyterlab-lsp/src/features/hover.ts index 4f80952f7..d5b652180 100644 --- a/packages/jupyterlab-lsp/src/features/hover.ts +++ b/packages/jupyterlab-lsp/src/features/hover.ts @@ -322,6 +322,7 @@ export class HoverCM extends CodeMirrorIntegration { // if over an empty space in a line (and not over a token) then not worth checking if (target.classList.contains('CodeMirror-line')) { + this.remove_range_highlight(); return; } From ff8951085e99cffefcbf8498b5131d7e83def615 Mon Sep 17 00:00:00 2001 From: krassowski Date: Fri, 18 Sep 2020 09:00:53 +0100 Subject: [PATCH 13/13] Slightly increase test timeouts --- atest/05_Features/Hover.robot | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/atest/05_Features/Hover.robot b/atest/05_Features/Hover.robot index 557657615..9004a12e7 100644 --- a/atest/05_Features/Hover.robot +++ b/atest/05_Features/Hover.robot @@ -27,7 +27,7 @@ Hover works in notebooks Hover can be triggered via modifier key once cursor stopped moving Enter Cell Editor 1 ${element} = Last Occurrence python_add - Wait Until Keyword Succeeds 4x 0.1 s Trigger Via Modifier Key Press ${element} + Wait Until Keyword Succeeds 5x 0.1 s Trigger Via Modifier Key Press ${element} Hover works in foreign code (javascript) Enter Cell Editor 2 @@ -63,7 +63,7 @@ Trigger Via Modifier Key Press [Arguments] ${sel} # bring the cursor to the element Mouse Over ${sel} - Wait Until Page Contains Element ${HOVER_SIGNAL} + Wait Until Page Contains Element ${HOVER_SIGNAL} timeout=10s Mouse Over And Wiggle ${sel} 5 Press Keys ${sel} CTRL Wait Until Keyword Succeeds 4x 0.1s Page Should Contain Element ${HOVER_BOX}