Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix highlighting in hover tooltips and start testing the hover feature #363

Merged
merged 15 commits into from
Sep 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
## CHANGELOG

### `@krassowski/jupyterlab-lsp 2.0.7` (unreleased)

- bug fixes

- 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])

[#363]: https://github.com/krassowski/jupyterlab-lsp/issues/363

### `@krassowski/jupyterlab-lsp 2.0.6` (2020-09-15)

- bug fixes
Expand Down
78 changes: 78 additions & 0 deletions atest/05_Features/Hover.robot
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
*** 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
Library ../mouse_over_extension.py

*** Variables ***
${HOVER_BOX} css:.lsp-hover
${HOVER_SIGNAL} css:.cm-lsp-hover-available

*** Test Cases ***
Hover works in notebooks
Enter Cell Editor 1
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)
# 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 can be triggered via modifier key once cursor stopped moving
Enter Cell Editor 1
${element} = Last Occurrence python_add
Wait Until Keyword Succeeds 5x 0.1 s Trigger Via Modifier Key Press ${element}

Hover works in foreign code (javascript)
Enter Cell Editor 2
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
# 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
Trigger Tooltip Math
Element Should Contain ${HOVER_BOX} const Math: Math

*** Keywords ***
Last Occurrence
[Arguments] ${symbol}
${sel} = Set Variable If "${symbol}".startswith(("xpath", "css")) ${symbol} xpath:(//span[@role="presentation"][contains(., "${symbol}")])[last()]
[Return] ${sel}

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 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} 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}

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
63 changes: 63 additions & 0 deletions atest/examples/Hover.ipynb
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
{
"cells": [
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"def python_add(a: int, b: int):\n",
" \"\"\"Add 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
}
28 changes: 28 additions & 0 deletions atest/mouse_over_extension.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
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):
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))
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()
12 changes: 12 additions & 0 deletions packages/jupyterlab-lsp/schema/hover.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,18 @@
"enum": ["Alt", "Control", "Shift", "Meta", "AltGraph"],
"default": "Control",
"description": "Keyboard key which activates the tooltip on hover."
},
"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": []
Expand Down
1 change: 1 addition & 0 deletions packages/jupyterlab-lsp/src/adapter_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
14 changes: 7 additions & 7 deletions packages/jupyterlab-lsp/src/components/free_tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -95,21 +96,19 @@ export namespace EditorTooltip {
markup: lsProtocol.MarkupContent;
ce_editor: CodeEditor.IEditor;
position: IEditorPosition;
adapter: WidgetAdapter<IDocumentWidget>;
className?: string;
}
}

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'
Expand All @@ -123,6 +122,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;
Expand Down
4 changes: 4 additions & 0 deletions packages/jupyterlab-lsp/src/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
}
Expand Down
42 changes: 39 additions & 3 deletions packages/jupyterlab-lsp/src/editor_integration/codemirror.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T, (event: HTMLElementEventMap[T]) => void> {
set<E extends T>(
k: E,
handler: (event: HTMLElementEventMap[E]) => void
): this;
get<E extends T>(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).
Expand All @@ -64,9 +93,16 @@ export abstract class CodeMirrorIntegration
is_registered: boolean;
feature: IFeature;

protected readonly editor_handlers: Map<string, CodeMirrorHandler>;
protected readonly connection_handlers: Map<string, any>;
protected readonly wrapper_handlers: Map<keyof HTMLElementEventMap, any>;
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;
Expand Down
Loading