From a8e624959e8481f4388bedeb041729d4db3187a1 Mon Sep 17 00:00:00 2001 From: Sawyer Blatz Date: Fri, 17 Apr 2020 13:32:20 -0700 Subject: [PATCH] For #6940: Fixes top dynamic toolbar behavior (#9900) --- .../fenix/browser/BaseBrowserFragment.kt | 8 +-- .../mozilla/fenix/browser/BrowserFragment.kt | 5 -- .../components/toolbar/BrowserInteractor.kt | 4 ++ .../toolbar/BrowserToolbarController.kt | 5 ++ .../components/toolbar/BrowserToolbarView.kt | 67 +++++++++++-------- .../SwipeRefreshScrollingViewBehavior.kt | 48 +++++++++++++ .../customtabs/ExternalAppBrowserFragment.kt | 4 -- .../layout/component_browser_top_toolbar.xml | 3 +- app/src/main/res/layout/fragment_browser.xml | 3 +- 9 files changed, 104 insertions(+), 43 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/components/toolbar/SwipeRefreshScrollingViewBehavior.kt diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 635423295..e98f20dae 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -19,7 +19,6 @@ import androidx.fragment.app.Fragment import androidx.fragment.app.activityViewModels import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.findNavController -import com.google.android.material.appbar.AppBarLayout import com.google.android.material.snackbar.Snackbar import kotlinx.android.synthetic.main.fragment_browser.* import kotlinx.android.synthetic.main.fragment_browser.view.* @@ -77,6 +76,7 @@ import org.mozilla.fenix.components.toolbar.BrowserInteractor import org.mozilla.fenix.components.toolbar.BrowserToolbarView import org.mozilla.fenix.components.toolbar.BrowserToolbarViewInteractor import org.mozilla.fenix.components.toolbar.DefaultBrowserToolbarController +import org.mozilla.fenix.components.toolbar.SwipeRefreshScrollingViewBehavior import org.mozilla.fenix.components.toolbar.ToolbarIntegration import org.mozilla.fenix.downloads.DownloadNotificationBottomSheetDialog import org.mozilla.fenix.downloads.DownloadService @@ -175,8 +175,6 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session val toolbarHeight = resources.getDimensionPixelSize(R.dimen.browser_toolbar_height) - initializeEngineView(toolbarHeight) - browserAnimator = BrowserAnimator( fragment = WeakReference(this), engineView = WeakReference(engineView), @@ -510,6 +508,8 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session view = view ) } + + initializeEngineView(toolbarHeight) } } @@ -520,7 +520,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session val behavior = if (requireContext().settings().shouldUseBottomToolbar) { EngineViewBottomBehavior(context, null) } else { - AppBarLayout.ScrollingViewBehavior(context, null) + SwipeRefreshScrollingViewBehavior(requireContext(), null, engineView, browserToolbarView) } (swipeRefresh.layoutParams as CoordinatorLayout.LayoutParams).behavior = behavior diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt index b6ac7a924..09ce6d620 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -23,7 +23,6 @@ import mozilla.components.feature.session.TrackingProtectionUseCases import mozilla.components.feature.sitepermissions.SitePermissions import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.feature.tabs.WindowFeature -import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.support.base.feature.UserInteractionHandler import mozilla.components.support.base.feature.ViewBoundFeatureWrapper import org.mozilla.fenix.FeatureFlags @@ -103,10 +102,6 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { owner = this, view = view ) - - consumeFrom(browserFragmentStore) { - browserToolbarView.update(it) - } } } diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserInteractor.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserInteractor.kt index 1bc49a349..0f049ecc6 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserInteractor.kt @@ -31,4 +31,8 @@ open class BrowserInteractor( override fun onBrowserMenuDismissed(lowPrioHighlightItems: List) { browserToolbarController.handleBrowserMenuDismissed(lowPrioHighlightItems) } + + override fun onScrolled(offset: Int) { + browserToolbarController.handleScroll(offset) + } } diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt index d4844d7d7..a19fa5e93 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt @@ -48,6 +48,7 @@ import org.mozilla.fenix.utils.Do * An interface that handles the view manipulation of the BrowserToolbar, triggered by the Interactor */ interface BrowserToolbarController { + fun handleScroll(offset: Int) fun handleToolbarPaste(text: String) fun handleToolbarPasteAndGo(text: String) fun handleToolbarItemInteraction(item: ToolbarMenu.Item) @@ -139,6 +140,10 @@ class DefaultBrowserToolbarController( } } + override fun handleScroll(offset: Int) { + engineView.setVerticalClipping(offset) + } + @ExperimentalCoroutinesApi @SuppressWarnings("ComplexMethod", "LongMethod") override fun handleToolbarItemInteraction(item: ToolbarMenu.Item) { diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt index a9e57b4b2..fa04fcece 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt @@ -23,6 +23,7 @@ import com.google.android.material.appbar.AppBarLayout.LayoutParams.SCROLL_FLAG_ import com.google.android.material.snackbar.Snackbar import kotlinx.android.extensions.LayoutContainer import kotlinx.android.synthetic.main.browser_toolbar_popup_window.view.* +import kotlinx.android.synthetic.main.component_browser_top_toolbar.* import kotlinx.android.synthetic.main.component_browser_top_toolbar.view.* import mozilla.components.browser.domains.autocomplete.ShippedDomainsProvider import mozilla.components.browser.session.Session @@ -48,6 +49,7 @@ interface BrowserToolbarViewInteractor { fun onBrowserToolbarMenuItemTapped(item: ToolbarMenu.Item) fun onTabCounterClicked() fun onBrowserMenuDismissed(lowPrioHighlightItems: List) + fun onScrolled(offset: Int) } class BrowserToolbarView( @@ -140,8 +142,17 @@ class BrowserToolbarView( with(container.context) { val sessionManager = components.core.sessionManager + if (!shouldUseBottomToolbar) { + val offsetChangedListener = + AppBarLayout.OnOffsetChangedListener { _: AppBarLayout?, verticalOffset: Int -> + interactor.onScrolled(verticalOffset) + } + + app_bar.addOnOffsetChangedListener(offsetChangedListener) + } + view.apply { - setScrollFlagsForTopToolbar() + setScrollFlags() elevation = TOOLBAR_ELEVATION.dpToFloat(resources.displayMetrics) @@ -241,11 +252,6 @@ class BrowserToolbarView( } } - @Suppress("UNUSED_PARAMETER") - fun update(state: BrowserFragmentState) { - // Intentionally leaving this as a stub for now since we don't actually want to update currently - } - fun expand() { if (settings.shouldUseBottomToolbar && FeatureFlags.dynamicBottomToolbar) { (view.layoutParams as CoordinatorLayout.LayoutParams).apply { @@ -256,33 +262,38 @@ class BrowserToolbarView( } } - companion object { - private const val TOOLBAR_ELEVATION = 16 - } -} + /** + * Dynamically sets scroll flags for the toolbar when the user does not have a screen reader enabled + * Note that the bottom toolbar has a feature flag for being dynamic, so it may not get flags set. + */ + fun setScrollFlags(shouldDisableScroll: Boolean = false) { + if (view.context.settings().shouldUseBottomToolbar) { + if (FeatureFlags.dynamicBottomToolbar && view.layoutParams is CoordinatorLayout.LayoutParams) { + (view.layoutParams as CoordinatorLayout.LayoutParams).apply { + behavior = BrowserToolbarBottomBehavior(view.context, null) + } + } + + return + } + + val params = view.layoutParams as AppBarLayout.LayoutParams -/** - * Dynamically sets scroll flags for the toolbar when the user does not have a screen reader enabled - * Note that the bottom toolbar has a feature flag for being dynamic, so it may not get flags set. - */ -fun BrowserToolbar.setScrollFlagsForTopToolbar() { - // Don't set scroll flags for bottom toolbar - if (context.settings().shouldUseBottomToolbar) { - if (FeatureFlags.dynamicBottomToolbar && layoutParams is CoordinatorLayout.LayoutParams) { - (layoutParams as CoordinatorLayout.LayoutParams).apply { - behavior = BrowserToolbarBottomBehavior(context, null) + params.scrollFlags = when (view.context.settings().shouldUseFixedTopToolbar || shouldDisableScroll) { + true -> { + // Force expand the toolbar so the user is not stuck with a hidden toolbar + expand() + 0 + } + false -> { + SCROLL_FLAG_SCROLL or SCROLL_FLAG_ENTER_ALWAYS or SCROLL_FLAG_SNAP or SCROLL_FLAG_EXIT_UNTIL_COLLAPSED } } - return + view.layoutParams = params } - val params = layoutParams as AppBarLayout.LayoutParams - params.scrollFlags = when (context.settings().shouldUseFixedTopToolbar) { - true -> 0 - false -> { - SCROLL_FLAG_SCROLL or SCROLL_FLAG_ENTER_ALWAYS or SCROLL_FLAG_SNAP or - SCROLL_FLAG_EXIT_UNTIL_COLLAPSED - } + companion object { + private const val TOOLBAR_ELEVATION = 16 } } diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/SwipeRefreshScrollingViewBehavior.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/SwipeRefreshScrollingViewBehavior.kt new file mode 100644 index 000000000..2135fd00a --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/SwipeRefreshScrollingViewBehavior.kt @@ -0,0 +1,48 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.components.toolbar + +import android.content.Context +import android.util.AttributeSet +import android.view.View +import androidx.coordinatorlayout.widget.CoordinatorLayout +import com.google.android.material.appbar.AppBarLayout +import mozilla.components.concept.engine.EngineView +import mozilla.components.concept.engine.EngineView.InputResult.INPUT_RESULT_UNHANDLED +import org.mozilla.fenix.ext.settings + +/** + * ScrollingViewBehavior that will setScrollFlags on BrowserToolbar based on EngineView touch handling + */ +class SwipeRefreshScrollingViewBehavior( + context: Context, + attrs: AttributeSet?, + private val engineView: EngineView, + private val browserToolbarView: BrowserToolbarView +) : AppBarLayout.ScrollingViewBehavior(context, attrs) { + override fun onStartNestedScroll( + coordinatorLayout: CoordinatorLayout, + child: View, + directTargetChild: View, + target: View, + axes: Int, + type: Int + ): Boolean { + + if (!browserToolbarView.view.context.settings().shouldUseBottomToolbar) { + val shouldDisable = engineView.getInputResult() == INPUT_RESULT_UNHANDLED + browserToolbarView.setScrollFlags(shouldDisable) + } + + return super.onStartNestedScroll( + coordinatorLayout, + child, + directTargetChild, + target, + axes, + type + ) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt index 108273281..b577285f9 100644 --- a/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt @@ -149,10 +149,6 @@ class ExternalAppBrowserFragment : BaseBrowserFragment(), UserInteractionHandler } } - consumeFrom(browserFragmentStore) { - browserToolbarView.update(it) - } - consumeFrom(components.core.customTabsStore) { state -> getSessionById() ?.let { session -> session.customTabConfig?.sessionToken } diff --git a/app/src/main/res/layout/component_browser_top_toolbar.xml b/app/src/main/res/layout/component_browser_top_toolbar.xml index ea19bfc1a..479bc9dd9 100644 --- a/app/src/main/res/layout/component_browser_top_toolbar.xml +++ b/app/src/main/res/layout/component_browser_top_toolbar.xml @@ -2,7 +2,8 @@ - diff --git a/app/src/main/res/layout/fragment_browser.xml b/app/src/main/res/layout/fragment_browser.xml index 79030138e..db398111e 100644 --- a/app/src/main/res/layout/fragment_browser.xml +++ b/app/src/main/res/layout/fragment_browser.xml @@ -2,7 +2,8 @@ -