fix(VirtualRenderer): fix scrollbar overlap on autocompletion (#5713)

src/autocomplete_test.js

@@ -1599,6 +1599,67 @@ module.exports = {
         var editor = initEditor("");
         var completer = Autocomplete.for(editor);
         assert.equal(Autocomplete.$sharedInstance == undefined, false);
+    },
+    "test: changing completion should render scrollbars correctly": function (done) {
+        var editor = initEditor("document");
+        var newLineCharacter = editor.session.doc.getNewLineCharacter();
+        var initialCompletions = [
+            {
+                caption: "small",
+                meta: "small",
+                value: "small"
+            }
+        ];
+        var longCompletions = Array(10)
+            .fill(null)
+            .map((_, i) => ({
+                caption: `this is a really long string that I want to use for testing horizontal scroll ${i}`,
+                meta: `meta ${i}`,
+                value: `value ${i}`
+            }));
+
+        var currentCompletions = initialCompletions;
+
+        editor.completers = [
+            {
+                getCompletions: function (editor, session, pos, prefix, callback) {
+                    var completions = currentCompletions;
+                    callback(null, completions);
+                },
+                triggerCharacters: [newLineCharacter]
+            }
+        ];
+
+        editor.moveCursorTo(0, 8);
+        user.type("Return"); // Accept suggestion
+        var popup = editor.completer.popup;
+        check(function () {
+            assert.equal(popup.data.length, 1);
+            assert.notOk(popup.renderer.scrollBar.isVisible);
+            user.type("Return"); // Accept suggestion
+            assert.equal(editor.getValue(), `document${newLineCharacter}small`);
+            // change completion values
+            currentCompletions = longCompletions;
+            check(function () {
+                user.type("Return"); // Enter new line
+                assert.equal(popup.renderer.layerConfig.height, popup.renderer.lineHeight * 1);
+                assert.equal(popup.data.length, 10);
+                check(function () {
+                    assert.ok(popup.renderer.scrollBar.isVisible);
+                    assert.equal(popup.renderer.layerConfig.height, popup.renderer.lineHeight * 8);
+                    user.type("Return"); // Accept suggestion
+                    assert.equal(editor.getValue(), `document${newLineCharacter}small${newLineCharacter}value 0`);
+                    done();
+                });
+            });
+        });
+        function check(callback) {
+            popup = editor.completer.popup;
+            popup.renderer.on("afterRender", function wait() {
+                popup.renderer.off("afterRender", wait);
+                callback();
+            });
+        }
     }
 };
 

src/virtual_renderer.js

@@ -1109,8 +1109,11 @@ class VirtualRenderer {
         }
         var vScrollBefore = this.$vScroll; // autosize can change vscroll value in which case we need to update longestLine
         // autoresize only after updating hscroll to include scrollbar height in desired height
-        if (this.$maxLines && this.lineHeight > 1)
+        if (this.$maxLines && this.lineHeight > 1){
             this.$autosize();
+            // recalculate this after $autosize so we take vertical scroll into account when calculating width
+            hideScrollbars = size.height <= 2 * this.lineHeight;
+        }
 
         var minHeight = size.scrollerHeight + this.lineHeight;
 

src/virtual_renderer_test.js

@@ -134,6 +134,56 @@ module.exports = {
         });
     },
 
+
+    "test scrollbars after value change": function() {
+        editor.container.style.height = "0px";
+        editor.setOptions({
+            maxLines: 8,
+        });
+        var renderCount = 0;
+        editor.renderer.on("afterRender", function(e) {
+            renderCount++;
+        });
+        // horizontal scroll
+        editor.setValue("\n");
+        editor.resize(true);
+        editor.renderer.$loop._flush(); // 1
+        assert.notOk(editor.renderer.scrollBarH.isVisible);
+        assert.notOk(editor.renderer.scrollBar.isVisible);
+        editor.setValue("\n\n\n\n" + "_".repeat(30));
+        editor.resize(true);
+        editor.renderer.$loop._flush(); // 2
+        assert.notOk(editor.renderer.scrollBarH.isVisible);
+        assert.notOk(editor.renderer.scrollBar.isVisible);
+        editor.setValue("\n\n\n\n" + "_".repeat(100));
+        editor.resize(true);
+        editor.renderer.$loop._flush(); // 3
+        assert.ok(editor.renderer.scrollBarH.isVisible);
+        assert.notOk(editor.renderer.scrollBar.isVisible);
+        // vertical scroll
+        editor.setValue("\n".repeat(9));
+        editor.resize(true);
+        editor.renderer.$loop._flush(); // 4
+        assert.notOk(editor.renderer.scrollBarH.isVisible);
+        assert.ok(editor.renderer.scrollBar.isVisible);
+        // vertical and horizontal scroll
+        editor.setValue("\n");
+        editor.resize(true);
+        editor.renderer.$loop._flush(); // 5
+        editor.setValue("\n".repeat(9) + "_".repeat(100));
+        editor.resize(true);
+        editor.renderer.$loop._flush(); // 6
+        assert.notOk(editor.renderer.scrollBarH.isVisible);
+        assert.ok(editor.renderer.scrollBar.isVisible);
+        editor.resize(true);
+        editor.renderer.$loop._flush(); // 7
+        // autosize changes vscroll value in which case updates longestLine
+        // this is why it renders an extra time
+        assert.ok(editor.renderer.scrollBarH.isVisible);
+        assert.ok(editor.renderer.scrollBar.isVisible);
+        assert.equal(renderCount, 7);
+    },
+
     "test autosize from 0 height": function() {
         editor.container.style.height = "0px";
         editor.textInput.getElement().style.position = "fixed";