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

[Web] Handle WebView Full Screen callbacks via state #1113

Closed

Conversation

sampengilly
Copy link
Contributor

I tried to write a test for this but so far haven't gotten things to work in that environment. I checked the behaviour during whilst implementing the feature by fullscreening youtube videos in the sample app and showing a dialog with the provided view.

@sampengilly
Copy link
Contributor Author

Probably best that I rebase this onto #1097 once that's merged

@bentrengrove
Copy link
Collaborator

This one might be venturing into an area we probably don't want to support from the Accompanist version, I almost feel like people should fork their own implementation if they need this functionality.

The biggest part that worries me is that custom view and what that could mean if you want to pass in a ComposeView for it or a normal view and all the can of worms involved in that.

@sampengilly
Copy link
Contributor Author

sampengilly commented Mar 29, 2022

Yep, with the exposure of the WebChromeClient it makes a little less sense to support this explicitly. As for the custom view, it's essentially just what the WebView has given us. There could be an argument over whether it should be exposed as-is or wrapped in a composable and then exposed.

There also seems to be some ambiguity in the docs stating that if the showCustomView/hideCustomView methods are overridden at all then it will be taken to mean that the WebView supports it, and if they're not overridden it will be taken to mean that it doesn't support it. I couldn't find anything concrete to determine if an empty override method with a single super call is somehow seen as supporting fullscreen just by existing even if there's no actual implementation.

@bentrengrove
Copy link
Collaborator

I'm going to go ahead and say that this is not something we want to support right now. I am hoping to get the Web wrapper in to a pretty stable state before Accompanist has it's next stable release so will be slowing down on adding new features.

Thanks for the PR again though.

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.

None yet

2 participants