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

Make Window.DialogResult a public property. #18284

Closed
wants to merge 1 commit into from

Conversation

rabbitism
Copy link
Contributor

@rabbitism rabbitism commented Feb 21, 2025

What does the pull request do?

Make Window.DialogResult a public property

What is the current behavior?

The only way to return a result when closing a dialog is to call Close(object?), which means we don't have any way to return a value when closing from system title bar (or press ALt+F4).

What is the updated/expected behavior with this PR?

User can set this property in advance if they want to return some value as default DialogResult. If user choose to close with Close(object?) method, then this paramter has a higher priority, which is the original behavior.

How was the solution implemented (if it's not obvious)?

Similar behaviors from other frameworks:

  1. Winform: there is a public property, but it is an enum.
  2. WPF: there is a public property, but a nullable boolean.
  3. WinUI: no such a behavior. But when close from close button(or press ESC), a ContentDilaogResult.None is returned

Checklist

Breaking changes

Obsoletions / Deprecations

Fixed issues

Closes #17691

@MrJul MrJul added feature needs-api-review The PR adds new public APIs that should be reviewed. labels Feb 21, 2025
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.3.999-cibuild0055026-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@MrJul
Copy link
Member

MrJul commented Mar 5, 2025

Public API for review:

namespace Avalonia.Controls
{
    public class Window : WindowBase, IFocusScope, ILayoutRoot
    {
+        public object? DialogResult { get; set; }
    }
}

@MrJul
Copy link
Member

MrJul commented Mar 6, 2025

During the API review meeting, it was noticed that in WPF, DialogResult immediately closes the window, which isn't the case at all in this PR. We aren't really comfortable adding the exact same API member as WPF with a totally different behavior.

We were against mimicking the WPF behavior as it would effectively be exactly the same as Close(DialogResult?).
We discussed calling the property something like DefaultDialogResult.

At the end of the day, we aren't sure this API is needed at all.
The caller of ShowDialog would need to handle a default/null result anyway and be prepared for that.
As mentioned above, if the WPF behavior is needed, Close(DialogResult?) works today.

Therefore, the API is rejected.

@MrJul MrJul removed the needs-api-review The PR adds new public APIs that should be reviewed. label Mar 6, 2025
@rabbitism rabbitism closed this Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Window] Expose Window.DialogResult as a property
3 participants