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

Reduce signature and hover flickering #836

Merged
merged 8 commits into from
Sep 27, 2022
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- use correct websocket URL if configured as different from base URL ([#820], thanks @MikeSem)
- clean up all completer styles when completer feature is disabled ([#829]).
- fix `undefined` being inserted for path-like completion items with no `insertText` ([#833])
- reduce signature flickering when typing and hover flicker when moving mouse ([#836])
- refactoring:
- move client capabilities to features ([#738])
- documentation:
Expand All @@ -31,6 +32,7 @@
[#826]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/826
[#829]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/829
[#833]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/833
[#836]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/836

### `@krassowski/jupyterlab-lsp 3.10.1` (2022-03-21)

Expand Down
27 changes: 22 additions & 5 deletions atest/05_Features/Signature.robot
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ ${SIGNATURE_DETAILS} css:${SIGNATURE_DETAILS_CSS}

*** Test Cases ***
Triggers Signature Help After A Keystroke
Enter Cell Editor 1 line=6
Enter Cell Editor 2 line=6
Capture Page Screenshot 01-entered-cell.png
Press Keys None (
Capture Page Screenshot 02-signature-shown.png
Wait Until Keyword Succeeds 20x 0.5s Page Should Contain Element ${SIGNATURE_BOX}
Element Should Be Visible ${SIGNATURE_BOX}
Wait Until Keyword Succeeds 10x 0.5s Element Should Contain ${SIGNATURE_BOX}
... Important docstring of abc()
Element Should Contain ${SIGNATURE_HIGHLIGHTED_ARG} x
Expand All @@ -40,32 +41,48 @@ Triggers Signature Help After A Keystroke
Press Keys None )
Wait Until Keyword Succeeds 20x 0.5s Page Should Not Contain Element ${SIGNATURE_BOX}


Triggered Signature Is Visible In First Cell
# test boundary conditions for out of view behaviour
Enter Cell Editor 1
Press Keys None (
Wait Until Keyword Succeeds 20x 0.5s Page Should Contain Element ${SIGNATURE_BOX}
Element Should Be Visible ${SIGNATURE_BOX}


Should Close After Moving Cursor Prior To Start
Enter Cell Editor 1 line=6
Enter Cell Editor 2 line=6
Press Keys None (
Wait Until Keyword Succeeds 20x 0.5s Page Should Contain Element ${SIGNATURE_BOX}
Press Keys None LEFT
Wait Until Keyword Succeeds 20x 0.5s Page Should Not Contain Element ${SIGNATURE_BOX}
# retrigger
Press Keys None DELETE
Press Keys None (
Wait Until Keyword Succeeds 20x 0.5s Page Should Contain Element ${SIGNATURE_BOX}
Press Keys None UP
Wait Until Keyword Succeeds 20x 0.5s Page Should Not Contain Element ${SIGNATURE_BOX}

Should Close After Executing The Cell
Enter Cell Editor 1 line=6
Enter Cell Editor 2 line=6
Press Keys None (
Wait Until Keyword Succeeds 20x 0.5s Page Should Contain Element ${SIGNATURE_BOX}
Press Keys None SHIFT+ENTER
Wait Until Keyword Succeeds 20x 0.5s Page Should Not Contain Element ${SIGNATURE_BOX}

Invalidates On Cell Change
Enter Cell Editor 1 line=6
Enter Cell Editor 2 line=6
Press Keys None (
Wait Until Keyword Succeeds 20x 0.5s Page Should Contain Element ${SIGNATURE_BOX}
Enter Cell Editor 2
Enter Cell Editor 1
Wait Until Keyword Succeeds 20x 0.5s Page Should Not Contain Element ${SIGNATURE_BOX}

Details Should Expand On Click
Configure JupyterLab Plugin {"maxLines": 4} plugin id=${SIGNATURE PLUGIN ID}
Enter Cell Editor 3 line=11
Press Keys None (
Wait Until Keyword Succeeds 20x 0.5s Page Should Contain Element ${SIGNATURE_BOX}
Element Should Be Visible ${SIGNATURE_BOX}
Wait Until Keyword Succeeds 10x 0.5s Element Should Contain ${SIGNATURE_BOX} Short description.
Page Should Contain Element ${SIGNATURE_DETAILS}
Details Should Be Collapsed ${SIGNATURE_DETAILS_CSS}
Expand Down
18 changes: 9 additions & 9 deletions atest/examples/Signature.ipynb
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
{
"cells": [
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"list"
]
},
{
"cell_type": "code",
"execution_count": null,
Expand All @@ -19,15 +28,6 @@
"abc"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"list"
]
},
{
"cell_type": "code",
"execution_count": null,
Expand Down
135 changes: 102 additions & 33 deletions packages/jupyterlab-lsp/src/components/free_tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import * as lsProtocol from 'vscode-languageserver-protocol';

import { WidgetAdapter } from '../adapters/adapter';
import { PositionConverter } from '../converter';
import { IEditorPosition } from '../positioning';
import { IEditorPosition, is_equal } from '../positioning';

const MIN_HEIGHT = 20;
const MAX_HEIGHT = 250;
Expand All @@ -41,6 +41,8 @@ interface IFreeTooltipOptions extends Tooltip.IOptions {
hideOnKeyPress?: boolean;
}

type Bundle = { 'text/plain': string } | { 'text/markdown': string };

/**
* Tooltip which can be placed at any character, not only at the current position (derived from getCursorPosition)
*/
Expand All @@ -50,8 +52,10 @@ export class FreeTooltip extends Tooltip {
constructor(protected options: IFreeTooltipOptions) {
super(options);
this._setGeometry();
// TODO: remove once https://github.com/jupyterlab/jupyterlab/pull/11010 is merged & released
const model = new MimeModel({ data: options.bundle });
}

setBundle(bundle: Bundle) {
const model = new MimeModel({ data: bundle });
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const content: IRenderMime.IRenderer = this._content;
Expand Down Expand Up @@ -99,35 +103,34 @@ export class FreeTooltip extends Tooltip {
this.options.position == null
? editor.getCursorPosition()
: this.options.position;

const end = editor.getOffsetAt(cursor);
const line = editor.getLine(cursor.line);

if (!line) {
return;
}

let position: CodeEditor.IPosition | undefined;

switch (this.options.alignment) {
case 'start': {
const tokens = line.substring(0, end).split(/\W+/);
const last = tokens[tokens.length - 1];
const start = last ? end - last.length : end;
position = editor.getPositionAt(start);
break;
}
case 'end': {
const tokens = line.substring(0, end).split(/\W+/);
const last = tokens[tokens.length - 1];
const start = last ? end - last.length : end;
position = editor.getPositionAt(start);
break;
if (this.options.alignment) {
const end = editor.getOffsetAt(cursor);
const line = editor.getLine(cursor.line);

if (!line) {
return;
}
default: {
position = cursor;
break;

switch (this.options.alignment) {
case 'start': {
const tokens = line.substring(0, end).split(/\W+/);
const last = tokens[tokens.length - 1];
const start = last ? end - last.length : end;
position = editor.getPositionAt(start);
break;
}
case 'end': {
const tokens = line.substring(0, end).split(/\W+/);
const last = tokens[tokens.length - 1];
const start = last ? end - last.length : end;
position = editor.getPositionAt(start);
break;
}
}
} else {
position = cursor;
}

if (!position) {
Expand Down Expand Up @@ -155,9 +158,23 @@ export class FreeTooltip extends Tooltip {
node: this.node,
offset: { horizontal: -1 * paddingLeft },
privilege: this.options.privilege || 'below',
style: style
style: style,
// TODO: remove `ts-ignore` once minimum version is >=3.5
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
outOfViewDisplay: {
left: 'stick-inside',
right: 'stick-outside',
top: 'stick-outside',
bottom: 'stick-inside'
}
});
}

setPosition(position: CodeEditor.IPosition) {
this.options.position = position;
this._setGeometry();
}
}

export namespace EditorTooltip {
Expand All @@ -172,6 +189,12 @@ export namespace EditorTooltip {
}
}

function markupToBundle(markup: lsProtocol.MarkupContent): Bundle {
return markup.kind === 'plaintext'
? { 'text/plain': markup.value }
: { 'text/markdown': markup.value };
}

export class EditorTooltipManager {
private currentTooltip: FreeTooltip | null = null;
private currentOptions: EditorTooltip.IOptions | null;
Expand All @@ -183,10 +206,7 @@ export class EditorTooltipManager {
this.currentOptions = options;
let { markup, position, adapter } = options;
let widget = adapter.widget;
const bundle: { 'text/plain': string } | { 'text/markdown': string } =
markup.kind === 'plaintext'
? { 'text/plain': markup.value }
: { 'text/markdown': markup.value };
const bundle = markupToBundle(markup);
const tooltip = new FreeTooltip({
...(options.tooltip || {}),
anchor: widget.content,
Expand All @@ -204,6 +224,43 @@ export class EditorTooltipManager {
return tooltip;
}

showOrCreate(options: EditorTooltip.IOptions): FreeTooltip {
const samePosition =
this.currentOptions &&
is_equal(this.currentOptions.position, options.position);
const sameMarkup =
this.currentOptions &&
this.currentOptions.markup.value === options.markup.value &&
this.currentOptions.markup.kind === options.markup.kind;
if (
this.currentTooltip !== null &&
!this.currentTooltip.isDisposed &&
this.currentOptions &&
this.currentOptions.adapter === options.adapter &&
(samePosition || sameMarkup) &&
this.currentOptions.ce_editor === options.ce_editor &&
this.currentOptions.id === options.id
) {
// we only allow either position or markup change, because if both changed,
// then we may get into problematic race condition in sizing after bundle update.
if (!sameMarkup) {
this.currentOptions.markup = options.markup;
this.currentTooltip.setBundle(markupToBundle(options.markup));
}
if (!samePosition) {
// setting geometry only works when visible
this.currentTooltip.setPosition(
PositionConverter.cm_to_ce(options.position)
);
}
this.show();
return this.currentTooltip;
} else {
this.remove();
return this.create(options);
}
}

get position(): IEditorPosition {
return this.currentOptions!.position;
}
Expand All @@ -219,6 +276,18 @@ export class EditorTooltipManager {
);
}

hide() {
if (this.currentTooltip !== null) {
this.currentTooltip.hide();
}
}

show() {
if (this.currentTooltip !== null) {
this.currentTooltip.show();
}
}

remove() {
if (this.currentTooltip !== null) {
this.currentTooltip.dispose();
Expand Down
3 changes: 1 addition & 2 deletions packages/jupyterlab-lsp/src/features/hover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,12 +342,11 @@ export class HoverCM extends CodeMirrorIntegration {
this.last_hover_response = response;

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

this.tooltip = this.lab_integration.tooltip.create({
this.tooltip = this.lab_integration.tooltip.showOrCreate({
markup,
position: editor_position,
ce_editor: response_data.ce_editor,
Expand Down
Loading