feat(lsp): redesign parameter hints popup, fix occlusion and flicker

src/features/ParameterHintsManager.js

@@ -53,6 +53,7 @@
         };
 
     let $hintContainer, // function hint container
+        $hintScroll, // single-line clipping/scrolling layer
         $hintContent, // function hint content holder
         hintState = {},
         lastChar = null,
@@ -66,12 +67,51 @@
     // keep jslint from complaining about handleCursorActivity being used before
     // it was defined.
     let handleCursorActivity,
-        popupShown = false;
+        popupShown = false,
+        // Monotonic id so a slow LSP response from an earlier caret position can be ignored
+        // once a newer cursor move has fired a fresh request.
+        pendingRequestId = 0;
+
+    /**
+     * A stable identity for the function being called (its parameter list), independent of which
+     * parameter the caret is currently in. Used to keep the popup anchored while only the active
+     * parameter changes.
+     * @param {{parameters: Array}} hint
+     * @return {string}
+     */
+    function _signatureKey(hint) {
+        return (hint.parameters || []).map(function (p) {
+            return p.label || p.type || "";
+        }).join(",");
+    }
 
     let _providerRegistrationHandler = new ProviderRegistrationHandler(),
         registerHintProvider = _providerRegistrationHandler.registerProvider.bind(_providerRegistrationHandler),
         removeHintProvider = _providerRegistrationHandler.removeProvider.bind(_providerRegistrationHandler);
 
