Skip to content

Conversation

@nwidynski
Copy link
Contributor

This PR improves scroll end detection in supported browsers with fallback to our previous way of detecting via timeout.
We also expose the isScrolling state for consumption, which will come in handy for carousel infinite loop 👍

✅ Pull Request Checklist:

  • 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:

🧢 Your Project:

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

seems reasonable, thanks!

@LFDanLu LFDanLu added this pull request to the merge queue Jul 25, 2025
Merged via the queue into adobe:main with commit e79c076 Jul 25, 2025
31 checks passed
@nwidynski
Copy link
Contributor Author

nwidynski commented Aug 10, 2025

@snowystinger Btw, onscrollend also delays the callback until pointers or keys have released. Do you guys know of any way to track pointer start/end (e.g. trackpad touchstart) manually so that we can delay onScrollEnd in our timeout impl?

In case there isn't I guess we just live with it firing too early in unsupported browsers.

@snowystinger
Copy link
Member

I understand the difference, but what is the issue it's causing for you to have them be at slightly different times? Is it a big enough of a difference we should reconsider this PR do you think? We can talk about it in the team, but would like your opinion as well.

I think we ran into this at one point and have a comment in the code somewhere, though I couldn't find it. I thought maybe in the usePress code, but maybe that logic was changed and the comment removed. I think there are no dependable events for this, which is a shame. That's why the fallback logic does polling :-/

@nwidynski
Copy link
Contributor Author

nwidynski commented Aug 11, 2025

I don't think it makes a difference for virtualizer. I planned on using useScrollView.onScrollEnd to notify the rotator to accept rotation inputs again. Controls are buffered while a programmatic animation is playing to prevent miscalculation of the intended target when controls are re-invoked during animation (e.g. 3 clicks on "next" shall land on n+3, even though selected key is still n).

I was concerned that an early onScrollEnd could result in a potential miscalculation of the target, if the user pressed the rotation control within our timeout window and just before a snap animation is started. This is very unlikely to happen, and can be worked around, but I wanted to check whether preventing it here was somehow possible.

In any way this use-case prefers the new behavior, so please don't revert this PR, haha.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants