From 1bbd69c060039899c222c6ca97476752fed04a7b Mon Sep 17 00:00:00 2001 From: Yihui Liao <44729383+yihuiliao@users.noreply.github.com> Date: Thu, 1 May 2025 16:54:55 -0700 Subject: [PATCH 1/4] sorta get selected item to scroll into view virtualized --- packages/@react-spectrum/s2/src/TableView.tsx | 3 +++ packages/@react-stately/layout/src/GridLayout.ts | 2 +- packages/@react-stately/layout/src/ListLayout.ts | 2 +- packages/@react-stately/layout/src/TableLayout.ts | 6 +++++- packages/@react-stately/virtualizer/src/Rect.ts | 4 +++- packages/@react-stately/virtualizer/src/Virtualizer.ts | 3 +-- packages/react-aria-components/src/Virtualizer.tsx | 4 ---- 7 files changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TableView.tsx b/packages/@react-spectrum/s2/src/TableView.tsx index 80953147990..91b18ce3204 100644 --- a/packages/@react-spectrum/s2/src/TableView.tsx +++ b/packages/@react-spectrum/s2/src/TableView.tsx @@ -194,6 +194,9 @@ export class S2TableLayout extends TableLayout { protected buildCollection(): LayoutNode[] { let [header, body] = super.buildCollection(); + if (!header) { + return []; + } let {children, layoutInfo} = body; // TableLayout's buildCollection always sets the body width to the max width between the header width, but // we want the body to be sticky and only as wide as the table so it is always in view if loading/empty diff --git a/packages/@react-stately/layout/src/GridLayout.ts b/packages/@react-stately/layout/src/GridLayout.ts index 14c8e0865b4..11c9e3d72c8 100644 --- a/packages/@react-stately/layout/src/GridLayout.ts +++ b/packages/@react-stately/layout/src/GridLayout.ts @@ -227,7 +227,7 @@ export class GridLayout exte // Find the closest item within on either side of the point using the gap width. let key: Key | null = null; if (this.numColumns === 1) { - let searchRect = new Rect(x, Math.max(0, y - this.gap.height), 1, this.gap.height * 2); + let searchRect = new Rect(x, Math.max(0, y - this.gap.height), 1, Math.max(1, this.gap.height * 2)); let candidates = this.getVisibleLayoutInfos(searchRect); let minDistance = Infinity; for (let candidate of candidates) { diff --git a/packages/@react-stately/layout/src/ListLayout.ts b/packages/@react-stately/layout/src/ListLayout.ts index 999bd12b82e..beb9c1f2996 100644 --- a/packages/@react-stately/layout/src/ListLayout.ts +++ b/packages/@react-stately/layout/src/ListLayout.ts @@ -506,7 +506,7 @@ export class ListLayout exte y += this.virtualizer!.visibleRect.y; // Find the closest item within on either side of the point using the gap width. - let searchRect = new Rect(x, Math.max(0, y - this.gap), 1, this.gap * 2); + let searchRect = new Rect(x, Math.max(0, y - this.gap), 1, Math.max(1, this.gap * 2)); let candidates = this.getVisibleLayoutInfos(searchRect); let key: Key | null = null; let minDistance = Infinity; diff --git a/packages/@react-stately/layout/src/TableLayout.ts b/packages/@react-stately/layout/src/TableLayout.ts index 4c820b5b1e9..68a92183a68 100644 --- a/packages/@react-stately/layout/src/TableLayout.ts +++ b/packages/@react-stately/layout/src/TableLayout.ts @@ -85,6 +85,10 @@ export class TableLayout exten this.stickyColumnIndices = []; let collection = this.virtualizer!.collection as TableCollection; + if (collection.head?.key === -1) { + return []; + } + for (let column of collection.columns) { // The selection cell and any other sticky columns always need to be visible. // In addition, row headers need to be in the DOM for accessibility labeling. @@ -543,7 +547,7 @@ export class TableLayout exten y += this.virtualizer!.visibleRect.y; // Find the closest item within on either side of the point using the gap width. - let searchRect = new Rect(x, Math.max(0, y - this.gap), 1, this.gap * 2); + let searchRect = new Rect(x, Math.max(0, y - this.gap), 1, Math.max(1, this.gap * 2)); let candidates = this.getVisibleLayoutInfos(searchRect); let key: Key | null = null; let minDistance = Infinity; diff --git a/packages/@react-stately/virtualizer/src/Rect.ts b/packages/@react-stately/virtualizer/src/Rect.ts index 58a74d0903d..65b55c194fb 100644 --- a/packages/@react-stately/virtualizer/src/Rect.ts +++ b/packages/@react-stately/virtualizer/src/Rect.ts @@ -92,7 +92,9 @@ export class Rect { * @param rect - The rectangle to check. */ intersects(rect: Rect): boolean { - return this.x <= rect.x + rect.width + return this.area > 0 + && rect.area > 0 + && this.x <= rect.x + rect.width && rect.x <= this.x + this.width && this.y <= rect.y + rect.height && rect.y <= this.y + this.height; diff --git a/packages/@react-stately/virtualizer/src/Virtualizer.ts b/packages/@react-stately/virtualizer/src/Virtualizer.ts index c627658a0bd..09e660dd5a7 100644 --- a/packages/@react-stately/virtualizer/src/Virtualizer.ts +++ b/packages/@react-stately/virtualizer/src/Virtualizer.ts @@ -193,8 +193,7 @@ export class Virtualizer { } else { rect = this._overscanManager.getOverscannedRect(); } - - let layoutInfos = rect.area === 0 ? [] : this.layout.getVisibleLayoutInfos(rect); + let layoutInfos = this.layout.getVisibleLayoutInfos(rect); let map = new Map; for (let layoutInfo of layoutInfos) { map.set(layoutInfo.key, layoutInfo); diff --git a/packages/react-aria-components/src/Virtualizer.tsx b/packages/react-aria-components/src/Virtualizer.tsx index affe32c2442..c8eabc015a0 100644 --- a/packages/react-aria-components/src/Virtualizer.tsx +++ b/packages/react-aria-components/src/Virtualizer.tsx @@ -102,10 +102,6 @@ function CollectionRoot({collection, persistedKeys, scrollRef, renderDropIndicat onScrollEnd: state.endScrolling }, scrollRef!); - if (state.contentSize.area === 0) { - return null; - } - return (
From bebc44bdf5b6cce068701ed1ffae25e4ec3411e7 Mon Sep 17 00:00:00 2001 From: Yihui Liao <44729383+yihuiliao@users.noreply.github.com> Date: Thu, 1 May 2025 16:55:05 -0700 Subject: [PATCH 2/4] fix tests --- .../autocomplete/test/SearchAutocomplete.test.js | 2 ++ packages/@react-spectrum/card/test/CardView.test.js | 5 +++++ packages/@react-spectrum/color/test/ColorPicker.test.js | 5 +++++ packages/@react-spectrum/combobox/test/ComboBox.test.js | 2 ++ packages/@react-spectrum/picker/test/Picker.test.js | 7 ++++++- packages/@react-spectrum/picker/test/TempUtilTest.test.js | 5 +++++ packages/@react-spectrum/tabs/test/Tabs.test.js | 1 + 7 files changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js b/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js index 553c7c38f6c..a94c20bea0c 100644 --- a/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js +++ b/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js @@ -143,6 +143,8 @@ describe('SearchAutocomplete', function () { user = userEvent.setup({delay: null, pointerMap}); jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 1000); jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 1000); + jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); + scrollHeight = jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); window.HTMLElement.prototype.scrollIntoView = jest.fn(); simulateDesktop(); jest.useFakeTimers(); diff --git a/packages/@react-spectrum/card/test/CardView.test.js b/packages/@react-spectrum/card/test/CardView.test.js index 9ed6a8df3e9..a05d69d963f 100644 --- a/packages/@react-spectrum/card/test/CardView.test.js +++ b/packages/@react-spectrum/card/test/CardView.test.js @@ -141,9 +141,14 @@ describe('CardView', function () { user = userEvent.setup({delay: null, pointerMap}); jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => mockWidth); jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => mockHeight); + // jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); jest.useFakeTimers(); }); + beforeEach(() => { + jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); + }) + afterEach(() => { jest.clearAllMocks(); act(() => jest.runAllTimers()); diff --git a/packages/@react-spectrum/color/test/ColorPicker.test.js b/packages/@react-spectrum/color/test/ColorPicker.test.js index a22b63aa124..51a930761f7 100644 --- a/packages/@react-spectrum/color/test/ColorPicker.test.js +++ b/packages/@react-spectrum/color/test/ColorPicker.test.js @@ -22,9 +22,14 @@ describe('ColorPicker', function () { beforeAll(() => { user = userEvent.setup({delay: null, pointerMap}); + jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); jest.useFakeTimers(); }); + afterAll(function () { + jest.restoreAllMocks(); + }); + afterEach(() => { act(() => {jest.runAllTimers();}); }); diff --git a/packages/@react-spectrum/combobox/test/ComboBox.test.js b/packages/@react-spectrum/combobox/test/ComboBox.test.js index 4b97d4c810d..3c0401ceb7b 100644 --- a/packages/@react-spectrum/combobox/test/ComboBox.test.js +++ b/packages/@react-spectrum/combobox/test/ComboBox.test.js @@ -257,6 +257,7 @@ describe('ComboBox', function () { }); beforeEach(() => { + jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); load = jest .fn() .mockImplementationOnce(getFilterItems) @@ -3667,6 +3668,7 @@ describe('ComboBox', function () { describe('mobile combobox', function () { beforeEach(() => { simulateMobile(); + }); afterEach(() => { diff --git a/packages/@react-spectrum/picker/test/Picker.test.js b/packages/@react-spectrum/picker/test/Picker.test.js index 36421aaf844..be16a4b0036 100644 --- a/packages/@react-spectrum/picker/test/Picker.test.js +++ b/packages/@react-spectrum/picker/test/Picker.test.js @@ -31,7 +31,7 @@ import {User} from '@react-aria/test-utils'; import userEvent from '@testing-library/user-event'; describe('Picker', function () { - let offsetWidth, offsetHeight; + let offsetWidth, offsetHeight, scrollHeight; let onSelectionChange = jest.fn(); let testUtilUser = new User(); let user; @@ -40,6 +40,7 @@ describe('Picker', function () { user = userEvent.setup({delay: null, pointerMap}); offsetWidth = jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 1000); offsetHeight = jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 1000); + scrollHeight = jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); simulateDesktop(); jest.useFakeTimers(); }); @@ -47,6 +48,7 @@ describe('Picker', function () { afterAll(function () { offsetWidth.mockReset(); offsetHeight.mockReset(); + scrollHeight.mockReset(); }); afterEach(() => { @@ -91,6 +93,9 @@ describe('Picker', function () { expect(queryByRole('listbox')).toBeNull(); let picker = selectTester.trigger; + // let picker = getByRole('button'); + // await user.click(picker); + // act(() => jest.runAllTimers()); await selectTester.open(); let listbox = selectTester.listbox; diff --git a/packages/@react-spectrum/picker/test/TempUtilTest.test.js b/packages/@react-spectrum/picker/test/TempUtilTest.test.js index a9fbc25eba4..0e135d7f4a5 100644 --- a/packages/@react-spectrum/picker/test/TempUtilTest.test.js +++ b/packages/@react-spectrum/picker/test/TempUtilTest.test.js @@ -27,9 +27,14 @@ describe('Picker/Select ', function () { beforeAll(function () { user = userEvent.setup({delay: null, pointerMap}); + jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); simulateDesktop(); }); + afterAll(function () { + jest.restoreAllMocks(); + }); + describe('with real timers', function () { beforeAll(function () { jest.useRealTimers(); diff --git a/packages/@react-spectrum/tabs/test/Tabs.test.js b/packages/@react-spectrum/tabs/test/Tabs.test.js index 3178a266774..802c7701e79 100644 --- a/packages/@react-spectrum/tabs/test/Tabs.test.js +++ b/packages/@react-spectrum/tabs/test/Tabs.test.js @@ -60,6 +60,7 @@ describe('Tabs', function () { beforeEach(() => { jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 1000); jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 1000); + jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); window.HTMLElement.prototype.scrollIntoView = jest.fn(); }); From 9976091ec6e9a26b0b2e2d4a85587bf91c17f980 Mon Sep 17 00:00:00 2001 From: Yihui Liao <44729383+yihuiliao@users.noreply.github.com> Date: Thu, 1 May 2025 16:58:36 -0700 Subject: [PATCH 3/4] cleanup fix lint --- .../autocomplete/test/SearchAutocomplete.test.js | 2 +- packages/@react-spectrum/card/test/CardView.test.js | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js b/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js index a94c20bea0c..141f9a33ca1 100644 --- a/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js +++ b/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js @@ -144,7 +144,7 @@ describe('SearchAutocomplete', function () { jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 1000); jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 1000); jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); - scrollHeight = jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); + jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); window.HTMLElement.prototype.scrollIntoView = jest.fn(); simulateDesktop(); jest.useFakeTimers(); diff --git a/packages/@react-spectrum/card/test/CardView.test.js b/packages/@react-spectrum/card/test/CardView.test.js index a05d69d963f..41260e2fa54 100644 --- a/packages/@react-spectrum/card/test/CardView.test.js +++ b/packages/@react-spectrum/card/test/CardView.test.js @@ -141,13 +141,12 @@ describe('CardView', function () { user = userEvent.setup({delay: null, pointerMap}); jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => mockWidth); jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => mockHeight); - // jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); jest.useFakeTimers(); }); beforeEach(() => { jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); - }) + }); afterEach(() => { jest.clearAllMocks(); From 9c51d0decb0d21707a568691ee4c3b9f681ffe78 Mon Sep 17 00:00:00 2001 From: Yihui Liao <44729383+yihuiliao@users.noreply.github.com> Date: Thu, 1 May 2025 17:03:44 -0700 Subject: [PATCH 4/4] more cleanup --- .../autocomplete/test/SearchAutocomplete.test.js | 1 - packages/@react-spectrum/combobox/test/ComboBox.test.js | 1 - packages/@react-spectrum/picker/test/Picker.test.js | 3 --- 3 files changed, 5 deletions(-) diff --git a/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js b/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js index 141f9a33ca1..4b2b42435f3 100644 --- a/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js +++ b/packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js @@ -144,7 +144,6 @@ describe('SearchAutocomplete', function () { jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 1000); jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 1000); jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); - jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50); window.HTMLElement.prototype.scrollIntoView = jest.fn(); simulateDesktop(); jest.useFakeTimers(); diff --git a/packages/@react-spectrum/combobox/test/ComboBox.test.js b/packages/@react-spectrum/combobox/test/ComboBox.test.js index 3c0401ceb7b..d4415598382 100644 --- a/packages/@react-spectrum/combobox/test/ComboBox.test.js +++ b/packages/@react-spectrum/combobox/test/ComboBox.test.js @@ -3668,7 +3668,6 @@ describe('ComboBox', function () { describe('mobile combobox', function () { beforeEach(() => { simulateMobile(); - }); afterEach(() => { diff --git a/packages/@react-spectrum/picker/test/Picker.test.js b/packages/@react-spectrum/picker/test/Picker.test.js index be16a4b0036..da7c4ad6960 100644 --- a/packages/@react-spectrum/picker/test/Picker.test.js +++ b/packages/@react-spectrum/picker/test/Picker.test.js @@ -93,9 +93,6 @@ describe('Picker', function () { expect(queryByRole('listbox')).toBeNull(); let picker = selectTester.trigger; - // let picker = getByRole('button'); - // await user.click(picker); - // act(() => jest.runAllTimers()); await selectTester.open(); let listbox = selectTester.listbox;