react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

fix: TableView jump to top on asynchronous load

Open obreitwi opened this issue 8 months ago • 5 comments

Closes #8132.

We confirmed that this PR resolves the issue internally, but I will provide a minimal example for easier verification.

✅ Pull Request Checklist:

  • [x] Included link to corresponding React Spectrum GitHub Issue.
  • [ ] Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • [ ] 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:

(will provide once minimal example is ready)

🧢 Your Project:

(not applicable)

obreitwi avatar Apr 23 '25 14:04 obreitwi

Thanks for the PR! You'll need to sign the CLA for all the checks to pass. If you could provide a minimal example (storybook) for easier verification that would be great. Otherwise, the changes make sense but we'll need to test.

yihuiliao avatar Apr 30 '25 22:04 yihuiliao

Let us know if you cannot sign the CLA so we know what action to take. Thank you!

snowystinger avatar May 28 '25 07:05 snowystinger

Hey, thank you for responding! I just signed the CLA (sorry for taking so long).

I am also a bit hard-pressed on time to provide a minimal solution, but the bug should become apparent whenever you use paged async loading on a table view and the individual rows are permitted to be of different heights (via overflowMode="wrap").

And just as a heads-up: We've noticed that while the original issue #8132 is indeed fixed, there are still occasional rendering issues where the table view does not get populated with all loaded rows at the bottom. A workaround for users is to scroll up and down a bit to trigger a re-render which typically fixes it.

An deeper investigation is pending on our side.

obreitwi avatar May 30 '25 11:05 obreitwi

right now this appears more as a performance optimization, which I'm fine with.

The issue was that since the quality is always wrong (since there are always two distinct objects to compare pointing to different locations in memory) the full re-render resets the vertical scroll positions on data load.

This was annoying for users scrolling through a list with several "pages" because loading each page reset their scroll view.

obreitwi avatar Jun 02 '25 09:06 obreitwi

https://github.com/adobe/react-spectrum/issues/8241 may be related with https://codesandbox.io/p/sandbox/lucid-parm-rhyz5n as an example, I'll try replicating locally soon.

LFDanLu avatar Jun 04 '25 16:06 LFDanLu