From 5d664b979d8c7b1722b0e645b12b1d4b6ba98371 Mon Sep 17 00:00:00 2001 From: ekager Date: Wed, 5 Aug 2020 18:33:43 -0400 Subject: [PATCH] For #6313 - Removes unused browser animations, improve delayed paint interactions --- .../java/org/mozilla/fenix/HomeActivity.kt | 2 +- .../addons/InstalledAddonDetailsFragment.kt | 3 +- .../fenix/browser/BaseBrowserFragment.kt | 10 ++- .../mozilla/fenix/browser/BrowserAnimator.kt | 56 +++-------------- .../toolbar/BrowserToolbarController.kt | 61 ++++++++++++------- .../org/mozilla/fenix/home/HomeFragment.kt | 4 +- app/src/main/res/navigation/nav_graph.xml | 10 --- 7 files changed, 58 insertions(+), 88 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index c095b28b8..7aaaa6eb3 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -554,7 +554,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { BrowserDirection.FromGlobal -> NavGraphDirections.actionGlobalBrowser(customTabSessionId) BrowserDirection.FromHome -> - HomeFragmentDirections.actionHomeFragmentToBrowserFragment(customTabSessionId, true) + HomeFragmentDirections.actionGlobalBrowser(customTabSessionId) BrowserDirection.FromSearch -> SearchFragmentDirections.actionGlobalBrowser(customTabSessionId) BrowserDirection.FromSearchDialog -> diff --git a/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt b/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt index 630b7592b..99da89b5a 100644 --- a/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt @@ -198,8 +198,7 @@ class InstalledAddonDetailsFragment : Fragment() { components.useCases.tabsUseCases.addTab(settingUrl) } - InstalledAddonDetailsFragmentDirections - .actionGlobalBrowser(null, false) + InstalledAddonDetailsFragmentDirections.actionGlobalBrowser(null) } else { InstalledAddonDetailsFragmentDirections .actionInstalledAddonFragmentToAddonInternalSettingsFragment(addon) 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 266bc7184..c7c350808 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -204,7 +204,6 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session engineView = WeakReference(engineView), swipeRefresh = WeakReference(swipeRefresh), viewLifecycleScope = WeakReference(viewLifecycleOwner.lifecycleScope), - arguments = requireArguments(), firstContentfulHappened = ::didFirstContentfulHappen ).apply { beginAnimateInIfNecessary() @@ -586,8 +585,15 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session } .ifChanged { it.content.firstContentfulPaint } .collect { - engineView?.asView()?.isVisible = + val showEngineView = it.content.firstContentfulPaint || it.content.progress == 100 + + if (showEngineView) { + engineView?.asView()?.isVisible = true + swipeRefresh.alpha = 1f + } else { + engineView?.asView()?.isVisible = false + } } } } diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt index c79c53a54..9a32bf1b6 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt @@ -4,19 +4,14 @@ package org.mozilla.fenix.browser -import android.animation.ValueAnimator import android.content.Context import android.graphics.Color import android.graphics.drawable.ColorDrawable -import android.os.Bundle import android.view.View -import android.view.animation.DecelerateInterpolator -import androidx.core.animation.doOnEnd import androidx.core.graphics.drawable.toDrawable import androidx.fragment.app.Fragment import androidx.lifecycle.LifecycleCoroutineScope import androidx.navigation.NavOptions -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay import kotlinx.coroutines.launch import mozilla.components.concept.engine.EngineView @@ -34,7 +29,6 @@ class BrowserAnimator( private val engineView: WeakReference, private val swipeRefresh: WeakReference, private val viewLifecycleScope: WeakReference, - private val arguments: Bundle, private val firstContentfulHappened: () -> Boolean ) { @@ -44,46 +38,19 @@ class BrowserAnimator( private val unwrappedSwipeRefresh: View? get() = swipeRefresh.get() - private val browserZoomInValueAnimator = ValueAnimator.ofFloat(0f, END_ANIMATOR_VALUE).apply { - addUpdateListener { - unwrappedSwipeRefresh?.scaleX = - STARTING_XY_SCALE + XY_SCALE_MULTIPLIER * it.animatedFraction - unwrappedSwipeRefresh?.scaleY = - STARTING_XY_SCALE + XY_SCALE_MULTIPLIER * it.animatedFraction - unwrappedSwipeRefresh?.alpha = it.animatedFraction - } - - doOnEnd { - if (firstContentfulHappened()) { - unwrappedEngineView?.asView()?.visibility = View.VISIBLE - } - unwrappedSwipeRefresh?.background = null - arguments.putBoolean(SHOULD_ANIMATE_FLAG, false) - } - - interpolator = DecelerateInterpolator() - duration = ANIMATION_DURATION - } - - /** - * Triggers the *zoom in* browser animation to run if necessary (based on the SHOULD_ANIMATE_FLAG). - * Also removes the flag from the bundle so it is not played on future entries into the fragment. - */ fun beginAnimateInIfNecessary() { - val shouldAnimate = arguments.getBoolean(SHOULD_ANIMATE_FLAG, false) - if (shouldAnimate) { - viewLifecycleScope.get()?.launch(Dispatchers.Main) { - delay(ANIMATION_DELAY) - captureEngineViewAndDrawStatically { - unwrappedSwipeRefresh?.alpha = 0f - browserZoomInValueAnimator.start() + if (unwrappedSwipeRefresh?.context?.settings()?.waitToShowPageUntilFirstPaint == true) { + if (firstContentfulHappened()) { + viewLifecycleScope.get()?.launch { + delay(100) + unwrappedEngineView?.asView()?.visibility = View.VISIBLE + unwrappedSwipeRefresh?.background = null + unwrappedSwipeRefresh?.alpha = 1f } } } else { unwrappedSwipeRefresh?.alpha = 1f - if (firstContentfulHappened()) { - unwrappedEngineView?.asView()?.visibility = View.VISIBLE - } + unwrappedEngineView?.asView()?.visibility = View.VISIBLE unwrappedSwipeRefresh?.background = null } } @@ -124,13 +91,6 @@ class BrowserAnimator( } companion object { - private const val SHOULD_ANIMATE_FLAG = "shouldAnimate" - private const val ANIMATION_DELAY = 50L - private const val ANIMATION_DURATION = 115L - private const val END_ANIMATOR_VALUE = 500f - private const val XY_SCALE_MULTIPLIER = .05f - private const val STARTING_XY_SCALE = .95f - fun getToolbarNavOptions(context: Context): NavOptions { val navOptions = NavOptions.Builder() 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 11d5faca1..953473302 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 @@ -94,18 +94,25 @@ class DefaultBrowserToolbarController( internal var ioScope: CoroutineScope = CoroutineScope(Dispatchers.IO) override fun handleToolbarPaste(text: String) { - val directions = if (useNewSearchExperience) { - BrowserFragmentDirections.actionGlobalSearchDialog( - sessionId = currentSession?.id, - pastedText = text + if (useNewSearchExperience) { + navController.nav( + R.id.browserFragment, BrowserFragmentDirections.actionGlobalSearchDialog( + sessionId = currentSession?.id, + pastedText = text + ), getToolbarNavOptions(activity) ) } else { - BrowserFragmentDirections.actionBrowserFragmentToSearchFragment( - sessionId = currentSession?.id, - pastedText = text - ) + browserAnimator.captureEngineViewAndDrawStatically { + navController.nav( + R.id.browserFragment, + BrowserFragmentDirections.actionBrowserFragmentToSearchFragment( + sessionId = currentSession?.id, + pastedText = text + ), + getToolbarNavOptions(activity) + ) + } } - navController.nav(R.id.browserFragment, directions, getToolbarNavOptions(activity)) } override fun handleToolbarPasteAndGo(text: String) { @@ -127,17 +134,23 @@ class DefaultBrowserToolbarController( Event.SearchBarTapped(Event.SearchBarTapped.Source.BROWSER) ) - val directions = if (useNewSearchExperience) { - BrowserFragmentDirections.actionGlobalSearchDialog( - currentSession?.id - ) - } else { - BrowserFragmentDirections.actionBrowserFragmentToSearchFragment( + if (useNewSearchExperience) { + navController.nav( + R.id.browserFragment, BrowserFragmentDirections.actionGlobalSearchDialog( currentSession?.id + ), getToolbarNavOptions(activity) + ) + } else { + browserAnimator.captureEngineViewAndDrawStatically { + navController.nav( + R.id.browserFragment, + BrowserFragmentDirections.actionBrowserFragmentToSearchFragment( + currentSession?.id + ), + getToolbarNavOptions(activity) ) } - - navController.nav(R.id.browserFragment, directions, getToolbarNavOptions(activity)) + } } override fun handleTabCounterClick() { @@ -162,7 +175,11 @@ class DefaultBrowserToolbarController( if (sessionManager.sessionsOfType(it.private).count() == 1) { // The tab tray always returns to normal mode so do that here too activity.browsingModeManager.mode = BrowsingMode.Normal - navController.navigate(BrowserFragmentDirections.actionGlobalHome(sessionToDelete = it.id)) + navController.navigate( + BrowserFragmentDirections.actionGlobalHome( + sessionToDelete = it.id + ) + ) } else { onCloseTab.invoke(it) activity.components.useCases.tabsUseCases.removeTab.invoke(it) @@ -211,7 +228,7 @@ class DefaultBrowserToolbarController( val directions = BrowserFragmentDirections.actionBrowserFragmentToSettingsFragment() navController.nav(R.id.browserFragment, directions) } - ToolbarMenu.Item.SyncedTabs -> { + ToolbarMenu.Item.SyncedTabs -> browserAnimator.captureEngineViewAndDrawStatically { navController.nav( R.id.browserFragment, BrowserFragmentDirections.actionBrowserFragmentToSyncedTabsFragment() @@ -272,7 +289,7 @@ class DefaultBrowserToolbarController( activity.components.analytics.metrics.track(Event.FindInPageOpened) } - ToolbarMenu.Item.AddonsManager -> { + ToolbarMenu.Item.AddonsManager -> browserAnimator.captureEngineViewAndDrawStatically { navController.nav( R.id.browserFragment, BrowserFragmentDirections.actionGlobalAddonsManagementFragment() @@ -348,13 +365,13 @@ class DefaultBrowserToolbarController( bookmarkTapped(it) } } - ToolbarMenu.Item.Bookmarks -> { + ToolbarMenu.Item.Bookmarks -> browserAnimator.captureEngineViewAndDrawStatically { navController.nav( R.id.browserFragment, BrowserFragmentDirections.actionGlobalBookmarkFragment(BookmarkRoot.Mobile.id) ) } - ToolbarMenu.Item.History -> { + ToolbarMenu.Item.History -> browserAnimator.captureEngineViewAndDrawStatically { navController.nav( R.id.browserFragment, BrowserFragmentDirections.actionGlobalHistoryFragment() diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index 643e6da94..9f659f5ca 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -478,9 +478,7 @@ class HomeFragment : Fragment() { engineSessionState = state ) findNavController().navigate( - HomeFragmentDirections.actionHomeFragmentToBrowserFragment( - null - ) + HomeFragmentDirections.actionGlobalBrowser(null) ) }, operation = { }, diff --git a/app/src/main/res/navigation/nav_graph.xml b/app/src/main/res/navigation/nav_graph.xml index 43cea13ed..c8bdb42d3 100644 --- a/app/src/main/res/navigation/nav_graph.xml +++ b/app/src/main/res/navigation/nav_graph.xml @@ -124,12 +124,6 @@ android:id="@+id/homeFragment" android:name="org.mozilla.fenix.home.HomeFragment" tools:layout="@layout/fragment_home"> - - -