dotnet / winforms

Windows Forms is a .NET UI framework for building Windows desktop applications.
MIT License
4.28k stars 954 forks source link

Button's DialogResult.Ok is not removed from the .Designer.cs even when this button stops from being the form's AcceptButton. #11537

Open nangelgr opened 2 weeks ago

nangelgr commented 2 weeks ago

.NET version

.net 8

Did it work in .NET Framework?

Yes

Did it work in any of the earlier releases of .NET Core or .NET 5+?

dont know

Issue description

I have a simple form with two buttons: SaveButton and CancelButton Initially I had set the savebutton as the form's acceptbutton. I run the app several times with no problems. At some point I didnt want the form to close when the savebutton is pressed so i put in the form's acceptbutton the none option. And here is where my problem arised. The form kept closing when the savebutton was pressed!!! despite the change in the form's accept button

I could not figure out what was the problem when i finally checked the .designer.cs file and i saw that despite having removed the savebutton from the acceptbutton property of the form, the SaveButton definition section was including the statement SaveButton.DialogResult = DialogResult.OK; Which is a bug When I manually removed this statement then the form stopped closing at the press of the save button Dont you agree that this should be corrected? I spent 3 hours chatting with the chatgpt which finally failed to resolve my issue so i had to do it by myself I dont know whether i am more upset with the chatgpt or the winforms designer itself? So so so many issues with a framework that used to work like a charm i am thinking of giving up with winforms and look for some other technology

Steps to reproduce

very easy to reproduce create a form place two buttons on it set the form's acceptbutton one of them run the app and then set the form's acceptbutton to none. The SaveButton.DialogResult = DialogResult.OK; will persist in the designer.cs guiding the form to close nevertheless when the button is clicked

elachlan commented 2 weeks ago

The DialogResult property on button is never via Form.AcceptButton when I tested this.

If you look at the docs for Form.AcceptButton, they say:

Gets or sets the button on the form that is clicked when the user presses the ENTER key. https://learn.microsoft.com/en-us/dotnet/api/system.windows.forms.form.acceptbutton?view=windowsdesktop-8.0

It is not used to close the form. That is controlled by two things:

See docs for Button.DialogResult:

If the DialogResult for this property is set to anything other than None, and if the parent form was displayed through the ShowDialog method, clicking the button closes the parent form without your having to hook up any events. The form's DialogResult property is then set to the DialogResult of the button when the button is clicked.

For example, to create a "Yes/No/Cancel" dialog box, simply add three buttons and set their DialogResult properties to Yes, No, and Cancel. https://learn.microsoft.com/en-us/dotnet/api/system.windows.forms.button.dialogresult?view=windowsdesktop-8.0

Interestingly what is not stated in the docs for Form.CancelButton is that when it is set, it also sets the buttons DialogResult to DialogResult.Cancel. https://github.com/dotnet/winforms/blob/79472b41928fe7147527f93651f828088f617c1e/src/System.Windows.Forms/src/System/Windows/Forms/Form.cs#L718-L741

elachlan commented 2 weeks ago

@merriemcgaw I suggest the team update the docs on Form.CancelButton so that its obvious that the buttons DialogResult is set to DialogResult.Cancel when there isn't a value present. It should also mention that if its set to anything, then that value will take precedence.

elachlan commented 2 weeks ago

Also, maybe we should change the AcceptButton to also set the Button.DialogResult to Ok if it isn't set to match the behavior of CancelButton?

merriemcgaw commented 1 week ago

@elachlan I really like the docs suggestion. I'll go file that now. I'll flag this as untriaged to bring it back to the team's radar.

KlausLoeffelmann commented 5 days ago

We should have Analyzers to guide proper Dialog handling. Most folks don't know that you can have modal Dialog control without a line of code. Let me create an Analyzer issue.

@elachlan (and not only!): When you have ideas of other scenarios where Analyzers would make sense for scenarios which have problems with discoverability even after 25 years, feel very free to capture those ideas as issues (and tag them with Analyzers).

KlausLoeffelmann commented 5 days ago

https://github.com/dotnet/winforms/issues/11586