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] Add properties for page title and icon to WebViewState #1029

Merged
merged 6 commits into from
Feb 27, 2022

Conversation

sampengilly
Copy link
Contributor

Add properties to the WebViewState class to supply any received title and favicon from the loaded page to the caller.

@bentrengrove bentrengrove self-requested a review February 17, 2022 21:39
@bentrengrove
Copy link
Collaborator

Thanks for the contribution. I'm just wondering if we have to worry about clearing that state on subsequent page loads. Do these callbacks always get called on page load so they will never be stale?

What about if you have loaded a url and then via a compose state change load a data string, would the title and favicon be set back to null?

@sampengilly
Copy link
Contributor Author

Good point, I'll update to reset the title and favicon on page started.

@bentrengrove
Copy link
Collaborator

You will also need to update the API file which can be done by running ./gradlew metalavaGenerateSignature

@bentrengrove
Copy link
Collaborator

Sorry to be a pain, this new state should also have tests. You could add some test html and test it that way, there is already an example in the project of that I believe. Let me know if you need more info.

@sampengilly
Copy link
Contributor Author

I've added a test for the title state fine. However the favicon state is a real challenge. Since the bitmap isn't necessarily loaded by the time onPageFinished() is called (and the idling resource resumes)

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.

Nice, thank you! Don't worry about that other test for now, I'll see what I can come up with.

Just need to fix up the one small thing and this will be good to go.

@bentrengrove bentrengrove changed the title [WebView] Add properties for page title and icon to WebViewState [Web] Add properties for page title and icon to WebViewState Feb 25, 2022
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.

Awesome. Thanks for the contribution!

@bentrengrove bentrengrove merged commit d4e9823 into google:main Feb 27, 2022
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.

2 participants