diff --git a/src/components/shared/VirtualList.js b/src/components/shared/VirtualList.js index b1d1dfb5e6..4253d06b0a 100644 --- a/src/components/shared/VirtualList.js +++ b/src/components/shared/VirtualList.js @@ -36,6 +36,7 @@ import * as React from 'react'; import classNames from 'classnames'; import range from 'array-range'; +import { getResizeObserverWrapper } from 'firefox-profiler/utils/resize-observer-wrapper'; import type { CssPixels } from 'firefox-profiler/types'; @@ -154,21 +155,6 @@ type VirtualListInnerProps = {| class VirtualListInner extends React.PureComponent< VirtualListInnerProps > { - _container: ?HTMLElement; - - _takeContainerRef = (element: ?HTMLDivElement) => { - this._container = element; - }; - - /* This method is used by users of this component. */ - /* eslint-disable-next-line react/no-unused-class-component-methods */ - getBoundingClientRect() { - if (this._container) { - return this._container.getBoundingClientRect(); - } - return new DOMRect(0, 0, 0, 0); - } - render() { const { itemHeight, @@ -194,7 +180,6 @@ class VirtualListInner extends React.PureComponent< return (
= {| +ariaActiveDescendant?: null | string, |}; -type Geometry = { - // getBoundingClientRect in the Flow definitions is wrong, and labels the return values - // as a ClientRect, and not a DOMRect. https://github.com/facebook/flow/issues/5475 - // - // Account for that here: - outerRect: DOMRect | ClientRect, - innerRectY: CssPixels, -}; +type VirtualListState = {| + // This value is updated from the scroll event. + scrollTop: CssPixels, + // This is updated from a resize observer. + containerHeight: CssPixels, +|}; export class VirtualList extends React.PureComponent< - VirtualListProps + VirtualListProps, + VirtualListState > { _container: {| current: HTMLDivElement | null |} = React.createRef(); - _inner: {| current: VirtualListInner | null |} = React.createRef(); - _geometry: ?Geometry; + state = { scrollTop: 0, containerHeight: 0 }; componentDidMount() { document.addEventListener('copy', this._onCopy, false); @@ -285,8 +268,8 @@ export class VirtualList extends React.PureComponent< 'The container was assumed to exist while mounting The VirtualList.' ); } - container.addEventListener('scroll', this._onScroll); - this._onScroll(); // for initial size + + getResizeObserverWrapper().subscribe(container, this._resizeListener); } componentWillUnmount() { @@ -297,12 +280,18 @@ export class VirtualList extends React.PureComponent< 'The container was assumed to exist while unmounting The VirtualList.' ); } - container.removeEventListener('scroll', this._onScroll); + getResizeObserverWrapper().unsubscribe(container, this._resizeListener); } - _onScroll = () => { - this._geometry = this._queryGeometry(); - this.forceUpdate(); + // The listener is only called when the document is visible. + _resizeListener = (contentRect: DOMRectReadOnly) => { + this.setState({ containerHeight: contentRect.height }); + }; + + _onScroll = (event: SyntheticEvent) => { + this.setState({ + scrollTop: event.currentTarget.scrollTop, + }); }; _onCopy = (event: ClipboardEvent) => { @@ -312,29 +301,14 @@ export class VirtualList extends React.PureComponent< } }; - _queryGeometry(): Geometry | void { - const container = this._container.current; - const inner = this._inner.current; - if (!container || !inner) { - return undefined; - } - const outerRect = container.getBoundingClientRect(); - const innerRectY = inner.getBoundingClientRect().top; - return { outerRect, innerRectY }; - } - computeVisibleRange() { const { itemHeight, disableOverscan } = this.props; - if (!this._geometry) { - return { visibleRangeStart: 0, visibleRangeEnd: 100 }; - } - const { outerRect, innerRectY } = this._geometry; + const { scrollTop, containerHeight } = this.state; const overscan = disableOverscan ? 0 : 25; const chunkSize = 16; - let visibleRangeStart = - Math.floor((outerRect.top - innerRectY) / itemHeight) - overscan; + let visibleRangeStart = Math.floor(scrollTop / itemHeight) - overscan; let visibleRangeEnd = - Math.ceil((outerRect.bottom - innerRectY) / itemHeight) + overscan; + Math.ceil((scrollTop + containerHeight) / itemHeight) + overscan; if (!disableOverscan) { visibleRangeStart = Math.floor(visibleRangeStart / chunkSize) * chunkSize; visibleRangeEnd = Math.ceil(visibleRangeEnd / chunkSize) * chunkSize; @@ -502,6 +476,7 @@ export class VirtualList extends React.PureComponent< role={ariaRole} aria-label={ariaLabel} aria-activedescendant={ariaActiveDescendant} + onScroll={this._onScroll} >
extends React.PureComponent< containerWidth={containerWidth} forceRender={forceRender} key={columnIndex} - ref={columnIndex === 0 ? this._inner : undefined} /> ))}
diff --git a/src/components/shared/WithSize.js b/src/components/shared/WithSize.js index bef02144c1..a6a33f500d 100644 --- a/src/components/shared/WithSize.js +++ b/src/components/shared/WithSize.js @@ -44,7 +44,6 @@ export function withSize< // See: https://github.com/firefox-devtools/profiler/issues/3062 // eslint-disable-next-line flowtype/no-existential-type return class WithSizeWrapper extends React.PureComponent<*, State> { - _dirtySize: DOMRectReadOnly | null = null; state = { width: 0, height: 0 }; _container: HTMLElement | null; @@ -55,37 +54,15 @@ export function withSize< } this._container = container; getResizeObserverWrapper().subscribe(container, this._resizeListener); - window.addEventListener( - 'visibilitychange', - this._visibilityChangeListener - ); } - // The size is only updated when the document is visible. - // In other cases resizing is registered in _dirtySize. + // The listener is only called when the document is visible. _resizeListener = (contentRect: DOMRectReadOnly) => { const container = this._container; if (!container) { return; } - if (document.hidden) { - this._dirtySize = contentRect; - } else { - this._updateSize(container, contentRect); - } - }; - - // If resizing was registered when the document wasn't visible, - // the size will be updated when the document becomes visible - _visibilityChangeListener = () => { - const container = this._container; - if (!container) { - return; - } - if (!document.hidden && this._dirtySize) { - this._updateSize(container, this._dirtySize); - this._dirtySize = null; - } + this._updateSize(container, contentRect); }; componentWillUnmount() { @@ -94,10 +71,6 @@ export function withSize< getResizeObserverWrapper().unsubscribe(container, this._resizeListener); } - window.removeEventListener( - 'visibilitychange', - this._visibilityChangeListener - ); this._container = null; } diff --git a/src/test/components/ProfileCallTreeView.test.js b/src/test/components/ProfileCallTreeView.test.js index c7df77e4f0..07d24b3bac 100644 --- a/src/test/components/ProfileCallTreeView.test.js +++ b/src/test/components/ProfileCallTreeView.test.js @@ -44,13 +44,14 @@ import { } from '../fixtures/profiles/processed-profile'; import { createGeckoProfile } from '../fixtures/profiles/gecko-profile'; import { autoMockElementSize } from '../fixtures/mocks/element-size'; +import { triggerResizeObservers } from '../fixtures/mocks/resize-observer'; import type { Profile } from 'firefox-profiler/types'; autoMockCanvasContext(); // This makes the bounding box large enough so that we don't trigger -// VirtualList's virtualization. We assert this above. +// VirtualList's virtualization. We assert this below. autoMockElementSize({ width: 1000, height: 2000 }); describe('calltree/ProfileCallTreeView', function () { @@ -381,6 +382,9 @@ describe('calltree/ProfileCallTreeView navigation keys', () => { ); + // This automatically uses the bounding box set in autoMockElementSize. + triggerResizeObservers(); + // Assert that we used a large enough bounding box to include all children. const renderedRows = container.querySelectorAll( '.treeViewRow.treeViewRowScrolledColumns' diff --git a/src/utils/resize-observer-wrapper.js b/src/utils/resize-observer-wrapper.js index 67986f0e81..3adfb228c9 100644 --- a/src/utils/resize-observer-wrapper.js +++ b/src/utils/resize-observer-wrapper.js @@ -17,21 +17,56 @@ export type ResizeObserverWrapper = {| function createResizeObserverWrapper() { // This keeps the list of callbacks for each observed element. const callbacks: Map> = new Map(); - const resizeObserver = new ResizeObserver((entries) => { + // This keeps the list of changes while the tab is hidden. + const dirtyChanges: Map = new Map(); + + let _resizeObserver = null; + + function notifyListenersForElement(element: Element, rect: DOMRectReadOnly) { + const callbacksForElement = callbacks.get(element); + if (callbacksForElement) { + callbacksForElement.forEach((callback) => callback(rect)); + } + } + + function resizeObserverCallback(entries) { for (const entry of entries) { - const callbacksForElement = callbacks.get(entry.target); - if (callbacksForElement) { - callbacksForElement.forEach((callback) => callback(entry.contentRect)); + if (document.hidden) { + dirtyChanges.set(entry.target, entry.contentRect); + } else { + notifyListenersForElement(entry.target, entry.contentRect); } } - }); + } + + function visibilityChangeListener() { + if (!document.hidden) { + dirtyChanges.forEach((rect, element) => + notifyListenersForElement(element, rect) + ); + dirtyChanges.clear(); + } + } + + function getResizeObserver() { + if (!_resizeObserver) { + _resizeObserver = new ResizeObserver(resizeObserverCallback); + window.addEventListener('visibilitychange', visibilityChangeListener); + } + return _resizeObserver; + } + + function stopResizeObserver() { + _resizeObserver = null; + window.removeEventListener('visibilitychange', visibilityChangeListener); + } return { subscribe(element: HTMLElement, callback: ResizeObserverCallback) { const callbacksForElement = callbacks.get(element) ?? new Set(); callbacks.set(element, callbacksForElement); callbacksForElement.add(callback); - resizeObserver.observe(element); + getResizeObserver().observe(element); }, unsubscribe(element: HTMLElement, callback: ResizeObserverCallback) { const callbacksForElement = callbacks.get(element); @@ -44,12 +79,12 @@ function createResizeObserverWrapper() { } if (callbacksForElement.size === 0) { callbacks.delete(element); - resizeObserver.unobserve(element); + getResizeObserver().unobserve(element); } if (callbacks.size === 0) { // It's important to clean this up properly so that tests are behaving // as expected. - _resizeObserverWrapper = null; + stopResizeObserver(); } } else { console.warn(