dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
7.05k stars 1.17k forks source link

[API Proposal]: Make additional MessageBoxButton and MessagBoxResult values available #9613

Open bstordrup opened 1 month ago

bstordrup commented 1 month ago

Background and motivation

Issue #5795 suggests getting more MessageBoxButton and MessageBoxResult values available and bringing Wpf repository more in sync with what is available in WinForms repository.

It will also make it easier to migrate from WinForms to WPF that you can practically directly make the same call to show a MessageBox in WPF as in WinForms.

Both repositories are calling into the same MessageBox function in Windows, so it should not be a technical problem to handle this.

API Proposal

Extend MessageBoxButton and MessageBoxResult enumerations

Add AbortRetryIgnore, RetryCancel and CancelTryContinue to the MessageBoxButton enumeration:

namespace System.Windows
{
    public enum MessageBoxButton
    {
        OK = 0,
        OKCancel = 1,
+       AbortRetryIgnore = 2,
        YesNoCancel = 3,
        YesNo = 4,
+       RetryCancel = 5,
+       CancelTryContinue = 6,
    }
}

Also add Abort, Retry, Ignore, TryAgain and Continue to the MessageBoxResult enumeration:

namespace System.Windows
{
    public enum MessageBoxResult
    {
        None = 0,
        OK = 1,
        Cancel = 2,
+       Abort = 3,
+       Retry = 4,
+       Ignore = 5,
        Yes = 6,
        No = 7,
+       TryAgain = 10,
+       Continue = 11,
    }
}

API Usage

// Call the MessageBox.Show with additional parameters

MessageBoxResult result = MessageBox.Show("Operation timed out. What would you like to do?", 
    "My application",
    MessageBoxButton.RetryCancel,
    MessageBoxIcon.Question,
    MessageBoxResult.Retry);
if (result == MessageBoxResult.Retry)
{
    /// Perfom code to retry operation
}
else
{
    // Log cancel request and exit
}

Alternative Designs

No response

Risks

In connection with the extra enumeration values, I don't see a risk, since the values already exist in the underlying Windows API (1).

  1. https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-messagebox
miloush commented 1 month ago

Winforms API approval: https://github.com/dotnet/winforms/issues/4712

I think the help button is slightly more problematic and also more complex for implementation. I suggest it is split out to a separate discussion. I also cannot immediately see it in the Winforms repo.

Editorial: It is not clear which code is new and which is existing. use ```diff and prefix lines with + or -

public enum MessageBoxButton
{
         OK = 0,
         OKCancel = 1,
+        AbortRetryIgnore = 2,
         YesNoCancel = 3,
         YesNo = 4,
+        RetryCancel = 5,
+        CancelTryContinue = 6,
}

Winforms also added default button 4, might be worth mentioning in comments.

Are any of these values gated by minimum Windows version?

bstordrup commented 1 month ago

Are any of these values gated by minimum Windows version?

According to the link, I posted, the minimum required Windows version is Windows 2000 Professional on client side and Windows 2000 Server on server side.

h3xds1nz commented 1 month ago

Those are untouched since at least WinXP, think it actually didn't change since Win2k, maybe the dialog IDs did between Win2k and XP.

bstordrup commented 1 month ago

The link also mentions API-set

ext-ms-win-ntuser-dialogbox-l1-1-0 (introduced in Windows 8)
bstordrup commented 1 month ago

Editorial: It is not clear which code is new and which is existing. use ```diff and prefix lines with + or -

Edited based on this. Learned something new 😊

h3xds1nz commented 1 month ago

The link also mentions API-set

ext-ms-win-ntuser-dialogbox-l1-1-0 (introduced in Windows 8)

That's just the umbrella library/ApiSetSchema you could link against; however WPF just PInvokes directly in all cases.

Sorry if I'm saying stuff you're familiar with. In case it's not; they were introduced in Win7, extended in Win8 and the implementation was remade in Win10; they're not real libraries, just stuff that's processed by the loader.

