Repository navigation
Fix toolbar floating at the top and not following web content observations - #4653
jonalmeida wants to merge 2 commits into
Conversation
jonalmeida
changed the title
…ppingBehavior This regressed in 94e84c3, which migrated to the two-height EngineViewClippingBehavior API and dropped the old ToolbarPosition.BOTTOM argument without moving the height to the matching slot.
dda3ba8 to
22fdd73
Compare
We aren't using it or actively working on it, so let's remove it. Additionally, we have a Compose toolbar that has taken a different direction for it's implementation.
22fdd73 to
efd5579
Compare
|
Tick the box to add this pull request to the merge queue (same as
|
rvandermeulen
left a comment
rvandermeulen
left a comment
There was a problem hiding this comment.
Thanks, the height fix looks right: toolbar.height was read before layout and went into the top slot, so the bottom translation was always clamped to 0. Two inline questions below, plus a nit: "for it's implementation" in the second commit message should be "its".
| ) | ||
|
|
||
| val bottomToolbarHeight = resources.getDimensionPixelSize(R.dimen.browser_toolbar_height) | ||
| engineView.setDynamicToolbarMaxHeight(bottomToolbarHeight) |
There was a problem hiding this comment.
Moving this into BaseBrowserFragment means custom tabs and PWAs (ExternalAppBrowserFragment) now get the full toolbar height instead of 0. That seems right, since they use the same bottom toolbar, but it's worth a mention in the commit message and a quick check on a custom tab.
There was a problem hiding this comment.
Confirmed it works in a custom tab.
Tested the first commit against a few examples listed here: https://jonalmeida.com/blog/browser-test-sites/bottom-toolbar.html
Buggy behaviour on FX-{01-03} (amongst others that I didn't record):
Screen_recording_20261109_171733.mp4
Fixed behaviour:
Screen_recording_20261109_174454.mp4
Pull Request checklist