Skip to content

fix(listBox): fix virtualized scrolling when mouse is used#118380

Merged
TkDodo merged 4 commits into
masterfrom
tkdodo/fix/virtualized-scrolling
Jun 25, 2026
Merged

fix(listBox): fix virtualized scrolling when mouse is used#118380
TkDodo merged 4 commits into
masterfrom
tkdodo/fix/virtualized-scrolling

Conversation

@TkDodo

@TkDodo TkDodo commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

the effect was meant for cmd-k keyboard navigation and should not have an impact on scrolling with the mouse / dragging the scrollbar

apparently, listState.selectionManager.focuseKey is empty string in those cases. The effect that scrolls elements into view was only meant for keyboard navigation, but it effectively stopped scrolling on virtualized lists when the scrollbar is dragged because it always instantly scrolls back to the focused element.

the fix is to widen the early return statement to include empty string
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 24, 2026
@TkDodo

TkDodo commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

@JonasBa FYI 😂

@TkDodo TkDodo marked this pull request as ready for review June 24, 2026 18:57
@TkDodo TkDodo requested a review from a team as a code owner June 24, 2026 18:57

@cursor cursor Bot left a comment

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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4130770. Configure here.

Comment thread static/app/components/core/compactSelect/listBox/index.tsx
@JonasBa

JonasBa commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Please kill this list component, I never want to see it ever again

@TkDodo

TkDodo commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

okay this is the wrong fix. I thought empty string is just another falsy value that we are getting, but it’s the actually selected value :/ I have to find another fix

TkDodo added 2 commits June 25, 2026 09:24
…keyboard

the effect was meant for cmd-k keyboard navigation and should not have an impact on scrolling with the mouse / dragging the scrollbar
Comment thread static/app/components/core/compactSelect/listBox/index.tsx
this includes keyboard and virtual (for screen reader navigation) for auto focus
@TkDodo TkDodo merged commit 9d376a8 into master Jun 25, 2026
76 checks passed
@TkDodo TkDodo deleted the tkdodo/fix/virtualized-scrolling branch June 25, 2026 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants