Skip to content

Commit

Permalink
Bug 1783561 - Stop BrowserToolbarBehavior positioning the snackbar
Browse files Browse the repository at this point in the history
The snackbar is placed depending on the toolbar so the relation should be
reversed. The toolbar should not know about and control the snackbar.
There could be other siblings that the snackbar wants to position itself by
and so removing this responsability from BrowserToolbarBehavior will help
better support current and future flows.
  • Loading branch information
Mugurell authored and mergify[bot] committed Jan 18, 2023
1 parent 6d95858 commit 04a464e
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 55 deletions.
Expand Up @@ -6,20 +6,16 @@ package mozilla.components.browser.toolbar.behavior

import android.content.Context
import android.util.AttributeSet
import android.view.Gravity
import android.view.MotionEvent
import android.view.View
import androidx.annotation.VisibleForTesting
import androidx.coordinatorlayout.widget.CoordinatorLayout
import androidx.core.view.ViewCompat
import com.google.android.material.snackbar.Snackbar
import mozilla.components.browser.toolbar.BrowserToolbar
import mozilla.components.concept.base.crash.CrashReporting
import mozilla.components.concept.engine.EngineView
import mozilla.components.support.ktx.android.view.findViewInHierarchy

private const val SMALL_ELEVATION_CHANGE = 0.01f

/**
* Where the toolbar is placed on the screen.
*/
Expand All @@ -36,7 +32,6 @@ enum class ToolbarPosition {
*
* This implementation will:
* - Show/Hide the [BrowserToolbar] automatically when scrolling vertically.
* - On showing a [Snackbar] position it above the [BrowserToolbar].
* - Snap the [BrowserToolbar] to be hidden or visible when the user stops scrolling.
*/
class BrowserToolbarBehavior(
Expand Down Expand Up @@ -130,14 +125,6 @@ class BrowserToolbarBehavior(
return false // allow events to be passed to below listeners
}

override fun layoutDependsOn(parent: CoordinatorLayout, child: BrowserToolbar, dependency: View): Boolean {
if (toolbarPosition == ToolbarPosition.BOTTOM && dependency is Snackbar.SnackbarLayout) {
positionSnackbar(child, dependency)
}

return super.layoutDependsOn(parent, child, dependency)
}

override fun onLayoutChild(
parent: CoordinatorLayout,
child: BrowserToolbar,
Expand Down Expand Up @@ -181,23 +168,6 @@ class BrowserToolbarBehavior(
isScrollEnabled = false
}

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun positionSnackbar(child: View, snackbarLayout: Snackbar.SnackbarLayout) {
val params = snackbarLayout.layoutParams as CoordinatorLayout.LayoutParams

// Position the snackbar above the toolbar so that it doesn't overlay the toolbar.
params.anchorId = child.id
params.anchorGravity = Gravity.TOP or Gravity.CENTER_HORIZONTAL
params.gravity = Gravity.TOP or Gravity.CENTER_HORIZONTAL

snackbarLayout.layoutParams = params

// In order to avoid the snackbar casting a shadow on the toolbar we adjust the elevation of the snackbar here.
// We still place it slightly behind the toolbar so that it will not animate over the toolbar but instead pop
// out from under the toolbar.
snackbarLayout.elevation = child.elevation - SMALL_ELEVATION_CHANGE
}

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun tryToScrollVertically(distance: Float) {
browserToolbar?.let { toolbar ->
Expand Down
Expand Up @@ -6,14 +6,12 @@ package mozilla.components.browser.toolbar.behavior

import android.content.Context
import android.graphics.Bitmap
import android.view.Gravity
import android.view.MotionEvent.ACTION_DOWN
import android.view.MotionEvent.ACTION_MOVE
import android.widget.FrameLayout
import androidx.coordinatorlayout.widget.CoordinatorLayout
import androidx.core.view.ViewCompat
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.google.android.material.snackbar.Snackbar
import mozilla.components.browser.toolbar.BrowserToolbar
import mozilla.components.concept.engine.EngineSession
import mozilla.components.concept.engine.EngineView
Expand Down Expand Up @@ -474,29 +472,6 @@ class BrowserToolbarBehaviorTest {
verify(yTranslator).collapseWithAnimation(toolbar)
}

@Test
fun `Behavior will position snackbar above toolbar`() {
val behavior = BrowserToolbarBehavior(testContext, null, ToolbarPosition.BOTTOM)

val toolbar: BrowserToolbar = mock()
doReturn(4223).`when`(toolbar).id

val layoutParams: CoordinatorLayout.LayoutParams = CoordinatorLayout.LayoutParams(0, 0)

val snackbarLayout: Snackbar.SnackbarLayout = mock()
doReturn(layoutParams).`when`(snackbarLayout).layoutParams

behavior.layoutDependsOn(
parent = mock(),
child = toolbar,
dependency = snackbarLayout,
)

assertEquals(4223, layoutParams.anchorId)
assertEquals(Gravity.TOP or Gravity.CENTER_HORIZONTAL, layoutParams.anchorGravity)
assertEquals(Gravity.TOP or Gravity.CENTER_HORIZONTAL, layoutParams.gravity)
}

@Test
fun `Behavior will forceExpand when scrolling up and !shouldScroll if the touch was handled in the browser`() {
val behavior = spy(BrowserToolbarBehavior(testContext, null, ToolbarPosition.BOTTOM))
Expand Down
3 changes: 3 additions & 0 deletions docs/changelog.md
Expand Up @@ -9,6 +9,9 @@ permalink: /changelog/
* [Gecko](https://github.com/mozilla-mobile/firefox-android/blob/main/android-components/plugins/dependencies/src/main/java/Gecko.kt)
* [Configuration](https://github.com/mozilla-mobile/firefox-android/blob/main/android-components/.config.yml)

* **browser-toolbar**
* ⚠️ **This is a breaking change**: `BrowserToolbarBehavior` will not position the `Snackbar.SnackbarLayout` anymore. The ownership for the positioning behavior should be reversed with the snackbar choosing whether it want to be shown above the toolbar and exactly how.

# 110.0.0
* [Commits](https://github.com/mozilla-mobile/firefox-android/compare/v109.0.0...v110.0.0)
* [Dependencies](https://github.com/mozilla-mobile/firefox-android/blob/v110.0.0/android-components/plugins/dependencies/src/main/java/DependenciesPlugin.kt)
Expand Down

0 comments on commit 04a464e

Please sign in to comment.