improvement(components): [virtual-list] preventDefault only when the scroll directions match by rzzf · Pull Request #22989 · element-plus/element-plus

rzzf · 2025-12-03T03:30:55Z

Sometimes we do need tree-v2 to support horizontal scrolling, and the simplest solution is to handle it with custom external styles.
However, the current implementation prevents the default wheel behavior, which makes it impossible to use Shift + scroll or a trackpad to trigger horizontal scrolling.
Therefore, I think we should only prevent the default behavior in the layout direction, and allow scrolling in other directions.

#9861
#20881

demo

Summary by CodeRabbit

Bug Fixes
- Improved wheel event handling in virtual list components to prevent default scroll behavior more intelligently, triggering only when actual scroll movement is detected on non-Firefox browsers.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

… scroll directions match

github-actions · 2025-12-03T03:31:04Z

github-actions · 2025-12-03T03:31:50Z

Hello, @rzzf, your PR title does not meet the standards, please refer to here.
你好，@rzzf，你的 PR 标题不符合规范，请参考这里。

pkg-pr-new · 2025-12-03T03:32:54Z

Open in StackBlitz

pnpm add https://pkg.pr.new/element-plus/element-plus@22989

npm i https://pkg.pr.new/element-plus/element-plus@22989

yarn add https://pkg.pr.new/element-plus/[email protected]

commit: e3e25a3

github-actions · 2025-12-03T03:34:07Z

Size Change: +13 B (0%)

Total Size: 1.6 MB

Filename	Size	Change
`./dist/element-plus/dist/index.full.js`	459 kB	+2 B (0%)
`./dist/element-plus/dist/index.full.min.js`	323 kB	+4 B (0%)
`./dist/element-plus/dist/index.full.min.mjs`	318 kB	+3 B (0%)
`./dist/element-plus/dist/index.full.mjs`	449 kB	+4 B (0%)

ℹ️ View Unchanged

Filename	Size
`./dist/element-plus/dist/index.css`	46.7 kB

_{compressed-size-action}

github-actions · 2025-12-03T03:37:48Z

🧪 Playground Preview: https://element-plus.run/?pr=22989
Please comment the example via this playground if needed.

warmthsea

shift + scroll it still seems unable to scroll horizontally.

rzzf · 2025-12-04T10:01:16Z

shift + scroll it still seems unable to scroll horizontally.

Yeah, this issue is known on Windows, and there is already a PR addressing it.

#22168 (comment)

Dsaquel · 2025-12-04T22:54:30Z

Sorry I don't see any difference too; feel like the same here.

Perhaps with trackpad it will but I currently don't have any right now.

rzzf · 2025-12-05T03:05:39Z

Sorry I don't see any difference too; feel like the same here.

I'll install a virtual machine and conduct a thorough investigation when I have time. I think this issue needs to be addressed.

MacOS Touchpad

Dsaquel · 2025-12-28T14:04:04Z

After rebasing with #22168 this patch works too 😃 .

…ault

coderabbitai · 2025-12-28T14:11:38Z

📝 Walkthrough

Walkthrough

Refines the wheel event preventDefault logic in the virtual-list hook to only prevent default behavior on non-Firefox browsers when an actual scroll delta occurs, rather than unconditionally on non-Firefox browsers.

Changes

Cohort / File(s)	Summary
Wheel event handling `packages/components/virtual-list/src/hooks/use-wheel.ts`	Added condition to preventDefault call: only triggers when newOffset ≠ 0 in addition to non-Firefox check, preventing unnecessary default prevention on zero-delta scroll events

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A rabbit bounced through scrolling code,
Found wheels that blocked the browser's road,
Now Firefox hops, and Chrome agrees—
Prevent only when the delta's not at ease! 🎡

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: preventing default wheel behavior only when scroll directions match, which directly corresponds to the modification in use-wheel.ts.
Description check	✅ Passed	The description provides clear context about the issue, references related GitHub issues, includes a working demo, and explains the rationale for the change. It addresses the contributing template requirements.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch improvement/virtual-list-use-wheel-preventDefault

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2355a19 and e3e25a3.

📒 Files selected for processing (1)

packages/components/virtual-list/src/hooks/use-wheel.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

GitHub Check: Build windows-latest LTS
GitHub Check: Unit Test (New)
GitHub Check: Build ubuntu-latest LTS
GitHub Check: Lint
GitHub Check: SSR rendering test
GitHub Check: Unit Test (LTS)
GitHub Check: Unit Test (Current)
GitHub Check: build
GitHub Check: size-report
GitHub Check: Coverage

🔇 Additional comments (1)

packages/components/virtual-list/src/hooks/use-wheel.ts (1)

45-47: The logic change is correct but references to unverified platform issues should be removed.

The addition of && newOffset !== 0 correctly prevents preventDefault() from being called when there is no scroll movement in the primary direction (newOffset = 0), which aligns with the stated objective. This allows perpendicular scrolling to pass through as intended.

However, references to PR #22168, unresolved Windows Shift+scroll behavior, and platform-specific issues mentioned in the review could not be verified in the codebase. The code already includes Windows Shift+scroll handling (lines 33-37), and the condition logic is sound. Remove speculative claims about unresolved platform problems unless they are substantiated with specific failing test cases or documented issues.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2025-12-28T14:15:35Z

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	85.37%	17788 / 20836
🔵	Statements	84.16%	18616 / 22119
🔵	Functions	83.82%	4793 / 5718
🔵	Branches	73.59%	10165 / 13812

File Coverage

