Skip to content

Fix ToolStripDropDownMenu scroll boundary issues#14532

Open
LeafShi1 wants to merge 6 commits intodotnet:mainfrom
LeafShi1:Fix_ToolStripDropDown_scroll_issue
Open

Fix ToolStripDropDownMenu scroll boundary issues#14532
LeafShi1 wants to merge 6 commits intodotnet:mainfrom
LeafShi1:Fix_ToolStripDropDown_scroll_issue

Conversation

@LeafShi1
Copy link
Copy Markdown
Member

@LeafShi1 LeafShi1 commented May 8, 2026

Fixes #14530, #14531

Root Cause

PR #14490 rewrote UpdateScrollButtonStatus() and disabled the Down scroll button as soon as indexOfLastDisplayedItem == lastAvailableItemIndex. The "last displayed item" was defined as any item that intersects the viewport — including items only partially visible at the bottom edge. As a result, the Down button was disabled one scroll step too early: the last item was already clipped into view, but _indexOfFirstDisplayedItem had not yet advanced to it.

Additionally, the previous implementation did not account for interspersed Available = false items (e.g., items whose Visible property was set to false). Scroll steps could land on an unavailable item, and the boundary indices could be computed incorrectly if hidden items were counted among the candidates.

Proposed changes

  • Removed indexOfLastDisplayedItem tracking. Down button is now enabled if FindNextScrollableItemIndex(_indexOfFirstDisplayedItem) >= 0, mirroring the actual scroll mechanic in ScrollInternal(bool up) which moves _indexOfFirstDisplayedItem forward by one available item.
  • Added firstAvailableItemIndex tracking to correctly skip scroll-button pseudo-items and unavailable items when determining scroll boundaries.
  • Unified the filtering rules for "scrolling candidates": exclude the "up/down" scroll button itself, as well as any items with Available = false.
  • Allow UpdateScrollButtonStatus() and ScrollInternal() to correctly skip over unavailable/hidden items when advancing the scroll anchor, preventing the scroll cursor from getting stuck on an invisible item.
  • Updated to use the new FindPreviousScrollableItemIndex / FindNextScrollableItemIndex helpers instead of naively doing _indexOfFirstDisplayedItem ± 1.

Customer Impact

  • Users can now correctly navigate to the last item in a ToolStripDropDownMenu using the Down Arrow key or scroll buttons without stopping when the last item is only partially visible.
  • Scroll buttons also work correctly when the menu contains hidden items (Available=false or Visible=false), preventing navigation from getting stuck on hidden items.

Regression?

  • Yes

Risk

  • Minimal

Screenshots

Before

When the last item appears within the visible area, the down arrow becomes disabled (grayed out).
Item B is hidden; consequently, clicking the up arrow triggers no scrolling response.

BeforeChanges.mp4

After

When the last item is not the first item in the visible area, the down arrow remains clickable.
Item B is hidden, yet both the up and down arrows remain fully functional.

AfterChanges.mp4

Test methodology

  • Manually and adding unit tests

Test environment(s)

  • .Net 11.0.0-preview.5.26256.105
Microsoft Reviewers: Open in CodeFlow

@LeafShi1 LeafShi1 added the waiting-review This item is waiting on review by one or more members of team label May 9, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes ToolStripDropDownMenu scrolling boundary logic so the scroll buttons reflect the actual scroll mechanics (advancing the “first displayed item”), including correctly skipping unavailable/hidden items.

Changes:

  • Updates UpdateScrollButtonStatus() to enable Up/Down based on whether a previous/next scrollable item exists relative to _indexOfFirstDisplayedItem.
  • Updates ScrollInternal(bool up) to scroll to the previous/next scrollable item (skipping unavailable items) via new helper methods.
  • Adjusts/adds unit tests covering the regression scenario and unavailable interleaved items.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/System.Windows.Forms/System/Windows/Forms/Controls/ToolStrips/ToolStripDropDownMenu.cs Aligns scroll-button enablement and scroll stepping with “previous/next scrollable item” semantics; skips unavailable items.
src/test/unit/System.Windows.Forms/ToolStripDropDownMenuTests.cs Updates an existing regression test expectation and adds coverage for interleaved unavailable items.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/test/unit/System.Windows.Forms/ToolStripDropDownMenuTests.cs Outdated
Comment thread src/test/unit/System.Windows.Forms/ToolStripDropDownMenuTests.cs
@SimonZhao888
Copy link
Copy Markdown
Member

Looks good! Please address the Copilot's suggestion first!

LeafShi1 added 3 commits May 9, 2026 15:47
… and align focused items to viewport top when scroll buttons are visible.
…gn focus to top, wrap-around) and scroll button interaction issues (click-through prevention, hit-test priority).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Infrastructure waiting-review This item is waiting on review by one or more members of team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression: ToolStrip dropdown scrolling stops one item too early when navigating with Down Arrow

3 participants