summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Matthew Phillips <matthew@skypack.dev> 2023-11-30 17:04:34 -0500
committerGravatar GitHub <noreply@github.com> 2023-11-30 17:04:34 -0500
commitb750a161e0e059de9cf814ce271d5891e4e97cbe (patch)
tree102d25d8d7262cc25f3ef60d19c2e9ac8c2f1268
parent9ea3e0b94f7c4813c52bffd78043f90fd87dffda (diff)
downloadastro-b750a161e0e059de9cf814ce271d5891e4e97cbe.tar.gz
astro-b750a161e0e059de9cf814ce271d5891e4e97cbe.tar.zst
astro-b750a161e0e059de9cf814ce271d5891e4e97cbe.zip
Use fixed positioning on x-ray when inside of fixed containers (#9254)
* Improve highlight/tooltip positioning * add changeset * Explain the perf issue
-rw-r--r--.changeset/grumpy-turtles-tickle.md5
-rw-r--r--packages/astro/src/runtime/client/dev-overlay/plugins/utils/highlight.ts22
-rw-r--r--packages/astro/src/runtime/client/dev-overlay/plugins/xray.ts12
3 files changed, 33 insertions, 6 deletions
diff --git a/.changeset/grumpy-turtles-tickle.md b/.changeset/grumpy-turtles-tickle.md
new file mode 100644
index 000000000..75f8f0eec
--- /dev/null
+++ b/.changeset/grumpy-turtles-tickle.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Improve highlight/tooltip positioning when in fixed positions
diff --git a/packages/astro/src/runtime/client/dev-overlay/plugins/utils/highlight.ts b/packages/astro/src/runtime/client/dev-overlay/plugins/utils/highlight.ts
index e2d74c0ff..565e58a1a 100644
--- a/packages/astro/src/runtime/client/dev-overlay/plugins/utils/highlight.ts
+++ b/packages/astro/src/runtime/client/dev-overlay/plugins/utils/highlight.ts
@@ -15,23 +15,37 @@ export function createHighlight(rect: DOMRect, icon?: Icon) {
return highlight;
}
-export function getHighlightZIndex(el: Element) {
+// Figures out the element's z-index and position, based on it's parents.
+export function getElementsPositionInDocument(el: Element) {
let highestZIndex = 0;
+ let fixed = false;
let current: Element | ParentNode | null = el;
while (current instanceof Element) {
- let zIndex = Number(getComputedStyle(current).zIndex);
+ // This is the expensive part, we are calling getComputedStyle which triggers layout
+ // all the way up the tree. We are only doing so when the app initializes, so the cost is one-time
+ // If perf becomes an issue we'll want to refactor this somehow so that it reads this info in a rAF
+ let style = getComputedStyle(current);
+ let zIndex = Number(style.zIndex);
if (!Number.isNaN(zIndex) && zIndex > highestZIndex) {
highestZIndex = zIndex;
}
+ if(style.position === 'fixed') {
+ fixed = true;
+ }
current = current.parentNode;
}
- return highestZIndex + 1;
+ return {
+ zIndex: highestZIndex + 1,
+ fixed,
+ };
}
export function positionHighlight(highlight: DevOverlayHighlight, rect: DOMRect) {
highlight.style.display = 'block';
+ // If the highlight is fixed, don't position based on scroll
+ const scrollY = highlight.style.position === 'fixed' ? 0 : window.scrollY;
// Make an highlight that is 10px bigger than the element on all sides
- highlight.style.top = `${Math.max(rect.top + window.scrollY - 10, 0)}px`;
+ highlight.style.top = `${Math.max(rect.top + scrollY - 10, 0)}px`;
highlight.style.left = `${Math.max(rect.left + window.scrollX - 10, 0)}px`;
highlight.style.width = `${rect.width + 15}px`;
highlight.style.height = `${rect.height + 15}px`;
diff --git a/packages/astro/src/runtime/client/dev-overlay/plugins/xray.ts b/packages/astro/src/runtime/client/dev-overlay/plugins/xray.ts
index 782ede4d2..e76da4de0 100644
--- a/packages/astro/src/runtime/client/dev-overlay/plugins/xray.ts
+++ b/packages/astro/src/runtime/client/dev-overlay/plugins/xray.ts
@@ -3,7 +3,7 @@ import type { DevOverlayHighlight } from '../ui-library/highlight.js';
import {
attachTooltipToHighlight,
createHighlight,
- getHighlightZIndex,
+ getElementsPositionInDocument,
positionHighlight,
} from './utils/highlight.js';
import { createWindowElement } from './utils/window.js';
@@ -83,8 +83,16 @@ export default {
attachTooltipToHighlight(highlight, tooltip, islandElement);
// Set the z-index to be 1 higher than the greatest z-index in the stack.
- const zIndex = getHighlightZIndex(islandElement);
+ // And also set the highlight/tooltip as being fixed position if they are inside
+ // a fixed container. We do this so that we don't mistakenly take scroll position
+ // into account when setting their position.
+ // We are only doing both of these things once, as they are relatively expensive
+ // to calculate, and are unlikely to change. If that turns out to be wrong, reconsider this.
+ const { zIndex, fixed } = getElementsPositionInDocument(islandElement);
tooltip.style.zIndex = highlight.style.zIndex = zIndex + '';
+ if(fixed) {
+ tooltip.style.position = highlight.style.position = 'fixed';
+ }
canvas.append(highlight);
islandsOverlays.push({ highlightElement: highlight, island: islandElement });