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

Epica3055 commented 2 months ago

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:

Create a Winforms .Net Core project with a ComboBox
Close and reopen form designer
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)

codecov[bot] commented 2 months ago

Codecov Report

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

Project coverage is 75.05220%. Comparing base (27d748e) to head (2092f3f). Report is 230 commits behind head on feature/10.0.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## feature/10.0 #11363 +/- ## ====================================================== + Coverage 74.27404% 75.05220% +0.77815% ====================================================== Files 3025 3120 +95 Lines 626857 656126 +29269 Branches 46741 51553 +4812 ====================================================== + Hits 465592 492437 +26845 - Misses 157914 160271 +2357 - Partials 3351 3418 +67 ``` | [Flag](https://app.codecov.io/gh/dotnet/winforms/pull/11363/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | Coverage Δ | | |---|---|---| | [Debug](https://app.codecov.io/gh/dotnet/winforms/pull/11363/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `75.05220% <100.00000%> (+0.77815%)` | :arrow_up: | | [integration](https://app.codecov.io/gh/dotnet/winforms/pull/11363/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `19.02403% <50.00000%> (+1.02387%)` | :arrow_up: | | [production](https://app.codecov.io/gh/dotnet/winforms/pull/11363/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `48.88259% <100.00000%> (+1.88792%)` | :arrow_up: | | [test](https://app.codecov.io/gh/dotnet/winforms/pull/11363/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `97.07646% <ø> (+0.07560%)` | :arrow_up: | | [unit](https://app.codecov.io/gh/dotnet/winforms/pull/11363/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `45.88838% <100.00000%> (+1.91903%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet#carryforward-flags-in-the-pull-request-comment) to find out more.

Tanya-Solyanik commented 2 months ago

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?

dotnet / winforms