codemirror popup flickering when updating content

[kasperpeulen]

This problems occurs in all browsers I have tested (IE11, chrome, firefox, safari) all latest version.

At dartpad we have this issue that the popup will flicker when the content is updating.

This is probably because the content is coming from a server, and it takes around 200-300 ms for that request to complete.

Now in this situation it is a bit unfortunate that codemirror is removing the popup from the dom in the showWidget.activity function:

function activity() {
  clearDebounce();
  var pos = completion.cm.getCursor(),
    line = completion.cm.getLine(pos.line);
  if (pos.line != startPos.line || line.length - pos.ch != startLen - startPos.ch ||
    pos.ch < startPos.ch || completion.cm.somethingSelected() ||
    (pos.ch && closeOn.test(line.charAt(pos.ch - 1)))) {
    completion.close();
  } else {
    debounce = requestAnimationFrame(update);
    if (completion.widget) completion.widget.close(); //completion popup removed here
  }
}

In a situation where it takes a long time for the update function to complete, this seems to cause flickering. To solve this, I think it would be best to only remove the popup from the dom just before it is updated again. So while analyzing it a bit more, it seems that in the finishUpdate function, does exactly that:

function finishUpdate(data_) {
  data = data_;
  if (finished) return;
  if (!data || !data.list.length) return done();
  if (completion.widget) completion.widget.close(); //completion widget also closed here
  completion.widget = new Widget(completion, data);
}

So either the line commented here, or the line commented seems to be redundant. To solve the flickering alltogether, I think it would be better to close the the widget in the finishUpdate function and remove the line in the showWidget.activity function. If I do this, the flickering disappears:

[marijnh]

The reason that call is there is that if the user keeps typing and then presses enter in the moment when the new results are being fetched, but the widget hasn't updated yet, they'll get a rather bogus result: the old text is replaced, and the text typed since the last refresh is left at the end. Similarly, if they delete text, the hint picking will overwrite unrelated text.

Concretely, create a slow (~500ms) async hinting function, and open the completion menu. Now type 'x' and quickly press enter. The result will be [completion]x.

I think this problem could be reasonably solved by disabling but not hiding the completion popup while waiting for an asynchronous completion. I.e. add a method to the widget object to remove its keymap, and call that instead of close.

[kasperpeulen]

@marijnh thanks for your response :)

I didn't think about that case. I've been trying to play around with your example, at dartpad, but this issue seems to also exist at dartpad without removing that line:

To reproduce, here is the screenshot taken: http://dev.dart-pad.appspot.com/9126d5d48ebabf5bf547

Edit: This seems to be caused by something different, I can now reproduce what you mean if I remove the line:

This doesn't happen when I don't remove the line.

[kasperpeulen]

@marijnh I I change this line
if (completion.widget) completion.widget.close();
for (as you suggested)
completion.cm.removeKeyMap(completion.widget.keyMap);
both problems seem to be solved.

Only now you have that if type to fast and press enter, you really will press enter, which may not be what you expect in a completion popup. It may be nice to prevent the default behaviour of tab and enter in this case. Something like this seems to work.

function activity() {
  clearDebounce();
  var pos = completion.cm.getCursor(),
    line = completion.cm.getLine(pos.line);
  if (pos.line != startPos.line || line.length - pos.ch != startLen - startPos.ch ||
    pos.ch < startPos.ch || completion.cm.somethingSelected() ||
    (pos.ch && closeOn.test(line.charAt(pos.ch - 1)))) {
    completion.close();
  } else {
    debounce = requestAnimationFrame(update);
    completion.cm.removeKeyMap(completion.widget.keyMap);
    completion.cm.addKeyMap(completion.widget.keyMap = buildKeyMap(completion, {
      pick: function() {}
    }));
  }
}

[marijnh]

Only now you have that if type to fast and press enter, you really will press enter,

I think this is best handled by remembering the enter press, and if the completions come up with a single result, automatically pick that. I've implemented this in attached patch.