From 7539caebc8c552dd015bac4ea9ae8c4066481a8c Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 5 Jun 2024 20:20:25 +1000 Subject: [PATCH 1/4] Fix Virtualizer infinite loop for height 0 --- .../list/stories/ListView.stories.tsx | 30 +++++++++++++++++++ .../virtualizer/src/OverscanManager.ts | 5 +++- .../virtualizer/src/Virtualizer.ts | 10 +++---- 3 files changed, 39 insertions(+), 6 deletions(-) diff --git a/packages/@react-spectrum/list/stories/ListView.stories.tsx b/packages/@react-spectrum/list/stories/ListView.stories.tsx index 4ce91b6d404..28c65d5925a 100644 --- a/packages/@react-spectrum/list/stories/ListView.stories.tsx +++ b/packages/@react-spectrum/list/stories/ListView.stories.tsx @@ -517,3 +517,33 @@ function Demo(props) { ); } + +const manyItems = []; +for (let i = 0; i < 500; i++) {manyItems.push({key: i, name: `item ${i}`});} + +function DisplayNoneComponent(args) { + const [isDisplay, setIsDisplay] = useState(false); + useEffect(() => { + const timeout = setTimeout(() => setIsDisplay(true), 10000); + return () => clearTimeout(timeout); + }, []); + + return ( +
+ + {(item: any) => ( + + + {item.name} + + + )} + +
+ ); +} + +export const DisplayNone: ListViewStory = { + render: (args) => , + name: 'display: none with many items' +}; diff --git a/packages/@react-stately/virtualizer/src/OverscanManager.ts b/packages/@react-stately/virtualizer/src/OverscanManager.ts index 8e5bf126a2a..8f527117ce1 100644 --- a/packages/@react-stately/virtualizer/src/OverscanManager.ts +++ b/packages/@react-stately/virtualizer/src/OverscanManager.ts @@ -17,7 +17,7 @@ export class OverscanManager { private startTime = 0; private velocity = new Point(0, 0); private visibleRect = new Rect(); - + setVisibleRect(rect: Rect) { let time = performance.now() - this.startTime; if (time < 500) { @@ -36,6 +36,9 @@ export class OverscanManager { getOverscannedRect() { let overscanned = this.visibleRect.copy(); + if (this.visibleRect.height === 0) { + return overscanned; + } let overscanY = this.visibleRect.height / 3; overscanned.height += overscanY; diff --git a/packages/@react-stately/virtualizer/src/Virtualizer.ts b/packages/@react-stately/virtualizer/src/Virtualizer.ts index 7940dcabbdb..ea108d2662e 100644 --- a/packages/@react-stately/virtualizer/src/Virtualizer.ts +++ b/packages/@react-stately/virtualizer/src/Virtualizer.ts @@ -23,8 +23,8 @@ import {Size} from './Size'; /** * The Virtualizer class renders a scrollable collection of data using customizable layouts. - * It supports very large collections by only rendering visible views to the DOM, reusing - * them as you scroll. Virtualizer can present any type of view, including non-item views + * It supports very large collections by only rendering visible views to the DOM, reusing + * them as you scroll. Virtualizer can present any type of view, including non-item views * such as section headers and footers. * * Virtualizer uses {@link Layout} objects to compute what views should be visible, and how @@ -133,7 +133,7 @@ export class Virtualizer { */ keyAtPoint(point: Point): Key | null { let rect = new Rect(point.x, point.y, 1, 1); - let layoutInfos = this.layout.getVisibleLayoutInfos(rect); + let layoutInfos = rect.height === 0 ? [] : this.layout.getVisibleLayoutInfos(rect); // Layout may return multiple layout infos in the case of // persisted keys, so find the first one that actually intersects. @@ -180,7 +180,7 @@ export class Virtualizer { rect = this._overscanManager.getOverscannedRect(); } - let layoutInfos = this.layout.getVisibleLayoutInfos(rect); + let layoutInfos = rect.height === 0 ? [] : this.layout.getVisibleLayoutInfos(rect); let map = new Map; for (let layoutInfo of layoutInfos) { map.set(layoutInfo.key, layoutInfo); @@ -273,7 +273,7 @@ export class Virtualizer { if (!this.visibleRect.equals(opts.visibleRect)) { this._overscanManager.setVisibleRect(opts.visibleRect); let shouldInvalidate = this.layout.shouldInvalidate(opts.visibleRect, this.visibleRect); - + if (shouldInvalidate) { offsetChanged = !opts.visibleRect.pointEquals(this.visibleRect); sizeChanged = !opts.visibleRect.sizeEquals(this.visibleRect); From ff46136b3c71663afb717ed9ce4622aadd0c85f3 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 5 Jun 2024 20:23:00 +1000 Subject: [PATCH 2/4] simplify --- packages/@react-stately/virtualizer/src/OverscanManager.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/@react-stately/virtualizer/src/OverscanManager.ts b/packages/@react-stately/virtualizer/src/OverscanManager.ts index 8f527117ce1..a12cc686b87 100644 --- a/packages/@react-stately/virtualizer/src/OverscanManager.ts +++ b/packages/@react-stately/virtualizer/src/OverscanManager.ts @@ -36,9 +36,6 @@ export class OverscanManager { getOverscannedRect() { let overscanned = this.visibleRect.copy(); - if (this.visibleRect.height === 0) { - return overscanned; - } let overscanY = this.visibleRect.height / 3; overscanned.height += overscanY; From 5b0f19bd5ba5c22ff9323e5907977ffcd6017adb Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Thu, 6 Jun 2024 07:16:54 +1000 Subject: [PATCH 3/4] check area --- packages/@react-stately/virtualizer/src/Virtualizer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@react-stately/virtualizer/src/Virtualizer.ts b/packages/@react-stately/virtualizer/src/Virtualizer.ts index ea108d2662e..36ed878f948 100644 --- a/packages/@react-stately/virtualizer/src/Virtualizer.ts +++ b/packages/@react-stately/virtualizer/src/Virtualizer.ts @@ -133,7 +133,7 @@ export class Virtualizer { */ keyAtPoint(point: Point): Key | null { let rect = new Rect(point.x, point.y, 1, 1); - let layoutInfos = rect.height === 0 ? [] : this.layout.getVisibleLayoutInfos(rect); + let layoutInfos = rect.area === 0 ? [] : this.layout.getVisibleLayoutInfos(rect); // Layout may return multiple layout infos in the case of // persisted keys, so find the first one that actually intersects. @@ -180,7 +180,7 @@ export class Virtualizer { rect = this._overscanManager.getOverscannedRect(); } - let layoutInfos = rect.height === 0 ? [] : this.layout.getVisibleLayoutInfos(rect); + let layoutInfos = rect.area === 0 ? [] : this.layout.getVisibleLayoutInfos(rect); let map = new Map; for (let layoutInfo of layoutInfos) { map.set(layoutInfo.key, layoutInfo); From 75d0cf4bcd2423d5c115cd3437dcf7469f2a798e Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Fri, 7 Jun 2024 11:04:33 +1000 Subject: [PATCH 4/4] add test and make story easier to use --- .../list/stories/ListView.stories.tsx | 29 +++++++++---------- .../list/test/ListView.test.js | 24 +++++++++++++++ 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/packages/@react-spectrum/list/stories/ListView.stories.tsx b/packages/@react-spectrum/list/stories/ListView.stories.tsx index 28c65d5925a..3c4cb6d822f 100644 --- a/packages/@react-spectrum/list/stories/ListView.stories.tsx +++ b/packages/@react-spectrum/list/stories/ListView.stories.tsx @@ -523,23 +523,22 @@ for (let i = 0; i < 500; i++) {manyItems.push({key: i, name: `item ${i}`});} function DisplayNoneComponent(args) { const [isDisplay, setIsDisplay] = useState(false); - useEffect(() => { - const timeout = setTimeout(() => setIsDisplay(true), 10000); - return () => clearTimeout(timeout); - }, []); return ( -
- - {(item: any) => ( - - - {item.name} - - - )} - -
+ <> + +
+ + {(item: any) => ( + + + {item.name} + + + )} + +
+ ); } diff --git a/packages/@react-spectrum/list/test/ListView.test.js b/packages/@react-spectrum/list/test/ListView.test.js index 6f4eab54f16..5ab989dfa75 100644 --- a/packages/@react-spectrum/list/test/ListView.test.js +++ b/packages/@react-spectrum/list/test/ListView.test.js @@ -1664,4 +1664,28 @@ describe('ListView', function () { }); }); }); + + describe('height 0', () => { + + it('should render and not infinite loop', function () { + offsetWidth = jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 0); + offsetHeight = jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 0); + scrollHeight = jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 0); + let tree = render( + ({key: k, name: `Item ${k}`}))}> + {item => {item.name}} + + ); + let grid = tree.getByRole('grid'); + act(() => { + jest.runAllTimers(); + }); + expect(grid.firstChild).toBeEmptyDOMElement(); + }); + }); });