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

Dialogs in Xamarin Forms can't RequestClose without a handler first being given to Show() [Bug] [XF] #1883

Closed
UnreachableCode opened this issue Aug 21, 2019 · 12 comments · Fixed by #1931
Labels

Comments

@UnreachableCode
Copy link

Description

Hi everyone. I've been trying out the new dialog service in 7.2.0.1367 for Xamarin Forms. I've been able to get it working but only when providing a callback handler. Otherwise I get a vague null reference exception.

This isn't explained in the release notes or the spec or PR but I think it should be. I think it should also be possible to not provide a handler and for the dialog to just dismiss.

Steps to Reproduce

  1. Create a dialog View and ViewModel as outlined in the release notes.
  2. Open your dialog when the app is running using dialogService.ShowDialog("MyDialog")
  3. Fire your close command which should call RequestClose(null).

Expected Behavior

Dialog should close.

Actual Behavior

Program crashes with null reference exception. If a callback is provided, then this doesn't happen, like so: dialogService.ShowDialog("MyDialog", CloseDialogCallback).

Basic Information

  • Version with issue: Prism 7.2.0.1367
  • Last known good version: N/A, new feature.
  • Xamarin.Forms version: 4.1.0.618606
  • IDE: Visual Studio for Mac Community 8.2.3
@brianlagunas
Copy link
Member

Closing as no reproduction app provided.

@brianlagunas
Copy link
Member

Also, make sure you check for null before calling the RequestClose. This is most likely your problem.

@UnreachableCode
Copy link
Author

I'm afraid this doesn't fix it

Screenshot 2019-08-21 at 17 39 42

@brianlagunas
Copy link
Member

We'll need a repo app then

@UnreachableCode
Copy link
Author

On it

@UnreachableCode
Copy link
Author

It's going to be a little longer until I can reproduce this on a repo app.

@hro-it
Copy link

hro-it commented Aug 28, 2019

I'm afraid this doesn't fix it

Screenshot 2019-08-21 at 17 39 42

Would like to second that I'm receiving the exact same NullReferenceException at the exact same line. Only solution is to provide a callback handler even if unnecessary.

@hro-it
Copy link

hro-it commented Aug 29, 2019

This issue needs to be reopened as I may have discovered the problem to it.

It appears that there is an extension helper interface known as IDialogServiceExtensions that aids the developer in offering overloads for a multitude of scenarios. However, they are merely calling the original method signature ShowDialog(string name, IDialogParameters parameters, Action<IDialogResult> callback.

The trick is that in this particular implementation of the interface IDialogService the callback parameter is always assumed to be valid instead of a potentially null argument.

Implementation of method signature ShowDialog(this IDialogService dialogService, string name) in IDialogServiceExtensions shows that you are deliberately just passing a null value. A check needs to be implemented into the DialogService class to ensure that the callback is valid prior to call when executing void DialogAware_RequestClose(IDialogParameters outParameters).

I do not want to perform a pull request as I do not have an extremely thorough understanding of all the code in this new feature yet, but hope that this will send everyone down the correct path in regards to debugging.

@dansiegel
Copy link
Member

@hro-it if you'd like to provide a sample that reproduces the issue I'd be happy to reopen the issue

@hro-it
Copy link

hro-it commented Aug 29, 2019

@hro-it if you'd like to provide a sample that reproduces the issue I'd be happy to reopen the issue

https://github.com/hro-it/PrismDialogIssueApp

Sorry that took so long. Had some issues with Android emulation and the new Windows Sandbox interfering with one another. This problem is reproducible on both Android and iOS.

The project is just setup as a .zip file for you to download, build, and run.

@dansiegel
Copy link
Member

@hro-it thanks for providing a sample. I'll look into this as soon as I get a moment.

@lock
Copy link

lock bot commented Jan 28, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants