Skip to content

Commit f142dbf

Browse files
committed
fix popups being covered by page scrollbar
1 parent cd78a94 commit f142dbf

File tree

6 files changed

+135
-31
lines changed

6 files changed

+135
-31
lines changed

Makefile.dryice.js

+1
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ function demo() {
272272
);
273273
}
274274

275+
require("rimraf").sync(BUILD_DIR + "/demo/kitchen-sink/docs/");
275276
copy(ACE_HOME +"/demo/kitchen-sink/docs/", BUILD_DIR + "/demo/kitchen-sink/docs/");
276277

277278
copy.file(ACE_HOME + "/demo/kitchen-sink/logo.png", BUILD_DIR + "/demo/kitchen-sink/logo.png");

src/autocomplete.js

+46-22
Original file line numberDiff line numberDiff line change
@@ -680,30 +680,54 @@ class Autocomplete {
680680

681681
var popup = this.popup;
682682
var rect = popup.container.getBoundingClientRect();
683-
tooltipNode.style.top = popup.container.style.top;
684-
tooltipNode.style.bottom = popup.container.style.bottom;
685-
686-
tooltipNode.style.display = "block";
687-
if (window.innerWidth - rect.right < 320) {
688-
if (rect.left < 320) {
689-
if(popup.isTopdown) {
690-
tooltipNode.style.top = rect.bottom + "px";
691-
tooltipNode.style.left = rect.left + "px";
692-
tooltipNode.style.right = "";
693-
tooltipNode.style.bottom = "";
694-
} else {
695-
tooltipNode.style.top = popup.container.offsetTop - tooltipNode.offsetHeight + "px";
696-
tooltipNode.style.left = rect.left + "px";
697-
tooltipNode.style.right = "";
698-
tooltipNode.style.bottom = "";
699-
}
683+
684+
var targetWidth = 400;
685+
var targetHeight = 300;
686+
var scrollBarSize = popup.renderer.scrollBar.width || 10;
687+
688+
var leftSize = rect.left;
689+
var rightSize = window.innerWidth - rect.right - scrollBarSize;
690+
var topSize = popup.isTopdown ? rect.top : window.innerHeight - scrollBarSize - rect.bottom;
691+
var scores = [
692+
Math.min(rightSize / targetWidth, 1),
693+
Math.min(leftSize / targetWidth, 1),
694+
Math.min(topSize / targetHeight * 0.9),
695+
];
696+
var max = Math.max.apply(Math, scores);
697+
var tooltipStyle = tooltipNode.style;
698+
tooltipStyle.display = "block";
699+
700+
if (max == scores[0]) {
701+
tooltipStyle.left = (rect.right + 1) + "px";
702+
tooltipStyle.right = "";
703+
tooltipStyle.maxWidth = targetWidth * max + "px";
704+
tooltipStyle.top = rect.top + "px";
705+
tooltipStyle.bottom = "";
706+
tooltipStyle.maxHeight = Math.min(window.innerHeight - scrollBarSize - rect.top, targetHeight) + "px";
707+
} else if (max == scores[1]) {
708+
tooltipStyle.right = window.innerWidth - rect.left + "px";
709+
tooltipStyle.left = "";
710+
tooltipStyle.maxWidth = targetWidth * max + "px";
711+
tooltipStyle.top = rect.top + "px";
712+
tooltipStyle.bottom = "";
713+
tooltipStyle.maxHeight = Math.min(window.innerHeight - scrollBarSize - rect.top, targetHeight) + "px";
714+
} else if (max == scores[2]) {
715+
tooltipStyle.left = window.innerWidth - rect.left + "px";
716+
tooltipStyle.maxWidth = Math.min(targetWidth, window.innerWidth) + "px";
717+
718+
if (popup.isTopdown) {
719+
tooltipStyle.top = rect.bottom + "px";
720+
tooltipStyle.left = rect.left + "px";
721+
tooltipStyle.right = "";
722+
tooltipStyle.bottom = "";
723+
tooltipStyle.maxHeight = Math.min(window.innerHeight - scrollBarSize - rect.bottom, targetHeight) + "px";
700724
} else {
701-
tooltipNode.style.right = window.innerWidth - rect.left + "px";
702-
tooltipNode.style.left = "";
725+
tooltipStyle.top = popup.container.offsetTop - tooltipNode.offsetHeight + "px";
726+
tooltipStyle.left = rect.left + "px";
727+
tooltipStyle.right = "";
728+
tooltipStyle.bottom = "";
729+
tooltipStyle.maxHeight = Math.min(popup.container.offsetTop, targetHeight) + "px";
703730
}
704-
} else {
705-
tooltipNode.style.left = (rect.right + 1) + "px";
706-
tooltipNode.style.right = "";
707731
}
708732
}
709733

src/autocomplete/popup.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -328,8 +328,9 @@ class AcePopup {
328328
}
329329

330330
var el = this.container;
331-
var screenHeight = window.innerHeight;
332-
var screenWidth = window.innerWidth;
331+
var scrollBarSize = this.renderer.scrollBar.width || 10;
332+
var screenHeight = window.innerHeight - scrollBarSize;
333+
var screenWidth = window.innerWidth - scrollBarSize;
333334
var renderer = this.renderer;
334335
// var maxLines = Math.min(renderer.$maxLines, this.session.getLength());
335336
var maxH = renderer.$maxLines * lineHeight * 1.4;
@@ -372,7 +373,7 @@ class AcePopup {
372373

373374
if (anchor === "top") {
374375
el.style.top = "";
375-
el.style.bottom = (screenHeight - dims.bottom) + "px";
376+
el.style.bottom = (screenHeight + scrollBarSize - dims.bottom) + "px";
376377
popup.isTopdown = false;
377378
} else {
378379
el.style.top = dims.top + "px";

src/autocomplete/popup_test.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ if (typeof process !== "undefined") {
88
var assert = require("../test/assertions");
99
var AcePopup = require("./popup").AcePopup;
1010

11+
/**@type {AcePopup} */
1112
var popup;
1213
var lineHeight = 14;
1314
var renderHeight = 8 * lineHeight;
@@ -42,6 +43,7 @@ var setupPopup = function() {
4243

4344
var tearDown = function(done) {
4445
if (popup) {
46+
popup.destroy();
4547
var el = popup.container;
4648
if (el && el.parentNode)
4749
el.parentNode.removeChild(el);
@@ -82,15 +84,15 @@ module.exports = {
8284
var result = tryShowAndRender({ top: 0, left: notEnoughSpaceOnRight }, lineHeight, "bottom");
8385
assert.strictEqual(result, true);
8486
assert.strictEqual(popup.isOpen, true);
85-
assert.strictEqual(popup.container.getBoundingClientRect().right, window.innerWidth);
87+
assert.strictEqual(popup.container.getBoundingClientRect().right, window.innerWidth - (popup.renderer.scrollBar.width || 10));
8688
assert.strictEqual(Math.abs(popup.container.getBoundingClientRect().width - renderWidth) < 5, true);
8789
popup.hide();
8890
assert.strictEqual(popup.isOpen, false);
8991

9092
result = tryShowAndRender({ top: notEnoughSpaceOnBottom, left: notEnoughSpaceOnRight }, lineHeight, "top");
9193
assert.strictEqual(result, true);
9294
assert.strictEqual(popup.isOpen, true);
93-
assert.strictEqual(popup.container.getBoundingClientRect().right, window.innerWidth);
95+
assert.strictEqual(popup.container.getBoundingClientRect().right, window.innerWidth - (popup.renderer.scrollBar.width || 10));
9496
assert.strictEqual(Math.abs(popup.container.getBoundingClientRect().width - renderWidth) < 5, true);
9597
popup.hide();
9698
assert.strictEqual(popup.isOpen, false);

src/autocomplete_test.js

+79-2
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ module.exports = {
215215
editor.onCommandKey(null, 0, 13);
216216
assert.equal(editor.getValue(), "{apple: }");
217217
done();
218-
});
218+
});
219219
},
220220
"test: should set correct aria attributes for popup items": function(done) {
221221
var editor = initEditor("");
@@ -1714,7 +1714,84 @@ module.exports = {
17141714
callback();
17151715
});
17161716
}
1717-
}
1717+
},
1718+
"test: doc tooltip positioning": function (done) {
1719+
var editor = initEditor("");
1720+
var longDoc = "This is a very long documentation text that should wrap and test the tooltip width constraints.";
1721+
1722+
editor.completers = [
1723+
{
1724+
getCompletions: function (editor, session, pos, prefix, callback) {
1725+
callback(null, [
1726+
{
1727+
caption: "completion1",
1728+
value: "completion1",
1729+
docHTML: longDoc
1730+
}
1731+
]);
1732+
}
1733+
}
1734+
];
1735+
1736+
user.type("c");
1737+
1738+
var popup = editor.completer.popup;
1739+
1740+
function checkTooltipPosition(positionCheck, message, next) {
1741+
afterRenderCheck(popup, function () {
1742+
editor.completer.onLayoutChange();
1743+
var tooltipNode = editor.completer.tooltipNode;
1744+
var popupRect = popup.container.getBoundingClientRect();
1745+
var tooltipRect = tooltipNode.getBoundingClientRect();
1746+
assert.ok(positionCheck(popupRect, tooltipRect), message);
1747+
next();
1748+
});
1749+
}
1750+
1751+
// Mock the CSS behaviour
1752+
popup.container.style.width = "300px";
1753+
popup.container.style.height = "300px";
1754+
const editorWidth = 400;
1755+
editor.container.style.width = editorWidth + "px";
1756+
editor.container.style.height = "100px";
1757+
editor.container.style.left = "0px";
1758+
editor.container.style.top = "0px";
1759+
1760+
checkTooltipPosition((popupRect, tooltipRect) => tooltipRect.left > popupRect.right,
1761+
"Tooltip should appear on the right", () => {
1762+
editor.container.style.left = (window.innerWidth - editorWidth) + "px";
1763+
user.type("o");
1764+
1765+
checkTooltipPosition((popupRect, tooltipRect) => tooltipRect.right < popupRect.left,
1766+
"Tooltip should appear on the left", () => {
1767+
editor.container.style.left = "400px";
1768+
editor.container.style.top = "0px";
1769+
popup.isTopdown = true;
1770+
user.type("Escape");
1771+
user.type("Enter");
1772+
user.type("c");
1773+
1774+
checkTooltipPosition((popupRect, tooltipRect) => tooltipRect.top > popupRect.bottom,
1775+
"Tooltip should appear below", () => {
1776+
editor.container.style.top = (window.innerHeight - 100) + "px";
1777+
editor.container.style.left = "0px";
1778+
popup.isTopdown = false;
1779+
user.type("Escape");
1780+
user.type("Enter");
1781+
user.type("c");
1782+
1783+
checkTooltipPosition((popupRect, tooltipRect) => tooltipRect.bottom <= popupRect.top,
1784+
"Tooltip should appear above", function () {
1785+
done();
1786+
}
1787+
);
1788+
}
1789+
);
1790+
}
1791+
);
1792+
}
1793+
);
1794+
},
17181795
};
17191796

17201797
if (typeof module !== "undefined" && module === require.main) {

src/css/editor-css.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,6 @@ module.exports = `
459459
border-radius: 1px;
460460
box-shadow: 0 1px 2px rgba(0, 0, 0, 0.3);
461461
color: black;
462-
max-width: 100%;
463462
padding: 3px 4px;
464463
position: fixed;
465464
z-index: 999999;
@@ -473,7 +472,7 @@ module.exports = `
473472
letter-spacing: normal;
474473
pointer-events: none;
475474
overflow: auto;
476-
max-width: min(60em, 66vw);
475+
max-width: min(33em, 66vw);
477476
overscroll-behavior: contain;
478477
}
479478
.ace_tooltip pre {

0 commit comments

Comments
 (0)