From 76bb2c81b667127675c0b2e75e8237601cfcd3ef Mon Sep 17 00:00:00 2001 From: Mugurell Date: Wed, 18 Jan 2023 13:05:39 +0200 Subject: [PATCH] [fenix] Bug 1812518 - Show the download dialog as an Android View Tried to mimic the UX of a modal dialog while using Android Views. This meant including a scrim that would consume all touches and theming the navigation bar and status bar. Avoiding a dialog and a separate window will allow the snackbar to see the new "dialog" as a sibling in a CoordinatorLayout parent and so be able to position itself based on the new "dialog". This patch also added "start_download_dialog_layout" from A-C as it leads to simpler and less code needed to style the layout - colors / shapes with everything happening in XML versus calculating the values then setting them programatically. --- .../mozilla/fenix/browser/BaseBrowserFragment.kt | 23 ++ .../fenix/components/FenixSnackbarBehavior.kt | 9 +- .../mozilla/fenix/downloads/StartDownloadDialog.kt | 212 ++++++++++++++++++ .../download_dialog_download_button_background.xml | 9 + .../fenix/app/src/main/res/layout/dialog_scrim.xml | 15 ++ .../app/src/main/res/layout/fragment_browser.xml | 11 +- .../res/layout/start_download_dialog_layout.xml | 96 ++++++++ .../fenix/app/src/main/res/values/colors.xml | 3 + .../fenix/app/src/main/res/values/dimens.xml | 2 + .../fenix/components/FenixSnackbarBehaviorTest.kt | 64 ++++++ .../downloads/FirstPartyDownloadDialogTest.kt | 115 ++++++++++ .../fenix/downloads/StartDownloadDialogTest.kt | 242 +++++++++++++++++++++ 12 files changed, 796 insertions(+), 5 deletions(-) create mode 100644 mobile/android/fenix/app/src/main/java/org/mozilla/fenix/downloads/StartDownloadDialog.kt create mode 100644 mobile/android/fenix/app/src/main/res/drawable/download_dialog_download_button_background.xml create mode 100644 mobile/android/fenix/app/src/main/res/layout/dialog_scrim.xml create mode 100644 mobile/android/fenix/app/src/main/res/layout/start_download_dialog_layout.xml create mode 100644 mobile/android/fenix/app/src/test/java/org/mozilla/fenix/downloads/FirstPartyDownloadDialogTest.kt create mode 100644 mobile/android/fenix/app/src/test/java/org/mozilla/fenix/downloads/StartDownloadDialogTest.kt diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 3937c5b5b93c..69551e846ed5 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -122,6 +122,8 @@ import org.mozilla.fenix.crashes.CrashContentIntegration import org.mozilla.fenix.databinding.FragmentBrowserBinding import org.mozilla.fenix.downloads.DownloadService import org.mozilla.fenix.downloads.DynamicDownloadDialog +import org.mozilla.fenix.downloads.FirstPartyDownloadDialog +import org.mozilla.fenix.downloads.StartDownloadDialog import org.mozilla.fenix.ext.accessibilityManager import org.mozilla.fenix.ext.breadcrumb import org.mozilla.fenix.ext.components @@ -216,6 +218,8 @@ abstract class BaseBrowserFragment : @VisibleForTesting internal val onboarding by lazy { FenixOnboarding(requireContext()) } + private var currentStartDownloadDialog: StartDownloadDialog? = null + @CallSuper override fun onCreateView( inflater: LayoutInflater, @@ -510,6 +514,19 @@ abstract class BaseBrowserFragment : onNeedToRequestPermissions = { permissions -> requestPermissions(permissions, REQUEST_CODE_DOWNLOAD_PERMISSIONS) }, + customFirstPartyDownloadDialog = { filename, contentSize, positiveAction, negativeAction -> + FirstPartyDownloadDialog( + activity = requireActivity(), + filename = filename.value, + contentSize = contentSize.value, + positiveButtonAction = positiveAction.value, + negativeButtonAction = negativeAction.value, + ).onDismiss { + currentStartDownloadDialog = null + }.show(binding.startDownloadDialogContainer).also { + currentStartDownloadDialog = it + } + }, ) downloadFeature.onDownloadStopped = { downloadState, _, downloadJobStatus -> @@ -1062,6 +1079,7 @@ abstract class BaseBrowserFragment : it.selectedTab } .collect { + currentStartDownloadDialog?.dismiss() handleTabSelected(it) } } @@ -1130,6 +1148,7 @@ abstract class BaseBrowserFragment : override fun onStop() { super.onStop() initUIJob?.cancel() + currentStartDownloadDialog?.dismiss() requireComponents.core.store.state.findTabOrCustomTabOrSelectedTab(customTabSessionId) ?.let { session -> @@ -1145,6 +1164,10 @@ abstract class BaseBrowserFragment : return findInPageIntegration.onBackPressed() || fullScreenFeature.onBackPressed() || promptsFeature.onBackPressed() || + currentStartDownloadDialog?.let { + it.dismiss() + true + } ?: false || sessionFeature.onBackPressed() || removeSessionIfNeeded() } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/FenixSnackbarBehavior.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/FenixSnackbarBehavior.kt index 7ee4dd54f727..d1f3f767d0f9 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/FenixSnackbarBehavior.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/FenixSnackbarBehavior.kt @@ -28,11 +28,12 @@ class FenixSnackbarBehavior( ) : CoordinatorLayout.Behavior(context, null) { private val dependenciesIds = listOf( + R.id.startDownloadDialogContainer, R.id.viewDynamicDownloadDialog, R.id.toolbar, ) - private var currentAnchorId = View.NO_ID + private var currentAnchorId: Int? = View.NO_ID override fun layoutDependsOn( parent: CoordinatorLayout, @@ -55,9 +56,9 @@ class FenixSnackbarBehavior( } } - private fun positionSnackbar(child: View, dependency: View?) { + private fun positionSnackbar(snackbar: View, dependency: View?) { currentAnchorId = dependency?.id ?: View.NO_ID - val params = child.layoutParams as CoordinatorLayout.LayoutParams + val params = snackbar.layoutParams as CoordinatorLayout.LayoutParams if (dependency == null || (dependency.id == R.id.toolbar && toolbarPosition == ToolbarPosition.TOP)) { // Position the snackbar at the bottom of the screen. @@ -71,6 +72,6 @@ class FenixSnackbarBehavior( params.gravity = Gravity.TOP or Gravity.CENTER_HORIZONTAL } - child.layoutParams = params + snackbar.layoutParams = params } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/downloads/StartDownloadDialog.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/downloads/StartDownloadDialog.kt new file mode 100644 index 000000000000..c821a75c7eaf --- /dev/null +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/downloads/StartDownloadDialog.kt @@ -0,0 +1,212 @@ +/* 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.app.Activity +import android.app.Dialog +import android.view.Gravity +import android.view.LayoutInflater +import android.view.View +import android.view.ViewGroup +import android.view.ViewTreeObserver.OnGlobalLayoutListener +import android.view.Window +import android.view.accessibility.AccessibilityEvent +import android.view.accessibility.AccessibilityNodeInfo +import androidx.annotation.VisibleForTesting +import androidx.coordinatorlayout.widget.CoordinatorLayout +import androidx.core.content.ContextCompat +import androidx.core.view.ViewCompat +import androidx.core.view.children +import androidx.viewbinding.ViewBinding +import mozilla.components.feature.downloads.toMegabyteOrKilobyteString +import mozilla.components.support.ktx.android.view.setNavigationBarTheme +import mozilla.components.support.ktx.android.view.setStatusBarTheme +import org.mozilla.fenix.R +import org.mozilla.fenix.databinding.DialogScrimBinding +import org.mozilla.fenix.databinding.StartDownloadDialogLayoutBinding +import org.mozilla.fenix.ext.settings + +/** + * Parent of all download views that can mimic a modal [Dialog]. + * + * @param activity The [Activity] in which the dialog will be shown. + * Used to update the activity [Window] to best mimic a modal dialog. + */ +abstract class StartDownloadDialog( + private val activity: Activity, +) { + @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED) + internal var binding: ViewBinding? = null + + @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED) + internal var container: ViewGroup? = null + private var scrim: DialogScrimBinding? = null + + @VisibleForTesting + internal var onDismiss: () -> Unit = {} + + @VisibleForTesting + internal var initialNavigationBarColor = activity.window.navigationBarColor + + @VisibleForTesting + internal var initialStatusBarColor = activity.window.statusBarColor + + /** + * Show the download view. + * + * @param container The [ViewGroup] in which the download view will be inflated. + */ + fun show(container: ViewGroup): StartDownloadDialog { + this.container = container + + val dialogParent = container.parent as? ViewGroup + dialogParent?.let { + scrim = DialogScrimBinding.inflate(LayoutInflater.from(activity), dialogParent, true).apply { + this.scrim.setOnClickListener { + // Empty listener needed to prevent clicking through. + } + } + } + + setupView() + + if (activity.settings().accessibilityServicesEnabled) { + disableSiblingsAccessibility(dialogParent) + } + + container.apply { + val params = layoutParams as CoordinatorLayout.LayoutParams + params.gravity = Gravity.BOTTOM or Gravity.CENTER_HORIZONTAL + layoutParams = params + + // Set a higher elevation than the toolbar sibling which we should cover. + elevation = activity.resources.getDimension(R.dimen.browser_fragment_download_dialog_elevation) + visibility = View.VISIBLE + } + + activity.window.setNavigationBarTheme(ContextCompat.getColor(activity, R.color.material_scrim_color)) + activity.window.setStatusBarTheme(ContextCompat.getColor(activity, R.color.material_scrim_color)) + + return this + } + + /** + * Set a callback for when the download view is dismissed. + * + * @param callback The callback for when the view is dismissed. + */ + fun onDismiss(callback: () -> Unit): StartDownloadDialog { + this.onDismiss = callback + return this + } + + /** + * Immediately dismiss the current download view if it is shown. + * This will restore the previous UI removing any other layout / window customizations. + */ + fun dismiss() { + scrim?.let { + (it.root.parent as? ViewGroup)?.removeView(it.root) + } + binding?.let { + (it.root.parent as? ViewGroup)?.removeView(it.root) + } + enableSiblingsAccessibility(container?.parent as? ViewGroup) + + container?.visibility = View.GONE + + activity.window.setNavigationBarTheme(initialNavigationBarColor) + activity.window.setStatusBarTheme(initialStatusBarColor) + + onDismiss() + } + + @VisibleForTesting + internal fun enableSiblingsAccessibility(parent: ViewGroup?) { + parent?.children + ?.filterNot { it.id == R.id.startDownloadDialogContainer } + ?.forEach { + ViewCompat.setImportantForAccessibility( + it, + ViewCompat.IMPORTANT_FOR_ACCESSIBILITY_YES, + ) + } + } + + @VisibleForTesting + internal fun disableSiblingsAccessibility(parent: ViewGroup?) { + parent?.children + ?.filterNot { it.id == R.id.startDownloadDialogContainer } + ?.forEach { + ViewCompat.setImportantForAccessibility( + it, + ViewCompat.IMPORTANT_FOR_ACCESSIBILITY_NO_HIDE_DESCENDANTS, + ) + } + } + + /** + * Bind all download data to the download view. + */ + @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED) + internal abstract fun setupView() +} + +/** + * A download view mimicking a modal dialog that allows the user to download a file with the current application. + * + * @param activity The [Activity] in which the dialog will be shown. + * Used to update the activity [Window] to best mimic a modal dialog. + * @param filename Name of the file to be downloaded. It wil be shown without any modification. + * @param contentSize Size of the file to be downloaded expressed as a number of bytes. + * It will automatically be parsed to the appropriate kilobyte or megabyte value before being shown. + * @param positiveButtonAction Callback for when the user interacts with the dialog to start the download. + * @param negativeButtonAction Callback for when the user interacts with the dialog to dismiss it. + */ +class FirstPartyDownloadDialog( + private val activity: Activity, + private val filename: String, + private val contentSize: Long, + private val positiveButtonAction: () -> Unit, + private val negativeButtonAction: () -> Unit, +) : StartDownloadDialog(activity) { + override fun setupView() { + val dialog = StartDownloadDialogLayoutBinding.inflate(LayoutInflater.from(activity), container, true) + .also { binding = it } + + if (contentSize > 0L) { + val contentSize = contentSize.toMegabyteOrKilobyteString() + dialog.title.text = + activity.getString(R.string.mozac_feature_downloads_dialog_title2, contentSize) + } + + dialog.filename.text = filename + + dialog.downloadButton.setOnClickListener { + positiveButtonAction() + dismiss() + } + + dialog.closeButton.setOnClickListener { + negativeButtonAction() + dismiss() + } + + if (activity.settings().accessibilityServicesEnabled) { + // Ensure the title of the dialog is focused and read by talkback first. + dialog.root.viewTreeObserver.addOnGlobalLayoutListener( + object : OnGlobalLayoutListener { + override fun onGlobalLayout() { + dialog.root.viewTreeObserver.removeOnGlobalLayoutListener(this) + dialog.title.run { + sendAccessibilityEvent(AccessibilityEvent.TYPE_VIEW_SELECTED) + performAccessibilityAction(AccessibilityNodeInfo.ACTION_ACCESSIBILITY_FOCUS, null) + } + } + }, + ) + } + } +} diff --git a/mobile/android/fenix/app/src/main/res/drawable/download_dialog_download_button_background.xml b/mobile/android/fenix/app/src/main/res/drawable/download_dialog_download_button_background.xml new file mode 100644 index 000000000000..55f324e92dc3 --- /dev/null +++ b/mobile/android/fenix/app/src/main/res/drawable/download_dialog_download_button_background.xml @@ -0,0 +1,9 @@ + + + + + + diff --git a/mobile/android/fenix/app/src/main/res/layout/dialog_scrim.xml b/mobile/android/fenix/app/src/main/res/layout/dialog_scrim.xml new file mode 100644 index 000000000000..9474438874af --- /dev/null +++ b/mobile/android/fenix/app/src/main/res/layout/dialog_scrim.xml @@ -0,0 +1,15 @@ + + + + diff --git a/mobile/android/fenix/app/src/main/res/layout/fragment_browser.xml b/mobile/android/fenix/app/src/main/res/layout/fragment_browser.xml index a8b3b29a00cb..a37f8d5a9fe7 100644 --- a/mobile/android/fenix/app/src/main/res/layout/fragment_browser.xml +++ b/mobile/android/fenix/app/src/main/res/layout/fragment_browser.xml @@ -67,9 +67,18 @@ android:visibility="gone" /> + + + android:layout_height="wrap_content" + android:elevation="@dimen/browser_fragment_toolbar_elevation"/> diff --git a/mobile/android/fenix/app/src/main/res/layout/start_download_dialog_layout.xml b/mobile/android/fenix/app/src/main/res/layout/start_download_dialog_layout.xml new file mode 100644 index 000000000000..999d3d6bce69 --- /dev/null +++ b/mobile/android/fenix/app/src/main/res/layout/start_download_dialog_layout.xml @@ -0,0 +1,96 @@ + + + + + + + + + + + + + +