From c81052bd6dae0f1c4b671f423584c0a9ee0005bb Mon Sep 17 00:00:00 2001 From: Sawyer Blatz Date: Fri, 17 Apr 2020 13:32:20 -0700 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/6940: Fixes top dynamic toolbar behavior (https://github.com/mozilla-mobile/fenix/pull/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 | 75 +++++++++++-------- .../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, 108 insertions(+), 47 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 6354232959..e98f20dae4 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 b6ac7a9248..09ce6d620f 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 1bc49a349e..0f049ecc62 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 d4844d7d70..a19fa5e935 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 a9e57b4b2e..fa04fcecee 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( } } + /** + * 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 + + 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 + } + } + + view.layoutParams = params + } + 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 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) - } - } - - return - } - - 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 - } - } -} 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 0000000000..2135fd00ae --- /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 1082732815..b577285f9b 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 ea19bfc1a5..479bc9dd91 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 79030138e4..db398111ea 100644 --- a/app/src/main/res/layout/fragment_browser.xml +++ b/app/src/main/res/layout/fragment_browser.xml @@ -2,7 +2,8 @@ -