maui icon indicating copy to clipboard operation
maui copied to clipboard

Published 20 hours ago • verified publisher icon dotnet

[Catalyst] Allow to use a custom WKUIDelegate on WebView

Open jsuarezruiz opened this issue 1 year ago • 13 comments

Description of Change

Error with a simple fix that I have seen doing triage taking a look to new issues. Allow to use a custom WKUIDelegate on WebView

Issues Fixed

Fixes #18394

jsuarezruiz avatar Nov 02 '23 13:11 jsuarezruiz

Do we have any tests for this - even for iOS?

mattleibow avatar Nov 02 '23 21:11 mattleibow

Line 31 should be || MacCatalyst, not && MacCatalyst. With AND operation, the conditional statement will not work on iOS and Mac. Please kindly change it! Thanks.

tekmun avatar Nov 03 '23 04:11 tekmun

Do we have any tests for this - even for iOS?

Yes. Use this project. https://github.com/dotnet/maui/issues/17431

Discussion was done in https://stackoverflow.com/questions/77170252/requestcapturemediapermission-for-wkwebview-in-maui-ios/77200328

tekmun avatar Nov 05 '23 09:11 tekmun

I mean do we have any unit or device tests in Maui? The fact that this PR is green and the wrong condition means that we don't and should. All changes should have a test to ensure that it never breaks again.

mattleibow avatar Nov 05 '23 12:11 mattleibow

With current main repository, Line 15 on Platforms/iOS/WebViewUIDelegate.cs of this project https://github.com/tekmun/WebViewIssue.git will be called for iOS.

I have not tested it on Mac though. Can someone test whether with the suggested changes, Line 15 on Platforms/iOS/WebViewUIDelegate.cs will be called on Mac/MacCatalyst? In theory, it should.

If it works, then this project https://github.com/tekmun/WebViewIssue.git can be easily converted to become a unit test because it needs to ensure Line 15 on Platforms/iOS/WebViewUIDelegate.cs and Platforms/MacCatalyst/WebViewUIDelegate.cs are called.

tekmun avatar Nov 07 '23 05:11 tekmun

Can this change make it into GA next week?

tekmun avatar Nov 09 '23 00:11 tekmun

This change is not destructive, meaning that it will not affect systems that are working now. It just allows the MacCatalyst app to have the same functionality as the iOS app. Please kindly merge the changes before GA next week. Thanks.

tekmun avatar Nov 10 '23 10:11 tekmun

Any updates on when the merging of this code into the main repository?

tekmun avatar Nov 13 '23 16:11 tekmun

I mean do we have any unit or device tests in Maui? The fact that this PR is green and the wrong condition means that we don't and should. All changes should have a test to ensure that it never breaks again.

Require a custom WKUIDelegate and handlers, already added a sample in the Gallery.

jsuarezruiz avatar Nov 14 '23 13:11 jsuarezruiz

Any updates on when the merging of this code into the main repository? I understand maui is now a nuget package and we can use <MauiVersion> to download a nightly build. Can I test it out soon?

tekmun avatar Nov 20 '23 03:11 tekmun

It looks like the test case has been added and all checks have passed. Can the reviewers approve this request into the main branch?

tekmun avatar Nov 23 '23 00:11 tekmun

The requested change has not been reflected in the main branch. Why is request marked as DONE?

tekmun avatar Jan 13 '24 12:01 tekmun

Yay, build is passing. Could we get another review here?

jsuarezruiz avatar Jan 25 '24 11:01 jsuarezruiz

https://github.com/dotnet/maui/pull/18483#discussion_r1536671820

PureWeen avatar Apr 12 '24 00:04 PureWeen