MicrosoftEdge / WebView2Feedback

Feedback and discussions about Microsoft Edge WebView2
https://aka.ms/webview2
451 stars 55 forks source link

WPF control should raise PropertyChanged on CanGoBack / CanGoForward / ZoomFactor #434

Closed Symbai closed 2 years ago

Symbai commented 4 years ago

Is your feature request related to a problem? Please describe.

The webview2 WPF control seems to not raise PropertyChanged on CanGoBack, CanGoForward and ZoomFactor. Instead its left for the developer to either double the work by creating properties for these properties or by calling CommandManager.InvalidateRequerySuggested() which however invalidates ALL commands in the application, so its costly.

Usually WPF developers would create for example a "Go Back" button and bind this button to a command, which is bound to the "CanGoBack" property. There are multiple ways how this can look like but one of it can be:

public DelegateCommand GoBackCommand => new DelegateCommand(() =>
{
    webView.GoBack();
}, () => webView?.CanGoBack == true);

If this CanGoBack property changes to true, the command gets notified which notify the button and change the button state to Enabled. The button changes the visual state and the end user sees its now clickable. This binding feature is what makes WPF so powerful. However the current issue is that bindings to them are not properly updated when they change. The command doesn't get notified and so the button does not. The "Go Back" button the WPF developer added still remains visually disabled.

You can see this issue on the WPF webview2 sample application directly: https://github.com/michael-russin/WebView2Samples/blob/7bfe71a86606075b4d9cc2f3742916520c98e030/WebView2WpfBrowser/MainWindow.xaml.cs#L107-L122

Describe the solution you'd like and alternatives you've considered

The WPF control needs to implement INotifyPropertyChanged and call it whenever CanGoBack, CanGoForward or ZoomFactor property changes.

AB#29358187

cgeier commented 4 years ago

I haven't really done much WPF programming, mostly WinForms, but I put together a demo that shows how one can use the CoreWebView2 HistoryChanged event and then use the CanGoBack (or CanGoForward) property of your WebView2 instance in WPF.

Here's an overview:

private async void Window_Loaded(object sender, RoutedEventArgs e)
{
    //initialize CoreWebView2
    await InitializeCoreWebView2Async();
}
public async Task InitializeCoreWebView2Async()
{
    //wait for CoreWebView2 initialization
    await webView21.EnsureCoreWebView2Async();
}
private void webView21_CoreWebView2Ready(object sender, EventArgs e)
{
    //subscribe to events (add event handlers) - CoreWebView2
    webView21.CoreWebView2.HistoryChanged += CoreWebView2_HistoryChanged;
}
private void CoreWebView2_HistoryChanged(object sender, object e)
{
    btnBack.IsEnabled = webView21.CanGoBack;
    btnForward.IsEnabled = webView21.CanGoForward;
}

See the demo project: WebView2WpfTest.zip for the complete code.

Symbai commented 4 years ago

I haven't said its impossible. I have only said its extremely unusual for WPF to manually refresh a binding, as its against everything WPF stands for. But I appreciate you showed an alterative to the CommandManager.InvalidateRequerySuggested at least.

champnic commented 4 years ago

Here are the change events you should listen to for each of those properties:

All of these properties are DependencyProperties, so I believe Binding should generally just work with them. If you are writing the code-behind yourself, then you'll need to listen to those events.

This is non-obvious. I'm going to open an item on our backlog to make this more clear, and potentially directly expose CanGoBackChanged and CanGoForwardChanged off of the WebView2 control.

darbid commented 3 years ago

As I indicated in my feature request #1146 I think that both of these properties should not be read only dependency properties. Sure in Winforms the properties might be read only but I believe to be able to use these in WPF for example in MVVM patterns they should be not read only.

If that is not the chosen path then the HistoryChanged event should be a RoutedEvent of WebView2.

Currently the only option I see is immediate sub classing of WebView2 or creating Attached Properties/Behaviors.

yunate commented 2 years ago

We have tried to reproduce the problem and found that: 1.We tried to implement INotifyPropertyChanged as suggested and it doesn't fix the problem. 2.We are able to successfully bind control properties directly to those WebView2 properties and have them update correctly (e.g. we could bind a button's Enabled property directly to CanGoBack and it would work). 3.We think that a command's CanExecute method cannot automatically be re-run based on a notification from a property, because the command doesn't automatically know which property to listen to.

champnic commented 2 years ago

Closing this as By Design behavior.

amaitland commented 2 years ago

The problem here sounds very much like you are clobbering the binding via updating the DependencyProperty using SetValue when SetCurrentValue should be used for internal value updates.

https://wpf.2000things.com/2010/12/06/147-use-setcurrentvalue-when-you-want-to-set-a-dependency-property-value-from-within-a-control/

champnic commented 2 years ago

Confirmed it is using SetValue. I'll also use this bug to track looking into any other calls to SetValue that should instead be SetCurrentValue. Thanks @amaitland!

champnic commented 2 years ago

Looks like the CanGoBack and CanGoForward are the only ones currently. The rest are calling SetCurrentValue.

yunate commented 2 years ago
darbid commented 2 years ago

@yunate so just checking in - you are saying in WPF you can bind to a read-only property?