Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WebView] ✨ Expose the postUrl function via the WebView Composable #1677

Merged
merged 3 commits into from
Aug 8, 2023

Conversation

MaxMichel2
Copy link
Contributor

Adds the postUrl function to the WebView

Fixes #1612

@bentrengrove
Copy link
Collaborator

Thanks for this contribution, it's looking good! We would also need a test please to be able to merge this

@bentrengrove bentrengrove self-requested a review July 23, 2023 22:59
@MaxMichel2
Copy link
Contributor Author

Hey @bentrengrove,

I totally forgot about the test! That's my bad. It should be added now so feel free to check and come back to me with other changes if required.

Copy link
Collaborator

@bentrengrove bentrengrove left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the contribution

@bentrengrove bentrengrove enabled auto-merge August 8, 2023 00:57
@MaxMichel2
Copy link
Contributor Author

It seems like the build failed for some reason. I'm not too sure on why that is however. I can take a look at the code if required 🤔

@bentrengrove
Copy link
Collaborator

e: file:///home/runner/work/accompanist/accompanist/web/src/main/java/com/google/accompanist/web/WebView.kt:332:5 Visibility must be specified in explicit API mode

e: file:///home/runner/work/accompanist/accompanist/web/src/main/java/com/google/accompanist/web/WebView.kt:577:5 Visibility must be specified in explicit API mode
e: file:///home/runner/work/accompanist/accompanist/web/src/main/java/com/google/accompanist/web/WebView.kt:700:1 Visibility must be specified in explicit API mode

It's running in explicit API mode so you probably just have to add public to all your assumed public methods and classes.

@MaxMichel2
Copy link
Contributor Author

MaxMichel2 commented Aug 8, 2023

e: file:///home/runner/work/accompanist/accompanist/web/src/main/java/com/google/accompanist/web/WebView.kt:332:5 Visibility must be specified in explicit API mode

e: file:///home/runner/work/accompanist/accompanist/web/src/main/java/com/google/accompanist/web/WebView.kt:577:5 Visibility must be specified in explicit API mode
e: file:///home/runner/work/accompanist/accompanist/web/src/main/java/com/google/accompanist/web/WebView.kt:700:1 Visibility must be specified in explicit API mode

It's running in explicit API mode so you probably just have to add public to all your assumed public methods and classes.

It seems as though the lines don't reference anything specific on my side... You can check the lines in the PR but 332, 577 and 700 don't point to anything where I could add the public keyword unfortunately

I've tried running the build as configured in the github actions on my side and it works just fine so I'm wondering if I'm missing a config that you may have locally 🤔

I also tried adding the explicit API mode locally but this seems to want keywords in many more places that what the build currently reports.

All help on this issue would be appreciated since I'd like to get this added soon ^^

@bentrengrove, could you try to re-run the job and if it fails again, I'll try to see what's going on on my side

auto-merge was automatically disabled August 8, 2023 14:59

Head branch was pushed to by a user without write access

@MaxMichel2 MaxMichel2 force-pushed the feature/web_view_post_url branch from c6b21c5 to d7e61b6 Compare August 8, 2023 14:59
@MaxMichel2 MaxMichel2 requested a review from bentrengrove August 8, 2023 15:00
@bentrengrove bentrengrove enabled auto-merge August 8, 2023 21:52
@bentrengrove bentrengrove merged commit 0b662c1 into google:main Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[WebView] postUrl not working anymore
2 participants