Listview display none

[richardbann]

Closes #6298

✅ Pull Request Checklist:

• [ x] Included link to corresponding React Spectrum GitHub Issue.
• [ x] Added/updated unit tests and storybook for this change (for new code or code which already has tests).
• [x ] Filled out test instructions.
• Updated documentation (if it already exists for this component).
• Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

ListView Story "display: none with many items" does not crash

🧢 Your Project:

[reidbarber]

GET_BUILD

[rspbot]

Build successful! 🎉

• View the storybook
• View the storybook-canary
• View the storybook-17
• View the storybook-16
• View the documentation

[rspbot]

## API Changes

unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }

[snowystinger]

what is the height at this point? 0? undefined?
I think it'd be better to be a little more explicit and check if it's > 0, and if it's undefined, it'd be good to add a !isNaN

[reidbarber]

If I go into the storybook controls and enable the isQuiet prop, this breaks, which makes me think this won't work beyond the first render. We may need to look a little closer at how to handle this.

[richardbann]

It's not clear if it is a safe change. Probably the height of a list item is 0 only if display:none set.
On the other hand if somehow the item size is small enough (say 1), react will throw the same exception even if the ListView is visible. This is because after render the size is calculated (1 in this case) and a re-render will add one more item to the visible item's list, which will notice it's size and render again... If the collection is large enough react will treat it as an infinite recursion.

[richardbann]

The page does not crash if padding is set to 1 on ListLayout, but this is not configurable. Can be verified by setting the padding in ListView.tsx.

[snowystinger]

Closing as resolved by #6497