Skip to content

Commit 8a5270d

Browse files
nightwingnightwing
authored
fix tooltip position in scaled elements (#5889)
* refactor async test * improve getBoundingClientRect implementation in mockdom * fix broken doc tooltip positioning logic * allow async/await in tests
1 parent 0ff33c1 commit 8a5270d

7 files changed

Lines changed: 119 additions & 76 deletions

File tree

‎.eslintrc‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,5 +152,19 @@
152152
// space-unary-ops: 2, // require or disallow spaces before/after unary operators (fixable)
153153
// spaced-comment: [2, "always", { markers: ["-", "*", "/", "{", "}", "#"], exceptions: ["}"] }]
154154

155-
}
155+
},
156+
157+
// allow async/await in tests only
158+
overrides: [
159+
{
160+
files: ["**/*_test.js"],
161+
parserOptions: {
162+
ecmaVersion: 2022,
163+
},
164+
env: {
165+
},
166+
rules: {
167+
},
168+
}
169+
]
156170
}

‎src/autocomplete.js‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -687,48 +687,46 @@ class Autocomplete {
687687

688688
var leftSize = rect.left;
689689
var rightSize = window.innerWidth - rect.right - scrollBarSize;
690-
var topSize = popup.isTopdown ? rect.top : window.innerHeight - scrollBarSize - rect.bottom;
690+
var topSize = popup.isTopdown ? window.innerHeight - scrollBarSize - rect.bottom : rect.top;
691691
var scores = [
692692
Math.min(rightSize / targetWidth, 1),
693693
Math.min(leftSize / targetWidth, 1),
694-
Math.min(topSize / targetHeight * 0.9),
694+
Math.min(topSize / targetHeight, 1) * 0.9,
695695
];
696696
var max = Math.max.apply(Math, scores);
697697
var tooltipStyle = tooltipNode.style;
698698
tooltipStyle.display = "block";
699699

700-
if (max == scores[0]) {
700+
if (max == scores[0] || scores[0] >= 1) {
701701
tooltipStyle.left = (rect.right + 1) + "px";
702702
tooltipStyle.right = "";
703703
tooltipStyle.maxWidth = targetWidth * max + "px";
704704
tooltipStyle.top = rect.top + "px";
705705
tooltipStyle.bottom = "";
706706
tooltipStyle.maxHeight = Math.min(window.innerHeight - scrollBarSize - rect.top, targetHeight) + "px";
707-
} else if (max == scores[1]) {
707+
} else if (max == scores[1] || scores[1] >= 1) {
708708
tooltipStyle.right = window.innerWidth - rect.left + "px";
709709
tooltipStyle.left = "";
710710
tooltipStyle.maxWidth = targetWidth * max + "px";
711711
tooltipStyle.top = rect.top + "px";
712712
tooltipStyle.bottom = "";
713713
tooltipStyle.maxHeight = Math.min(window.innerHeight - scrollBarSize - rect.top, targetHeight) + "px";
714714
} else if (max == scores[2]) {
715-
tooltipStyle.left = window.innerWidth - rect.left + "px";
716-
tooltipStyle.maxWidth = Math.min(targetWidth, window.innerWidth) + "px";
715+
tooltipStyle.left = rect.left + "px";
716+
tooltipStyle.right = "";
717+
tooltipStyle.maxWidth = Math.min(targetWidth, window.innerWidth - rect.left) + "px";
717718

718719
if (popup.isTopdown) {
719720
tooltipStyle.top = rect.bottom + "px";
720-
tooltipStyle.left = rect.left + "px";
721-
tooltipStyle.right = "";
722721
tooltipStyle.bottom = "";
723722
tooltipStyle.maxHeight = Math.min(window.innerHeight - scrollBarSize - rect.bottom, targetHeight) + "px";
724723
} else {
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";
724+
tooltipStyle.top = "";
725+
tooltipStyle.bottom = (window.innerHeight - rect.top) + "px";
726+
tooltipStyle.maxHeight = Math.min(rect.top, targetHeight) + "px";
730727
}
731728
}
729+
dom.$fixPositionBug(tooltipNode);
732730
}
733731

734732
hideDocTooltip() {

‎src/autocomplete/popup.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@ class AcePopup {
389389

390390
el.style.left = left + "px";
391391
el.style.right = "";
392+
dom.$fixPositionBug(el);
392393

393394
if (!popup.isOpen) {
394395
popup.isOpen = true;
@@ -399,7 +400,6 @@ class AcePopup {
399400
popup.anchorPos = pos;
400401
popup.anchor = anchor;
401402

402-
dom.$fixPositionBug(el);
403403

404404
return true;
405405
};

‎src/autocomplete_test.js‎

Lines changed: 61 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ module.exports = {
614614
editor.destroy();
615615
editor.container.remove();
616616
},
617-
"test: selection should follow hovermarker if setSelectOnHover true": function(done) {
617+
"test: selection should follow hovermarker if setSelectOnHover true": function() {
618618
editor = initEditor("hello world\n");
619619

620620
editor.completers = [
@@ -643,19 +643,20 @@ module.exports = {
643643
assert.equal(editor.completer.popup.isOpen, true);
644644
assert.equal(completer.popup.getRow(), 0);
645645

646+
editor.completer.popup.renderer.$loop._flush();
647+
646648
var text = completer.popup.renderer.content.childNodes[2];
647649
var rect = text.getBoundingClientRect();
648650

649651
// We need two mouse events to trigger the updating of the hover marker.
650652
text.dispatchEvent(new MouseEvent("move", {x: rect.left, y: rect.top}));
651653
// Hover over the second row.
652-
text.dispatchEvent(new MouseEvent("move", {x: rect.left + 1, y: rect.top + 20}));
654+
var lineHeight = completer.popup.renderer.lineHeight;
655+
text.dispatchEvent(new MouseEvent("move", {x: rect.left + 1, y: rect.top + 1.5 * lineHeight}));
653656

654657
// Selected row should follow mouse.
655658
editor.completer.popup.renderer.$loop._flush();
656-
assert.equal(completer.popup.getRow(), 2);
657-
658-
done();
659+
assert.equal(completer.popup.getRow(), 1);
659660
},
660661
"test: selection should not follow hovermarker if setSelectOnHover not set": function(done) {
661662
editor = initEditor("hello world\n");
@@ -1705,7 +1706,7 @@ module.exports = {
17051706
});
17061707
}
17071708
},
1708-
"test: doc tooltip positioning": function (done) {
1709+
"test: doc tooltip positioning": async function(done) {
17091710
var editor = initEditor("");
17101711
var longDoc = "This is a very long documentation text that should wrap and test the tooltip width constraints.";
17111712

@@ -1717,70 +1718,79 @@ module.exports = {
17171718
caption: "completion1",
17181719
value: "completion1",
17191720
docHTML: longDoc
1721+
},
1722+
{
1723+
caption: "completion2",
1724+
value: "completion2"
1725+
},
1726+
{
1727+
caption: "completion3",
1728+
value: "completion3"
17201729
}
17211730
]);
17221731
}
17231732
}
17241733
];
17251734

1726-
user.type("c");
1735+
1736+
editor.resize();
1737+
await editor.renderer.once("afterRender");
1738+
editor.completer.showPopup(editor);
17271739

17281740
var popup = editor.completer.popup;
1741+
var popupRect, tooltipRect;
17291742

1730-
function checkTooltipPosition(positionCheck, message, next) {
1731-
afterRenderCheck(popup, function () {
1732-
editor.completer.onLayoutChange();
1733-
var tooltipNode = editor.completer.tooltipNode;
1734-
var popupRect = popup.container.getBoundingClientRect();
1735-
var tooltipRect = tooltipNode.getBoundingClientRect();
1736-
assert.ok(positionCheck(popupRect, tooltipRect), message);
1737-
next();
1738-
});
1743+
async function waitForDocTooltip() {
1744+
editor.renderer.$loop._flush();
1745+
await new Promise(resolve => {setTimeout(resolve, 50);});
1746+
editor.completer.onLayoutChange();
1747+
var tooltipNode = editor.completer.tooltipNode;
1748+
popupRect = popup.container.getBoundingClientRect();
1749+
tooltipRect = tooltipNode.getBoundingClientRect();
17391750
}
17401751

17411752
// Mock the CSS behaviour
17421753
popup.container.style.width = "300px";
17431754
popup.container.style.height = "300px";
1744-
const editorWidth = 400;
1755+
var editorWidth = 400;
17451756
editor.container.style.width = editorWidth + "px";
17461757
editor.container.style.height = "100px";
17471758
editor.container.style.left = "0px";
17481759
editor.container.style.top = "0px";
1760+
popup.container.style.positionHint = "fixed";
1761+
1762+
1763+
user.type("c");
1764+
await waitForDocTooltip();
1765+
assert.ok(tooltipRect.left > popupRect.right, "Tooltip should appear on the right");
17491766

1750-
checkTooltipPosition((popupRect, tooltipRect) => tooltipRect.left > popupRect.right,
1751-
"Tooltip should appear on the right", () => {
1752-
editor.container.style.left = (window.innerWidth - editorWidth) + "px";
1753-
user.type("o");
1754-
1755-
checkTooltipPosition((popupRect, tooltipRect) => tooltipRect.right < popupRect.left,
1756-
"Tooltip should appear on the left", () => {
1757-
editor.container.style.left = "400px";
1758-
editor.container.style.top = "0px";
1759-
popup.isTopdown = true;
1760-
user.type("Escape");
1761-
user.type("Enter");
1762-
user.type("c");
1763-
1764-
checkTooltipPosition((popupRect, tooltipRect) => tooltipRect.top > popupRect.bottom,
1765-
"Tooltip should appear below", () => {
1766-
editor.container.style.top = (window.innerHeight - 100) + "px";
1767-
editor.container.style.left = "0px";
1768-
popup.isTopdown = false;
1769-
user.type("Escape");
1770-
user.type("Enter");
1771-
user.type("c");
1772-
1773-
checkTooltipPosition((popupRect, tooltipRect) => tooltipRect.bottom <= popupRect.top,
1774-
"Tooltip should appear above", function () {
1775-
done();
1776-
}
1777-
);
1778-
}
1779-
);
1780-
}
1781-
);
1782-
}
1783-
);
1767+
editor.container.style.left = (window.innerWidth - editorWidth) + "px";
1768+
user.type("o");
1769+
1770+
await waitForDocTooltip();
1771+
1772+
assert.ok(tooltipRect.right <= popupRect.left, "Tooltip should appear on the left");
1773+
1774+
editor.container.style.left = "400px";
1775+
editor.container.style.top = "0px";
1776+
popup.container.style.width = (window.innerWidth - 100) + "px";
1777+
user.type("Escape");
1778+
user.type("Enter");
1779+
user.type("c");
1780+
1781+
await waitForDocTooltip();
1782+
assert.ok(tooltipRect.top >= popupRect.bottom, "Tooltip should appear below");
1783+
1784+
editor.container.style.top = (window.innerHeight - 100) + "px";
1785+
editor.container.style.left = "0px";
1786+
user.type("Escape");
1787+
user.type("Enter");
1788+
user.type("c");
1789+
1790+
await waitForDocTooltip();
1791+
assert.ok(tooltipRect.bottom <= popupRect.top, "Tooltip should appear above");
1792+
1793+
done();
17841794
},
17851795
};
17861796

‎src/marker_group_test.js‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ module.exports = {
2727
document.body.appendChild(editor.container);
2828
editor.container.style.height = "200px";
2929
editor.container.style.width = "300px";
30-
dom.importCssString('.ace_tooltip-marker_test { position: absolute; }');
30+
dom.importCssString('.ace_tooltip-marker_test { position: absolute; background: rgba(255, 0, 0, 0.3); }', 'marker_group_test');
3131

3232
next();
3333
},
@@ -86,7 +86,7 @@ module.exports = {
8686
// Height should be two lines
8787
assert.equal(markerSize.height, 2 * lineHeight);
8888
// Should start at the beginning of the line
89-
assert.equal(markerSize.left, 0);
89+
assert.equal(markerSize.left, editor.renderer.scroller.getBoundingClientRect().left);
9090
// Shoud be as wide as the marker layer itself.
9191
assert.equal(markerSize.width, editor.renderer.$markerBack.element.getBoundingClientRect().width);
9292
},
@@ -110,12 +110,13 @@ module.exports = {
110110
var lineHeight = editor.renderer.lineHeight;
111111
var characterWidth = editor.renderer.characterWidth;
112112

113+
var baseRect = editor.renderer.$markerBack.element.getBoundingClientRect();
113114
// Height should be one lines
114115
assert.equal(markerSize.height, lineHeight);
115116
// Should start at the 13th character (including 4px offset)
116-
assert.equal(markerSize.left, 12 * characterWidth + 4);
117+
assert.equal(markerSize.left, 12 * characterWidth + 4 + baseRect.left);
117118
// Shoud be as wide as the marker layer - 12 characters and the offset on both sides.
118-
assert.equal(markerSize.width, editor.renderer.$markerBack.element.getBoundingClientRect().width - 12 * characterWidth - 4 - 4);
119+
assert.equal(markerSize.width, baseRect.width - 12 * characterWidth - 4 - 4);
119120
},
120121
"test: should default to markers of text type": function() {
121122
editor.resize(true);
@@ -138,13 +139,14 @@ module.exports = {
138139
var markerSize = editor.container.querySelectorAll(".m")[0].getBoundingClientRect();
139140
var lineHeight = editor.renderer.lineHeight;
140141
var characterWidth = editor.renderer.characterWidth;
142+
var baseRect = editor.renderer.$markerBack.element.getBoundingClientRect();
141143

142144
// Height should be one lines
143145
assert.equal(markerSize.height, lineHeight);
144146
// Should start at the 13th character (including 4px offset)
145-
assert.equal(markerSize.left, 12 * characterWidth + 4);
147+
assert.equal(markerSize.left, 12 * characterWidth + 4 + baseRect.left);
146148
// Shoud be as wide as the remaining characters in the range on the first line.
147-
assert.equal(markerSize.width, 6 * characterWidth);
149+
assert.equal(Math.round(markerSize.width), Math.round(6 * characterWidth));
148150
},
149151
tearDown: function() {
150152
editor.destroy();

‎src/test/mockdom.js‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -425,11 +425,18 @@ function Node(name) {
425425
if (!height) height = CHAR_HEIGHT;
426426
}
427427
else if (this.parentNode) {
428+
var isFixed = this.style.position == "fixed"
429+
|| this.style.positionHint == "fixed"
430+
|| this.getAttribute("role") == "tooltip";
428431
// prevent recursion by passing -1
429-
var rect = fromChild == -1
432+
var rect = fromChild == -1 || isFixed
430433
? {top: 0, left: 0, width: 0, height: 0, right: 0, bottom: 0}
431434
: this.parentNode.getBoundingClientRect();
432-
435+
if (isFixed) {
436+
rect.height = rect.bottom = WINDOW_HEIGHT;
437+
rect.width = rect.right = WINDOW_WIDTH;
438+
}
439+
433440
left = parseCssLength(this.style.left || "0", rect.width);
434441
top = parseCssLength(this.style.top || "0", rect.height);
435442
var right = parseCssLength(this.style.right || "0", rect.width);
@@ -458,9 +465,16 @@ function Node(name) {
458465
if (!height && !this.style.height && this.firstChild && this.firstChild.getBoundingClientRect && !fromChild) {
459466
height = this.firstChild.getBoundingClientRect(-1).height;
460467
}
468+
469+
if (!this.style.left && this.style.right) {
470+
left = rect.width - right - width;
471+
}
472+
if (!this.style.top && this.style.bottom) {
473+
top = rect.height - bottom - height;
474+
}
461475

462476
top += rect.top;
463-
bottom += rect.bottom;
477+
left += rect.left;
464478
}
465479
return {top: top, left: left, width: width, height: height, right: left + width, bottom: top + height};
466480
};
@@ -919,7 +933,11 @@ exports.load = function() {
919933
loaded = true;
920934
};
921935

922-
exports.loadInBrowser = function(global) {
936+
exports.loadInBrowser = function(global, $setSize) {
937+
if ($setSize) {
938+
WINDOW_HEIGHT = global.innerHeight;
939+
WINDOW_WIDTH = global.innerWidth;
940+
}
923941
delete global.ResizeObserver;
924942
global.__origRoot__ = global.document.documentElement;
925943
global.__origBody__ = global.document.body;

‎src/tooltip_test.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ module.exports = {
2121
document.body.appendChild(editor.container);
2222
editor.container.style.height = "200px";
2323
editor.container.style.width = "300px";
24+
editor.container.style.position = "absolute";
2425

2526
docTooltip.setDataProvider(function(e, editor) {
2627
let session = editor.session;

0 commit comments

Comments
 (0)