From 6d09a8757c0d8d1fc65e4dd78d7291945bf974ee Mon Sep 17 00:00:00 2001 From: Jonathan Almeida Date: Wed, 7 Apr 2021 21:51:21 -0400 Subject: [PATCH] Close #18845: Adds swipe-to-delete to tabs tray refactor Copied the TabsTouchHelper from the `tabtray` package here so we don't need to re-write our own because there's nothing more to add. We can hook this up with our tabs tray here by putting it in the `BaseBrowserTrayList` for our normal and private tabs list. --- .../fenix/tabstray/TrayPagerAdapter.kt | 3 +- .../tabstray/browser/BaseBrowserTrayList.kt | 75 ++++++---- .../tabstray/browser/SwipeToDeleteBinding.kt | 42 ++++++ .../fenix/tabstray/browser/TabsTouchHelper.kt | 139 ++++++++++++++++++ .../viewholders/BaseBrowserTabViewHolder.kt | 7 +- .../viewholders/NormalBrowserTabViewHolder.kt | 4 +- .../PrivateBrowserTabViewHolder.kt | 4 +- .../browser/SwipeToDeleteBindingTest.kt | 48 ++++++ .../tabstray/browser/TabsTouchHelperTest.kt | 60 ++++++++ 9 files changed, 347 insertions(+), 35 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/tabstray/browser/SwipeToDeleteBinding.kt create mode 100644 app/src/main/java/org/mozilla/fenix/tabstray/browser/TabsTouchHelper.kt create mode 100644 app/src/test/java/org/mozilla/fenix/tabstray/browser/SwipeToDeleteBindingTest.kt create mode 100644 app/src/test/java/org/mozilla/fenix/tabstray/browser/TabsTouchHelperTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt index f106475a7..668e9662c 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt @@ -34,12 +34,13 @@ class TrayPagerAdapter( return when (viewType) { NormalBrowserTabViewHolder.LAYOUT_ID -> NormalBrowserTabViewHolder( - store, itemView, + store, interactor ) PrivateBrowserTabViewHolder.LAYOUT_ID -> PrivateBrowserTabViewHolder( itemView, + store, interactor ) SyncedTabViewHolder.LAYOUT_ID -> SyncedTabViewHolder( diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BaseBrowserTrayList.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BaseBrowserTrayList.kt index fdc33ad4e..03f70a11e 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BaseBrowserTrayList.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BaseBrowserTrayList.kt @@ -8,12 +8,11 @@ import android.content.Context import android.util.AttributeSet import androidx.recyclerview.widget.RecyclerView import mozilla.components.feature.tabs.tabstray.TabsFeature -import mozilla.components.support.base.feature.ViewBoundFeatureWrapper import org.mozilla.fenix.ext.components import org.mozilla.fenix.tabstray.TabsTrayInteractor +import org.mozilla.fenix.tabstray.TabsTrayStore import org.mozilla.fenix.tabstray.TrayItem import org.mozilla.fenix.tabstray.ext.filterFromConfig -import org.mozilla.fenix.utils.view.LifecycleViewProvider abstract class BaseBrowserTrayList @JvmOverloads constructor( context: Context, @@ -33,44 +32,62 @@ abstract class BaseBrowserTrayList @JvmOverloads constructor( abstract val configuration: Configuration - var interactor: TabsTrayInteractor? = null + lateinit var interactor: TabsTrayInteractor + lateinit var tabsTrayStore: TabsTrayStore - private val lifecycleProvider = LifecycleViewProvider(this) + private val tabsFeature by lazy { + // NB: The use cases here are duplicated because there isn't a nicer + // way to share them without a better dependency injection solution. + val selectTabUseCase = SelectTabUseCaseWrapper( + context.components.analytics.metrics, + context.components.useCases.tabsUseCases.selectTab + ) { + interactor.navigateToBrowser() + } + + val removeTabUseCase = RemoveTabUseCaseWrapper( + context.components.analytics.metrics + ) { sessionId -> + interactor.tabRemoved(sessionId) + } - private val selectTabUseCase = SelectTabUseCaseWrapper( - context.components.analytics.metrics, - context.components.useCases.tabsUseCases.selectTab - ) { - interactor?.navigateToBrowser() + TabsFeature( + adapter as TabsAdapter, + context.components.core.store, + selectTabUseCase, + removeTabUseCase, + { it.filterFromConfig(configuration) }, + { } + ) } - private val removeTabUseCase = RemoveTabUseCaseWrapper( - context.components.analytics.metrics - ) { sessionId -> - interactor?.tabRemoved(sessionId) + private val swipeToDelete by lazy { + SwipeToDeleteBinding(tabsTrayStore) } - private val tabsFeature by lazy { - ViewBoundFeatureWrapper( - feature = TabsFeature( - adapter as BrowserTabsAdapter, - context.components.core.store, - selectTabUseCase, - removeTabUseCase, - { it.filterFromConfig(configuration) }, - { } - ), - owner = lifecycleProvider, - view = this + private val touchHelper by lazy { + TabsTouchHelper( + observable = adapter as TabsAdapter, + onViewHolderTouched = { swipeToDelete.isSwipeable }, + onViewHolderDraw = { context.components.settings.listTabView } ) } override fun onAttachedToWindow() { super.onAttachedToWindow() - // This is weird, but I don't have a better solution right now: We need to keep a - // lazy reference to the feature/adapter so that we do not re-create - // it every time it's attached. This reference is our way to init. - tabsFeature + tabsFeature.start() + swipeToDelete.start() + + touchHelper.attachToRecyclerView(this) + } + + override fun onDetachedFromWindow() { + super.onDetachedFromWindow() + + tabsFeature.stop() + swipeToDelete.stop() + + touchHelper.attachToRecyclerView(null) } } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/SwipeToDeleteBinding.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/SwipeToDeleteBinding.kt new file mode 100644 index 000000000..fd7052e81 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/SwipeToDeleteBinding.kt @@ -0,0 +1,42 @@ +/* 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.tabstray.browser + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.map +import mozilla.components.lib.state.ext.flowScoped +import mozilla.components.support.base.feature.LifecycleAwareFeature +import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged +import org.mozilla.fenix.tabstray.TabsTrayState +import org.mozilla.fenix.tabstray.TabsTrayStore + +/** + * Notifies whether a tab is accessible for using the swipe-to-delete gesture. + */ +class SwipeToDeleteBinding( + private val store: TabsTrayStore +) : LifecycleAwareFeature { + private var scope: CoroutineScope? = null + var isSwipeable = false + private set + + @OptIn(ExperimentalCoroutinesApi::class) + override fun start() { + scope = store.flowScoped { flow -> + flow.map { it.mode } + .ifChanged() + .collect { mode -> + isSwipeable = mode == TabsTrayState.Mode.Normal + } + } + } + + override fun stop() { + scope?.cancel() + } +} diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabsTouchHelper.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabsTouchHelper.kt new file mode 100644 index 000000000..25fc74c19 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabsTouchHelper.kt @@ -0,0 +1,139 @@ +/* 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.tabstray.browser + +import android.graphics.Canvas +import android.graphics.drawable.Drawable +import androidx.appcompat.content.res.AppCompatResources +import androidx.recyclerview.widget.ItemTouchHelper +import androidx.recyclerview.widget.ItemTouchHelper.ACTION_STATE_IDLE +import androidx.recyclerview.widget.RecyclerView +import mozilla.components.browser.tabstray.TabTouchCallback +import mozilla.components.concept.tabstray.TabsTray +import mozilla.components.support.base.observer.Observable +import mozilla.components.support.ktx.android.content.getColorFromAttr +import mozilla.components.support.ktx.android.content.getDrawableWithTint +import mozilla.components.support.ktx.android.util.dpToPx +import org.mozilla.fenix.R +import org.mozilla.fenix.home.sessioncontrol.SwipeToDeleteCallback + +/** + * A callback for consumers to know when a [RecyclerView.ViewHolder] is about to be touched. + * Return false if the custom behaviour should be ignored. + */ +typealias OnViewHolderTouched = (RecyclerView.ViewHolder) -> Boolean + +/** + * A callback for consumers to know when a [RecyclerView.ViewHolder] is about to be drawn. + * Return false if the custom drawing should be ignored. + */ +typealias OnViewHolderToDraw = (RecyclerView.ViewHolder) -> Boolean + +/** + * An [ItemTouchHelper] for handling tab swiping to delete. + * + * @param onViewHolderTouched See [OnViewHolderTouched]. + */ +class TabsTouchHelper( + observable: Observable, + onViewHolderTouched: OnViewHolderTouched = { true }, + onViewHolderDraw: OnViewHolderToDraw = { true }, + delegate: Callback = TouchCallback(observable, onViewHolderTouched, onViewHolderDraw) +) : ItemTouchHelper(delegate) + +/** + * An [ItemTouchHelper.Callback] for drawing custom layouts on [RecyclerView.ViewHolder] interactions. + * + * @param onViewHolderTouched invoked when a tab is about to be swiped. See [OnViewHolderTouched]. + */ +class TouchCallback( + observable: Observable, + private val onViewHolderTouched: OnViewHolderTouched, + private val onViewHolderDraw: OnViewHolderToDraw +) : TabTouchCallback(observable) { + + override fun getMovementFlags( + recyclerView: RecyclerView, + viewHolder: RecyclerView.ViewHolder + ): Int { + if (!onViewHolderTouched.invoke(viewHolder)) { + return ItemTouchHelper.Callback.makeFlag(ACTION_STATE_IDLE, 0) + } + + return super.getMovementFlags(recyclerView, viewHolder) + } + + override fun onChildDraw( + c: Canvas, + recyclerView: RecyclerView, + viewHolder: RecyclerView.ViewHolder, + dX: Float, + dY: Float, + actionState: Int, + isCurrentlyActive: Boolean + ) { + super.onChildDraw(c, recyclerView, viewHolder, dX, dY, actionState, isCurrentlyActive) + + if (!onViewHolderDraw.invoke(viewHolder)) { + return + } + + val icon = recyclerView.context.getDrawableWithTint( + R.drawable.ic_delete, + recyclerView.context.getColorFromAttr(R.attr.destructive) + )!! + val background = AppCompatResources.getDrawable( + recyclerView.context, + R.drawable.swipe_delete_background + )!! + val itemView = viewHolder.itemView + val iconLeft: Int + val iconRight: Int + val margin = + SwipeToDeleteCallback.MARGIN.dpToPx(recyclerView.resources.displayMetrics) + val iconWidth = icon.intrinsicWidth + val iconHeight = icon.intrinsicHeight + val cellHeight = itemView.bottom - itemView.top + val iconTop = itemView.top + (cellHeight - iconHeight) / 2 + val iconBottom = iconTop + iconHeight + + when { + dX > 0 -> { // Swiping to the right + iconLeft = itemView.left + margin + iconRight = itemView.left + margin + iconWidth + background.setBounds( + itemView.left, itemView.top, + (itemView.left + dX).toInt() + SwipeToDeleteCallback.BACKGROUND_CORNER_OFFSET, + itemView.bottom + ) + icon.setBounds(iconLeft, iconTop, iconRight, iconBottom) + draw(background, icon, c) + } + dX < 0 -> { // Swiping to the left + iconLeft = itemView.right - margin - iconWidth + iconRight = itemView.right - margin + background.setBounds( + (itemView.right + dX).toInt() - SwipeToDeleteCallback.BACKGROUND_CORNER_OFFSET, + itemView.top, itemView.right, itemView.bottom + ) + icon.setBounds(iconLeft, iconTop, iconRight, iconBottom) + draw(background, icon, c) + } + else -> { // View not swiped + background.setBounds(0, 0, 0, 0) + icon.setBounds(0, 0, 0, 0) + } + } + } + + private fun draw( + background: Drawable, + icon: Drawable, + c: Canvas + ) { + background.draw(c) + icon.draw(c) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/BaseBrowserTabViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/BaseBrowserTabViewHolder.kt index 6bb89ea54..f3a432ad6 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/BaseBrowserTabViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/BaseBrowserTabViewHolder.kt @@ -9,6 +9,7 @@ import androidx.annotation.CallSuper import androidx.recyclerview.widget.RecyclerView import org.mozilla.fenix.R import org.mozilla.fenix.tabstray.TabsTrayInteractor +import org.mozilla.fenix.tabstray.TabsTrayStore import org.mozilla.fenix.tabstray.browser.BaseBrowserTrayList /** @@ -16,13 +17,15 @@ import org.mozilla.fenix.tabstray.browser.BaseBrowserTrayList */ abstract class BaseBrowserTabViewHolder( containerView: View, - interactor: TabsTrayInteractor + interactor: TabsTrayInteractor, + tabsTrayStore: TabsTrayStore ) : AbstractTrayViewHolder(containerView) { - protected val trayList: BaseBrowserTrayList = itemView.findViewById(R.id.tray_list_item) + private val trayList: BaseBrowserTrayList = itemView.findViewById(R.id.tray_list_item) init { trayList.interactor = interactor + trayList.tabsTrayStore = tabsTrayStore } @CallSuper diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/NormalBrowserTabViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/NormalBrowserTabViewHolder.kt index f7ac2b551..dc129ad72 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/NormalBrowserTabViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/NormalBrowserTabViewHolder.kt @@ -17,10 +17,10 @@ import org.mozilla.fenix.tabstray.browser.BrowserTabsAdapter * View holder for the normal tabs tray list. */ class NormalBrowserTabViewHolder( - private val store: TabsTrayStore, containerView: View, + private val store: TabsTrayStore, interactor: TabsTrayInteractor -) : BaseBrowserTabViewHolder(containerView, interactor), SelectionHolder { +) : BaseBrowserTabViewHolder(containerView, interactor, store), SelectionHolder { /** * Holds the list of selected tabs. diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/PrivateBrowserTabViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/PrivateBrowserTabViewHolder.kt index 1820c112a..f32f44437 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/PrivateBrowserTabViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/PrivateBrowserTabViewHolder.kt @@ -7,14 +7,16 @@ package org.mozilla.fenix.tabstray.viewholders import android.view.View import org.mozilla.fenix.R import org.mozilla.fenix.tabstray.TabsTrayInteractor +import org.mozilla.fenix.tabstray.TabsTrayStore /** * View holder for the private tabs tray list. */ class PrivateBrowserTabViewHolder( containerView: View, + store: TabsTrayStore, interactor: TabsTrayInteractor -) : BaseBrowserTabViewHolder(containerView, interactor) { +) : BaseBrowserTabViewHolder(containerView, interactor, store) { companion object { const val LAYOUT_ID = R.layout.private_browser_tray_list } diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/SwipeToDeleteBindingTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/SwipeToDeleteBindingTest.kt new file mode 100644 index 000000000..672738a76 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/SwipeToDeleteBindingTest.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.tabstray.browser + +import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestCoroutineDispatcher +import mozilla.components.support.test.libstate.ext.waitUntilIdle +import mozilla.components.support.test.rule.MainCoroutineRule +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.mozilla.fenix.tabstray.TabsTrayAction +import org.mozilla.fenix.tabstray.TabsTrayState +import org.mozilla.fenix.tabstray.TabsTrayStore + +class SwipeToDeleteBindingTest { + + @OptIn(ExperimentalCoroutinesApi::class) + @get:Rule + val coroutinesTestRule = MainCoroutineRule(TestCoroutineDispatcher()) + + @Test + fun `WHEN started THEN update the swipeable state`() { + val store = TabsTrayStore(TabsTrayState(mode = TabsTrayState.Mode.Select(emptySet()))) + val binding = SwipeToDeleteBinding(store) + + binding.start() + + assertFalse(binding.isSwipeable) + + store.dispatch(TabsTrayAction.ExitSelectMode) + + store.waitUntilIdle() + + assertTrue(binding.isSwipeable) + } + + @Test + fun `default state of binding is false`() { + val binding = SwipeToDeleteBinding(mockk()) + + assertFalse(binding.isSwipeable) + } +} diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabsTouchHelperTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabsTouchHelperTest.kt new file mode 100644 index 000000000..9cab8e53f --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabsTouchHelperTest.kt @@ -0,0 +1,60 @@ +/* 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.tabstray.browser + +import android.widget.FrameLayout +import androidx.recyclerview.widget.ItemTouchHelper +import androidx.recyclerview.widget.ItemTouchHelper.ACTION_STATE_IDLE +import androidx.recyclerview.widget.ItemTouchHelper.Callback.makeMovementFlags +import androidx.recyclerview.widget.RecyclerView +import io.mockk.mockk +import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +import org.mozilla.fenix.tabstray.viewholders.SyncedTabViewHolder + +@RunWith(FenixRobolectricTestRunner::class) +class TabsTouchHelperTest { + + @Test + fun `movement flags remain unchanged if onSwipeToDelete is true`() { + val recyclerView = RecyclerView(testContext) + val layout = FrameLayout(testContext) + val viewHolder = SyncedTabViewHolder(layout, mockk()) + val callback = TouchCallback(mockk(), { true }, mockk()) + + assertEquals(0, callback.getDragDirs(recyclerView, viewHolder)) + assertEquals( + ItemTouchHelper.LEFT or ItemTouchHelper.RIGHT, + callback.getSwipeDirs(recyclerView, viewHolder) + ) + + val actual = callback.getMovementFlags(recyclerView, viewHolder) + val expected = makeMovementFlags(0, ItemTouchHelper.LEFT or ItemTouchHelper.RIGHT) + + assertEquals(expected, actual) + } + + @Test + fun `movement flags remain unchanged if onSwipeToDelete is false`() { + val recyclerView = RecyclerView(testContext) + val layout = FrameLayout(testContext) + val viewHolder = SyncedTabViewHolder(layout, mockk()) + val callback = TouchCallback(mockk(), { false }, mockk()) + + assertEquals(0, callback.getDragDirs(recyclerView, viewHolder)) + assertEquals( + ItemTouchHelper.LEFT or ItemTouchHelper.RIGHT, + callback.getSwipeDirs(recyclerView, viewHolder) + ) + + val actual = callback.getMovementFlags(recyclerView, viewHolder) + val expected = ItemTouchHelper.Callback.makeFlag(ACTION_STATE_IDLE, 0) + + assertEquals(expected, actual) + } +}