Skip to content

Commit 22e2be7

Browse files
krassowskikrassowski
authored
Merge pull request #363 from krassowski/fix-hover
Fix highlighting in hover tooltips and start testing the hover feature
2 parents 344fc7f + ff89510 commit 22e2be7

File tree

16 files changed

+647
-190
lines changed

16 files changed

+647
-190
lines changed

‎CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,16 @@
11
## CHANGELOG
22

3+
### `@krassowski/jupyterlab-lsp 2.0.7` (unreleased)
4+
5+
- bug fixes
6+
7+
- fix syntax highlighting in hover tooltips and reduce unnecessary padding and margin ([#363])
8+
- greatly improve performance of hover action ([#363])
9+
- improve support for expanded hovers tooltips using deprecated API ([#363])
10+
- do not hide hover tooltips too eagerly (allowing selecting text/easy scrolling of longer tooltips) ([#363])
11+
12+
[#363]: https://github.com/krassowski/jupyterlab-lsp/issues/363
13+
314
### `@krassowski/jupyterlab-lsp 2.0.6` (2020-09-15)
415

516
- bug fixes

‎atest/05_Features/Hover.robot

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
*** Settings ***
2+
Suite Setup Setup Suite For Screenshots hover
3+
Test Setup Setup Hover Test
4+
Test Teardown Clean Up After Working With File Hover.ipynb
5+
Force Tags feature:hover
6+
Resource ../Keywords.robot
7+
Library ../mouse_over_extension.py
8+
9+
*** Variables ***
10+
${HOVER_BOX} css:.lsp-hover
11+
${HOVER_SIGNAL} css:.cm-lsp-hover-available
12+
13+
*** Test Cases ***
14+
Hover works in notebooks
15+
Enter Cell Editor 1
16+
Trigger Tooltip python_add
17+
Element Text Should Be ${HOVER_SIGNAL} python_add
18+
Capture Page Screenshot 02-hover-shown.png
19+
Element Should Contain ${HOVER_BOX} python_add(a: int, b: int)
20+
# syntax highlight should work
21+
Page Should Contain Element ${HOVER_BOX} code.language-python
22+
# testing multi-element responses
23+
Element Should Contain ${HOVER_BOX} Add documentation
24+
# it should be possible to move the mouse over the tooltip in order to copy/scroll
25+
Mouse Over ${HOVER_BOX}
26+
27+
Hover can be triggered via modifier key once cursor stopped moving
28+
Enter Cell Editor 1
29+
${element} = Last Occurrence python_add
30+
Wait Until Keyword Succeeds 5x 0.1 s Trigger Via Modifier Key Press ${element}
31+
32+
Hover works in foreign code (javascript)
33+
Enter Cell Editor 2
34+
Trigger Tooltip js_add
35+
Capture Page Screenshot 02-hover-shown.png
36+
Element Should Contain ${HOVER_BOX} function js_add(a: any, b: any): any
37+
Page Should Contain Element ${HOVER_BOX} code.language-typescript
38+
# should be hidden once moving the mouse away
39+
Mouse Over ${STATUSBAR}
40+
Page Should Not Contain Element ${HOVER_BOX}
41+
Page Should Not Contain Element ${HOVER_SIGNAL}
42+
# also for multiple cells of the same document
43+
Enter Cell Editor 3
44+
Trigger Tooltip Math
45+
Element Should Contain ${HOVER_BOX} const Math: Math
46+
47+
*** Keywords ***
48+
Last Occurrence
49+
[Arguments] ${symbol}
50+
${sel} = Set Variable If "${symbol}".startswith(("xpath", "css")) ${symbol} xpath:(//span[@role="presentation"][contains(., "${symbol}")])[last()]
51+
[Return] ${sel}
52+
53+
Trigger Via Hover With Modifier
54+
[Arguments] ${sel}
55+
# bring the cursor to the element
56+
Mouse Over ${sel}
57+
# move it back and forth (wiggle) while hodling the ctrl modifier
58+
Mouse Over With Control ${sel} x_wiggle=5
59+
Wait Until Page Contains Element ${HOVER_SIGNAL}
60+
Wait Until Keyword Succeeds 4x 0.1s Page Should Contain Element ${HOVER_BOX}
61+
62+
Trigger Via Modifier Key Press
63+
[Arguments] ${sel}
64+
# bring the cursor to the element
65+
Mouse Over ${sel}
66+
Wait Until Page Contains Element ${HOVER_SIGNAL} timeout=10s
67+
Mouse Over And Wiggle ${sel} 5
68+
Press Keys ${sel} CTRL
69+
Wait Until Keyword Succeeds 4x 0.1s Page Should Contain Element ${HOVER_BOX}
70+
71+
Trigger Tooltip
72+
[Arguments] ${symbol}
73+
[Documentation] The default way to trigger the hover tooltip
74+
${sel} = Last Occurrence ${symbol}
75+
Wait Until Keyword Succeeds 4x 0.1 s Trigger Via Hover With Modifier ${sel}
76+
77+
Setup Hover Test
78+
Setup Notebook Python Hover.ipynb

‎atest/examples/Hover.ipynb

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
{
2+
"cells": [
3+
{
4+
"cell_type": "code",
5+
"execution_count": null,
6+
"metadata": {},
7+
"outputs": [],
8+
"source": [
9+
"def python_add(a: int, b: int):\n",
10+
" \"\"\"Add documentation\"\"\"\n",
11+
" return\n",
12+
"\n",
13+
"\n",
14+
"python_add(1, 2)"
15+
]
16+
},
17+
{
18+
"cell_type": "code",
19+
"execution_count": null,
20+
"metadata": {},
21+
"outputs": [],
22+
"source": [
23+
"%%javascript\n",
24+
"function js_add(a, b) {\n",
25+
" return a + b\n",
26+
"}\n",
27+
"\n",
28+
"js_add(1, 2)"
29+
]
30+
},
31+
{
32+
"cell_type": "code",
33+
"execution_count": null,
34+
"metadata": {},
35+
"outputs": [],
36+
"source": [
37+
"%%javascript\n",
38+
"Math"
39+
]
40+
}
41+
],
42+
"metadata": {
43+
"kernelspec": {
44+
"display_name": "Python 3",
45+
"language": "python",
46+
"name": "python3"
47+
},
48+
"language_info": {
49+
"codemirror_mode": {
50+
"name": "ipython",
51+
"version": 3
52+
},
53+
"file_extension": ".py",
54+
"mimetype": "text/x-python",
55+
"name": "python",
56+
"nbconvert_exporter": "python",
57+
"pygments_lexer": "ipython3",
58+
"version": "3.7.5"
59+
}
60+
},
61+
"nbformat": 4,
62+
"nbformat_minor": 4
63+
}

‎atest/mouse_over_extension.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
from robot.libraries.BuiltIn import BuiltIn
2+
from selenium.webdriver import ActionChains
3+
from selenium.webdriver.common.keys import Keys
4+
from SeleniumLibrary import SeleniumLibrary
5+
6+
7+
def wiggle(action_chains, x_wiggle):
8+
if x_wiggle:
9+
action_chains.move_by_offset(xoffset=x_wiggle, yoffset=0)
10+
action_chains.move_by_offset(xoffset=-x_wiggle, yoffset=0)
11+
12+
13+
def mouse_over_with_control(locator, x_wiggle=0):
14+
sl: SeleniumLibrary = BuiltIn().get_library_instance("SeleniumLibrary")
15+
action_chains = ActionChains(sl.driver)
16+
action_chains.key_down(Keys.CONTROL)
17+
action_chains.move_to_element(sl.find_element(locator))
18+
wiggle(action_chains, x_wiggle)
19+
action_chains.key_up(Keys.CONTROL)
20+
return action_chains.perform()
21+
22+
23+
def mouse_over_and_wiggle(locator, x_wiggle=5):
24+
sl: SeleniumLibrary = BuiltIn().get_library_instance("SeleniumLibrary")
25+
action_chains = ActionChains(sl.driver)
26+
action_chains.move_to_element(sl.find_element(locator))
27+
wiggle(action_chains, x_wiggle)
28+
return action_chains.perform()

‎packages/jupyterlab-lsp/schema/hover.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,18 @@
1111
"enum": ["Alt", "Control", "Shift", "Meta", "AltGraph"],
1212
"default": "Control",
1313
"description": "Keyboard key which activates the tooltip on hover."
14+
},
15+
"throttlerDelay": {
16+
"title": "Throttler delay",
17+
"type": "number",
18+
"default": 50,
19+
"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."
20+
},
21+
"cacheSize": {
22+
"title": "Cache size",
23+
"type": "number",
24+
"default": 25,
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."
1426
}
1527
},
1628
"jupyter.lab.shortcuts": []

‎packages/jupyterlab-lsp/src/adapter_manager.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ export class WidgetAdapterManager implements ILSPAdapterManager {
125125
widget.context.pathChanged.connect(reconnect);
126126

127127
// TODO: maybe emit adapterCreated. Should it be handled by statusbar?
128+
this.refreshAdapterFromCurrentWidget();
128129
}
129130

130131
isAnyActive() {

‎packages/jupyterlab-lsp/src/components/free_tooltip.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ import { IEditorPosition } from '../positioning';
99
import { PositionConverter } from '../converter';
1010
import { Widget } from '@lumino/widgets';
1111
import { IRenderMimeRegistry } from '@jupyterlab/rendermime';
12-
import { ILSPAdapterManager } from '../tokens';
12+
import { WidgetAdapter } from '../adapters/adapter';
13+
import { IDocumentWidget } from '@jupyterlab/docregistry';
1314

1415
const MIN_HEIGHT = 20;
1516
const MAX_HEIGHT = 250;
@@ -95,21 +96,19 @@ export namespace EditorTooltip {
9596
markup: lsProtocol.MarkupContent;
9697
ce_editor: CodeEditor.IEditor;
9798
position: IEditorPosition;
99+
adapter: WidgetAdapter<IDocumentWidget>;
100+
className?: string;
98101
}
99102
}
100103

101104
export class EditorTooltipManager {
102105
private currentTooltip: FreeTooltip = null;
103106

104-
constructor(
105-
private rendermime_registry: IRenderMimeRegistry,
106-
private adapterManager: ILSPAdapterManager
107-
) {}
107+
constructor(private rendermime_registry: IRenderMimeRegistry) {}
108108

109109
create(options: EditorTooltip.IOptions): FreeTooltip {
110110
this.remove();
111-
let { markup, position } = options;
112-
let adapter = this.adapterManager.currentAdapter;
111+
let { markup, position, adapter } = options;
113112
let widget = adapter.widget;
114113
const bundle =
115114
markup.kind === 'plaintext'
@@ -123,6 +122,7 @@ export class EditorTooltipManager {
123122
position: PositionConverter.cm_to_ce(position),
124123
moveToLineEnd: false
125124
});
125+
tooltip.addClass(options.className);
126126
Widget.attach(tooltip, document.body);
127127
this.currentTooltip = tooltip;
128128
return tooltip;

‎packages/jupyterlab-lsp/src/converter.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ export class PositionConverter {
77
return { line: position.line, ch: position.character };
88
}
99

10+
static cm_to_lsp(position: CodeMirror.Position): lsProtocol.Position {
11+
return { line: position.line, character: position.ch };
12+
}
13+
1014
static lsp_to_ce(position: lsProtocol.Position): CodeEditor.IPosition {
1115
return { line: position.line, column: position.character };
1216
}

‎packages/jupyterlab-lsp/src/editor_integration/codemirror.ts

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,35 @@ export interface IEditOutcome {
5555
errors: string[];
5656
}
5757

58+
/**
59+
* Interface for storage of HTMLElement event specifications (event name + handler).
60+
*/
61+
interface IHTMLEventMap<
62+
T extends keyof HTMLElementEventMap = keyof HTMLElementEventMap
63+
> extends Map<T, (event: HTMLElementEventMap[T]) => void> {
64+
set<E extends T>(
65+
k: E,
66+
handler: (event: HTMLElementEventMap[E]) => void
67+
): this;
68+
get<E extends T>(k: E): (event: HTMLElementEventMap[E]) => void;
69+
}
70+
71+
type CodeMirrorEventName =
72+
| CodeMirror.DOMEvent
73+
| 'change'
74+
| 'changes'
75+
| 'beforeChange'
76+
| 'cursorActivity'
77+
| 'beforeSelectionChange'
78+
| 'viewportChange'
79+
| 'gutterClick'
80+
| 'focus'
81+
| 'blur'
82+
| 'scroll'
83+
| 'update'
84+
| 'renderLine'
85+
| 'overwriteToggle';
86+
5887
/**
5988
* One feature of each type exists per VirtualDocument
6089
* (the initialization is performed by the adapter).
@@ -64,9 +93,16 @@ export abstract class CodeMirrorIntegration
6493
is_registered: boolean;
6594
feature: IFeature;
6695

67-
protected readonly editor_handlers: Map<string, CodeMirrorHandler>;
68-
protected readonly connection_handlers: Map<string, any>;
69-
protected readonly wrapper_handlers: Map<keyof HTMLElementEventMap, any>;
96+
protected readonly editor_handlers: Map<
97+
CodeMirrorEventName,
98+
CodeMirrorHandler
99+
>;
100+
// TODO use better type constraints for connection event names and for responses
101+
protected readonly connection_handlers: Map<
102+
string,
103+
(response: object) => void
104+
>;
105+
protected readonly wrapper_handlers: IHTMLEventMap;
70106
protected wrapper: HTMLElement;
71107

72108
protected virtual_editor: CodeMirrorVirtualEditor;

0 commit comments

Comments
 (0)