Skip to content

fix: selected item in virtualized rac select to scroll into view #8178

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ 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);
window.HTMLElement.prototype.scrollIntoView = jest.fn();
simulateDesktop();
jest.useFakeTimers();
Expand Down
4 changes: 4 additions & 0 deletions packages/@react-spectrum/card/test/CardView.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ describe('CardView', function () {
jest.useFakeTimers();
});

beforeEach(() => {
jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50);
});

afterEach(() => {
jest.clearAllMocks();
act(() => jest.runAllTimers());
Expand Down
5 changes: 5 additions & 0 deletions packages/@react-spectrum/color/test/ColorPicker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();});
});
Expand Down
1 change: 1 addition & 0 deletions packages/@react-spectrum/combobox/test/ComboBox.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ describe('ComboBox', function () {
});

beforeEach(() => {
jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50);
load = jest
.fn()
.mockImplementationOnce(getFilterItems)
Expand Down
4 changes: 3 additions & 1 deletion packages/@react-spectrum/picker/test/Picker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -40,13 +40,15 @@ 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();
});

afterAll(function () {
offsetWidth.mockReset();
offsetHeight.mockReset();
scrollHeight.mockReset();
});

afterEach(() => {
Expand Down
5 changes: 5 additions & 0 deletions packages/@react-spectrum/picker/test/TempUtilTest.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 3 additions & 0 deletions packages/@react-spectrum/s2/src/TableView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,9 @@ export class S2TableLayout<T> extends TableLayout<T> {

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
Expand Down
1 change: 1 addition & 0 deletions packages/@react-spectrum/tabs/test/Tabs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

Expand Down
2 changes: 1 addition & 1 deletion packages/@react-stately/layout/src/GridLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ export class GridLayout<T, O extends GridLayoutOptions = GridLayoutOptions> 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) {
Expand Down
2 changes: 1 addition & 1 deletion packages/@react-stately/layout/src/ListLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ export class ListLayout<T, O extends ListLayoutOptions = ListLayoutOptions> 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;
Expand Down
6 changes: 5 additions & 1 deletion packages/@react-stately/layout/src/TableLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ export class TableLayout<T, O extends TableLayoutProps = TableLayoutProps> exten
this.stickyColumnIndices = [];

let collection = this.virtualizer!.collection as TableCollection<T>;
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.
Expand Down Expand Up @@ -543,7 +547,7 @@ export class TableLayout<T, O extends TableLayoutProps = TableLayoutProps> 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;
Expand Down
4 changes: 3 additions & 1 deletion packages/@react-stately/virtualizer/src/Rect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions packages/@react-stately/virtualizer/src/Virtualizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,7 @@ export class Virtualizer<T extends object, V> {
} 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);
Expand Down
4 changes: 0 additions & 4 deletions packages/react-aria-components/src/Virtualizer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,6 @@ function CollectionRoot({collection, persistedKeys, scrollRef, renderDropIndicat
onScrollEnd: state.endScrolling
}, scrollRef!);

if (state.contentSize.area === 0) {
return null;
}

return (
<div {...contentProps}>
<VirtualizerContext.Provider value={state}>
Expand Down