From 6637f610884bf4c6826886b4a413a23f3400622e Mon Sep 17 00:00:00 2001 From: Christian Sadilek Date: Wed, 8 Dec 2021 11:36:49 -0500 Subject: [PATCH] Support tab movement/reordering (#22751) * Adds basic support for tab reordering via drag-and-drop selected tabs * ktlint/detekt formatting * Use defaultTabsFilter (now not private) instead of getter * Convert from position+filter API to target+placeAfter Unfortunately I still need the filter passed around a bit * Handle inactive tabs' holder being children of the RecyclerView of the normal tabs Don't go through LayoutManager needlessly * Non-working use tabID the whole way. Does not compile. * Fix to do direct tab ID and use grid setting directly * Remove non-null assertion. Now fully works for "other" tabs. * Prevent grouped tabs from being dragged * Remove unused import * Add/fix comments * Do API version check and use deprecated startDrag if too old. * Build process fails: both outdated and too new, so reverting to just too new * Use deprecated function and suppress warning * fix space * Suppress "TooManyFunctions" on DefaultTabsTrayController * Repeatedly update tab movement during drag * Remove multi-tab movement, only allow dragging if tab groups disabled, fix tab positioning during movement I'm forced to suppress LongParameterList to get the settings information where it needs to go though * Remove settings argument and corresponding long args suppression: instead get settings from parent AbstractBrowserTrayList's context * New UI: Select a tab and then, while holding down, start dragging * Revert to using before/after boolean to accomodate delays Move drag transparency to start of drag * Use new BlankDragShadowBuilder and DraggableItemAnimator to handle tab movement * Replace Pair<>s with data classes * Only drag if exactly 1 tab selected, don't consume drag event if not used * Auto-scroll tab tray while dragging near top/bottom edge * Remove unexpected scrolling on tab bind (triggered when tab is selected) * Fix broken scroll behavior during dragging * Cleanup for ktlint/detekt * Constantly set elevation during drag in case of update Clean code at drag start * Add custom drag start behavior * Add drag distance constant, do all touch-drag behavior in OnTouchListener * Disable parent vertical scrolling on drag start, fix detekt ComplexCondition * Minor cleanup/comments * Revert removal of scroll on bind, this was related to something different * Correction to prepareForDrop to match documentation- doesn't seem to have any effect * Simplify via unchecked typecast, use ViewCompat * Use ViewConfiguration.scaledTouchSlop instead of arbitrary 30px * Added tabReorderingFeature flag, split drag interactor to separate function to satisfy complexity requirement Co-authored-by: Steven Knipe Co-authored-by: ssk97 Co-authored-by: Sebastian Kaspari --- .../java/org/mozilla/fenix/FeatureFlags.kt | 5 + .../fenix/tabstray/TabsTrayController.kt | 27 +++ .../fenix/tabstray/TabsTrayInteractor.kt | 17 ++ .../browser/AbstractBrowserTabViewHolder.kt | 55 +++++++ .../browser/AbstractBrowserTrayList.kt | 155 ++++++++++++++++++ .../browser/BlankDragShadowBuilder.kt | 22 +++ .../tabstray/browser/DraggableItemAnimator.kt | 26 +++ .../fenix/tabstray/browser/TabDragData.kt | 10 ++ 8 files changed, 317 insertions(+) create mode 100644 app/src/main/java/org/mozilla/fenix/tabstray/browser/BlankDragShadowBuilder.kt create mode 100644 app/src/main/java/org/mozilla/fenix/tabstray/browser/DraggableItemAnimator.kt create mode 100644 app/src/main/java/org/mozilla/fenix/tabstray/browser/TabDragData.kt diff --git a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt index 9d514cc914..865941a598 100644 --- a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt +++ b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt @@ -73,6 +73,11 @@ object FeatureFlags { */ val tabGroupFeature = Config.channel.isNightlyOrDebug + /** + * Allows tabs to be dragged around as long as tab groups are disabled + */ + val tabReorderingFeature = Config.channel.isNightlyOrDebug + /** * Enables showing search groupings in the History. */ diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt index bce694ae82..f61f1e1581 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt @@ -59,6 +59,15 @@ interface TabsTrayController { */ fun handleMultipleTabsDeletion(tabs: Collection) + /** + * Moves [tabId] next to before/after [targetId] + * + * @param tabId The tabs to be moved + * @param targetId The id of the tab that the [tab] will be placed next to + * @param placeAfter Place [tabs] before or after the target + */ + fun handleTabsMove(tabId: String, targetId: String?, placeAfter: Boolean) + /** * Navigate from TabsTray to Recently Closed section in the History fragment. */ @@ -82,6 +91,7 @@ interface TabsTrayController { fun handleDeleteAllInactiveTabs() } +@Suppress("TooManyFunctions") class DefaultTabsTrayController( private val trayStore: TabsTrayStore, private val browserStore: BrowserStore, @@ -171,6 +181,23 @@ class DefaultTabsTrayController( showUndoSnackbarForTab(isPrivate) } + /** + * Moves [tabId] next to before/after [targetId] + * + * @param tabId The tabs to be moved + * @param targetId The id of the tab that the [tab] will be placed next to + * @param placeAfter Place [tabs] before or after the target + */ + override fun handleTabsMove( + tabId: String, + targetId: String?, + placeAfter: Boolean + ) { + if (targetId != null && tabId != targetId) { + tabsUseCases.moveTabs(listOf(tabId), targetId, placeAfter) + } + } + /** * Dismisses the tabs tray and navigates to the Recently Closed section in the History fragment. */ diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt index 620f54a02e..b78933fbac 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt @@ -35,6 +35,15 @@ interface TabsTrayInteractor { */ fun onInactiveDebugClicked(tabs: Collection) + /** + * Invoked when [tabId] should be moved to before/after [targetId] from a drag-drop operation + */ + fun onTabsMove( + tabId: String, + targetId: String?, + placeAfter: Boolean + ) + /** * Deletes all inactive tabs. */ @@ -65,6 +74,14 @@ class DefaultTabsTrayInteractor( controller.handleMultipleTabsDeletion(tabs) } + override fun onTabsMove( + tabId: String, + targetId: String?, + placeAfter: Boolean + ) { + controller.handleTabsMove(tabId, targetId, placeAfter) + } + override fun onInactiveDebugClicked(tabs: Collection) { controller.forceTabsAsInactive(tabs) } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt index 7157c3df32..0b9e51c4b1 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt @@ -4,13 +4,18 @@ package org.mozilla.fenix.tabstray.browser +import android.annotation.SuppressLint +import android.graphics.PointF +import android.view.MotionEvent import android.view.View +import android.view.ViewConfiguration import android.widget.ImageButton import android.widget.ImageView import android.widget.TextView import androidx.annotation.VisibleForTesting import androidx.appcompat.content.res.AppCompatResources import androidx.appcompat.widget.AppCompatImageButton +import androidx.core.view.ViewCompat import androidx.core.view.isInvisible import androidx.core.view.isVisible import mozilla.components.browser.state.selector.findTabOrCustomTab @@ -24,6 +29,7 @@ import mozilla.components.browser.toolbar.MAX_URI_LENGTH import mozilla.components.concept.base.images.ImageLoadRequest import mozilla.components.concept.base.images.ImageLoader import mozilla.components.concept.engine.mediasession.MediaSession +import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController @@ -31,6 +37,7 @@ import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.increaseTapArea import org.mozilla.fenix.ext.removeAndDisable import org.mozilla.fenix.ext.removeTouchDelegate +import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showAndEnable import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.selection.SelectionHolder @@ -77,6 +84,9 @@ abstract class AbstractBrowserTabViewHolder( override var tab: TabSessionState? = null + internal var beingDragged: Boolean = false + private var touchStartPoint: PointF? = null + /** * Displays the data of the given session and notifies the given observable about events. */ @@ -88,6 +98,7 @@ abstract class AbstractBrowserTabViewHolder( delegate: TabsTray.Delegate ) { this.tab = tab + beingDragged = false updateTitle(tab) updateUrl(tab) @@ -226,6 +237,50 @@ abstract class AbstractBrowserTabViewHolder( false } } + setDragInteractor(item, holder, interactor) + } + + @SuppressLint("ClickableViewAccessibility") + private fun setDragInteractor( + item: TabSessionState, + holder: SelectionHolder, + interactor: BrowserTrayInteractor + ) { + // Since I immediately pass the event to onTouchEvent if it's not a move + // The ClickableViewAccessibility warning isn't useful + itemView.setOnTouchListener { view, motionEvent -> + when (motionEvent.actionMasked) { + MotionEvent.ACTION_DOWN -> { + touchStartPoint = PointF(motionEvent.x, motionEvent.y) + } + MotionEvent.ACTION_UP, MotionEvent.ACTION_CANCEL -> { + touchStartPoint = null + } + MotionEvent.ACTION_MOVE -> { + val parent = itemView.parent as AbstractBrowserTrayList + val touchStart = touchStartPoint + val selected = holder.selectedItems + val selectsOnlyThis = (selected.size == 1 && selected.contains(item)) + val featureEnabled = FeatureFlags.tabReorderingFeature && + !parent.context.settings().searchTermTabGroupsAreEnabled + if (featureEnabled && selectsOnlyThis && touchStart != null) { + // Prevent scrolling if the user tries to start drag vertically + parent.requestDisallowInterceptTouchEvent(true) + // Only start deselect+drag if the user drags far enough + val dist = PointF.length(touchStart.x - motionEvent.x, touchStart.y - motionEvent.y) + if (dist > ViewConfiguration.get(parent.context).scaledTouchSlop) { + interactor.deselect(item) // Exit selection mode + touchStartPoint = null + val dragOffset = PointF(motionEvent.x, motionEvent.y) + val shadow = BlankDragShadowBuilder() + ViewCompat.startDragAndDrop(itemView, null, shadow, TabDragData(item, dragOffset), 0) + } + return@setOnTouchListener true + } + } + } + view.onTouchEvent(motionEvent) + } } companion object { diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTrayList.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTrayList.kt index 1ad3518188..542749d773 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTrayList.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTrayList.kt @@ -5,10 +5,17 @@ package org.mozilla.fenix.tabstray.browser import android.content.Context +import android.graphics.PointF +import android.graphics.Rect import android.util.AttributeSet +import android.view.DragEvent +import android.view.View +import androidx.recyclerview.widget.ItemTouchHelper import androidx.recyclerview.widget.RecyclerView +import mozilla.components.browser.tabstray.TabViewHolder import org.mozilla.fenix.tabstray.TabsTrayInteractor import org.mozilla.fenix.tabstray.TabsTrayStore +import kotlin.math.abs /** * The base class for a tabs tray list that wants to display browser tabs. @@ -22,6 +29,9 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( lateinit var interactor: TabsTrayInteractor lateinit var tabsTrayStore: TabsTrayStore + private var lastDragPos: PointF? = null + private var lastDragData: TabDragData? = null + protected val swipeToDelete by lazy { SwipeToDeleteBinding(tabsTrayStore) } @@ -32,6 +42,8 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( swipeToDelete.start() adapter?.onAttachedToRecyclerView(this) + this.setOnDragListener(dragListen) + itemAnimator = DraggableItemAnimator() } override fun onDetachedFromWindow() { @@ -41,5 +53,148 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( // Notify the adapter that it is released from the view preemptively. adapter?.onDetachedFromRecyclerView(this) + this.setOnDragListener(null) + } + + // Find the closest item to the x/y position of the drop. + private data class DropPositionData(val id: String, val placeAfter: Boolean, val view: View) + private fun getDropPosition(x: Float, y: Float, source: String): DropPositionData? { + if (childCount < 2) return null // If there's 0 or 1 tabs visible, can't reorder + var bestDist = Float.MAX_VALUE + var bestOut: DropPositionData? = null + var seenSource = false + for (i in 0 until childCount) { + val proposedTarget = getChildAt(i) + val targetHolder = findContainingViewHolder(proposedTarget) + if (targetHolder is TabViewHolder) { + val rect = Rect() // Get post-animation positioning + getDecoratedBoundsWithMargins(proposedTarget, rect) + val targetX = (rect.left + rect.right) / 2 + val targetY = (rect.top + rect.bottom) / 2 + val xDiff = x - targetX + val yDiff = y - targetY + val dist = abs(xDiff) + abs(yDiff) + val id = targetHolder.tab?.id + // Determine before/after drop placement + // based on if source tab is coming from before/after the target + if (id == source) seenSource = true + if (dist < bestDist && id != null) { + bestDist = dist + bestOut = DropPositionData(id, seenSource, proposedTarget) + } + } + } + return bestOut + } + private fun findSourceViewAndHolder(id: String): Pair? { + for (i in 0 until childCount) { + val proposed = getChildAt(i) + val targetHolder = findContainingViewHolder(proposed) + if (targetHolder is AbstractBrowserTabViewHolder && targetHolder.tab?.id == id) { + return Pair(proposed, targetHolder) + } + } + return null + } + private val dragListen = OnDragListener { _, event -> + if (event.localState is TabDragData) { + val (tab, _) = event.localState as TabDragData + val sourceId = tab.id + val sources = findSourceViewAndHolder(sourceId) + + when (event.action) { + DragEvent.ACTION_DRAG_STARTED -> { + // Put the dragged tab on top of all other tabs + if (sources != null) { + val (sourceView, sourceViewHolder) = sources + sourceViewHolder.beingDragged = true + sourceView.elevation = DRAGGED_TAB_ELEVATION + } + // Setup the scrolling/updating loop + lastDragPos = PointF(event.x, event.y) + lastDragData = event.localState as TabDragData + handler.postDelayed(dragRunnable, DRAG_UPDATE_PERIOD_MS) + true + } + DragEvent.ACTION_DRAG_ENTERED -> { + true + } + DragEvent.ACTION_DRAG_LOCATION -> { + lastDragPos = PointF(event.x, event.y) + true + } + DragEvent.ACTION_DRAG_EXITED -> { + true + } + DragEvent.ACTION_DROP -> { + true + } + DragEvent.ACTION_DRAG_ENDED -> { + // Move tab to center, set dragging to false, return tab to normal height + if (sources != null) { + val (sourceView, sourceViewHolder) = sources + sourceViewHolder.beingDragged = false + sourceView.elevation = 0f + sourceView.animate() + .translationX(0f).translationY(0f).duration = + itemAnimator?.moveDuration ?: 0 + } + // This will stop the scroll/update loop + lastDragPos = null + lastDragData = null + true + } + else -> { // Unknown action + false + } + } + } else false + } + + private val dragRunnable: Runnable = object : Runnable { + override fun run() { + val pos = lastDragPos + val data = lastDragData + if (pos == null || data == null) return + val (tab, dragOffset) = data + val sourceId = tab.id + val sources = findSourceViewAndHolder(sourceId) + // Move the tab's visual position + if (sources != null) { + val (sourceView, sourceViewHolder) = sources + sourceView.x = pos.x - dragOffset.x + sourceView.y = pos.y - dragOffset.y + sourceViewHolder.beingDragged = true + sourceView.elevation = DRAGGED_TAB_ELEVATION + + // Move the tab's position in the list + val target = getDropPosition(pos.x, pos.y, tab.id) + if (target != null) { + val (targetId, placeAfter, targetView) = target + if (sourceView != targetView) { + interactor.onTabsMove(tab.id, targetId, placeAfter) + // Deal with https://issuetracker.google.com/issues/37018279 + // See also https://stackoverflow.com/questions/27992427 + (layoutManager as? ItemTouchHelper.ViewDropHandler)?.prepareForDrop( + sourceView, targetView, sourceView.left, sourceView.top + ) + } + } + } + // Scroll the tray + var scroll = 0 + if (pos.y < SCROLL_AREA) scroll = -SCROLL_SPEED + if (pos.y > height - SCROLL_AREA) scroll = SCROLL_SPEED + scrollBy(0, scroll) + + // Repeats forever, until lastDragPos/Data are null + handler.postDelayed(this, DRAG_UPDATE_PERIOD_MS) + } + } + companion object { + internal const val DRAGGED_TAB_ELEVATION = 10f + internal const val DRAG_UPDATE_PERIOD_MS = 10L + internal const val SCROLL_SPEED = 20 + internal const val SCROLL_AREA = 200 } } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BlankDragShadowBuilder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BlankDragShadowBuilder.kt new file mode 100644 index 0000000000..565da74bd5 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BlankDragShadowBuilder.kt @@ -0,0 +1,22 @@ +/* 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.Point +import android.view.View + +class BlankDragShadowBuilder : View.DragShadowBuilder() { + override fun onProvideShadowMetrics(outShadowSize: Point?, outShadowTouchPoint: Point?) { + outShadowSize?.x = 1 + outShadowSize?.y = 1 + outShadowTouchPoint?.x = 0 + outShadowTouchPoint?.y = 0 + } + + override fun onDrawShadow(canvas: Canvas?) { + // Do nothing + } +} diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/DraggableItemAnimator.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/DraggableItemAnimator.kt new file mode 100644 index 0000000000..cbd1153d32 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/DraggableItemAnimator.kt @@ -0,0 +1,26 @@ +/* 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 androidx.annotation.NonNull +import androidx.recyclerview.widget.DefaultItemAnimator +import androidx.recyclerview.widget.RecyclerView + +class DraggableItemAnimator : DefaultItemAnimator() { + override fun animatePersistence( + @NonNull viewHolder: RecyclerView.ViewHolder, + @NonNull preLayoutInfo: RecyclerView.ItemAnimator.ItemHolderInfo, + @NonNull postLayoutInfo: RecyclerView.ItemAnimator.ItemHolderInfo + ): Boolean { + // While being dragged, keep the tab visually in place + if (viewHolder is AbstractBrowserTabViewHolder && viewHolder.beingDragged) { + viewHolder.itemView.translationX -= postLayoutInfo.left - preLayoutInfo.left + viewHolder.itemView.translationY -= postLayoutInfo.top - preLayoutInfo.top + dispatchAnimationFinished(viewHolder) + return false + } + return super.animatePersistence(viewHolder, preLayoutInfo, postLayoutInfo) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabDragData.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabDragData.kt new file mode 100644 index 0000000000..8111188572 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabDragData.kt @@ -0,0 +1,10 @@ +/* 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.PointF +import mozilla.components.browser.state.state.TabSessionState + +data class TabDragData(val tab: TabSessionState, val dragOffset: PointF)