From 7feae9894409291d99f4090f2e4455779b2daa10 Mon Sep 17 00:00:00 2001 From: "codrut.topliceanu" Date: Thu, 7 May 2020 16:57:18 +0300 Subject: [PATCH] For #9044 - Replace DownloadNotificationBottomSheetDialog with unobtrusive view - Renamed DownloadNotification and removed DownloadState.dismissed dependency - Improved DynamicDownloadDialog behaviour when scrolling - DynamicDownloadDialog remains attached to tab until dismissed - Fixed onTryAgain not working for resumed DownloadDialogs --- .../mozilla/fenix/ui/robots/DownloadRobot.kt | 4 +- .../fenix/browser/BaseBrowserFragment.kt | 95 +++++++++- .../DownloadNotificationBottomSheetDialog.kt | 113 ----------- .../fenix/downloads/DynamicDownloadDialog.kt | 140 ++++++++++++++ .../DynamicDownloadDialogBehavior.kt | 161 ++++++++++++++++ .../org/mozilla/fenix/home/SharedViewModel.kt | 7 + ..._layout.xml => download_dialog_layout.xml} | 40 ++-- app/src/main/res/layout/fragment_browser.xml | 8 + .../DynamicDownloadDialogBehaviorTest.kt | 178 ++++++++++++++++++ 9 files changed, 601 insertions(+), 145 deletions(-) delete mode 100644 app/src/main/java/org/mozilla/fenix/downloads/DownloadNotificationBottomSheetDialog.kt create mode 100644 app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialog.kt create mode 100644 app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialogBehavior.kt rename app/src/main/res/layout/{download_notification_layout.xml => download_dialog_layout.xml} (70%) create mode 100644 app/src/test/java/org/mozilla/fenix/downloads/DynamicDownloadDialogBehaviorTest.kt diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/DownloadRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/DownloadRobot.kt index 5d8d2a2bf..3a111bc93 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/DownloadRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/DownloadRobot.kt @@ -112,7 +112,7 @@ private fun assertDownloadNotificationShade() { private fun assertDownloadNotificationPopup() { val mDevice = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()) mDevice.waitNotNull(Until.findObjects(By.text("Open")), TestAssetHelper.waitingTime) - onView(withId(R.id.download_notification_title)) + onView(withId(R.id.download_dialog_title)) .check(matches(withText(CoreMatchers.containsString("Download completed")))) } @@ -123,7 +123,7 @@ private fun clickDownloadButton() = onView(withText("Download")).inRoot(isDialog()).check(matches(isDisplayed())) private fun clickOpenButton() = - onView(withId(R.id.download_notification_action_button)).inRoot(isDialog()).check( + onView(withId(R.id.download_dialog_action_button)).inRoot(isDialog()).check( matches(isDisplayed()) ) 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 7a92205f8..359fd514a 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -32,6 +32,9 @@ import mozilla.appservices.places.BookmarkRoot import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager import mozilla.components.browser.session.runWithSessionIdOrSelected +import mozilla.components.browser.state.action.ContentAction +import mozilla.components.browser.state.state.content.DownloadState +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.feature.accounts.FxaCapability import mozilla.components.feature.accounts.FxaWebChannelFeature @@ -78,7 +81,7 @@ 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.DynamicDownloadDialog import org.mozilla.fenix.downloads.DownloadService import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.enterToImmersiveMode @@ -286,15 +289,18 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session } ) - downloadFeature.onDownloadStopped = { download, _, downloadJobStatus -> + downloadFeature.onDownloadStopped = { downloadState, _, downloadJobStatus -> // If the download is just paused, don't show any in-app notification if (downloadJobStatus == AbstractFetchDownloadService.DownloadJobStatus.COMPLETED || downloadJobStatus == AbstractFetchDownloadService.DownloadJobStatus.FAILED ) { - val dialog = DownloadNotificationBottomSheetDialog( - context = context, + + saveDownloadDialogState(session, downloadState, downloadJobStatus) + + DynamicDownloadDialog( + container = view.browserLayout, + downloadState = downloadState, didFail = downloadJobStatus == AbstractFetchDownloadService.DownloadJobStatus.FAILED, - download = download, tryAgain = downloadFeature::tryAgain, onCannotOpenFile = { FenixSnackbar.make( @@ -304,9 +310,12 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session ) .setText(context.getString(R.string.mozac_feature_downloads_could_not_open_file)) .show() - } - ) - dialog.show() + }, + view = view.viewDynamicDownloadDialog, + toolbarHeight = toolbarHeight, + onDismiss = { sharedViewModel.downloadDialogState.remove(session.id) } + ).show() + browserToolbarView.expand() } } @@ -316,6 +325,8 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session view = view ) + resumeDownloadDialogState(session, store, view, context, toolbarHeight) + pipFeature = PictureInPictureFeature( requireComponents.core.sessionManager, requireActivity(), @@ -503,6 +514,74 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session } } + /** + * Preserves current state of the [DynamicDownloadDialog] to persist through tab changes and + * other fragments navigation. + * */ + private fun saveDownloadDialogState( + session: Session, + downloadState: DownloadState, + downloadJobStatus: AbstractFetchDownloadService.DownloadJobStatus + ) { + sharedViewModel.downloadDialogState[session.id] = Pair( + downloadState, + downloadJobStatus == AbstractFetchDownloadService.DownloadJobStatus.FAILED + ) + } + + /** + * Re-initializes [DynamicDownloadDialog] if the user hasn't dismissed the dialog + * before navigating away from it's original tab. + * onTryAgain it will use [ContentAction.UpdateDownloadAction] to re-enqueue the former failed + * download, because [DownloadsFeature] clears any queued downloads onStop. + * */ + private fun resumeDownloadDialogState( + session: Session, + store: BrowserStore, + view: View, + context: Context, + toolbarHeight: Int + ) { + val savedDownloadState = + sharedViewModel.downloadDialogState[session.id] ?: return + + val onTryAgain: (Long) -> Unit = { + savedDownloadState.first?.let { dlState -> + store.dispatch( + ContentAction.UpdateDownloadAction( + session.id, dlState.copy(skipConfirmation = true) + ) + ) + } + } + + val onCannotOpenFile = { + FenixSnackbar.make( + view = view, + duration = Snackbar.LENGTH_SHORT, + isDisplayedWithBrowserToolbar = true + ) + .setText(context.getString(R.string.mozac_feature_downloads_could_not_open_file)) + .show() + } + + val onDismiss: () -> Unit = + { sharedViewModel.downloadDialogState.remove(session.id) } + + DynamicDownloadDialog( + container = view.browserLayout, + downloadState = savedDownloadState.first, + didFail = savedDownloadState.second, + tryAgain = onTryAgain, + onCannotOpenFile = onCannotOpenFile, + view = view.viewDynamicDownloadDialog, + toolbarHeight = toolbarHeight, + onDismiss = onDismiss + ).show() + + browserToolbarView.expand() + } + private fun initializeEngineView(toolbarHeight: Int) { if (FeatureFlags.dynamicBottomToolbar) { engineView.setDynamicToolbarMaxHeight(toolbarHeight) diff --git a/app/src/main/java/org/mozilla/fenix/downloads/DownloadNotificationBottomSheetDialog.kt b/app/src/main/java/org/mozilla/fenix/downloads/DownloadNotificationBottomSheetDialog.kt deleted file mode 100644 index bacf0c92c..000000000 --- a/app/src/main/java/org/mozilla/fenix/downloads/DownloadNotificationBottomSheetDialog.kt +++ /dev/null @@ -1,113 +0,0 @@ -/* 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.downloads - -import android.content.Context -import android.graphics.Color -import android.graphics.drawable.ColorDrawable -import android.os.Bundle -import android.view.View -import android.view.ViewGroup -import android.widget.FrameLayout -import androidx.core.content.ContextCompat -import com.google.android.material.bottomsheet.BottomSheetBehavior -import com.google.android.material.bottomsheet.BottomSheetDialog -import kotlinx.android.synthetic.main.download_notification_layout.* -import mozilla.components.browser.state.state.content.DownloadState -import mozilla.components.feature.downloads.AbstractFetchDownloadService -import mozilla.components.feature.downloads.toMegabyteString -import org.mozilla.fenix.R -import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.ext.metrics -import org.mozilla.fenix.theme.ThemeManager - -class DownloadNotificationBottomSheetDialog( - context: Context, - private val download: DownloadState, - private val didFail: Boolean, - private val tryAgain: (Long) -> Unit, - private val onCannotOpenFile: () -> Unit - // We must pass in the BottomSheetDialog theme for the transparent window background to apply properly -) : BottomSheetDialog(context, R.style.Theme_MaterialComponents_BottomSheetDialog) { - override fun onCreate(savedInstanceState: Bundle?) { - setContentView(R.layout.download_notification_layout) - - if (didFail) { - download_notification_title.text = - context.getString(R.string.mozac_feature_downloads_failed_notification_text2) - - download_notification_icon.setImageResource( - mozilla.components.feature.downloads.R.drawable.mozac_feature_download_ic_download_failed - ) - - download_notification_action_button.apply { - text = context.getString( - mozilla.components.feature.downloads.R.string.mozac_feature_downloads_button_try_again - ) - setOnClickListener { - tryAgain(download.id) - context.metrics.track(Event.InAppNotificationDownloadTryAgain) - dismiss() - } - } - } else { - val titleText = context.getString( - R.string.mozac_feature_downloads_completed_notification_text2 - ) + " (${download.contentLength?.toMegabyteString()})" - - download_notification_title.text = titleText - - download_notification_icon.setImageResource( - mozilla.components.feature.downloads.R.drawable.mozac_feature_download_ic_download_complete - ) - - download_notification_action_button.apply { - text = context.getString( - mozilla.components.feature.downloads.R.string.mozac_feature_downloads_button_open - ) - setOnClickListener { - val fileWasOpened = AbstractFetchDownloadService.openFile( - context = context, - contentType = download.contentType, - filePath = download.filePath - ) - - if (!fileWasOpened) { - onCannotOpenFile() - } - - context.metrics.track(Event.InAppNotificationDownloadOpen) - dismiss() - } - } - } - - download_notification_close_button.setOnClickListener { - dismiss() - } - - download_notification_filename.text = download.fileName - - setOnShowListener { - val bottomSheet = - findViewById(com.google.android.material.R.id.design_bottom_sheet) as FrameLayout - val behavior = BottomSheetBehavior.from(bottomSheet) - behavior.state = BottomSheetBehavior.STATE_EXPANDED - window?.apply { - setBackgroundDrawable(ColorDrawable(Color.TRANSPARENT)) - setLayout( - ViewGroup.LayoutParams.MATCH_PARENT, - ViewGroup.LayoutParams.MATCH_PARENT - ) - navigationBarColor = ContextCompat.getColor( - context, - ThemeManager.resolveAttribute( - R.attr.foundation, context - ) - ) - } - } - } -} diff --git a/app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialog.kt b/app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialog.kt new file mode 100644 index 000000000..4b8ba2ee2 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialog.kt @@ -0,0 +1,140 @@ +/* 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.downloads + +import android.view.View +import android.view.ViewGroup +import androidx.coordinatorlayout.widget.CoordinatorLayout +import kotlinx.android.extensions.LayoutContainer +import kotlinx.android.synthetic.main.download_dialog_layout.view.* +import mozilla.components.browser.state.state.content.DownloadState +import mozilla.components.feature.downloads.AbstractFetchDownloadService +import mozilla.components.feature.downloads.toMegabyteString +import org.mozilla.fenix.FeatureFlags +import org.mozilla.fenix.R +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.ext.metrics +import org.mozilla.fenix.ext.settings + +/** + * [DynamicDownloadDialog] is used to show a view in the current tab to the user, triggered when + * downloadFeature.onDownloadStopped gets invoked. It uses [DynamicDownloadDialogBehavior] to + * hide when the users scrolls through a website as to not impede his activities. + * */ + +class DynamicDownloadDialog( + private val container: ViewGroup, + private val downloadState: DownloadState?, + private val didFail: Boolean, + private val tryAgain: (Long) -> Unit, + private val onCannotOpenFile: () -> Unit, + private val view: View, + private val toolbarHeight: Int, + private val onDismiss: () -> Unit +) : LayoutContainer { + + override val containerView: View? + get() = container + + private val settings = container.context.settings() + + init { + setupDownloadDialog(view) + } + + private fun setupDownloadDialog(view: View) { + if (downloadState == null) return + view.apply { + if (FeatureFlags.dynamicBottomToolbar && layoutParams is CoordinatorLayout.LayoutParams) { + (layoutParams as CoordinatorLayout.LayoutParams).apply { + + behavior = + DynamicDownloadDialogBehavior( + context, + null, + toolbarHeight.toFloat() + ) + } + } + } + + if (settings.shouldUseBottomToolbar) { + val params: ViewGroup.MarginLayoutParams = + view.layoutParams as ViewGroup.MarginLayoutParams + params.bottomMargin = toolbarHeight + } + + if (didFail) { + view.download_dialog_title.text = + container.context.getString(R.string.mozac_feature_downloads_failed_notification_text2) + + view.download_dialog_icon.setImageResource( + mozilla.components.feature.downloads.R.drawable.mozac_feature_download_ic_download_failed + ) + + view.download_dialog_action_button.apply { + text = context.getString( + mozilla.components.feature.downloads.R.string.mozac_feature_downloads_button_try_again + ) + setOnClickListener { + tryAgain(downloadState.id) + context.metrics.track(Event.InAppNotificationDownloadTryAgain) + dismiss(view) + } + } + } else { + val titleText = container.context.getString( + R.string.mozac_feature_downloads_completed_notification_text2 + ) + " (${downloadState.contentLength?.toMegabyteString()})" + + view.download_dialog_title.text = titleText + + view.download_dialog_icon.setImageResource( + mozilla.components.feature.downloads.R.drawable.mozac_feature_download_ic_download_complete + ) + + view.download_dialog_action_button.apply { + text = context.getString( + mozilla.components.feature.downloads.R.string.mozac_feature_downloads_button_open + ) + setOnClickListener { + val fileWasOpened = AbstractFetchDownloadService.openFile( + context = context, + contentType = downloadState.contentType, + filePath = downloadState.filePath + ) + + if (!fileWasOpened) { + onCannotOpenFile() + } + + context.metrics.track(Event.InAppNotificationDownloadOpen) + dismiss(view) + } + } + } + + view.download_dialog_close_button.setOnClickListener { + dismiss(view) + } + + view.download_dialog_filename.text = downloadState.fileName + } + + fun show() { + view.visibility = View.VISIBLE + + if (FeatureFlags.dynamicBottomToolbar) { + (view.layoutParams as CoordinatorLayout.LayoutParams).apply { + (behavior as DynamicDownloadDialogBehavior).forceExpand(view) + } + } + } + + private fun dismiss(view: View) { + view.visibility = View.GONE + onDismiss() + } +} diff --git a/app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialogBehavior.kt b/app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialogBehavior.kt new file mode 100644 index 000000000..a5d640bae --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/downloads/DynamicDownloadDialogBehavior.kt @@ -0,0 +1,161 @@ +/* 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.downloads + +import android.animation.ValueAnimator +import android.content.Context +import android.util.AttributeSet +import android.view.View +import android.view.animation.DecelerateInterpolator +import androidx.annotation.VisibleForTesting +import androidx.coordinatorlayout.widget.CoordinatorLayout +import androidx.core.view.ViewCompat +import mozilla.components.concept.engine.EngineView +import mozilla.components.support.ktx.android.view.findViewInHierarchy +import kotlin.math.max +import kotlin.math.min + +/** + * A [CoordinatorLayout.Behavior] implementation to be used when placing [DynamicDownloadDialog] + * at the bottom of the screen. Based off of BrowserToolbarBottomBehavior. + * + * This implementation will: + * - Show/Hide the [DynamicDownloadDialog] automatically when scrolling vertically. + * - Snap the [DynamicDownloadDialog] to be hidden or visible when the user stops scrolling. + */ + +private const val SNAP_ANIMATION_DURATION = 150L + +class DynamicDownloadDialogBehavior( + context: Context?, + attrs: AttributeSet?, + private val bottomToolbarHeight: Float = 0f +) : CoordinatorLayout.Behavior(context, attrs) { + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal var expanded: Boolean = true + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal var shouldSnapAfterScroll: Boolean = false + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal var snapAnimator: ValueAnimator = ValueAnimator() + .apply { + interpolator = DecelerateInterpolator() + duration = SNAP_ANIMATION_DURATION + } + + /** + * Reference to [EngineView] used to check user's [android.view.MotionEvent]s. + */ + private var engineView: EngineView? = null + + /** + * Depending on how user's touch was consumed by EngineView / current website, + * + * we will animate the dynamic download notification dialog if: + * - touches were used for zooming / panning operations in the website. + * + * We will do nothing if: + * - the website is not scrollable + * - the website handles the touch events itself through it's own touch event listeners. + */ + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal val shouldScroll: Boolean + get() = engineView?.getInputResult() == EngineView.InputResult.INPUT_RESULT_HANDLED + + override fun onStartNestedScroll( + coordinatorLayout: CoordinatorLayout, + child: V, + directTargetChild: View, + target: View, + axes: Int, + type: Int + ): Boolean { + return if (shouldScroll && axes == ViewCompat.SCROLL_AXIS_VERTICAL) { + shouldSnapAfterScroll = type == ViewCompat.TYPE_TOUCH + snapAnimator.cancel() + true + } else if (engineView?.getInputResult() == EngineView.InputResult.INPUT_RESULT_UNHANDLED) { + // Force expand the notification dialog if event is unhandled, otherwise user could get stuck in a + // state where they cannot show it + forceExpand(child) + snapAnimator.cancel() + false + } else { + false + } + } + + override fun onStopNestedScroll( + coordinatorLayout: CoordinatorLayout, + child: V, + target: View, + type: Int + ) { + if (shouldSnapAfterScroll || type == ViewCompat.TYPE_NON_TOUCH) { + if (expanded) { + if (child.translationY >= bottomToolbarHeight / 2) + animateSnap(child, SnapDirection.DOWN) + else animateSnap(child, SnapDirection.UP) + } else { + if (child.translationY < (bottomToolbarHeight + child.height.toFloat() / 2)) + animateSnap(child, SnapDirection.UP) + else animateSnap(child, SnapDirection.DOWN) + } + } + } + + override fun onNestedPreScroll( + coordinatorLayout: CoordinatorLayout, + child: V, + target: View, + dx: Int, + dy: Int, + consumed: IntArray, + type: Int + ) { + if (shouldScroll) { + super.onNestedPreScroll(coordinatorLayout, child, target, dx, dy, consumed, type) + child.translationY = max( + 0f, + min( + child.height.toFloat() + bottomToolbarHeight, + child.translationY + dy + ) + ) + } + } + + override fun layoutDependsOn( + parent: CoordinatorLayout, + child: V, + dependency: View + ): Boolean { + engineView = parent.findViewInHierarchy { it is EngineView } as? EngineView + return super.layoutDependsOn(parent, child, dependency) + } + + fun forceExpand(view: View) { + animateSnap(view, SnapDirection.UP) + } + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal fun animateSnap(child: View, direction: SnapDirection) = with(snapAnimator) { + expanded = direction == SnapDirection.UP + addUpdateListener { child.translationY = it.animatedValue as Float } + setFloatValues( + child.translationY, + if (direction == SnapDirection.UP) 0f else child.height.toFloat() + bottomToolbarHeight + ) + start() + } + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal enum class SnapDirection { + UP, + DOWN + } +} diff --git a/app/src/main/java/org/mozilla/fenix/home/SharedViewModel.kt b/app/src/main/java/org/mozilla/fenix/home/SharedViewModel.kt index 9b3f9bf9f..d94c42565 100644 --- a/app/src/main/java/org/mozilla/fenix/home/SharedViewModel.kt +++ b/app/src/main/java/org/mozilla/fenix/home/SharedViewModel.kt @@ -5,6 +5,7 @@ package org.mozilla.fenix.home import androidx.lifecycle.ViewModel +import mozilla.components.browser.state.state.content.DownloadState class SharedViewModel : ViewModel() { @@ -12,4 +13,10 @@ class SharedViewModel : ViewModel() { * Used to remember if we need to scroll to the selected tab in the homeFragment's recycleView see #7356 * */ var shouldScrollToSelectedTab: Boolean = false + + /** + * Stores data needed for [DynamicDownloadDialog]. See #9044 + * Format: HashMap + * */ + var downloadDialogState: HashMap> = HashMap() } diff --git a/app/src/main/res/layout/download_notification_layout.xml b/app/src/main/res/layout/download_dialog_layout.xml similarity index 70% rename from app/src/main/res/layout/download_notification_layout.xml rename to app/src/main/res/layout/download_dialog_layout.xml index 2c22f8f5b..a24206c2e 100644 --- a/app/src/main/res/layout/download_notification_layout.xml +++ b/app/src/main/res/layout/download_dialog_layout.xml @@ -8,14 +8,14 @@ android:layout_width="match_parent" android:layout_height="wrap_content" android:background="?foundation" - android:paddingBottom="16dp"> + android:paddingBottom="4dp">