diff options
author | 2023-11-30 17:04:34 -0500 | |
---|---|---|
committer | 2023-11-30 17:04:34 -0500 | |
commit | b750a161e0e059de9cf814ce271d5891e4e97cbe (patch) | |
tree | 102d25d8d7262cc25f3ef60d19c2e9ac8c2f1268 | |
parent | 9ea3e0b94f7c4813c52bffd78043f90fd87dffda (diff) | |
download | astro-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
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 }); |