Use MouseWheel scrolling in ToolStripDropDownMenu#13303
Use MouseWheel scrolling in ToolStripDropDownMenu#13303toehead2001 wants to merge 3 commits intodotnet:mainfrom
Conversation
|
Error: src\System.Windows.Forms\System\Windows\Forms\Controls\ToolStrips\ToolStripDropDownMenu.cs(847,29): error RS0016: (NETCORE_ENGINEERING_TELEMETRY=Build) |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #13303 +/- ##
===================================================
- Coverage 76.90689% 76.89944% -0.00746%
===================================================
Files 3260 3260
Lines 643088 643110 +22
Branches 47601 47605 +4
===================================================
- Hits 494579 494548 -31
- Misses 144845 144893 +48
- Partials 3664 3669 +5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for scrolling a ToolStripDropDownMenu using the mouse wheel, addressing issue #13302.
- Implements an override of OnMouseWheel in ToolStripDropDownMenu to calculate and apply a scroll amount.
- Updates PublicAPI.Unshipped.txt to document the new override.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/System.Windows.Forms/System/Windows/Forms/Controls/ToolStrips/ToolStripDropDownMenu.cs | Added OnMouseWheel override handling scroll logic. |
| src/System.Windows.Forms/PublicAPI.Unshipped.txt | Added API declaration for the new override. |
Comments suppressed due to low confidence (1)
src/System.Windows.Forms/System/Windows/Forms/Controls/ToolStrips/ToolStripDropDownMenu.cs:865
- [nitpick] Consider renaming 'delta' to 'scrollDelta' to clarify that it represents the scroll amount calculated from the mouse wheel event.
int delta = firstItemBounds.Height * SystemInformation.MouseWheelScrollLines * -Math.Sign(e.Delta);
f1ad6c1 to
bf72ef2
Compare
|
System.Windows.Forms.Tests.ClipboardTests.SetDataObject_InvokeObjectBoolNotIComDataObject_GetReturnsExpected(data: "data", copy: True) Error message Stack trace |
|
That error looks unrelated to the code changes in this PR. |
@toehead2001 - I'm adding these errors for our internal tracking. ClipboardTests is a known flaky set of tests because they read from a machine-wide resource, we ignore any non-recurring failures there. |
|
|
||
| protected override void OnMouseWheel(MouseEventArgs e) | ||
| { | ||
| base.OnMouseWheel(e); |
There was a problem hiding this comment.
Could you please explain why the base class handling in insufficient?
There was a problem hiding this comment.
The direct base class (ToolStripDropDown) doesn't have a implementation of OnMouseWheel, so what gets called here is the implementation all the way down in ScrollableControl.
Obviously, ScrollableControl doesn't know how to handle the re-positioning of ToolStripItems, and just handles normal Controls.
There was a problem hiding this comment.
Right, but why does scrollable control need to know about the ToolStripItems. Is ClientRectangle or Display rectangle calculated incorrectly in this case?
There was a problem hiding this comment.
The parent ToolStrip class has it own scroll implementation that doesn't use OnScroll() of ScrollableControl.
I am not sure why it was originally designed that way.
b2b133d to
be95b05
Compare
Fixes #13302
Proposed changes
ToolStripDropDownMenuto handle scrolling the menu items.Customer Impact
ToolStripDropDownMenuwith the Mouse Wheel.Regression?
Risk
ToolStripDropDownMenu.Test methodology
Microsoft Reviewers: Open in CodeFlow