But it ain't relevant for the availability of the options for MessageBox(Ex)[A/W] itself.

bstordrup commented 1 month ago

I think the help button is slightly more problematic and also more complex for implementation. I suggest it is split out to a separate discussion. I also cannot immediately see it in the Winforms repo.

Created #9619 for this purpose and edited the original proposal.

Winforms also added default button 4, might be worth mentioning in comments.

Included in the new API Proposal

miloush commented 1 month ago

Thanks @bstordrup! I think the default button discussion belongs to this one. I wasn't suggesting a new enum/API should be added, I was suggesting - as you did - to just say we don't need to add default button 4 because WPF does not use them in public API.

It is a bit unfortunate that the button is called Try but the result is called TryAgain but it follows the native API and this is one of the few places I agree that alignment with Winforms is justified.

ThomasGoulet73 commented 1 month ago

@miloush I replaced the https://github.com/dotnet/wpf/labels/api-ready-for-review label with the https://github.com/dotnet/wpf/labels/API%20suggestion. I believe this label is not up to us to add, it's up to the repo owners aka the WPF team. This label marks it as ready to be reviewed by the .Net API Review Team (I don't know their official name) and adds the issue in their tracking I believe (It makes it show up on this website: https://apireview.net/)

bstordrup commented 1 month ago

@ThomasGoulet73, according to https://apireview.net/request, the label to use is api-ready-for-review

Get into the backlog. Generally speaking, filing an issue in dotnet/runtime and applying the label api-ready-for-review on it will make your issue show up during API reviews.

ThomasGoulet73 commented 1 month ago

This issue is not ready for review by the .Net API Review Team because the WPF team hasn't signed off on it yet. See step 4 substep "Mark for review" here: https://github.com/dotnet/runtime/blob/main/docs/project/api-review-process.md#steps.

bstordrup commented 1 month ago

@miloush, @ThomasGoulet73 I no longer see the proposal in the https://apireview.net/ page. What does that mean?

All that is visible in the link, has the api-ready-for-review set. And they have had the api-suggestion label set at some point (at least the ones I looked at).

bstordrup commented 1 month ago

I'm a bit confused about the process here. What is the review process. If I read here, I get confused about the reference to corefx review process and mentioning about assigning to area owners.

But in the note in the list, it says that the WPF API process is not finalized yet. Does that mean that there is no area owner for WPF? Or is it not the review process described in https://apireview.net/? I cannot figure out who will actually need to review this and take a decision about it.

ThomasGoulet73 commented 1 month ago

I no longer see the proposal in the https://apireview.net/ page. What does that mean?

All that is visible in the link, has the api-ready-for-review set. And they have had the api-suggestion label set at some point (at least the ones I looked at).

https://apireview.net/ should only contain issue ready to be reviewed by the FXDC because they're the ones that use that site for tracking. The WPF team needs to review it first before the FXDC reviews it. The WPF team will review issues with the https://github.com/dotnet/wpf/labels/API%20suggestion label and when they approve it they will replace the label with https://github.com/dotnet/wpf/labels/api-ready-for-review.

I'm a bit confused about the process here. What is the review process. If I read here, I get confused about the reference to corefx review process and mentioning about assigning to area owners.

But in the note in the list, it says that the WPF API process is not finalized yet. Does that mean that there is no area owner for WPF? Or is it not the review process described in https://apireview.net/? I cannot figure out who will actually need to review this and take a decision about it.

A lot of the documentation in this repo is obsolete but this repo follows the same API review process as the dotnet/runtime here: https://github.com/dotnet/runtime/blob/main/docs/project/api-review-process.md. Your issue is currently at step 1.

This issue is how it's supposed to be, the only thing to do now is to wait for the WPF team to review it.

bstordrup commented 3 weeks ago

@ThomasGoulet73, @miloush, any news on reviewing this?