Skip to content

Conversation

LeafShi1
Copy link
Member

@LeafShi1 LeafShi1 commented Jul 10, 2025

Fixes #13633

Root Cause

The original logic, set the needRefresh= true directly when currentSelection.IsOnDropDown, this should be a bug in the code transplant process. The original code logic is here.

Proposed changes

  • Correct the logic of the OnSelectionChanged of the ToolStripItemDesigner

Customer Impact

  • ToolStripTextBox can be focused normally

Regression?

  • No

Risk

  • Minimal

Screenshots

Before

ToolStripTextBox cannot get focus
Image

After

ToolStripTextBox can be selected normally
AfterChange

Test methodology

  • Manually

Test environment(s)

  • .net 10.0.0-preview.7.25320.118
Microsoft Reviewers: Open in CodeFlow

@LeafShi1 LeafShi1 requested a review from a team as a code owner July 10, 2025 08:25
@LeafShi1 LeafShi1 requested review from Copilot and removed request for a team July 10, 2025 08:25
Copy link
Contributor

@Copilot 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 refines the refresh logic in OnSelectionChanged of ToolStripItemDesigner so that a refresh only occurs when the first drop-down isn’t a ContextMenuStrip, fixing focus issues with ToolStripTextBox.

  • Adjusted needRefresh to a conditional check instead of always true when on a drop-down.
  • Ensures ContextMenuStrip selections do not trigger unnecessary refreshes.
Comments suppressed due to low confidence (1)

src/System.Windows.Forms.Design/src/System/Windows/Forms/Design/ToolStripItemDesigner.cs:996

  • Add unit tests for OnSelectionChanged covering both ContextMenuStrip and other drop-down scenarios to verify that needRefresh is set correctly.
                        needRefresh = GetFirstDropDown(currentSelection) is not null and not ContextMenuStrip;

Copy link

codecov bot commented Jul 10, 2025

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 51.33223%. Comparing base (9856a7c) to head (dfa7ae3).
Report is 19 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (9856a7c) and HEAD (dfa7ae3). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (9856a7c) HEAD (dfa7ae3)
Debug 3 2
test 1 0
Additional details and impacted files
@@                 Coverage Diff                  @@
##                main      #13695          +/-   ##
====================================================
- Coverage   76.60680%   51.33223%   -25.27458%     
====================================================
  Files           3253        2063        -1190     
  Lines         641011      287751      -353260     
  Branches       47442       42057        -5385     
====================================================
- Hits          491058      147709      -343349     
+ Misses        146303      137156        -9147     
+ Partials        3650        2886         -764     
Flag Coverage Δ
Debug 51.33223% <0.00000%> (-25.27458%) ⬇️
integration 18.99211% <0.00000%> (+0.00455%) ⬆️
production 51.33223% <0.00000%> (+0.26774%) ⬆️
test ?
unit 48.70947% <0.00000%> (+0.25980%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@ricardobossan ricardobossan left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@Epica3055 Epica3055 left a comment

Choose a reason for hiding this comment

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

💯

@LeafShi1 LeafShi1 merged commit 32489a2 into dotnet:main Jul 11, 2025
9 checks passed
@LeafShi1 LeafShi1 deleted the Fix_13633_Update_ToolStripItemDesigner branch July 17, 2025 08:26
@github-actions github-actions bot locked and limited conversation to collaborators Aug 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ToolStripTextBox cannot get focus in DemoConsole application
3 participants