Discussion: MVVM-friendly ContentDialog

[omikhailov]

Discussion: Current API of ContentDialog is not MVVM-friendly

It forces developers to call ShowAsync to open dialog. But ContentDialog is obviously part of View layer and this makes it incompatible or inconvenient for use with MVVM, because:

• If this method is called in code-behind of the View, this is dirty style of coding
• If it is called from ViewModel, this is violation of MVVM rules, because lower layer should not depend on higher layers (although it looks like this API was meant to be used exactly this way for small ad-hoc dialogs)

So, usually developers have to create some DialogService and pretend that it's fine, because services live apart from Models, Views and ViewModels.

I believe there should have been just a bindable property "IsOpen" here instead of the method. In addition, it is strange that ContentDialog provides limited set of predefined buttons because it is Content dialog after all and developer can add whatever he needs as content.

So, what I would be happy to see instead of current ContentDialog would look like following:

<ContentDialog IsOpen="{x:Bind pageViewModel.DialogViewModel.IsOpen, Mode=OneWay}" ...>
    <Grid>
        ...
        <Button Content="OK" Click="{x:Bind pageViewModel.DialogViewModel.CloseDialog}" />  
    <Grid />
<ContentDialog />

Related Links

Current ContentDialog API

[StephenLPeters]

@MikeHillberg FYI

[MikeHillberg]

I think this makes sense too. Across Windows we generally avoid a property for showing UI, preferring a Show method instead (there's a lot of those). But we treat Xaml special specifically to enable data binding and MVVM, thus flyouts have both the IsOpen property and the ShowAt method.

I'd just change the example here to listen to the ContentDialog's built-in OK button (PrimaryButtonClick) rather than having one in its content area.

[omikhailov]

I agree, if it is possible to add a property and don't change other APIs, it would be good backward compatible solution.

[omikhailov]

Just noticed one related thing about IsOpen: this property cannot be set for FlyoutBase.