search

KevinnZou / compose-webview-multiplatform

WebView for JetBrains Compose Multiplatform
https://kevinnzou.github.io/compose-webview-multiplatform/
Apache License 2.0
305 stars 39 forks source link

Upgrade deps version #120

Closed alirezat775 closed 3 weeks ago

alirezat775 commented 2 months ago

Hey @KevinnZou In this PR, I updated dependencies to the new available version, and, since the new compose version has been released I updated it, Plus the new version had some breaking changes and I fixed them.

Thank you in advance for reviewing this PR

KevinnZou commented 2 months ago

@alirezat775 Thank you for your contribution! That is exactly what I am planning to do! I will review and test it soon!

alirezat775 commented 2 months ago

@KevinnZou Thank you for the review, the new resource directory has been applied based on your feedback and Documentation also, I moved the HTML path's decision to the app level from the common level, to make sure we do not restrict any platform, but we need to mention in the doc or release note that after this change, using resources must be compatible with new compose version.

KevinnZou commented 2 months ago

@alirezat775 Thank you for your work! However, when I tested it locally, I found that it did not work on Android. I think the issue may be that the HTML files are not being packaged to the Android file system after being moved to the composeResource folder. I have already submitted this issue to Jetbrains.

To resolve the issue, we can revert to the previous version where the file was stored in the resources folder. But this will violate the official instructions. Do you have any suggestions?

alirezat775 commented 1 month ago

@KevinnZou Thanks for submitting the issue on the CMP side, I'm not sure about the side effect of reverting the composeResource to the previous version, it opens the incompatibility door for webview's users. they have to maintain 2 packaging structures, one for KMP, and one for webview library because we made it non-standard.

KevinnZou commented 1 month ago

@alirezat775 Yes, I agree that it would be best to align with the KMP resource structure. Let's wait for an official response and work together to find a solution for this issue.

KevinnZou commented 3 weeks ago

@alirezat775 I found a solution to the previous issue. We just need to place the HTML files under the composeResource/assets folder, and everything will work as before. I have submitted a new pull request based on this one, can you please review it?

alirezat775 commented 3 weeks ago

I'm declining this PR because of another implementation