-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix: Prevent repetitive viewport analysis in PointerMoved #7366
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
If you select a lot of items by scrolling without stopping, items in the middle won't get selected (by the time you stop some items will be virtualized again / ContainerFromItem() will be null). |
@gave92 This was part of the safeguard to ensure we don't lag when dilating the selection rectangle over the y axis. Otherwise, we would be adding every item in the viewport for each tiny vertical offset change. Waiting until the user stops scrolling seemed like a decent temporary solution until someone can figure out a better method, but considering how bad the behavior was before in large directories, we should go ahead with this imo |
Made a small change that should fix the above issue. It should not affect performance, could you confirm if it's ok? |
@gave92 Not really sure how you did it, but the issue mentioned above is indeed fixed. Thanks! |
Resolved / Related Issues
Items resolved / related issues by this PR.
Details of Changes
Analyze the ListView viewport for item rectangles a single time, upfront, rather than every time the pointer is moved. Further, do this rather intensive work on selection rectangle y-axis dilation only when the scrolling gesture becomes determinate for 1 second.
English: Drag-to-select will no longer lag in very large folders (e.g. System32), and similar safeguards are put in place when attempting to select more items outside the bottom (or top) of the ListView.
Validation
How did you test these changes?
Tested the changes for accessibilityScreenshots (optional)
N/A