+    /**
+     * Keep the active parameter visible inside the single-line, width-capped popup: scroll it
+     * into view (centered) when the signature overflows, and fade whichever edge is clipped so
+     * it's clear there's more of the signature off-screen.
+     */
+    function _revealCurrentParameter() {
+        let el = $hintScroll && $hintScroll[0];
+        if (!el) {
+            return;
+        }
+        let maxScroll = el.scrollWidth - el.clientWidth;
+        let $cur = $hintContent.find(".current-parameter");
+        if (maxScroll > 0 && $cur.length) {
+            let elRect = el.getBoundingClientRect(),
+                curRect = $cur[0].getBoundingClientRect(),
+                curLeftInContent = (curRect.left - elRect.left) + el.scrollLeft,
+                target = curLeftInContent - (el.clientWidth - curRect.width) / 2;
+            el.scrollLeft = Math.max(0, Math.min(target, maxScroll));
+        }
+        $hintScroll.toggleClass("fade-left", el.scrollLeft > 1);
+        $hintScroll.toggleClass("fade-right", el.scrollLeft < maxScroll - 1);
+    }
+
     /**
      * Position a function hint.
      *
@@ -80,11 +120,7 @@
      * @param {number} ybot
      */
     function positionHint(xpos, ypos, ybot) {
-        let hintWidth = $hintContainer.width(),
-            hintHeight = $hintContainer.height(),
-            top = ypos - hintHeight - POINTER_TOP_OFFSET,
-            left = xpos,
-            $editorHolder = $("#editor-holder"),
+        let $editorHolder = $("#editor-holder"),
             editorLeft;
 
         if ($editorHolder.offset() === undefined) {
@@ -94,8 +130,22 @@
         }
 
         editorLeft = $editorHolder.offset().left;
-        left = Math.max(left, editorLeft);
+
+        // Cap the popup to the editor width so a long signature can never run off-screen (and
+        // underneath the central control bar). The signature stays on one line and scrolls.
+        let maxWidth = Math.min(700, $editorHolder.width() - 24);
+        $hintContainer.css("max-width", maxWidth + "px");
+        _revealCurrentParameter();
+
+        let hintWidth = $hintContainer.outerWidth(),
+            hintHeight = $hintContainer.outerHeight(),
+            top = ypos - hintHeight - POINTER_TOP_OFFSET,
+            left = xpos;
+
+        // Clamp within the editor area: never left of the editor (keeps it clear of the CCB),
+        // never past the right edge.
         left = Math.min(left, editorLeft + $editorHolder.width() - hintWidth);
+        left = Math.max(left, editorLeft);
 
         if (top < 0) {
             $hintContainer.removeClass("preview-bubble-above");
@@ -232,6 +282,8 @@
      */
     function dismissHint(editor) {
         popupShown = false;
+        // Invalidate any in-flight request so a late response can't re-show a dismissed popup.
+        pendingRequestId++;
         if (hintState.visible) {
             $hintContainer.hide();
             $hintContent.empty();
@@ -262,7 +314,6 @@
         let $deferredPopUp = $.Deferred();
         let sessionProvider = null;
 
-        dismissHint(editor);
         popupShown = true;
         // Find a suitable provider, if any
         let language = editor.getLanguageForSelection(),
@@ -279,25 +330,67 @@
             request = sessionProvider.getParameterHints(explicit, onCursorActivity);
         }
 
-        if (request) {
-            request.done(function (parameterHint) {
+        // No hint at the caret (no provider, or none available here) - take down any existing popup.
+        if (!request) {
+            dismissHint(editor);
+            return $deferredPopUp;
+        }
+
+        let requestId = ++pendingRequestId;
+
+        request.done(function (parameterHint) {
+            // A newer cursor move already fired a fresh request; drop this stale response.
+            if (requestId !== pendingRequestId) {
+                return;
+            }
+
+            let signature = _signatureKey(parameterHint),
+                renderKey = parameterHint.currentIndex + "|" + signature;
+
+            // Already showing this exact signature with this exact active parameter: leave the
+            // popup untouched. Moving the caret within one parameter must not dismiss+redraw it
+            // (that is what made arrow-key presses flicker).
+            if (hintState.visible && hintState.renderKey === renderKey) {
+                $deferredPopUp.resolveWith(null);
+                return;
+            }
+
+            _formatHint(editor, parameterHint);
+            $hintContainer.show(); // no-op when already visible -> content updates in place, no blink
+
+            if (hintState.visible && hintState.signature === signature && hintState.anchor) {
+                // Same call, just a different active parameter: keep the popup anchored where it
+                // is and only let the highlight move (positionHint re-reveals the active param).
+                positionHint(hintState.anchor.left, hintState.anchor.top, hintState.anchor.bottom);
+            } else {
                 let cm = editor._codeMirror,
                     pos = parameterHint.functionCallPos || editor.getCursorPos();
-
                 pos = cm.charCoords(pos);
-                _formatHint(editor, parameterHint);
-
-                $hintContainer.show();
                 positionHint(pos.left, pos.top, pos.bottom);
-                hintState.visible = true;
+                hintState.anchor = pos;
+                hintState.signature = signature;
+            }
+
+            hintState.visible = true;
+            hintState.renderKey = renderKey;
 
+            // Attach the cursor-tracking listener once per editor (not on every refresh).
+            if (sessionEditor !== editor) {
+                if (sessionEditor) {
+                    sessionEditor.off("cursorActivity.ParameterHinting", handleCursorActivity);
+                }
                 sessionEditor = editor;
+                editor.off("cursorActivity.ParameterHinting", handleCursorActivity);
                 editor.on("cursorActivity.ParameterHinting", handleCursorActivity);
-                $deferredPopUp.resolveWith(null);
-            }).fail(function () {
-                hintState = {};
-            });
-        }
+            }
+            $deferredPopUp.resolveWith(null);
+        }).fail(function () {
+            // The caret moved off the call (or the request failed) - dismiss, unless a newer
+            // request has since taken over.
+            if (requestId === pendingRequestId) {
+                dismissHint(editor);
+            }
+        });
 
         return $deferredPopUp;
     }
@@ -407,6 +500,7 @@
         }
         // Create the function hint container
         $hintContainer = $(hintContainerHTML).appendTo($("body"));
+        $hintScroll = $hintContainer.find(".function-hint-scroll");
         $hintContent = $hintContainer.find(".function-hint-content-new");
         activeEditorChangeHandler(null, EditorManager.getActiveEditor(), null);
 

src/htmlContent/parameter-hint-template.html

@@ -1,4 +1,6 @@
 <div id="function-hint-container-new">
-    <div class="function-hint-content-new">
+    <div class="function-hint-scroll">
+        <div class="function-hint-content-new">
+        </div>
     </div>
 </div>

src/languageTools/DefaultProviders.js

@@ -50,13 +50,68 @@
     // is shown in a separate popup beside the hint list so the list itself never reflows while
     // navigating with the arrow keys.
     var $lspDocPopup = null;
+    // The hint list reflows after _showDocPopup runs - it flips above/below the caret near a screen
+    // edge, shifts as the cursor moves, and moves on scroll. To keep the doc popup glued beside it
+    // (and never overlapping it), we re-derive its position from the list's *current* rect on every
+    // animation frame while it is visible, instead of positioning it once.
+    var _docTrackRAF = null,   // active requestAnimationFrame handle, or null when not tracking
+        _docTrackList = null,  // the ul.dropdown-menu the popup is anchored to
+        _docLastPos = "";      // last applied "left,top" - skip the css write when unchanged
 
     function _hideDocPopup() {
+        if (_docTrackRAF) {
+            cancelAnimationFrame(_docTrackRAF);
+            _docTrackRAF = null;
+        }
+        _docTrackList = null;
+        _docLastPos = "";
         if ($lspDocPopup) {
             $lspDocPopup.hide().empty();
         }
     }
 
+    /**
+     * Place the doc popup flush beside the hint list's current position: to the right, flipping to
+     * the left when there isn't room, and clamped vertically to stay on screen. Reads the list's
+     * live rect so it stays correct no matter how the list has since moved.
+     */
+    function _positionDocPopup() {
+        if (!$lspDocPopup || !_docTrackList || !_docTrackList.length) {
+            return;
+        }
+        var listEl = _docTrackList[0];
+        if (!listEl.isConnected) {
+            // The hint menu (and this popup, its child) was torn down - stop tracking.
+            _hideDocPopup();
+            return;
+        }
+        var anchor = listEl.getBoundingClientRect();
+        if (anchor.width === 0 && anchor.height === 0) {
+            return; // list not laid out yet (mid-reflow) - try again next frame
+        }
+        var GAP = 6,
+            winW = $(window).width(),
+            winH = $(window).height(),
+            pw = $lspDocPopup.outerWidth(),
+            ph = $lspDocPopup.outerHeight(),
+            left = anchor.right + GAP;
+        if (left + pw > winW - 8) {
+            left = anchor.left - pw - GAP; // not enough room on the right - flip to the left
+        }
+        left = Math.max(8, left);
+        var top = Math.max(8, Math.min(anchor.top, winH - ph - 8));
+        var pos = Math.round(left) + "," + Math.round(top);
+        if (pos !== _docLastPos) {
+            _docLastPos = pos;
+            $lspDocPopup.css({ left: left, top: top });
+        }
+    }
+
+    function _trackDocPopup() {
+        _positionDocPopup();
+        _docTrackRAF = requestAnimationFrame(_trackDocPopup);
+    }
+
     // Syntax-highlight fenced code blocks (the signature/examples in completion docs) with the
     // globally available highlight.js, so they read like code instead of flat monospace. Theme-aware
     // token colours live in src/styles/brackets.less (.lsp-hint-doc-popup, shared with the hover).
@@ -117,21 +172,16 @@
         if (!$list.length) {
             $list = $menu;
         }
-        var anchor = $list[0].getBoundingClientRect();
 
-        var GAP = 6;
-        // Measure, then place to the right of the hint list - flipping to the left when there
-        // isn't enough room.
-        $lspDocPopup.css({ display: "block", visibility: "hidden", left: 0, top: 0 });
-        var winW = $(window).width(), winH = $(window).height(),
-            pw = $lspDocPopup.outerWidth(), ph = $lspDocPopup.outerHeight(),
-            left = anchor.right + GAP;
-        if (left + pw > winW - 8) {
-            left = anchor.left - pw - GAP; // not enough room on the right - flip to the left
+        // Position now, then keep re-positioning every frame so the popup follows the list wherever
+        // it reflows to (and never ends up overlapping it).
+        _docTrackList = $list;
+        _docLastPos = "";
+        $lspDocPopup.css({ display: "block" });
+        _positionDocPopup();
+        if (!_docTrackRAF) {
+            _trackDocPopup();
         }
-        left = Math.max(8, left);
-        var top = Math.min(anchor.top, Math.max(8, winH - ph - 8));
-        $lspDocPopup.css({ left: left, top: top, visibility: "visible" });
     }
 
     function _injectInlineSignature($labelSpan, detail) {
@@ -140,7 +190,7 @@
         }
         // Append the signature as a sibling of the label (inside .codehint-item) so the flex row
         // layout (see CSS) lays out label + signature side-by-side without overlap or width change.
         var text = detail.split("->").join(":").replace(/\s+/g, " ").trim();
         var $container = $labelSpan.parent(); // .codehint-item
         if (!$container.length) {
             $container = $labelSpan;