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

EngineView doesn't render content below the toolbar #975

Open
jonalmeida opened this issue Dec 20, 2019 · 3 comments
Open

EngineView doesn't render content below the toolbar #975

jonalmeida opened this issue Dec 20, 2019 · 3 comments
Labels
🐞 bug Something isn't working

Comments

@jonalmeida
Copy link
Collaborator

jonalmeida commented Dec 20, 2019

This PR added dynamic adjustment for the engineView, but it wasn't in the right place for when the toolbar scrolls away. This ends up showing blank white boxes where the engine view is when the toolbar is scrolled away.

Screen Shot 2019-12-20 at 10 06 23 AM

We need to add that call into the EngineViewBehaviour probably so that we can have it invoked when the toolbar scrolls away.

I did a quick test of the solution I proposed but that doesn't seem like the best way forward.

Note: consider if this has a perf loss to it.

cc: @hiikezoe for reference

@jonalmeida jonalmeida added the 🐞 bug Something isn't working label Dec 20, 2019
@jonalmeida
Copy link
Collaborator Author

Revert the previous addition of the new API that is invoked statically: #977

Add the API to be invoked when the toolbar is dynamically changing height instead in the CoordinatorLayout.Behaviour: mozilla-mobile/android-components#5404

@hiikezoe
Copy link
Contributor

The #977 looks totally wrong to me. The max height shouldn't be changed during the dynamic toolbar transitions.

I suppose the issue here is that the content in question is not scrollable, but the toolbar is able to be hidden? I'd say, in such cases, we should stop the dynamic toolbar transition.

@jonalmeida
Copy link
Collaborator Author

I suppose the issue here is that the content in question is not scrollable, but the toolbar is able to be hidden? I'd say, in such cases, we should stop the dynamic toolbar transition.

Yes, I think you're right that the toolbar shouldn't be hidden which is something we might know from bug 1557411.

I was playing around with this last evening and AC#134 (which I think is what you're referring to) looked much better than a white box on some sites, so I put up that PR but I don't feel strongly about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants