From d461e6843338991bd397b930050f8646ffa21e8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Claud=C3=A9ric=20Demers?= Date: Fri, 13 Sep 2024 17:18:41 -0400 Subject: [PATCH] PositionObserver bug fixes --- .../src/core/entities/droppable/droppable.ts | 4 +- .../core/plugins/scrolling/ScrollListener.ts | 9 +- .../src/core/plugins/scrolling/Scroller.ts | 3 +- .../bounding-rectangle/PositionObserver.ts | 134 ++++++++---------- .../getViewportBoundingRectangle.ts | 5 + .../getVisibleBoundingRectangle.ts | 63 ++++++++ .../bounding-rectangle/isOverflowVisible.ts | 16 +++ .../bounding-rectangle/isRectEqual.ts | 7 +- .../dom/src/utilities/scheduling/throttle.ts | 27 ++++ .../dom/src/utilities/scroll/isScrollable.ts | 10 +- packages/geometry/src/shapes/Rectangle.ts | 8 ++ 11 files changed, 200 insertions(+), 86 deletions(-) create mode 100644 packages/dom/src/utilities/bounding-rectangle/getVisibleBoundingRectangle.ts create mode 100644 packages/dom/src/utilities/bounding-rectangle/isOverflowVisible.ts create mode 100644 packages/dom/src/utilities/scheduling/throttle.ts diff --git a/packages/dom/src/core/entities/droppable/droppable.ts b/packages/dom/src/core/entities/droppable/droppable.ts index cc9e0372..711dad14 100644 --- a/packages/dom/src/core/entities/droppable/droppable.ts +++ b/packages/dom/src/core/entities/droppable/droppable.ts @@ -6,7 +6,7 @@ import type { import {defaultCollisionDetection} from '@dnd-kit/collision'; import type {CollisionDetector} from '@dnd-kit/collision'; import {reactive, untracked} from '@dnd-kit/state'; -import type {Shape} from '@dnd-kit/geometry'; +import type {BoundingRectangle, Shape} from '@dnd-kit/geometry'; import {DOMRectangle, PositionObserver} from '@dnd-kit/dom/utilities'; import type {DragDropManager} from '../../manager/manager.ts'; @@ -28,7 +28,7 @@ export class Droppable extends AbstractDroppable< manager: DragDropManager | undefined ) { const {collisionDetector = defaultCollisionDetection} = input; - const updateShape = (boundingClientRect?: DOMRectReadOnly | null) => { + const updateShape = (boundingClientRect?: BoundingRectangle | null) => { const {element} = this; if (!element || boundingClientRect === null) { diff --git a/packages/dom/src/core/plugins/scrolling/ScrollListener.ts b/packages/dom/src/core/plugins/scrolling/ScrollListener.ts index cafad99c..cdab77ff 100644 --- a/packages/dom/src/core/plugins/scrolling/ScrollListener.ts +++ b/packages/dom/src/core/plugins/scrolling/ScrollListener.ts @@ -1,4 +1,4 @@ -import {DragOperationStatus, CorePlugin} from '@dnd-kit/abstract'; +import {CorePlugin} from '@dnd-kit/abstract'; import {effect} from '@dnd-kit/state'; import type {DragDropManager} from '../../manager/index.ts'; @@ -20,10 +20,12 @@ export class ScrollListener extends CorePlugin { const enabled = dragOperation.status.dragging; if (enabled) { - document.addEventListener('scroll', this.handleScroll, listenerOptions); + const root = dragOperation.source?.element?.ownerDocument ?? document; + + root.addEventListener('scroll', this.handleScroll, listenerOptions); return () => { - document.removeEventListener( + root.removeEventListener( 'scroll', this.handleScroll, listenerOptions @@ -37,7 +39,6 @@ export class ScrollListener extends CorePlugin { if (this.#timeout == null) { this.#timeout = setTimeout(() => { this.manager.collisionObserver.forceUpdate(); - this.#timeout = undefined; }, 50); } diff --git a/packages/dom/src/core/plugins/scrolling/Scroller.ts b/packages/dom/src/core/plugins/scrolling/Scroller.ts index f2848bc5..047ba635 100644 --- a/packages/dom/src/core/plugins/scrolling/Scroller.ts +++ b/packages/dom/src/core/plugins/scrolling/Scroller.ts @@ -115,7 +115,8 @@ export class Scroller extends CorePlugin { const {element, by} = this.#meta; - element.scrollBy(by.x, by.y); + if (by.y) element.scrollTop += by.y; + if (by.x) element.scrollLeft += by.x; }; public scroll = (options?: {by: Coordinates}): boolean => { diff --git a/packages/dom/src/utilities/bounding-rectangle/PositionObserver.ts b/packages/dom/src/utilities/bounding-rectangle/PositionObserver.ts index 931bd272..a45f63b5 100644 --- a/packages/dom/src/utilities/bounding-rectangle/PositionObserver.ts +++ b/packages/dom/src/utilities/bounding-rectangle/PositionObserver.ts @@ -1,14 +1,17 @@ -import {isRectEqual} from './isRectEqual.ts'; +import {BoundingRectangle, Rectangle} from '@dnd-kit/geometry'; + +import {throttle} from '../scheduling/throttle.ts'; -const THRESHOLD = [ - 0, 0.05, 0.1, 0.15, 0.2, 0.25, 0.3, 0.35, 0.4, 0.45, 0.5, 0.55, 0.6, 0.65, - 0.7, 0.75, 0.8, 0.85, 0.9, 0.95, 0.99, 1, -]; +import {isRectEqual} from './isRectEqual.ts'; +import {getVisibleBoundingRectangle} from './getVisibleBoundingRectangle.ts'; type PositionObserverCallback = ( - boundingClientRect: DOMRectReadOnly | null + boundingClientRect: BoundingRectangle | null ) => void; +const threshold = Array.from({length: 100}, (_, index) => index / 100); +const THROTTLE_INTERVAL = 75; + export class PositionObserver { constructor( private element: Element, @@ -18,58 +21,46 @@ export class PositionObserver { this.#callback = callback; this.boundingClientRect = element.getBoundingClientRect(); + const root = element.ownerDocument; + if (options?.debug) { this.#debug = document.createElement('div'); this.#debug.style.background = 'rgba(0,0,0,0.15)'; this.#debug.style.position = 'fixed'; this.#debug.style.pointerEvents = 'none'; - element.ownerDocument.body.appendChild(this.#debug); + root.body.appendChild(this.#debug); } this.#visibilityObserver = new IntersectionObserver( (entries: IntersectionObserverEntry[]) => { const entry = entries[entries.length - 1]; - const { - boundingClientRect, - intersectionRect, - isIntersecting: visible, - intersectionRatio, - } = entry; + const {boundingClientRect, isIntersecting: visible} = entry; const {width, height} = boundingClientRect; + const previousVisible = this.#visible; - if (!width && !height) return; - - if (intersectionRatio < 1 && intersectionRatio > 0) { - this.#visibleRect = intersectionRect; - this.#offsetLeft = intersectionRect.left - boundingClientRect.left; - this.#offsetTop = intersectionRect.top - boundingClientRect.top; - } else { - this.#visibleRect = undefined; - this.#offsetLeft = 0; - this.#offsetTop = 0; - } + this.#visible = visible; - this.#observePosition(); + if (!width && !height) return; - if (this.#visible && !visible) { + if (previousVisible && !visible) { this.#positionObserver?.disconnect(); this.#callback(null); this.#resizeObserver?.disconnect(); this.#resizeObserver = undefined; if (this.#debug) this.#debug.style.visibility = 'hidden'; + } else { + this.#observePosition(); } if (visible && !this.#resizeObserver) { this.#resizeObserver = new ResizeObserver(this.#observePosition); this.#resizeObserver.observe(element); } - - this.#visible = visible; }, { - threshold: THRESHOLD, - root: element.ownerDocument ?? document, + threshold, + root, } ); @@ -80,6 +71,7 @@ export class PositionObserver { public boundingClientRect: DOMRectReadOnly; public disconnect() { + this.#disconnected = true; this.#resizeObserver?.disconnect(); this.#positionObserver?.disconnect(); this.#visibilityObserver.disconnect(); @@ -88,59 +80,55 @@ export class PositionObserver { #callback: PositionObserverCallback; #visible = true; - #offsetTop = 0; - #offsetLeft = 0; - #visibleRect: DOMRectReadOnly | undefined; #previousBoundingClientRect: DOMRectReadOnly | undefined; #resizeObserver: ResizeObserver | undefined; #positionObserver: IntersectionObserver | undefined; #visibilityObserver: IntersectionObserver; #debug: HTMLElement | undefined; + #disconnected = false; - #observePosition = () => { + #observePosition = throttle(() => { const {element} = this; - if (!element.isConnected) { - this.disconnect(); + this.#positionObserver?.disconnect(); + + if (this.#disconnected || !this.#visible || !element.isConnected) { return; } const root = element.ownerDocument ?? document; const {innerHeight, innerWidth} = root.defaultView ?? window; - const {width, height} = this.#visibleRect ?? this.boundingClientRect; - const rect = element.getBoundingClientRect(); - const top = rect.top + this.#offsetTop; - const left = rect.left + this.#offsetLeft; - const bottom = top + height; - const right = left + width; - const insetTop = Math.floor(top); - const insetLeft = Math.floor(left); - const insetRight = Math.floor(innerWidth - right); - const insetBottom = Math.floor(innerHeight - bottom); - const rootMargin = `${-insetTop}px ${-insetRight}px ${-insetBottom}px ${-insetLeft}px`; - - this.#positionObserver?.disconnect(); + const clientRect = element.getBoundingClientRect(); + const visibleRect = getVisibleBoundingRectangle(element, clientRect); + const {top, left, bottom, right} = visibleRect; + const insetTop = -Math.floor(top); + const insetLeft = -Math.floor(left); + const insetRight = -Math.floor(innerWidth - right); + const insetBottom = -Math.floor(innerHeight - bottom); + const rootMargin = `${insetTop}px ${insetRight}px ${insetBottom}px ${insetLeft}px`; + + this.boundingClientRect = clientRect; this.#positionObserver = new IntersectionObserver( (entries: IntersectionObserverEntry[]) => { const [entry] = entries; - const {boundingClientRect, intersectionRatio} = entry; - - const previous = this.boundingClientRect; - this.boundingClientRect = - intersectionRatio === 1 - ? boundingClientRect - : element.getBoundingClientRect(); - - if ( - previous.width > width || - previous.height > height || - !isRectEqual(this.boundingClientRect, previous) - ) { + const {intersectionRect} = entry; + /* + * The intersection ratio returned by the intersection observer entry + * represents the ratio of the intersectionRect to the boundingClientRect, + * which is not what we want. We want the ratio of the intersectionRect + * to the rootBounds (visible rect). + */ + const intersectionRatio = Rectangle.intersectionRatio( + intersectionRect, + visibleRect + ); + + if (intersectionRatio !== 1) { this.#observePosition(); } }, { - threshold: [0, 1], + threshold, rootMargin, root, } @@ -148,26 +136,30 @@ export class PositionObserver { this.#positionObserver.observe(element); this.#notify(); - }; + }, THROTTLE_INTERVAL); + + #notify() { + this.#updateDebug(); - async #notify() { if (isRectEqual(this.boundingClientRect, this.#previousBoundingClientRect)) return; - this.#updateDebug(); this.#callback(this.boundingClientRect); this.#previousBoundingClientRect = this.boundingClientRect; } #updateDebug() { if (this.#debug) { - const {top, left, width, height} = this.boundingClientRect; + const {top, left, width, height} = getVisibleBoundingRectangle( + this.element + ); + this.#debug.style.overflow = 'hidden'; this.#debug.style.visibility = 'visible'; - this.#debug.style.top = `${top}px`; - this.#debug.style.left = `${left}px`; - this.#debug.style.width = `${width}px`; - this.#debug.style.height = `${height}px`; + this.#debug.style.top = `${Math.floor(top)}px`; + this.#debug.style.left = `${Math.floor(left)}px`; + this.#debug.style.width = `${Math.floor(width)}px`; + this.#debug.style.height = `${Math.floor(height)}px`; } } } diff --git a/packages/dom/src/utilities/bounding-rectangle/getViewportBoundingRectangle.ts b/packages/dom/src/utilities/bounding-rectangle/getViewportBoundingRectangle.ts index 4b3cdf21..451a86b2 100644 --- a/packages/dom/src/utilities/bounding-rectangle/getViewportBoundingRectangle.ts +++ b/packages/dom/src/utilities/bounding-rectangle/getViewportBoundingRectangle.ts @@ -2,6 +2,11 @@ import type {BoundingRectangle} from '@dnd-kit/geometry'; import {getDocument} from '../execution-context/index.ts'; +/** + * Returns the bounding rectangle of the viewport + * @param element + * @returns BoundingRectangle + */ export function getViewportBoundingRectangle( element: Element ): BoundingRectangle { diff --git a/packages/dom/src/utilities/bounding-rectangle/getVisibleBoundingRectangle.ts b/packages/dom/src/utilities/bounding-rectangle/getVisibleBoundingRectangle.ts new file mode 100644 index 00000000..d0cf4ec4 --- /dev/null +++ b/packages/dom/src/utilities/bounding-rectangle/getVisibleBoundingRectangle.ts @@ -0,0 +1,63 @@ +import type {BoundingRectangle} from '@dnd-kit/geometry'; + +import {isOverflowVisible} from './isOverflowVisible.ts'; + +/* + * Get the currently visible bounding rectangle of an element + * @param element + * @param boundingClientRect + * @returns Rect + */ +export function getVisibleBoundingRectangle( + element: Element, + boundingClientRect = element.getBoundingClientRect() +): BoundingRectangle { + // Get the initial bounding client rect of the element + let rect: BoundingRectangle = boundingClientRect; + const {ownerDocument} = element; + const ownerWindow = ownerDocument.defaultView ?? window; + + // Traverse up the DOM tree to clip the rect based on ancestors' bounding rects + let ancestor: HTMLElement | null = element.parentElement; + + while (ancestor && ancestor !== ownerDocument.documentElement) { + if (!isOverflowVisible(ancestor)) { + const ancestorRect = ancestor.getBoundingClientRect(); + // Clip the rect based on the ancestor's bounding rect + rect = { + top: Math.max(rect.top, ancestorRect.top), + right: Math.min(rect.right, ancestorRect.right), + bottom: Math.min(rect.bottom, ancestorRect.bottom), + left: Math.max(rect.left, ancestorRect.left), + width: 0, // Will be calculated next + height: 0, // Will be calculated next + }; + + // Calculate the width and height after clipping + rect.width = rect.right - rect.left; + rect.height = rect.bottom - rect.top; + } + + // Move to the next ancestor + ancestor = ancestor.parentElement; + } + + // Clip the rect based on the viewport (window) + const viewportWidth = ownerWindow.innerWidth; + const viewportHeight = ownerWindow.innerHeight; + + rect = { + top: Math.max(rect.top, 0), + right: Math.min(rect.right, viewportWidth), + bottom: Math.min(rect.bottom, viewportHeight), + left: Math.max(rect.left, 0), + width: 0, // Will be calculated next + height: 0, // Will be calculated next + }; + + // Calculate the width and height after clipping + rect.width = rect.right - rect.left; + rect.height = rect.bottom - rect.top; + + return rect; +} diff --git a/packages/dom/src/utilities/bounding-rectangle/isOverflowVisible.ts b/packages/dom/src/utilities/bounding-rectangle/isOverflowVisible.ts new file mode 100644 index 00000000..f57c2eb8 --- /dev/null +++ b/packages/dom/src/utilities/bounding-rectangle/isOverflowVisible.ts @@ -0,0 +1,16 @@ +/* + * Check if an element has visible overflow. + * @param element + * @param style + * @returns boolean + */ +export function isOverflowVisible( + element: Element, + style = getComputedStyle(element) +) { + const {overflow, overflowX, overflowY} = style; + + return ( + overflow === 'visible' && overflowX === 'visible' && overflowY === 'visible' + ); +} diff --git a/packages/dom/src/utilities/bounding-rectangle/isRectEqual.ts b/packages/dom/src/utilities/bounding-rectangle/isRectEqual.ts index 742bc114..4ca80227 100644 --- a/packages/dom/src/utilities/bounding-rectangle/isRectEqual.ts +++ b/packages/dom/src/utilities/bounding-rectangle/isRectEqual.ts @@ -1,6 +1,9 @@ -type Rect = Pick; +import type {BoundingRectangle} from '@dnd-kit/geometry'; -export function isRectEqual(a: Rect | undefined, b: Rect | undefined) { +export function isRectEqual( + a: BoundingRectangle | undefined, + b: BoundingRectangle | undefined +) { if (a === b) return true; if (!a || !b) return false; diff --git a/packages/dom/src/utilities/scheduling/throttle.ts b/packages/dom/src/utilities/scheduling/throttle.ts new file mode 100644 index 00000000..caa1ecda --- /dev/null +++ b/packages/dom/src/utilities/scheduling/throttle.ts @@ -0,0 +1,27 @@ +import {timeout} from './timeout.ts'; + +export function throttle any>( + func: T, + limit: number +): (...args: Parameters) => void { + const time = () => performance.now(); + let cancel: () => void | undefined; + let lastRan: number; + + return function (this: any, ...args: Parameters) { + const context = this; + if (!lastRan) { + func.apply(context, args); + lastRan = time(); + } else { + cancel?.(); + cancel = timeout( + () => { + func.apply(context, args); + lastRan = time(); + }, + limit - (time() - lastRan) + ); + } + }; +} diff --git a/packages/dom/src/utilities/scroll/isScrollable.ts b/packages/dom/src/utilities/scroll/isScrollable.ts index d79d07ef..4077c7ba 100644 --- a/packages/dom/src/utilities/scroll/isScrollable.ts +++ b/packages/dom/src/utilities/scroll/isScrollable.ts @@ -7,11 +7,9 @@ export function isScrollable( const overflowRegex = /(auto|scroll|overlay)/; const properties = ['overflow', 'overflowX', 'overflowY']; - return ( - properties.find((property) => { - const value = computedStyle[property as keyof CSSStyleDeclaration]; + return properties.some((property) => { + const value = computedStyle[property as keyof CSSStyleDeclaration]; - return typeof value === 'string' ? overflowRegex.test(value) : false; - }) != null - ); + return typeof value === 'string' ? overflowRegex.test(value) : false; + }); } diff --git a/packages/geometry/src/shapes/Rectangle.ts b/packages/geometry/src/shapes/Rectangle.ts index ca01dd89..a6825983 100644 --- a/packages/geometry/src/shapes/Rectangle.ts +++ b/packages/geometry/src/shapes/Rectangle.ts @@ -100,6 +100,14 @@ export class Rectangle implements Shape { return left + width; } + + static from({top, left, width, height}: BoundingRectangle) { + return new Rectangle(left, top, width, height); + } + + static intersectionRatio(a: BoundingRectangle, b: BoundingRectangle): number { + return Rectangle.from(a).intersectionRatio(Rectangle.from(b)); + } } function rectangleRectangleIntersection(