Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
bstordrup opened this issue Aug 21, 2024 · 18 comments
Labels
API suggestion Early API idea and discussion, it is NOT ready for implementation

Comments

@bstordrup
Copy link

bstordrup commented Aug 21, 2024

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
Copy link
Contributor

miloush commented Aug 21, 2024

Winforms API approval: dotnet/winforms#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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

The link also mentions API-set

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

@bstordrup
Copy link
Author

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
Copy link
Contributor

h3xds1nz commented Aug 21, 2024

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
Copy link
Author

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
Copy link
Contributor

miloush commented Aug 22, 2024

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.

@miloush miloush added the api-ready-for-review API is ready for review, it is NOT ready for implementation label Aug 22, 2024
@ThomasGoulet73 ThomasGoulet73 added API suggestion Early API idea and discussion, it is NOT ready for implementation and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Aug 22, 2024
@ThomasGoulet73
Copy link
Contributor

@miloush I replaced the api-ready-for-review API is ready for review, it is NOT ready for implementation label with the API suggestion Early API idea and discussion, it is NOT ready for implementation . 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
Copy link
Author

@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
Copy link
Contributor

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
Copy link
Author

bstordrup commented Aug 23, 2024

@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
Copy link
Author

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
Copy link
Contributor

ThomasGoulet73 commented Aug 26, 2024

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 API suggestion Early API idea and discussion, it is NOT ready for implementation label and when they approve it they will replace the label with api-ready-for-review API is ready for review, it is NOT ready for implementation .

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
Copy link
Author

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

@bstordrup
Copy link
Author

@ThomasGoulet73, @miloush any news?

@himgoyalmicro
Copy link
Contributor

Thanks @bstordrup for the API proposal. As these are enumeration additions and the impact also appears good, we are inclined to take this proposal forward for .NET10. However, we can't commit on timelines due to the process and dependencies associated with API proposals. We will keep this issue posted.

@bstordrup
Copy link
Author

@himgoyalmicro, can I do some work on it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API suggestion Early API idea and discussion, it is NOT ready for implementation
Projects
None yet
Development

No branches or pull requests

5 participants