Skip to content

Commit 573568f

Browse files
committed
feat(lsp): redesign parameter hints popup, fix occlusion and flicker
Cap the function-hint popup to the editor width and keep the signature on a single scrollable line, so a long TS signature can no longer run off-screen underneath the central control bar. Clamp it into the editor area and raise its z-index above the CCB. Theme the surface to match the other LSP popups, use a monospace font, and highlight the active parameter clearly. When the signature overflows, scroll the active parameter into view and fade the clipped edge(s). Stop the popup flickering on every caret move: cursorActivity no longer dismisses (hides) the popup before firing the async request. It now updates in place - skipping the redraw entirely when the same signature and active parameter are already shown, and keeping the popup anchored while only the highlight moves. A monotonic request id drops stale responses and prevents a late response from re-showing a dismissed popup.
1 parent 974c8de commit 573568f

4 files changed

Lines changed: 181 additions & 36 deletions

File tree

src/features/ParameterHintsManager.js

Lines changed: 114 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ define(function (require, exports, module) {
5353
};
5454

5555
let $hintContainer, // function hint container
56+
$hintScroll, // single-line clipping/scrolling layer
5657
$hintContent, // function hint content holder
5758
hintState = {},
5859
lastChar = null,
@@ -66,12 +67,51 @@ define(function (require, exports, module) {
6667
// keep jslint from complaining about handleCursorActivity being used before
6768
// it was defined.
6869
let handleCursorActivity,
69-
popupShown = false;
70+
popupShown = false,
71+
// Monotonic id so a slow LSP response from an earlier caret position can be ignored
72+
// once a newer cursor move has fired a fresh request.
73+
pendingRequestId = 0;
74+
75+
/**
76+
* A stable identity for the function being called (its parameter list), independent of which
77+
* parameter the caret is currently in. Used to keep the popup anchored while only the active
78+
* parameter changes.
79+
* @param {{parameters: Array}} hint
80+
* @return {string}
81+
*/
82+
function _signatureKey(hint) {
83+
return (hint.parameters || []).map(function (p) {
84+
return p.label || p.type || "";
85+
}).join(",");
86+
}
7087

7188
let _providerRegistrationHandler = new ProviderRegistrationHandler(),
7289
registerHintProvider = _providerRegistrationHandler.registerProvider.bind(_providerRegistrationHandler),
7390
removeHintProvider = _providerRegistrationHandler.removeProvider.bind(_providerRegistrationHandler);
7491

92+
/**
93+
* Keep the active parameter visible inside the single-line, width-capped popup: scroll it
94+
* into view (centered) when the signature overflows, and fade whichever edge is clipped so
95+
* it's clear there's more of the signature off-screen.
96+
*/
97+
function _revealCurrentParameter() {
98+
let el = $hintScroll && $hintScroll[0];
99+
if (!el) {
100+
return;
101+
}
102+
let maxScroll = el.scrollWidth - el.clientWidth;
103+
let $cur = $hintContent.find(".current-parameter");
104+
if (maxScroll > 0 && $cur.length) {
105+
let elRect = el.getBoundingClientRect(),
106+
curRect = $cur[0].getBoundingClientRect(),
107+
curLeftInContent = (curRect.left - elRect.left) + el.scrollLeft,
108+
target = curLeftInContent - (el.clientWidth - curRect.width) / 2;
109+
el.scrollLeft = Math.max(0, Math.min(target, maxScroll));
110+
}
111+
$hintScroll.toggleClass("fade-left", el.scrollLeft > 1);
112+
$hintScroll.toggleClass("fade-right", el.scrollLeft < maxScroll - 1);
113+
}
114+
75115
/**
76116
* Position a function hint.
77117
*
@@ -80,11 +120,7 @@ define(function (require, exports, module) {
80120
* @param {number} ybot
81121
*/
82122
function positionHint(xpos, ypos, ybot) {
83-
let hintWidth = $hintContainer.width(),
84-
hintHeight = $hintContainer.height(),
85-
top = ypos - hintHeight - POINTER_TOP_OFFSET,
86-
left = xpos,
87-
$editorHolder = $("#editor-holder"),
123+
let $editorHolder = $("#editor-holder"),
88124
editorLeft;
89125

90126
if ($editorHolder.offset() === undefined) {
@@ -94,8 +130,22 @@ define(function (require, exports, module) {
94130
}
95131

96132
editorLeft = $editorHolder.offset().left;
97-
left = Math.max(left, editorLeft);
133+
134+
// Cap the popup to the editor width so a long signature can never run off-screen (and
135+
// underneath the central control bar). The signature stays on one line and scrolls.
136+
let maxWidth = Math.min(700, $editorHolder.width() - 24);
137+
$hintContainer.css("max-width", maxWidth + "px");
138+
_revealCurrentParameter();
139+
140+
let hintWidth = $hintContainer.outerWidth(),
141+
hintHeight = $hintContainer.outerHeight(),
142+
top = ypos - hintHeight - POINTER_TOP_OFFSET,
143+
left = xpos;
144+
145+
// Clamp within the editor area: never left of the editor (keeps it clear of the CCB),
146+
// never past the right edge.
98147
left = Math.min(left, editorLeft + $editorHolder.width() - hintWidth);
148+
left = Math.max(left, editorLeft);
99149

100150
if (top < 0) {
101151
$hintContainer.removeClass("preview-bubble-above");
@@ -232,6 +282,8 @@ define(function (require, exports, module) {
232282
*/
233283
function dismissHint(editor) {
234284
popupShown = false;
285+
// Invalidate any in-flight request so a late response can't re-show a dismissed popup.
286+
pendingRequestId++;
235287
if (hintState.visible) {
236288
$hintContainer.hide();
237289
$hintContent.empty();
@@ -262,7 +314,6 @@ define(function (require, exports, module) {
262314
let $deferredPopUp = $.Deferred();
263315
let sessionProvider = null;
264316

265-
dismissHint(editor);
266317
popupShown = true;
267318
// Find a suitable provider, if any
268319
let language = editor.getLanguageForSelection(),
@@ -279,25 +330,67 @@ define(function (require, exports, module) {
279330
request = sessionProvider.getParameterHints(explicit, onCursorActivity);
280331
}
281332

282-
if (request) {
283-
request.done(function (parameterHint) {
333+
// No hint at the caret (no provider, or none available here) - take down any existing popup.
334+
if (!request) {
335+
dismissHint(editor);
336+
return $deferredPopUp;
337+
}
338+
339+
let requestId = ++pendingRequestId;
340+
341+
request.done(function (parameterHint) {
342+
// A newer cursor move already fired a fresh request; drop this stale response.
343+
if (requestId !== pendingRequestId) {
344+
return;
345+
}
346+
347+
let signature = _signatureKey(parameterHint),
348+
renderKey = parameterHint.currentIndex + "|" + signature;
349+
350+
// Already showing this exact signature with this exact active parameter: leave the
351+
// popup untouched. Moving the caret within one parameter must not dismiss+redraw it
352+
// (that is what made arrow-key presses flicker).
353+
if (hintState.visible && hintState.renderKey === renderKey) {
354+
$deferredPopUp.resolveWith(null);
355+
return;
356+
}
357+
358+
_formatHint(editor, parameterHint);
359+
$hintContainer.show(); // no-op when already visible -> content updates in place, no blink
360+
361+
if (hintState.visible && hintState.signature === signature && hintState.anchor) {
362+
// Same call, just a different active parameter: keep the popup anchored where it
363+
// is and only let the highlight move (positionHint re-reveals the active param).
364+
positionHint(hintState.anchor.left, hintState.anchor.top, hintState.anchor.bottom);
365+
} else {
284366
let cm = editor._codeMirror,
285367
pos = parameterHint.functionCallPos || editor.getCursorPos();
286-
287368
pos = cm.charCoords(pos);
288-
_formatHint(editor, parameterHint);
289-
290-
$hintContainer.show();
291369
positionHint(pos.left, pos.top, pos.bottom);
292-
hintState.visible = true;
370+
hintState.anchor = pos;
371+
hintState.signature = signature;
372+
}
373+
374+
hintState.visible = true;
375+
hintState.renderKey = renderKey;
293376

377+
// Attach the cursor-tracking listener once per editor (not on every refresh).
378+
if (sessionEditor !== editor) {
379+
if (sessionEditor) {
380+
sessionEditor.off("cursorActivity.ParameterHinting", handleCursorActivity);
381+
}
294382
sessionEditor = editor;
383+
editor.off("cursorActivity.ParameterHinting", handleCursorActivity);
295384
editor.on("cursorActivity.ParameterHinting", handleCursorActivity);
296-
$deferredPopUp.resolveWith(null);
297-
}).fail(function () {
298-
hintState = {};
299-
});
300-
}
385+
}
386+
$deferredPopUp.resolveWith(null);
387+
}).fail(function () {
388+
// The caret moved off the call (or the request failed) - dismiss, unless a newer
389+
// request has since taken over.
390+
if (requestId === pendingRequestId) {
391+
dismissHint(editor);
392+
}
393+
});
301394

302395
return $deferredPopUp;
303396
}
@@ -407,6 +500,7 @@ define(function (require, exports, module) {
407500
}
408501
// Create the function hint container
409502
$hintContainer = $(hintContainerHTML).appendTo($("body"));
503+
$hintScroll = $hintContainer.find(".function-hint-scroll");
410504
$hintContent = $hintContainer.find(".function-hint-content-new");
411505
activeEditorChangeHandler(null, EditorManager.getActiveEditor(), null);
412506

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
<div id="function-hint-container-new">
2-
<div class="function-hint-content-new">
2+
<div class="function-hint-scroll">
3+
<div class="function-hint-content-new">
4+
</div>
35
</div>
46
</div>

src/styles/brackets.less

Lines changed: 61 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2667,33 +2667,80 @@ span.brackets-hints-with-type-details {
26672667

26682668
#function-hint-container-new {
26692669
display: none;
2670-
background: #fff;
26712670
position: absolute;
26722671
z-index: var(--z-index-parameter-hints);
2673-
left: 400px;
2674-
top: 40px;
2672+
box-sizing: border-box;
2673+
// Never wider than the editor (JS caps to the editor width too); the signature
2674+
// stays on one line and scrolls so it can never run off-screen under the CCB.
2675+
max-width: 700px;
26752676
height: auto;
2676-
width: auto;
2677-
overflow: scroll;
2678-
padding: 1px 6px;
2679-
text-align: center;
2680-
border-radius: 3px;
2681-
box-shadow: 0 3px 9px rgba(0, 0, 0, 0.24);
2677+
overflow: hidden;
2678+
padding: 5px 11px;
2679+
text-align: left;
2680+
border-radius: 6px;
2681+
color: @bc-text;
2682+
background: @bc-panel-bg;
2683+
border: 1px solid rgba(0, 0, 0, 0.12);
2684+
box-shadow: 0 4px 15px @bc-shadow-large;
26822685

26832686
.dark & {
2684-
background: #000;
2685-
border: 1px solid rgba(255, 255, 255, 0.15);
2686-
color: #fff;
2687-
box-shadow: 0 3px 9px rgba(0, 0, 0, 0.24);
2687+
color: @dark-bc-text;
2688+
background: @dark-bc-panel-bg;
2689+
border-color: rgba(255, 255, 255, 0.12);
2690+
box-shadow: 0 4px 15px @dark-bc-shadow-large;
2691+
}
2692+
}
2693+
2694+
// Clipping/scrolling layer: the (potentially very long) signature lives here on a single
2695+
// line. When it overflows we scroll the active parameter into view and fade the clipped
2696+
// edge(s) to signal there is more - see positionHint() in ParameterHintsManager.
2697+
#function-hint-container-new .function-hint-scroll {
2698+
overflow: hidden;
2699+
white-space: nowrap;
2700+
2701+
&.fade-left {
2702+
-webkit-mask-image: linear-gradient(to right, transparent 0, #000 22px);
2703+
mask-image: linear-gradient(to right, transparent 0, #000 22px);
2704+
}
2705+
&.fade-right {
2706+
-webkit-mask-image: linear-gradient(to left, transparent 0, #000 22px);
2707+
mask-image: linear-gradient(to left, transparent 0, #000 22px);
2708+
}
2709+
&.fade-left.fade-right {
2710+
-webkit-mask-image: linear-gradient(to right, transparent 0, #000 22px, #000 calc(100% - 22px), transparent 100%);
2711+
mask-image: linear-gradient(to right, transparent 0, #000 22px, #000 calc(100% - 22px), transparent 100%);
26882712
}
26892713
}
26902714

26912715
#function-hint-container-new .function-hint-content-new {
2716+
display: inline-block;
26922717
text-align: left;
2718+
font-family: 'SourceCodePro', 'SF Mono', Menlo, Consolas, monospace;
2719+
font-size: 12px;
2720+
line-height: 1.6;
2721+
white-space: pre;
26932722
}
26942723

2724+
// Non-active parameters and the function name / separators read as muted code.
2725+
.brackets-hints .parameter {
2726+
color: @bc-text;
2727+
.dark & {
2728+
color: @dark-bc-text;
2729+
}
2730+
}
2731+
2732+
// The parameter the caret is currently inside: clearly emphasized so it's easy to track.
26952733
.brackets-hints .current-parameter {
2696-
font-weight: 500;
2734+
font-weight: 700;
2735+
color: @accent-keyword;
2736+
background: fade(@accent-keyword, 12%);
2737+
border-radius: 3px;
2738+
padding: 1px 2px;
2739+
2740+
.dark & {
2741+
color: #6cb6ff;
2742+
background: rgba(108, 182, 255, 0.16);
2743+
}
26972744
}
26982745

26992746
// selection view

src/styles/brackets_variables.less

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222

2323
/* Brackets Variables */
2424
:root {
25-
--z-index-parameter-hints: 19;
25+
/* Above the central control bar (@z-index-brackets-main-content + 1 = 20) so the
26+
parameter hint is never occluded by it when it sits near the editor's left edge. */
27+
--z-index-parameter-hints: 21;
2628
--editor-line-height: 1.5;
2729
}
2830

0 commit comments

Comments
 (0)