[Preview-4] Change the ContentDialog XamlRoot not set error message to something more helpful.

[nzstevem]

Describe the bug

Steps to reproduce the bug

Steps to reproduce the behavior:

1. Create a standard winui in desktop program
2. Create a ContentDialog object in the button click event and execute the show() method.
3. System.ArgumentException - Value not in expected range is raised.

Expected behavior Exception with a useful error message.

Screenshots See above

Version Info

NuGet package version:

Windows app type:	UWP	Win32

Windows 10 version	Saw the problem?
Insider Build (xxxxx)
October 2020 Update (19042)
May 2020 Update (19041)
November 2019 Update (18363)
May 2019 Update (18362)
October 2018 Update (17763)
April 2018 Update (17134)
Fall Creators Update (16299)
Creators Update (15063)

Device form factor	Saw the problem?
Desktop
Xbox
Surface Hub
IoT

Additional context

[nzstevem]

Not sure what happened to the version info.

Observed in Winui in Desktop

Edition Windows 10 Pro Version 20H2 Installed on ‎30/‎07/‎2020 OS build 19042.804 Experience Windows Feature Experience Pack 120.2212.551.0

[dotMorten]

I also called that out in this comment: https://github.com/microsoft/microsoft-ui-xaml/issues/4167#issuecomment-778485861 Had the same happen with MessageDialog, but the workaround presented in #4167 doesn't work for ContentDialog.

It's worth pointing out that if you declare the ContentDialog in the Page xaml instead, it'll work.

[crramirez]

It's worth pointing out that if you declare the ContentDialog in the Page xaml instead, it'll work.

Interesting I didn't know and can simplify the work

[lhak]

I think it is required to set the XamlRoot property of the dialog to get it to work in a desktop app. The error message is certainly not helpful at all.

[StephenLPeters]

Agreed, the error message is not helpful, but this is the problem and solution.

[nzstevem]

Stephen, Excuse me if I misunderstand I’m new to this. I don’t understand why you have closed this item. Has the code been fixed and tested so that when the next preview rolls out this feature will work as documented? Rgds

[StephenLPeters]

This issue is expected. When content dialog is contructed in a page the XamlRoot gets set automatically, however if you create a ContentDialog from code behind or by some other method you might have to tell the content dialog which xaml root it is to be associated with. You do this by setting the XamlRoot property.

Thus I closed this issue because this isn't an issue with ContentDialog, its expected. Is there documentation that says this should work? If so we should change that documentation. We could also repurpose this issue as an issue to have the error message be a little more useful.

[dotMorten]

@StephenLPeters Before closing it, please change the error thrown to clearly state that the XamlRoot isn't set. The current behavior is extremely unhelpful.

[nzstevem]

The documentation example does not mention this requirement.

Seems to me that both the error message and the docs need to be updated...

https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.controls.contentdialog?view=winrt-19041

[dotMorten]

@nzstevem it actually does: https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.controls.contentdialog?view=winrt-19041#contentdialog-in-appwindow-or-xaml-islands (Also this is the UWP doc not the WinUI doc) Remember in a WinUI Win32 app you don't have an AppWindow so the xamlisland doc applies to this here too. I'm sure it'll be better once we have dedicated winui doc

[nzstevem]

Thanks. Yes, I see that down in the lower part of the page. All the more important that these easily fixed migration incompatibilities generate clear and obvious error messages. Perhaps VS should enforce what is in effect a mandatory field for this app type.

[patrickklaeren]

The documentation has this buried pretty far down - definitely not easy for anyone to find. I only found out the XamlRoot had to be set by finding this issue.

[nzstevem]

I think this a particularly bad issue because when you are new to winui, and particularly if you come from uwp, the first thing you want to do once you get past xaml layouts without a designer is add some interactivity and potentially some debugging using a pop up message box.

When you do this you immediately get this error. It took me 2 days to find the cause! I glossed over the docs without noticing the requirement several times.

If you are trying to get developers to take this product seriously they need an easy onramp and a good experience from the get go.

When something that should be so easy is so hard you are going to lose many potential advocates at the first step.

The real issue here is that you don’t expect to need to provide this information. Might it not be better to build in a default that at least produces a non-crash default behaviour that can be changed if required.

Rgds Steve 021-956-996

On 13/05/2022, at 7:19 AM, Patrick Klaeren @.***> wrote:

 The documentation has this buried pretty far down - definitely not easy for anyone to find. I only found out the XamlRoot had to be set by finding this issue.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

[github-actions[bot]]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[dotMorten]

Error message has changed in v1.3.230724000, however the error is just as unhelpful as before:

Drilling in a bit deeper to the data of the exception, you can find a bit more info, albeit still not helpful wrt what you did wrong: At least it gives you a hint that it is related to the xamlroot, so it's closer considering the fix is literally to set the xamlroot

   cd.XamlRoot = this.Content.XamlRoot;

At the very least it would be good to expose this error message, since most don't drill into the data array of the exception.

[nzstevem]

I raised this issue originally when, as a new person to this environment with deep IT experience but limited hands on programming experience with the latest tools, I was trying to get my head round the WinUI environment. Once I had scanned through the documents I tried to build the simplest classic "hello world" type program. It took me TWO DAYS to work out why my simple program was failing. If Microsoft wants these products adopted it needs to do some classic useability testing using fresh subjects to establish the hurdles newbies strike and then clear them. The on road needs to be easy. If setting XamlRoot explicitly is mandatory then it should be part of the dialog constructor. If that is not possible then the error needs to be explicit - "ERROR - XamlRoot not set !!".

[codendone]

Same issue as #3678. I agree this (and error messages and visibility to error messages in general) needs to be better.