Reduce signature and hover flickering (#836)

krassowski · web-flow · commit 4da6697c93b4 · 2022-09-27T01:54:33.000+01:00
* Prevent signature flickering

* Eliminate flickering when position stays but content needs updating

* Make sure we dispose previous tooltip

* Ensure disposed tooltip is regenerated,

and that typing at the end does not produce flicker either

* Add changelog entry

* Reduce signature flicker

* Reduce hover flicker on mouse move
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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:
@@ -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)
 
diff --git a/atest/05_Features/Signature.robot b/atest/05_Features/Signature.robot
@@ -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
@@ -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}
diff --git a/atest/examples/Signature.ipynb b/atest/examples/Signature.ipynb
@@ -1,5 +1,14 @@
 {
  "cells": [
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "list"
+   ]
+  },
   {
    "cell_type": "code",
    "execution_count": null,
@@ -19,15 +28,6 @@
     "abc"
    ]
   },
-  {
-   "cell_type": "code",
-   "execution_count": null,
-   "metadata": {},
-   "outputs": [],
-   "source": [
-    "list"
-   ]
-  },
   {
    "cell_type": "code",
    "execution_count": null,
diff --git a/packages/jupyterlab-lsp/src/components/free_tooltip.ts b/packages/jupyterlab-lsp/src/components/free_tooltip.ts
@@ -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;
@@ -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)
  */
@@ -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;
@@ -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) {
@@ -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 {
@@ -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;
@@ -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,
@@ -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;
   }
@@ -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();
diff --git a/packages/jupyterlab-lsp/src/features/hover.ts b/packages/jupyterlab-lsp/src/features/hover.ts
@@ -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,
diff --git a/packages/jupyterlab-lsp/src/features/signature.ts b/packages/jupyterlab-lsp/src/features/signature.ts
