Skip to content

Commit

Permalink
Use fixed positioning on x-ray when inside of fixed containers (#9254)
Browse files Browse the repository at this point in the history
* Improve highlight/tooltip positioning

* add changeset

* Explain the perf issue
  • Loading branch information
matthewp authored Nov 30, 2023
1 parent 9ea3e0b commit b750a16
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/grumpy-turtles-tickle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Improve highlight/tooltip positioning when in fixed positions
Original file line number Diff line number Diff line change
Expand Up @@ -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`;
Expand Down
12 changes: 10 additions & 2 deletions packages/astro/src/runtime/client/dev-overlay/plugins/xray.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 });
Expand Down

0 comments on commit b750a16

Please sign in to comment.