Skip to content
Draft
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
24 changes: 24 additions & 0 deletions packages/react-aria-components/stories/GridList.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -953,3 +953,27 @@ export const AsyncGridListGridVirtualized: StoryObj<typeof AsyncGridListGridVirt
}
}
};

function VirtualizedDisplayNoneToggleRender() {
let [visible, setVisible] = useState(true);

return (
<div>
<Button onPress={() => setVisible(v => !v)} style={{marginBottom: 8}}>
{visible ? 'Hide' : 'Show'}
</Button>
<div style={{display: visible ? undefined : 'none'}}>
<VirtualizedGridDnD />
</div>
</div>
);
}

export const VirtualizedDisplayNoneToggle: StoryObj = {
render: VirtualizedDisplayNoneToggleRender,
parameters: {
description: {
data: 'toggling hide and show should not cause the items to disappear'
}
}
};
58 changes: 57 additions & 1 deletion packages/react-aria-components/test/GridList.browser.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@
*/

import {expect, it} from 'vitest';
import {GridLayout} from '../src/GridLayout';
import {GridList, GridListItem} from '../src/GridList';
import React from 'react';
import React, {useState} from 'react';
import {render} from 'vitest-browser-react';
import {Size} from 'react-stately/useVirtualizerState';
import {User} from '@react-aria/test-utils';
import {Virtualizer} from '../src/Virtualizer';

function Grid() {
return (
Expand All @@ -36,6 +39,37 @@ function Grid() {
);
}

function VirtualizedDisplayNone() {
let [visible, setVisible] = useState(true);
let items = Array.from({length: 100}, (_, i) => ({id: i, name: `Item ${i}`}));
return (
<div>
<button data-testid="toggle" onClick={() => setVisible(v => !v)}>
Toggle
</button>
<div style={{display: visible ? undefined : 'none'}}>
<Virtualizer
layout={GridLayout}
layoutOptions={{
minItemSize: new Size(80, 40),
maxItemSize: new Size(200, 40),
minSpace: new Size(8, 8)
}}>
<GridList
layout="grid"
aria-label="virtualized list"
style={{height: 200, width: 400}}
items={items}>
{(item: {id: number; name: string}) => (
<GridListItem id={item.id}>{item.name}</GridListItem>
)}
</GridList>
</Virtualizer>
</div>
</div>
);
}

it.each`
interactionType
${'mouse'}
Expand Down Expand Up @@ -64,3 +98,25 @@ it.each`
expect(rows[8].getAttribute('aria-selected')).toBe('true');
expect(document.activeElement).toBe(rows[8]);
});

it('virtualizer renders items after toggling display:none', async () => {
let testUtilUser = new User();
let {container} = await render(<VirtualizedDisplayNone />);

let gridlist = container.querySelector('[role=grid]') as HTMLElement;
let tester = testUtilUser.createTester('GridList', {
root: gridlist,
layout: 'grid'
});

await expect.poll(() => tester.getRows().length).toBeGreaterThan(0);
let button = container.querySelector('[data-testid=toggle]') as HTMLElement;

await button.click();
await button.click();
await expect.poll(() => tester.getRows().length).toBeGreaterThan(0);

await button.click();
await button.click();
await expect.poll(() => tester.getRows().length).toBeGreaterThan(0);
});
7 changes: 7 additions & 0 deletions packages/react-aria/src/virtualizer/useVirtualizerItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ export function useVirtualizerItem(options: VirtualizerItemOptions): {updateSize

let updateSize = useCallback(() => {
if (key != null && ref.current) {
// offsetParent is null if element or ancestor has display: none, see https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/offsetParent
// for that case we want to avoid reporting size 0 otherwise we get into a state
// where the virtualizer renders 0 items when it is hidden and thus won't remeasure when it is is unhidden
// in jsdom tests, offsetParent can be null, so skip the check there.
if (!navigator.userAgent.includes('jsdom') && ref.current.offsetParent === null) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will need to think about this check since jsdom is probably not the only one used in tests

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe check isElementVisible() instead? This will inherit display and should work in test environments.

return;
}
let size = getSize(ref.current);
virtualizer.updateItemSize(key, size);
}
Expand Down
4 changes: 4 additions & 0 deletions vitest.browser.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ function iconWrapperPlugin(): Plugin {
}

export default defineConfig({
define: {
// make sure virtualizer actually runs since this is in browser
'process.env.VIRT_ON': '"1"'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isnt VIRT_ON only relevant for test environments? This is to say, why are the browser tests using the test environment instead of dev or prod?

A bunch of logic behind the test flag is tailored around JDOM and synthetic event quirks etc, which are not applicable to CDP and a real browser.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh fair point, didn't really consider that but makes sense

},
plugins: [
// @ts-expect-error
macros.vite(), // Must be first!
Expand Down