winforms icon indicating copy to clipboard operation
winforms copied to clipboard

fix winforms-designer/issues/2707 The comboBox incorrect display on form designer when setting its DropDownStyle as Simple after building the project

Open Epica3055 opened this issue 1 year ago • 2 comments

Fixes designer/issues/2707

Root Cause

We use _requestedHeight to set the height of a ComboBox when DropDownStyle is ComboBoxStyle.Simple

https://github.com/dotnet/winforms/blob/70a6a7b4a9af452160e636838f946b3f4327383c/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ComboBox/ComboBox.cs#L61-L63

Let's go through the repro steps that was described here https://github.com/microsoft/winforms-designer/issues/2707#issuecomment-799222994

The following steps also can repro this issue:

  1. Create a Winforms .Net Core project with a ComboBox
  2. Close and reopen form designer
  3. Set the DropDownStyle of comboBox as Simple

Step I : Create a Winforms .Net Core project with a ComboBox

I think this is the generated code

        private void InitializeComponent()
        {
            comboBox1 = new ComboBox();
            SuspendLayout();
            // 
            // comboBox1
            // 
            comboBox1.FormattingEnabled = true;
            comboBox1.Location = new Point(44, 71);
            comboBox1.Name = "comboBox1";
            comboBox1.Size = new Size(121, 23);
            comboBox1.TabIndex = 0;

Step II: Close and reopen form designer First ComboBox constrictor will be invoked

https://github.com/dotnet/winforms/blob/70a6a7b4a9af452160e636838f946b3f4327383c/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ComboBox/ComboBox.cs#L116-L126

_requestedHeight will be assigned to DefaultSimpleStyleHeight, which is 150.

https://github.com/dotnet/winforms/blob/70a6a7b4a9af452160e636838f946b3f4327383c/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ComboBox/ComboBox.cs#L47

then comboBox1.Size = new Size(121, 23); will be executed, which will involve SetBoundsCore method

https://github.com/dotnet/winforms/blob/70a6a7b4a9af452160e636838f946b3f4327383c/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ComboBox/ComboBox.cs#L3371-L3382

then height(23) will be assigned to _requestedHeight.

Step III: Set the DropDownStyle of comboBox as Simple

please see line 1170-1173

https://github.com/dotnet/winforms/blob/70a6a7b4a9af452160e636838f946b3f4327383c/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ComboBox/ComboBox.cs#L1135-L1177

So step 3 will trigger HandleRecreate. please see line 2435-2438, so eventually SetBoundsCore method will be triggered. And _requestedHeight(23) will be assigned to Height

https://github.com/dotnet/winforms/blob/70a6a7b4a9af452160e636838f946b3f4327383c/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ComboBox/ComboBox.cs#L2380-L2469

So the parameter height here is 23, which is supposed to be 150.

https://github.com/dotnet/winforms/blob/70a6a7b4a9af452160e636838f946b3f4327383c/src/System.Windows.Forms/src/System/Windows/Forms/Controls/ComboBox/ComboBox.cs#L3371-L3382

Proposed changes

  • Add a condition when _requestedHeight is assigned.

Before

Core

After

https://github.com/dotnet/winforms/assets/135201996/aaef3b5d-2ca0-4722-bbe1-f4b643eb7e1a

Test methodology

  • manually (build the binaries and copy System.windows.forms.dll to the program files\dotnet\shared)

Epica3055 avatar May 13 '24 10:05 Epica3055

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 75.01959%. Comparing base (9feb143) to head (363f1a9). Report is 86 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                  @@
##                main      #11363          +/-   ##
====================================================
- Coverage   97.00820%   75.01959%   -21.98862%     
====================================================
  Files           1065        3047        +1982     
  Lines         345678      631639      +285961     
  Branches        5034       46767       +41733     
====================================================
+ Hits          335336      473853      +138517     
- Misses          9622      154421      +144799     
- Partials         720        3365        +2645     
Flag Coverage Δ
Debug 75.01959% <100.00000%> (-21.98862%) :arrow_down:
integration 17.93579% <50.00000%> (?)
production 48.13100% <100.00000%> (?)
test 97.01632% <100.00000%> (+0.00811%) :arrow_up:
unit 45.15756% <100.00000%> (?)

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

codecov[bot] avatar May 13 '24 10:05 codecov[bot]

I haven't tested it yet. because this change is made in runtime repo while Designer is in another repo.

build the binaries and copy System.windows.forms.dll to the program files\dotnet\shared

But more importantly, how do you prove that this is not regressing any runtime scenarios?

Tanya-Solyanik avatar May 14 '24 18:05 Tanya-Solyanik

@Epica3055 - pleas add a unit test.

Tanya-Solyanik avatar Aug 08 '24 18:08 Tanya-Solyanik

/azp run

lonitra avatar Aug 20 '24 21:08 lonitra

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Aug 20 '24 21:08 azure-pipelines[bot]