File	Stmts	% Branch	% Funcs	% Lines	Uncovered Lines
Changed Files
packages/components/virtual-list/src/hooks/use-wheel.ts	23.8%	0%	25%	25%	24, 26, 30, 32, 34-37, 35-36, 39, 41, 41, 43, 45-47, 46, 49-52, 50-51

Generated in workflow #398

coderabbitai

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/components/virtual-list/src/hooks/use-wheel.ts (1)
41-41: Critical: Early return blocks perpendicular scrolls when at edge.

When newOffset is 0 (perpendicular scroll), the condition hasReachedEdge(offset) && hasReachedEdge(offset + newOffset) simplifies to hasReachedEdge(offset) && hasReachedEdge(offset), which returns early if the accumulated offset has reached an edge in the primary direction.

Example scenario:

User scrolls to the top of a vertical list (offset becomes negative, atStartEdge is true)

Before rAF fires, user tries to scroll horizontally via Shift+wheel

newOffset = 0 (vertical layout uses deltaY, but Shift+wheel produces deltaX)

Line 41 evaluates: hasReachedEdge(offset) = true, hasReachedEdge(offset + 0) = true

Function returns early, blocking the horizontal scroll

This defeats the purpose of this PR, which is to allow perpendicular scrolling.
🔎 Proposed fix
-    if (hasReachedEdge(offset) && hasReachedEdge(offset + newOffset)) return
+    if (newOffset !== 0 && hasReachedEdge(offset) && hasReachedEdge(offset + newOffset)) return
This ensures the early return only applies when actually scrolling in the primary direction.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2355a19 and e3e25a3.

📒 Files selected for processing (1)

packages/components/virtual-list/src/hooks/use-wheel.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

GitHub Check: Build windows-latest LTS
GitHub Check: Unit Test (New)
GitHub Check: Build ubuntu-latest LTS
GitHub Check: Lint
GitHub Check: SSR rendering test
GitHub Check: Unit Test (LTS)
GitHub Check: Unit Test (Current)
GitHub Check: build
GitHub Check: size-report
GitHub Check: Coverage

🔇 Additional comments (1)

packages/components/virtual-list/src/hooks/use-wheel.ts (1)

45-47: The logic change is correct but references to unverified platform issues should be removed.

The addition of && newOffset !== 0 correctly prevents preventDefault() from being called when there is no scroll movement in the primary direction (newOffset = 0), which aligns with the stated objective. This allows perpendicular scrolling to pass through as intended.

However, references to PR #22168, unresolved Windows Shift+scroll behavior, and platform-specific issues mentioned in the review could not be verified in the codebase. The code already includes Windows Shift+scroll handling (lines 33-37), and the condition logic is sound. Remove speculative claims about unresolved platform problems unless they are substantiated with specific failing test cases or documented issues.

github-actions · 2025-12-29T03:07:36Z

@rzzf Thanks for your contribution! ❤️

improvement(components): [virtual-list] prevent default only when the…

fcddee5

… scroll directions match

pull-request-triage Bot assigned rzzf Dec 3, 2025

github-actions Bot added the CommitMessage::Unqualified Unqualified commit message label Dec 3, 2025

warmthsea reviewed Dec 4, 2025

View reviewed changes

Dsaquel approved these changes Dec 28, 2025

View reviewed changes

Merge branch 'dev' into improvement/virtual-list-use-wheel-preventDef…

e3e25a3

…ault

rzzf changed the title ~~improvement(components): [virtual-list] prevent default only when the scroll directions match~~ improvement(components): [virtual-list] preventDefault only when the scroll directions match Dec 28, 2025

coderabbitai Bot reviewed Dec 28, 2025

View reviewed changes

btea approved these changes Dec 29, 2025

View reviewed changes

btea merged commit 27743a3 into dev Dec 29, 2025
18 checks passed

btea deleted the improvement/virtual-list-use-wheel-preventDefault branch December 29, 2025 03:07

coderabbitai Bot mentioned this pull request Jan 3, 2026

fix(components): [virtual-list] incorrect scroll boundary check #23289

Merged

element-bot mentioned this pull request Jan 9, 2026

D2M #23355

Merged

3 tasks

coderabbitai Bot mentioned this pull request Feb 20, 2026

fix(components): [table-v2] navigation back triggered by trackpad scrolling #23673

Merged

coderabbitai Bot mentioned this pull request Mar 25, 2026

feat(components): [cascader] support virtual-scroll & fitInputWidth #23877

Merged

3 tasks

Uh oh!

Conversation

rzzf commented Dec 3, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

github-actions Bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pkg-pr-new Bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

warmthsea left a comment

Choose a reason for hiding this comment

Uh oh!

rzzf commented Dec 4, 2025

Uh oh!

Dsaquel commented Dec 4, 2025

Uh oh!

rzzf commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dsaquel commented Dec 28, 2025

Uh oh!

coderabbitai Bot commented Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

github-actions Bot commented Dec 28, 2025

Coverage Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Dec 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rzzf commented Dec 3, 2025 •

edited by coderabbitai Bot

Loading

github-actions Bot commented Dec 3, 2025 •

edited

Loading

github-actions Bot commented Dec 3, 2025 •

edited

Loading

pkg-pr-new Bot commented Dec 3, 2025 •

edited

Loading

github-actions Bot commented Dec 3, 2025 •

edited

Loading

github-actions Bot commented Dec 3, 2025 •

edited

Loading

rzzf commented Dec 5, 2025 •

edited

Loading

coderabbitai Bot commented Dec 28, 2025 •

edited

Loading