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

feat: Keep focus on same item in completion popup when slow completer delivers results. #5322

Merged
merged 10 commits into from
Sep 22, 2023
2 changes: 2 additions & 0 deletions ace.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,8 @@ export namespace Ace {
exactMatch?: boolean;
inlineEnabled?: boolean;
parentNode?: HTMLElement;
setSelectOnHover?: Boolean;
stickySelectionDelay?: Number;
emptyMessage?(prefix: String): String;
getPopup(): AcePopup;
showPopup(editor: Editor, options: CompletionOptions): void;
Expand Down
32 changes: 30 additions & 2 deletions src/autocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ class Autocomplete {
this.parentNode = null;
this.setSelectOnHover = false;

/**
* @property {number} stickySelectionDelay - a numerical value that determines after how many ms the popup selection will become 'sticky'.
* Normally, when new elements are added to an open popup, the selection is reset to the first row of the popup. If sticky, the focus will remain
* on the currently selected item when new items are added to the popup. Set to a negative value to disable this feature and never set selection to sticky.
*/
this.stickySelectionDelay = 500;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add JSDoc comment somehow. for this option, since it's a bit hard to understand from just the name without prior context, but it's a pretty cool functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a (hopefully) descriptive comment about this feature


this.blurListener = this.blurListener.bind(this);
this.changeListener = this.changeListener.bind(this);
this.mousedownListener = this.mousedownListener.bind(this);
Expand All @@ -74,6 +81,10 @@ class Autocomplete {
}.bind(this));

this.tooltipTimer = lang.delayedCall(this.updateDocTooltip.bind(this), 50);

this.stickySelectionTimer = lang.delayedCall(function() {
this.stickySelection = true;
}.bind(this), this.stickySelectionDelay);
}

$init() {
Expand All @@ -83,7 +94,7 @@ class Autocomplete {
e.stop();
}.bind(this));
this.popup.focus = this.editor.focus.bind(this.editor);
this.popup.on("show", this.$onPopupChange.bind(this));
this.popup.on("show", this.$onPopupShow.bind(this));
this.popup.on("hide", this.$onHidePopup.bind(this));
this.popup.on("select", this.$onPopupChange.bind(this));
this.popup.on("changeHoverMarker", this.tooltipTimer.bind(null, null));
Expand All @@ -106,6 +117,8 @@ class Autocomplete {
this.inlineRenderer.hide();
}
this.hideDocTooltip();
this.stickySelectionTimer.cancel();
this.stickySelection = false;
}

$onPopupChange(hide) {
Expand All @@ -120,6 +133,13 @@ class Autocomplete {
this.tooltipTimer.call(null, null);
}

$onPopupShow(hide) {
this.$onPopupChange(hide);
this.stickySelection = false;
if (this.stickySelectionDelay >= 0)
this.stickySelectionTimer.schedule(this.stickySelectionDelay);
}

observeLayoutChanges() {
if (this.$elements || !this.editor) return;
window.addEventListener("resize", this.onLayoutChange, {passive: true});
Expand Down Expand Up @@ -194,6 +214,8 @@ class Autocomplete {
this.popup.autoSelect = this.autoSelect;
this.popup.setSelectOnHover(this.setSelectOnHover);

var previousSelectedItem = this.popup.data[this.popup.getRow()];

this.popup.setData(this.completions.filtered, this.completions.filterText);
if (this.editor.textInput.setAriaOptions) {
this.editor.textInput.setAriaOptions({
Expand All @@ -204,7 +226,13 @@ class Autocomplete {

editor.keyBinding.addKeyboardHandler(this.keyboardHandler);

this.popup.setRow(this.autoSelect ? 0 : -1);
var newRow = this.popup.data.indexOf(previousSelectedItem);

if (newRow && this.stickySelection)
this.popup.setRow(this.autoSelect ? newRow : -1);
else
this.popup.setRow(this.autoSelect ? 0 : -1);

if (!keepPopupPosition) {
this.popup.setTheme(editor.getTheme());
this.popup.setFontSize(editor.getFontSize());
Expand Down
112 changes: 112 additions & 0 deletions src/autocomplete_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"use strict";

var sendKey = require("./test/user").type;
var {buildDom} = require("./lib/dom");

Check warning on line 8 in src/autocomplete_test.js

View workflow job for this annotation

GitHub Actions / build (16.x)

'buildDom' is assigned a value but never used
var ace = require("./ace");
var assert = require("./test/assertions");
var user = require("./test/user");
Expand Down Expand Up @@ -614,6 +614,118 @@


done();
},
"test: should maintain selection on fast completer item when slow completer results come in": function(done) {
var editor = initEditor("hello world\n");

var slowCompleter = {
getCompletions: function (editor, session, pos, prefix, callback) {
var completions = [
{
caption: "slow option 1",
value: "s1",
score: 3
}, {
caption: "slow option 2",
value: "s2",
score: 0
}
];
setTimeout(() => {
callback(null, completions);
}, 200);
}
};

var fastCompleter = {
getCompletions: function (editor, session, pos, prefix, callback) {
var completions = [
{
caption: "fast option 1",
value: "f1",
score: 2
}, {
caption: "fast option 2",
value: "f2",
score: 1
}
];
callback(null, completions);
}
};

editor.completers = [fastCompleter, slowCompleter];

var completer = Autocomplete.for(editor);
completer.stickySelectionDelay = 100;
user.type("Ctrl-Space");
assert.equal(completer.popup.isOpen, true);
assert.equal(completer.popup.data.length, 2);
assert.equal(completer.popup.getRow(), 0);

setTimeout(() => {
completer.popup.renderer.$loop._flush();
assert.equal(completer.popup.data.length, 4);
assert.equal(completer.popup.getRow(), 1);

done();
}, 500);
},
"test: should not maintain selection on fast completer item when slow completer results come in when stickySelectionDelay negative": function(done) {
var editor = initEditor("hello world\n");

var slowCompleter = {
getCompletions: function (editor, session, pos, prefix, callback) {
var completions = [
{
caption: "slow option 1",
value: "s1",
score: 3
}, {
caption: "slow option 2",
value: "s2",
score: 0
}
];
setTimeout(() => {
callback(null, completions);
}, 200);
}
};

var fastCompleter = {
getCompletions: function (editor, session, pos, prefix, callback) {
var completions = [
{
caption: "fast option 1",
value: "f1",
score: 2
}, {
caption: "fast option 2",
value: "f2",
score: 1
}
];
callback(null, completions);
}
};

editor.completers = [fastCompleter, slowCompleter];

var completer = Autocomplete.for(editor);
completer.stickySelectionDelay = -1;
user.type("Ctrl-Space");
assert.equal(completer.popup.isOpen, true);
assert.equal(completer.popup.data.length, 2);
assert.equal(completer.popup.getRow(), 0);

setTimeout(() => {
completer.popup.renderer.$loop._flush();
assert.equal(completer.popup.data.length, 4);
assert.equal(completer.popup.getRow(), 0);

done();
}, 500);
}
};

Expand Down
Loading