From 6628943ccecfe723f208d3f4f5034635230c6978 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Sun, 5 Sep 2021 00:10:50 -0700 Subject: [PATCH 01/38] Adds basic support for tab reordering via drag-and-drop selected tabs --- .../fenix/tabstray/TabsTrayController.kt | 18 +++++ .../fenix/tabstray/TabsTrayInteractor.kt | 10 +++ .../browser/AbstractBrowserTabViewHolder.kt | 4 ++ .../browser/AbstractBrowserTrayList.kt | 69 +++++++++++++++++++ 4 files changed, 101 insertions(+) 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 f273008c0..e216c3c77 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt @@ -60,6 +60,14 @@ interface TabsTrayController { */ fun handleMultipleTabsDeletion(tabs: Collection) + /** + * Moves tabs to a new position + * + * @param tabs The tabs to be moved + * @param position The new position to put the tabs + */ + fun handleTabsMove(tabs: Collection, position: Int,filter: (TabSessionState) -> Boolean) + /** * Navigate from TabsTray to Recently Closed section in the History fragment. */ @@ -167,6 +175,16 @@ class DefaultTabsTrayController( showUndoSnackbarForTab(isPrivate) } + /** + * Moves currently selected tabs to position + * + * @param position The position to move the tabs to + */ + override fun handleTabsMove(tabs: Collection, position: Int,filter: (TabSessionState) -> Boolean) { + val tabIDs = tabs.map{it.id} + tabsUseCases.moveTabs(tabIDs,position,filter) + } + /** * 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 9923b0eb0..2b4745066 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt @@ -4,6 +4,7 @@ package org.mozilla.fenix.tabstray +import mozilla.components.browser.state.state.TabSessionState import mozilla.components.concept.tabstray.Tab interface TabsTrayInteractor { @@ -34,6 +35,11 @@ interface TabsTrayInteractor { * Called when clicking the debug menu option for inactive tabs. */ fun onInactiveDebugClicked(tabs: Collection) + + /** + * Invoked when [tabs] should be moved to [position] from a drag-drop operation + */ + fun onTabsMove(tabs: Collection, position: Int, filter: (TabSessionState) -> Boolean = {true}) } /** @@ -60,6 +66,10 @@ class DefaultTabsTrayInteractor( controller.handleMultipleTabsDeletion(tabs) } + override fun onTabsMove(tabs: Collection, position: Int,filter: (TabSessionState) -> Boolean) { + controller.handleTabsMove(tabs, position, filter) + } + 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 34c5c1dbd..cd59dd504 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 @@ -227,6 +227,10 @@ abstract class AbstractBrowserTabViewHolder( metrics.track(Event.CollectionTabLongPressed) interactor.select(item) true + } else if (holder.selectedItems.contains(item)){ + val shadow = View.DragShadowBuilder(itemView); + itemView.startDragAndDrop(null,shadow,holder.selectedItems,0); + true } else { false } 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 6b40df526..a26f1f124 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 @@ -6,11 +6,14 @@ package org.mozilla.fenix.tabstray.browser import android.content.Context import android.util.AttributeSet +import android.view.DragEvent import androidx.recyclerview.widget.RecyclerView +import mozilla.components.concept.tabstray.Tab import mozilla.components.feature.tabs.tabstray.TabsFeature import org.mozilla.fenix.ext.components 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. @@ -55,6 +58,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( swipeToDelete.start() adapter?.onAttachedToRecyclerView(this) + this.setOnDragListener(dragListen) } override fun onDetachedFromWindow() { @@ -64,5 +68,70 @@ 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. + // Then, based on the offset between the first and second + //determine if it goes before or after that item. + // This will fail if there's a spiral layout or something + //but should work perfectly for lists and grids. + private fun getDropPosition(x: Float, y: Float): Int{ + val lm = layoutManager?: return 0//No layoutManager should never happen + val targetCount = lm.childCount + if (targetCount < 1) return 0//There's nothing here! + if (targetCount < 2) return lm.getPosition(lm.getChildAt(0)!!)//only one item, can't get offset + val first = lm.getChildAt(0)!! + val second = lm.getChildAt(1)!! + val xOffset = second.x-first.x + val yOffset = second.y-first.y + + var bestDist = Float.MAX_VALUE + var bestPos = 0 + for (i in 0 until targetCount){ + val proposedTarget = lm.getChildAt(i)!! + val xDiff = x-(proposedTarget.x+proposedTarget.width/2) + val yDiff = y-(proposedTarget.y+proposedTarget.height/2) + val dist = abs(xDiff)+abs(yDiff) + if (dist < bestDist){ + bestDist = dist + bestPos = getChildAdapterPosition(proposedTarget) + val modifier = (xDiff*xOffset)+(yDiff*yOffset) + if (modifier > 0) bestPos += 1 + } + } + return bestPos + } + private val dragListen = OnDragListener { _, event -> + when (event.action) { + DragEvent.ACTION_DRAG_STARTED -> { + //This check is required for the unchecked cast later on + if (event.localState is Collection<*>){ + (event.localState as Collection<*>).all { it is Tab } + } else false + } + DragEvent.ACTION_DRAG_ENTERED -> { + true + } + DragEvent.ACTION_DRAG_LOCATION -> { + true + } + DragEvent.ACTION_DRAG_EXITED -> { + true + } + DragEvent.ACTION_DROP -> { + val target = getDropPosition(event.x, event.y) + @Suppress("UNCHECKED_CAST") //Cast is checked on drag start + interactor.onTabsMove(event.localState as Collection,target,tabsFeature.getFilter()) + true + } + DragEvent.ACTION_DRAG_ENDED -> { + true + } + else -> { //Unknown action + false + } + } + } + } From 8841c19d99de07c9f03d16e421e215aade446f0f Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Sun, 5 Sep 2021 01:52:46 -0700 Subject: [PATCH 02/38] ktlint/detekt formatting --- .../fenix/tabstray/TabsTrayController.kt | 8 ++-- .../fenix/tabstray/TabsTrayInteractor.kt | 4 +- .../browser/AbstractBrowserTabViewHolder.kt | 6 +-- .../browser/AbstractBrowserTrayList.kt | 39 +++++++++---------- 4 files changed, 27 insertions(+), 30 deletions(-) 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 e216c3c77..87cd43be1 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt @@ -66,7 +66,7 @@ interface TabsTrayController { * @param tabs The tabs to be moved * @param position The new position to put the tabs */ - fun handleTabsMove(tabs: Collection, position: Int,filter: (TabSessionState) -> Boolean) + fun handleTabsMove(tabs: Collection, position: Int, filter: (TabSessionState) -> Boolean) /** * Navigate from TabsTray to Recently Closed section in the History fragment. @@ -180,9 +180,9 @@ class DefaultTabsTrayController( * * @param position The position to move the tabs to */ - override fun handleTabsMove(tabs: Collection, position: Int,filter: (TabSessionState) -> Boolean) { - val tabIDs = tabs.map{it.id} - tabsUseCases.moveTabs(tabIDs,position,filter) + override fun handleTabsMove(tabs: Collection, position: Int, filter: (TabSessionState) -> Boolean) { + val tabIDs = tabs.map { it.id } + tabsUseCases.moveTabs(tabIDs, position, filter) } /** 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 2b4745066..03644a66f 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt @@ -39,7 +39,7 @@ interface TabsTrayInteractor { /** * Invoked when [tabs] should be moved to [position] from a drag-drop operation */ - fun onTabsMove(tabs: Collection, position: Int, filter: (TabSessionState) -> Boolean = {true}) + fun onTabsMove(tabs: Collection, position: Int, filter: (TabSessionState) -> Boolean = { true }) } /** @@ -66,7 +66,7 @@ class DefaultTabsTrayInteractor( controller.handleMultipleTabsDeletion(tabs) } - override fun onTabsMove(tabs: Collection, position: Int,filter: (TabSessionState) -> Boolean) { + override fun onTabsMove(tabs: Collection, position: Int, filter: (TabSessionState) -> Boolean) { controller.handleTabsMove(tabs, position, filter) } 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 cd59dd504..4ce972d7d 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 @@ -227,9 +227,9 @@ abstract class AbstractBrowserTabViewHolder( metrics.track(Event.CollectionTabLongPressed) interactor.select(item) true - } else if (holder.selectedItems.contains(item)){ - val shadow = View.DragShadowBuilder(itemView); - itemView.startDragAndDrop(null,shadow,holder.selectedItems,0); + } else if (holder.selectedItems.contains(item)) { + val shadow = View.DragShadowBuilder(itemView) + itemView.startDragAndDrop(null, shadow, holder.selectedItems, 0) true } else { false 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 a26f1f124..4f0797a54 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 @@ -73,30 +73,28 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( // Find the closest item to the x/y position. // Then, based on the offset between the first and second - //determine if it goes before or after that item. + // determine if it goes before or after that item. // This will fail if there's a spiral layout or something - //but should work perfectly for lists and grids. - private fun getDropPosition(x: Float, y: Float): Int{ - val lm = layoutManager?: return 0//No layoutManager should never happen - val targetCount = lm.childCount - if (targetCount < 1) return 0//There's nothing here! - if (targetCount < 2) return lm.getPosition(lm.getChildAt(0)!!)//only one item, can't get offset + // but should work perfectly for lists and grids. + private fun getDropPosition(x: Float, y: Float): Int { + val lm = layoutManager ?: return 0 // No layoutManager should never happen + if (lm.childCount < 2) return 0 // If there's 0 or 1 tabs visible, can't reorder val first = lm.getChildAt(0)!! val second = lm.getChildAt(1)!! - val xOffset = second.x-first.x - val yOffset = second.y-first.y + val xOffset = second.x - first.x + val yOffset = second.y - first.y var bestDist = Float.MAX_VALUE var bestPos = 0 - for (i in 0 until targetCount){ + for (i in 0 until lm.childCount) { val proposedTarget = lm.getChildAt(i)!! - val xDiff = x-(proposedTarget.x+proposedTarget.width/2) - val yDiff = y-(proposedTarget.y+proposedTarget.height/2) - val dist = abs(xDiff)+abs(yDiff) - if (dist < bestDist){ + val xDiff = x - (proposedTarget.x + proposedTarget.width / 2) + val yDiff = y - (proposedTarget.y + proposedTarget.height / 2) + val dist = abs(xDiff) + abs(yDiff) + if (dist < bestDist) { bestDist = dist bestPos = getChildAdapterPosition(proposedTarget) - val modifier = (xDiff*xOffset)+(yDiff*yOffset) + val modifier = (xDiff * xOffset) + (yDiff * yOffset) if (modifier > 0) bestPos += 1 } } @@ -105,8 +103,8 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( private val dragListen = OnDragListener { _, event -> when (event.action) { DragEvent.ACTION_DRAG_STARTED -> { - //This check is required for the unchecked cast later on - if (event.localState is Collection<*>){ + // This check is required for the unchecked cast later on + if (event.localState is Collection<*>) { (event.localState as Collection<*>).all { it is Tab } } else false } @@ -121,17 +119,16 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( } DragEvent.ACTION_DROP -> { val target = getDropPosition(event.x, event.y) - @Suppress("UNCHECKED_CAST") //Cast is checked on drag start - interactor.onTabsMove(event.localState as Collection,target,tabsFeature.getFilter()) + @Suppress("UNCHECKED_CAST") // Cast is checked on drag start + interactor.onTabsMove(event.localState as Collection, target, tabsFeature.getFilter()) true } DragEvent.ACTION_DRAG_ENDED -> { true } - else -> { //Unknown action + else -> { // Unknown action false } } } - } From 1881122ec099842cbea4f1918eb0c2fe9d1e9104 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Sun, 5 Sep 2021 13:51:18 -0700 Subject: [PATCH 03/38] Use defaultTabsFilter (now not private) instead of getter --- .../mozilla/fenix/tabstray/browser/AbstractBrowserTrayList.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 4f0797a54..78e2ad88c 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 @@ -120,7 +120,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( DragEvent.ACTION_DROP -> { val target = getDropPosition(event.x, event.y) @Suppress("UNCHECKED_CAST") // Cast is checked on drag start - interactor.onTabsMove(event.localState as Collection, target, tabsFeature.getFilter()) + interactor.onTabsMove(event.localState as Collection, target, tabsFeature.defaultTabsFilter) true } DragEvent.ACTION_DRAG_ENDED -> { From d0030d8417bde42af503a226727987e76f758a24 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Thu, 16 Sep 2021 16:39:54 -0700 Subject: [PATCH 04/38] Convert from position+filter API to target+placeAfter Unfortunately I still need the filter passed around a bit --- .../fenix/tabstray/TabsTrayController.kt | 23 ++++++++++++++----- .../fenix/tabstray/TabsTrayInteractor.kt | 19 +++++++++++---- .../browser/AbstractBrowserTrayList.kt | 19 +++++++++------ 3 files changed, 44 insertions(+), 17 deletions(-) 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 87cd43be1..d85d880a0 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt @@ -64,9 +64,11 @@ interface TabsTrayController { * Moves tabs to a new position * * @param tabs The tabs to be moved - * @param position The new position to put the tabs + * @param targetPos Place the moved tabs next to the target position + * @param placeAfter Place the moved tabs before or after the target + * @param filter Filter the full tab list to whatever the displayed tabs are to find the target */ - fun handleTabsMove(tabs: Collection, position: Int, filter: (TabSessionState) -> Boolean) + fun handleTabsMove(tabs: Collection, targetPos: Int, placeAfter: Boolean, filter: (TabSessionState) -> Boolean) /** * Navigate from TabsTray to Recently Closed section in the History fragment. @@ -176,13 +178,22 @@ class DefaultTabsTrayController( } /** - * Moves currently selected tabs to position + * Moves a set of tabs to a position * - * @param position The position to move the tabs to + * @param tabs The tabs to be moved + * @param targetPos The position to move the tabs to in the displayed list + * @param placeAfter Place the tabs before or after the target + * @param filter The filter to go from the full tab list to the displayed list */ - override fun handleTabsMove(tabs: Collection, position: Int, filter: (TabSessionState) -> Boolean) { + override fun handleTabsMove( + tabs: Collection, + targetPos: Int, + placeAfter: Boolean, + filter: (TabSessionState) -> Boolean + ) { val tabIDs = tabs.map { it.id } - tabsUseCases.moveTabs(tabIDs, position, filter) + val target = browserStore.state.tabs.filter(filter)[targetPos] + tabsUseCases.moveTabs(tabIDs, target.id, placeAfter) } /** 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 03644a66f..6cab7beb8 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt @@ -37,9 +37,15 @@ interface TabsTrayInteractor { fun onInactiveDebugClicked(tabs: Collection) /** - * Invoked when [tabs] should be moved to [position] from a drag-drop operation + * Invoked when [tabs] should be moved to before/after position [targetPos] from a drag-drop operation + * Filter modifies [targetPos] in the controller. */ - fun onTabsMove(tabs: Collection, position: Int, filter: (TabSessionState) -> Boolean = { true }) + fun onTabsMove( + tabs: Collection, + targetPos: Int, + placeAfter: Boolean, + filter: (TabSessionState) -> Boolean = { true } + ) } /** @@ -66,8 +72,13 @@ class DefaultTabsTrayInteractor( controller.handleMultipleTabsDeletion(tabs) } - override fun onTabsMove(tabs: Collection, position: Int, filter: (TabSessionState) -> Boolean) { - controller.handleTabsMove(tabs, position, filter) + override fun onTabsMove( + tabs: Collection, + targetPos: Int, + placeAfter: Boolean, + filter: (TabSessionState) -> Boolean + ) { + controller.handleTabsMove(tabs, targetPos, placeAfter, filter) } override fun onInactiveDebugClicked(tabs: Collection) { 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 78e2ad88c..b38a67844 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 @@ -76,9 +76,9 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( // determine if it goes before or after that item. // This will fail if there's a spiral layout or something // but should work perfectly for lists and grids. - private fun getDropPosition(x: Float, y: Float): Int { - val lm = layoutManager ?: return 0 // No layoutManager should never happen - if (lm.childCount < 2) return 0 // If there's 0 or 1 tabs visible, can't reorder + private fun getDropPosition(x: Float, y: Float): Pair? { + val lm = layoutManager ?: return null // No layoutManager should never happen + if (lm.childCount < 2) return null // If there's 0 or 1 tabs visible, can't reorder val first = lm.getChildAt(0)!! val second = lm.getChildAt(1)!! val xOffset = second.x - first.x @@ -86,6 +86,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( var bestDist = Float.MAX_VALUE var bestPos = 0 + var placeAfter = false for (i in 0 until lm.childCount) { val proposedTarget = lm.getChildAt(i)!! val xDiff = x - (proposedTarget.x + proposedTarget.width / 2) @@ -95,10 +96,10 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( bestDist = dist bestPos = getChildAdapterPosition(proposedTarget) val modifier = (xDiff * xOffset) + (yDiff * yOffset) - if (modifier > 0) bestPos += 1 + placeAfter = (modifier > 0) } } - return bestPos + return Pair(bestPos, placeAfter) } private val dragListen = OnDragListener { _, event -> when (event.action) { @@ -119,8 +120,12 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( } DragEvent.ACTION_DROP -> { val target = getDropPosition(event.x, event.y) - @Suppress("UNCHECKED_CAST") // Cast is checked on drag start - interactor.onTabsMove(event.localState as Collection, target, tabsFeature.defaultTabsFilter) + if (target != null) { + val (targetPos, placeAfter) = target + val filter = tabsFeature.defaultTabsFilter + @Suppress("UNCHECKED_CAST") // Cast is checked on drag start + interactor.onTabsMove(event.localState as Collection, targetPos, placeAfter, filter) + } true } DragEvent.ACTION_DRAG_ENDED -> { From 63c3a8911d37c8e1dd0533c64b46463f1c7de84f Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Thu, 16 Sep 2021 23:01:43 -0700 Subject: [PATCH 05/38] Handle inactive tabs' holder being children of the RecyclerView of the normal tabs Don't go through LayoutManager needlessly --- .../org/mozilla/fenix/tabstray/TabsTrayController.kt | 8 ++++++-- .../fenix/tabstray/browser/AbstractBrowserTrayList.kt | 11 +++++------ 2 files changed, 11 insertions(+), 8 deletions(-) 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 d85d880a0..ce32ea56c 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt @@ -192,8 +192,12 @@ class DefaultTabsTrayController( filter: (TabSessionState) -> Boolean ) { val tabIDs = tabs.map { it.id } - val target = browserStore.state.tabs.filter(filter)[targetPos] - tabsUseCases.moveTabs(tabIDs, target.id, placeAfter) + val filteredTabs = browserStore.state.tabs.filter(filter) + if (targetPos ? { - val lm = layoutManager ?: return null // No layoutManager should never happen - if (lm.childCount < 2) return null // If there's 0 or 1 tabs visible, can't reorder - val first = lm.getChildAt(0)!! - val second = lm.getChildAt(1)!! + if (childCount < 2) return null // If there's 0 or 1 tabs visible, can't reorder + val first = getChildAt(0)!! + val second = getChildAt(1)!! val xOffset = second.x - first.x val yOffset = second.y - first.y var bestDist = Float.MAX_VALUE var bestPos = 0 var placeAfter = false - for (i in 0 until lm.childCount) { - val proposedTarget = lm.getChildAt(i)!! + for (i in 0 until childCount) { + val proposedTarget = getChildAt(i)!! val xDiff = x - (proposedTarget.x + proposedTarget.width / 2) val yDiff = y - (proposedTarget.y + proposedTarget.height / 2) val dist = abs(xDiff) + abs(yDiff) From adfd6702901d95d7e8e2f288c6b23e8c80efd641 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Tue, 21 Sep 2021 14:34:05 -0700 Subject: [PATCH 06/38] Non-working use tabID the whole way. Does not compile. --- .../fenix/tabstray/TabsTrayController.kt | 12 +++---- .../fenix/tabstray/TabsTrayInteractor.kt | 8 ++--- .../browser/AbstractBrowserTrayList.kt | 31 ++++++++++--------- 3 files changed, 24 insertions(+), 27 deletions(-) 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 ce32ea56c..e57ced78c 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt @@ -68,7 +68,7 @@ interface TabsTrayController { * @param placeAfter Place the moved tabs before or after the target * @param filter Filter the full tab list to whatever the displayed tabs are to find the target */ - fun handleTabsMove(tabs: Collection, targetPos: Int, placeAfter: Boolean, filter: (TabSessionState) -> Boolean) + fun handleTabsMove(tabs: Collection, targetId: String?, placeAfter: Boolean) /** * Navigate from TabsTray to Recently Closed section in the History fragment. @@ -187,16 +187,12 @@ class DefaultTabsTrayController( */ override fun handleTabsMove( tabs: Collection, - targetPos: Int, + targetId: String?, placeAfter: Boolean, - filter: (TabSessionState) -> Boolean ) { val tabIDs = tabs.map { it.id } - val filteredTabs = browserStore.state.tabs.filter(filter) - if (targetPos , - targetPos: Int, + targetId: String?, placeAfter: Boolean, - filter: (TabSessionState) -> Boolean = { true } ) } @@ -74,11 +73,10 @@ class DefaultTabsTrayInteractor( override fun onTabsMove( tabs: Collection, - targetPos: Int, + targetId: String?, placeAfter: Boolean, - filter: (TabSessionState) -> Boolean ) { - controller.handleTabsMove(tabs, targetPos, placeAfter, filter) + controller.handleTabsMove(tabs, targetId, placeAfter) } override fun onInactiveDebugClicked(tabs: Collection) { 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 c5620312c..87172ff7d 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 @@ -8,6 +8,7 @@ import android.content.Context import android.util.AttributeSet import android.view.DragEvent import androidx.recyclerview.widget.RecyclerView +import mozilla.components.browser.tabstray.TabViewHolder import mozilla.components.concept.tabstray.Tab import mozilla.components.feature.tabs.tabstray.TabsFeature import org.mozilla.fenix.ext.components @@ -76,7 +77,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( // determine if it goes before or after that item. // This will fail if there's a spiral layout or something // but should work perfectly for lists and grids. - private fun getDropPosition(x: Float, y: Float): Pair? { + private fun getDropPosition(x: Float, y: Float): Pair? { if (childCount < 2) return null // If there's 0 or 1 tabs visible, can't reorder val first = getChildAt(0)!! val second = getChildAt(1)!! @@ -84,21 +85,24 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( val yOffset = second.y - first.y var bestDist = Float.MAX_VALUE - var bestPos = 0 + var bestId: String? = null var placeAfter = false for (i in 0 until childCount) { val proposedTarget = getChildAt(i)!! - val xDiff = x - (proposedTarget.x + proposedTarget.width / 2) - val yDiff = y - (proposedTarget.y + proposedTarget.height / 2) - val dist = abs(xDiff) + abs(yDiff) - if (dist < bestDist) { - bestDist = dist - bestPos = getChildAdapterPosition(proposedTarget) - val modifier = (xDiff * xOffset) + (yDiff * yOffset) - placeAfter = (modifier > 0) + if (proposedTarget is TabViewHolder) { + val targetTabId = (proposedTarget as TabViewHolder).tab?.id + val xDiff = x - (proposedTarget.x + proposedTarget.width / 2) + val yDiff = y - (proposedTarget.y + proposedTarget.height / 2) + val dist = abs(xDiff) + abs(yDiff) + if (dist < bestDist) { + bestDist = dist + bestId = targetTabId + val modifier = (xDiff * xOffset) + (yDiff * yOffset) + placeAfter = (modifier > 0) + } } } - return Pair(bestPos, placeAfter) + return Pair(bestId, placeAfter) } private val dragListen = OnDragListener { _, event -> when (event.action) { @@ -120,10 +124,9 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( DragEvent.ACTION_DROP -> { val target = getDropPosition(event.x, event.y) if (target != null) { - val (targetPos, placeAfter) = target - val filter = tabsFeature.defaultTabsFilter + val (targetId, placeAfter) = target @Suppress("UNCHECKED_CAST") // Cast is checked on drag start - interactor.onTabsMove(event.localState as Collection, targetPos, placeAfter, filter) + interactor.onTabsMove(event.localState as Collection, targetId, placeAfter) } true } From 3b8748375b48c3966d9d0c041bfddaf10d5a7005 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Tue, 21 Sep 2021 15:34:10 -0700 Subject: [PATCH 07/38] Fix to do direct tab ID and use grid setting directly --- .../browser/AbstractBrowserTrayList.kt | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) 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 87172ff7d..c71490170 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 @@ -79,27 +79,22 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( // but should work perfectly for lists and grids. private fun getDropPosition(x: Float, y: Float): Pair? { if (childCount < 2) return null // If there's 0 or 1 tabs visible, can't reorder - val first = getChildAt(0)!! - val second = getChildAt(1)!! - val xOffset = second.x - first.x - val yOffset = second.y - first.y - + val isGrid = context.components.settings.gridTabView var bestDist = Float.MAX_VALUE var bestId: String? = null var placeAfter = false for (i in 0 until childCount) { + val targetHolder = findViewHolderForAdapterPosition(i)!! val proposedTarget = getChildAt(i)!! - if (proposedTarget is TabViewHolder) { - val targetTabId = (proposedTarget as TabViewHolder).tab?.id - val xDiff = x - (proposedTarget.x + proposedTarget.width / 2) - val yDiff = y - (proposedTarget.y + proposedTarget.height / 2) - val dist = abs(xDiff) + abs(yDiff) - if (dist < bestDist) { - bestDist = dist - bestId = targetTabId - val modifier = (xDiff * xOffset) + (yDiff * yOffset) - placeAfter = (modifier > 0) - } + val xDiff = x - (proposedTarget.x + proposedTarget.width / 2) + val yDiff = y - (proposedTarget.y + proposedTarget.height / 2) + val dist = abs(xDiff) + abs(yDiff) + if (dist < bestDist && targetHolder is TabViewHolder) { + val targetTabId = targetHolder.tab?.id + bestDist = dist + bestId = targetTabId + val modifier = if (isGrid) xDiff else yDiff + placeAfter = (modifier > 0) } } return Pair(bestId, placeAfter) From 92d3208087cb8dc1da96253f7e6809c10079d2a2 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Tue, 21 Sep 2021 16:02:38 -0700 Subject: [PATCH 08/38] Remove non-null assertion. Now fully works for "other" tabs. --- .../mozilla/fenix/tabstray/browser/AbstractBrowserTrayList.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c71490170..937b7b910 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 @@ -84,7 +84,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( var bestId: String? = null var placeAfter = false for (i in 0 until childCount) { - val targetHolder = findViewHolderForAdapterPosition(i)!! + val targetHolder = findViewHolderForAdapterPosition(i) val proposedTarget = getChildAt(i)!! val xDiff = x - (proposedTarget.x + proposedTarget.width / 2) val yDiff = y - (proposedTarget.y + proposedTarget.height / 2) From ea082b3a494beab707ee9100842865e7a94aa67b Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Wed, 22 Sep 2021 17:49:10 -0700 Subject: [PATCH 09/38] Prevent grouped tabs from being dragged --- .../fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 4ce972d7d..dfb17a4d8 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 @@ -227,7 +227,7 @@ abstract class AbstractBrowserTabViewHolder( metrics.track(Event.CollectionTabLongPressed) interactor.select(item) true - } else if (holder.selectedItems.contains(item)) { + } else if (holder.selectedItems.contains(item) && itemView.parent is NormalBrowserTrayList) { val shadow = View.DragShadowBuilder(itemView) itemView.startDragAndDrop(null, shadow, holder.selectedItems, 0) true From 0c3513ef70e4fe03ea0402eb46e6d3406faa07f5 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Wed, 22 Sep 2021 17:58:24 -0700 Subject: [PATCH 10/38] Remove unused import --- .../main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt | 1 - 1 file changed, 1 deletion(-) 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 35f029430..6d1403eb0 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt @@ -4,7 +4,6 @@ package org.mozilla.fenix.tabstray -import mozilla.components.browser.state.state.TabSessionState import mozilla.components.concept.tabstray.Tab interface TabsTrayInteractor { From 64020863d5ebabb03e885836eed6c2cdf56ffe02 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Thu, 23 Sep 2021 12:36:26 -0700 Subject: [PATCH 11/38] Add/fix comments --- .../mozilla/fenix/tabstray/TabsTrayController.kt | 14 ++++++-------- .../mozilla/fenix/tabstray/TabsTrayInteractor.kt | 3 +-- .../browser/AbstractBrowserTabViewHolder.kt | 3 +++ .../tabstray/browser/AbstractBrowserTrayList.kt | 9 ++++----- 4 files changed, 14 insertions(+), 15 deletions(-) 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 e57ced78c..1f00251b6 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt @@ -61,12 +61,11 @@ interface TabsTrayController { fun handleMultipleTabsDeletion(tabs: Collection) /** - * Moves tabs to a new position + * Moves [tabs] to before/after the target * * @param tabs The tabs to be moved - * @param targetPos Place the moved tabs next to the target position - * @param placeAfter Place the moved tabs before or after the target - * @param filter Filter the full tab list to whatever the displayed tabs are to find the target + * @param targetId The id of the tab that the [tabs] will be placed next to + * @param placeAfter Place [tabs] before or after the target */ fun handleTabsMove(tabs: Collection, targetId: String?, placeAfter: Boolean) @@ -178,12 +177,11 @@ class DefaultTabsTrayController( } /** - * Moves a set of tabs to a position + * Moves [tabs] to before/after the target * * @param tabs The tabs to be moved - * @param targetPos The position to move the tabs to in the displayed list - * @param placeAfter Place the tabs before or after the target - * @param filter The filter to go from the full tab list to the displayed list + * @param targetId The id of the tab that the [tabs] will be placed next to + * @param placeAfter Place [tabs] before or after the target */ override fun handleTabsMove( tabs: Collection, 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 6d1403eb0..086099d62 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt @@ -36,8 +36,7 @@ interface TabsTrayInteractor { fun onInactiveDebugClicked(tabs: Collection) /** - * Invoked when [tabs] should be moved to before/after position [targetPos] from a drag-drop operation - * Filter modifies [targetPos] in the controller. + * Invoked when [tabs] should be moved to before/after [targetId] from a drag-drop operation */ fun onTabsMove( tabs: Collection, 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 dfb17a4d8..d2661f946 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 @@ -228,6 +228,9 @@ abstract class AbstractBrowserTabViewHolder( interactor.select(item) true } else if (holder.selectedItems.contains(item) && itemView.parent is NormalBrowserTrayList) { + // Only allow a drag to start if the tab is a non-grouped tab (parent is normal list) + // Non-grouped tabs can be selected, but they can't be drop targets + // so it's useless to try to move them (and they're auto-sorted anyway) val shadow = View.DragShadowBuilder(itemView) itemView.startDragAndDrop(null, shadow, holder.selectedItems, 0) true 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 937b7b910..09b7e42fc 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 @@ -72,11 +72,10 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( this.setOnDragListener(null) } - // Find the closest item to the x/y position. - // Then, based on the offset between the first and second - // determine if it goes before or after that item. - // This will fail if there's a spiral layout or something - // but should work perfectly for lists and grids. + // Find the closest item to the x/y position of the drop. + // This gets all the children of the tab tray, which will not include grouped tabs + // Since those are children of the group which is a child of the tab tray. + // Determine if the drop is before or after based on the current grid/list view setting private fun getDropPosition(x: Float, y: Float): Pair? { if (childCount < 2) return null // If there's 0 or 1 tabs visible, can't reorder val isGrid = context.components.settings.gridTabView From 3fd6f62209e6f38094e1430f4fb5ef5f6e6bdd58 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Thu, 23 Sep 2021 12:45:07 -0700 Subject: [PATCH 12/38] Do API version check and use deprecated startDrag if too old. --- .../fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 d2661f946..6fe8f9256 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,6 +4,7 @@ package org.mozilla.fenix.tabstray.browser +import android.os.Build import android.view.View import android.widget.ImageButton import android.widget.ImageView @@ -232,7 +233,11 @@ abstract class AbstractBrowserTabViewHolder( // Non-grouped tabs can be selected, but they can't be drop targets // so it's useless to try to move them (and they're auto-sorted anyway) val shadow = View.DragShadowBuilder(itemView) - itemView.startDragAndDrop(null, shadow, holder.selectedItems, 0) + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + itemView.startDragAndDrop(null, shadow, holder.selectedItems, 0) + } else { + itemView.startDrag(null, shadow, holder.selectedItems, 0) + } true } else { false From a0c0547a6442a50b4230a2772ba7a17f28f1cd4f Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Thu, 23 Sep 2021 13:04:26 -0700 Subject: [PATCH 13/38] Build process fails: both outdated and too new, so reverting to just too new --- .../fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) 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 6fe8f9256..d2661f946 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,7 +4,6 @@ package org.mozilla.fenix.tabstray.browser -import android.os.Build import android.view.View import android.widget.ImageButton import android.widget.ImageView @@ -233,11 +232,7 @@ abstract class AbstractBrowserTabViewHolder( // Non-grouped tabs can be selected, but they can't be drop targets // so it's useless to try to move them (and they're auto-sorted anyway) val shadow = View.DragShadowBuilder(itemView) - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - itemView.startDragAndDrop(null, shadow, holder.selectedItems, 0) - } else { - itemView.startDrag(null, shadow, holder.selectedItems, 0) - } + itemView.startDragAndDrop(null, shadow, holder.selectedItems, 0) true } else { false From 97498b2df752b7985790af0e8d0c505ea9a877fd Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Thu, 23 Sep 2021 20:48:50 -0700 Subject: [PATCH 14/38] Use deprecated function and suppress warning --- .../fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 d2661f946..5c17788eb 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 @@ -232,7 +232,9 @@ abstract class AbstractBrowserTabViewHolder( // Non-grouped tabs can be selected, but they can't be drop targets // so it's useless to try to move them (and they're auto-sorted anyway) val shadow = View.DragShadowBuilder(itemView) - itemView.startDragAndDrop(null, shadow, holder.selectedItems, 0) + @Suppress("DEPRECATION") + itemView.startDrag(null, shadow, holder.selectedItems, 0) + //startDragAndDrop is the non-deprecated version, but requires API 24 true } else { false From 7f1b63f54a36d98fafcfa0614b0e0702426f77bf Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Thu, 23 Sep 2021 21:00:05 -0700 Subject: [PATCH 15/38] fix space --- .../fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5c17788eb..f884920f1 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 @@ -234,7 +234,7 @@ abstract class AbstractBrowserTabViewHolder( val shadow = View.DragShadowBuilder(itemView) @Suppress("DEPRECATION") itemView.startDrag(null, shadow, holder.selectedItems, 0) - //startDragAndDrop is the non-deprecated version, but requires API 24 + // startDragAndDrop is the non-deprecated version, but requires API 24 true } else { false From 52598370650c68da0ea3bf898f78a5979b260c44 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Fri, 1 Oct 2021 13:26:12 -0700 Subject: [PATCH 16/38] Suppress "TooManyFunctions" on DefaultTabsTrayController --- .../main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt | 1 + 1 file changed, 1 insertion(+) 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 c70bb108b..4ce4d40ad 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt @@ -93,6 +93,7 @@ interface TabsTrayController { fun handleDeleteAllInactiveTabs() } +@Suppress("TooManyFunctions") class DefaultTabsTrayController( private val trayStore: TabsTrayStore, private val browserStore: BrowserStore, From bff73b6a76d9cde206b9e83ec02bd38cd14c4d12 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Wed, 3 Nov 2021 17:34:03 -0700 Subject: [PATCH 17/38] Repeatedly update tab movement during drag --- .../fenix/tabstray/browser/AbstractBrowserTrayList.kt | 6 ++++++ 1 file changed, 6 insertions(+) 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 310180225..c25c8b581 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 @@ -110,6 +110,12 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( true } DragEvent.ACTION_DRAG_LOCATION -> { + val target = getDropPosition(event.x, event.y) + if (target != null) { + val (targetId, placeAfter) = target + @Suppress("UNCHECKED_CAST") // Cast is checked on drag start + interactor.onTabsMove(event.localState as Collection, targetId, placeAfter) + } true } DragEvent.ACTION_DRAG_EXITED -> { From 0a5a60fae7a760f276206d47fc9117422ae4cbc0 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Thu, 4 Nov 2021 20:54:18 -0700 Subject: [PATCH 18/38] 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 --- .../fenix/tabstray/TabsTrayController.kt | 24 ++++----- .../fenix/tabstray/TabsTrayInteractor.kt | 11 ++-- .../browser/AbstractBrowserTabViewHolder.kt | 8 +-- .../browser/AbstractBrowserTrayList.kt | 52 +++++++++---------- .../tabstray/browser/BrowserTabViewHolder.kt | 13 +++-- .../tabstray/browser/BrowserTabsAdapter.kt | 7 ++- 6 files changed, 58 insertions(+), 57 deletions(-) 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 9477566b7..3e0c3e410 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt @@ -60,13 +60,12 @@ interface TabsTrayController { fun handleMultipleTabsDeletion(tabs: Collection) /** - * Moves [tabs] to before/after the target + * Moves [tabId] to replace position of [targetId] * - * @param tabs The tabs to be moved - * @param targetId The id of the tab that the [tabs] will be placed next to - * @param placeAfter Place [tabs] before or after the target + * @param tabId The tabs to be moved + * @param targetId The id of the tab that the [tab] will replace */ - fun handleTabsMove(tabs: Collection, targetId: String?, placeAfter: Boolean) + fun handleTabsMove(tabId: String, targetId: String?) /** * Navigate from TabsTray to Recently Closed section in the History fragment. @@ -182,20 +181,17 @@ class DefaultTabsTrayController( } /** - * Moves [tabs] to before/after the target + * Moves [tabId] to replace position of [targetId] * - * @param tabs The tabs to be moved - * @param targetId The id of the tab that the [tabs] will be placed next to - * @param placeAfter Place [tabs] before or after the target + * @param tabId The tabs to be moved + * @param targetId The id of the tab that the [tab] will replace */ override fun handleTabsMove( - tabs: Collection, + tabId: String, targetId: String?, - placeAfter: Boolean, ) { - val tabIDs = tabs.map { it.id } - if (targetId != null) { - tabsUseCases.moveTabs(tabIDs, targetId, placeAfter) + if (targetId != null && tabId != targetId) { + tabsUseCases.moveTabs(tabId, targetId) } } 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 9ea0eaf3f..d6db8963d 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt @@ -36,12 +36,12 @@ interface TabsTrayInteractor { fun onInactiveDebugClicked(tabs: Collection) /** - * Invoked when [tabs] should be moved to before/after [targetId] from a drag-drop operation + * Invoked when [tabId] should be moved replace [targetId]'s position + * due to a drag-drop operation */ fun onTabsMove( - tabs: Collection, + tabId: String, targetId: String?, - placeAfter: Boolean, ) /** @@ -75,11 +75,10 @@ class DefaultTabsTrayInteractor( } override fun onTabsMove( - tabs: Collection, + tabId: String, targetId: String?, - placeAfter: Boolean, ) { - controller.handleTabsMove(tabs, targetId, placeAfter) + controller.handleTabsMove(tabId, targetId) } override fun onInactiveDebugClicked(tabs: Collection) { 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 c765bc834..65779e4e0 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 @@ -31,6 +31,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.utils.Settings import org.mozilla.fenix.ext.showAndEnable import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.selection.SelectionHolder @@ -56,8 +57,9 @@ abstract class AbstractBrowserTabViewHolder( private val selectionHolder: SelectionHolder?, @VisibleForTesting internal val featureName: String, + private val settings: Settings?, private val store: BrowserStore = itemView.context.components.core.store, - private val metrics: MetricController = itemView.context.components.analytics.metrics + private val metrics: MetricController = itemView.context.components.analytics.metrics, ) : TabViewHolder(itemView) { private val faviconView: ImageView? = @@ -222,13 +224,13 @@ abstract class AbstractBrowserTabViewHolder( metrics.track(Event.CollectionTabLongPressed) interactor.select(item) true - } else if (holder.selectedItems.contains(item) && itemView.parent is NormalBrowserTrayList) { + } else if (holder.selectedItems.contains(item) && settings?.searchTermTabGroupsAreEnabled == false) { // Only allow a drag to start if the tab is a non-grouped tab (parent is normal list) // Non-grouped tabs can be selected, but they can't be drop targets // so it's useless to try to move them (and they're auto-sorted anyway) val shadow = View.DragShadowBuilder(itemView) @Suppress("DEPRECATION") - itemView.startDrag(null, shadow, holder.selectedItems, 0) + itemView.startDrag(null, shadow, item, 0) // startDragAndDrop is the non-deprecated version, but requires API 24 true } else { 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 c25c8b581..c9d52f976 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,6 +5,7 @@ package org.mozilla.fenix.tabstray.browser import android.content.Context +import android.graphics.Rect import android.util.AttributeSet import android.view.DragEvent import androidx.recyclerview.widget.RecyclerView @@ -73,38 +74,35 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( } // Find the closest item to the x/y position of the drop. - // This gets all the children of the tab tray, which will not include grouped tabs - // Since those are children of the group which is a child of the tab tray. - // Determine if the drop is before or after based on the current grid/list view setting - private fun getDropPosition(x: Float, y: Float): Pair? { + private fun getDropPosition(x: Float, y: Float): String? { if (childCount < 2) return null // If there's 0 or 1 tabs visible, can't reorder - val isGrid = context.components.settings.gridTabView + if (layoutManager == null) return null + val lm = layoutManager!! var bestDist = Float.MAX_VALUE var bestId: String? = null - var placeAfter = false for (i in 0 until childCount) { - val targetHolder = findViewHolderForAdapterPosition(i) - val proposedTarget = getChildAt(i)!! - val xDiff = x - (proposedTarget.x + proposedTarget.width / 2) - val yDiff = y - (proposedTarget.y + proposedTarget.height / 2) - val dist = abs(xDiff) + abs(yDiff) - if (dist < bestDist && targetHolder is TabViewHolder) { - val targetTabId = targetHolder.tab?.id - bestDist = dist - bestId = targetTabId - val modifier = if (isGrid) xDiff else yDiff - placeAfter = (modifier > 0) + val proposedTarget = getChildAt(i) + val targetHolder = findContainingViewHolder(proposedTarget) + if (targetHolder is TabViewHolder) { + var rect = Rect() // Use layoutManager to get post-animation positioning + lm.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) + if (dist < bestDist) { + bestDist = dist + bestId = targetHolder.tab?.id + } } } - return Pair(bestId, placeAfter) + return bestId } private val dragListen = OnDragListener { _, event -> when (event.action) { DragEvent.ACTION_DRAG_STARTED -> { - // This check is required for the unchecked cast later on - if (event.localState is Collection<*>) { - (event.localState as Collection<*>).all { it is TabSessionState } - } else false + (event.localState is TabSessionState) } DragEvent.ACTION_DRAG_ENTERED -> { true @@ -112,9 +110,8 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( DragEvent.ACTION_DRAG_LOCATION -> { val target = getDropPosition(event.x, event.y) if (target != null) { - val (targetId, placeAfter) = target - @Suppress("UNCHECKED_CAST") // Cast is checked on drag start - interactor.onTabsMove(event.localState as Collection, targetId, placeAfter) + val source = (event.localState as TabSessionState) + interactor.onTabsMove(source.id, target) } true } @@ -124,9 +121,8 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( DragEvent.ACTION_DROP -> { val target = getDropPosition(event.x, event.y) if (target != null) { - val (targetId, placeAfter) = target - @Suppress("UNCHECKED_CAST") // Cast is checked on drag start - interactor.onTabsMove(event.localState as Collection, targetId, placeAfter) + val source = (event.localState as TabSessionState) + interactor.onTabsMove(source.id, target) } true } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabViewHolder.kt index ca0ae0405..354b79ba1 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabViewHolder.kt @@ -16,6 +16,7 @@ import mozilla.components.concept.base.images.ImageLoader import org.mozilla.fenix.R import org.mozilla.fenix.databinding.TabTrayGridItemBinding import org.mozilla.fenix.ext.increaseTapArea +import org.mozilla.fenix.utils.Settings import kotlin.math.max import org.mozilla.fenix.selection.SelectionHolder import org.mozilla.fenix.tabstray.TabsTrayStore @@ -32,14 +33,16 @@ sealed class BrowserTabViewHolder(itemView: View) : RecyclerView.ViewHolder(item * @param itemView [View] that displays a "tab". * @param featureName [String] representing the name of the feature displaying tabs. Used in telemetry reporting. */ + @Suppress("LongParameterList") class GridViewHolder( imageLoader: ImageLoader, override val browserTrayInteractor: BrowserTrayInteractor, store: TabsTrayStore, selectionHolder: SelectionHolder? = null, itemView: View, - featureName: String - ) : AbstractBrowserTabViewHolder(itemView, imageLoader, store, selectionHolder, featureName) { + featureName: String, + settings: Settings? = null + ) : AbstractBrowserTabViewHolder(itemView, imageLoader, store, selectionHolder, featureName, settings) { private val closeButton: AppCompatImageButton = itemView.findViewById(R.id.mozac_browser_tabstray_close) @@ -86,14 +89,16 @@ sealed class BrowserTabViewHolder(itemView: View) : RecyclerView.ViewHolder(item * @param itemView [View] that displays a "tab". * @param featureName [String] representing the name of the feature displaying tabs. Used in telemetry reporting. */ + @Suppress("LongParameterList") class ListViewHolder( imageLoader: ImageLoader, override val browserTrayInteractor: BrowserTrayInteractor, store: TabsTrayStore, selectionHolder: SelectionHolder? = null, itemView: View, - featureName: String - ) : AbstractBrowserTabViewHolder(itemView, imageLoader, store, selectionHolder, featureName) { + featureName: String, + settings: Settings? = null + ) : AbstractBrowserTabViewHolder(itemView, imageLoader, store, selectionHolder, featureName, settings) { override val thumbnailSize: Int get() = max( itemView.resources.getDimensionPixelSize(R.dimen.tab_tray_list_item_thumbnail_height), diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt index 6b5d57019..2f0fabaa4 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt @@ -17,6 +17,7 @@ import org.mozilla.fenix.components.Components import org.mozilla.fenix.databinding.TabTrayGridItemBinding import org.mozilla.fenix.databinding.TabTrayItemBinding import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.settings import org.mozilla.fenix.selection.SelectionHolder import org.mozilla.fenix.tabstray.TabsTrayStore @@ -67,9 +68,11 @@ class BrowserTabsAdapter( return when (viewType) { ViewType.GRID.layoutRes -> - BrowserTabViewHolder.GridViewHolder(imageLoader, interactor, store, selectionHolder, view, featureName) + BrowserTabViewHolder.GridViewHolder(imageLoader, interactor, store, + selectionHolder, view, featureName, context.settings()) else -> - BrowserTabViewHolder.ListViewHolder(imageLoader, interactor, store, selectionHolder, view, featureName) + BrowserTabViewHolder.ListViewHolder(imageLoader, interactor, store, + selectionHolder, view, featureName, context.settings()) } } From f3bfca7218ed18a5f5bf1aa4eeb01e33ec1cbaa5 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Thu, 4 Nov 2021 21:07:03 -0700 Subject: [PATCH 19/38] Remove settings argument and corresponding long args suppression: instead get settings from parent AbstractBrowserTrayList's context --- .../browser/AbstractBrowserTabViewHolder.kt | 20 +++++++++---------- .../tabstray/browser/BrowserTabViewHolder.kt | 11 +++------- .../tabstray/browser/BrowserTabsAdapter.kt | 6 ++---- 3 files changed, 14 insertions(+), 23 deletions(-) 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 65779e4e0..dc169cf36 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 @@ -31,7 +31,6 @@ 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.utils.Settings import org.mozilla.fenix.ext.showAndEnable import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.selection.SelectionHolder @@ -57,7 +56,6 @@ abstract class AbstractBrowserTabViewHolder( private val selectionHolder: SelectionHolder?, @VisibleForTesting internal val featureName: String, - private val settings: Settings?, private val store: BrowserStore = itemView.context.components.core.store, private val metrics: MetricController = itemView.context.components.analytics.metrics, ) : TabViewHolder(itemView) { @@ -224,15 +222,15 @@ abstract class AbstractBrowserTabViewHolder( metrics.track(Event.CollectionTabLongPressed) interactor.select(item) true - } else if (holder.selectedItems.contains(item) && settings?.searchTermTabGroupsAreEnabled == false) { - // Only allow a drag to start if the tab is a non-grouped tab (parent is normal list) - // Non-grouped tabs can be selected, but they can't be drop targets - // so it's useless to try to move them (and they're auto-sorted anyway) - val shadow = View.DragShadowBuilder(itemView) - @Suppress("DEPRECATION") - itemView.startDrag(null, shadow, item, 0) - // startDragAndDrop is the non-deprecated version, but requires API 24 - true + } else if (holder.selectedItems.contains(item)) { + val parent = itemView.parent as? AbstractBrowserTrayList + if (parent?.context?.settings()?.searchTermTabGroupsAreEnabled == false) { + val shadow = View.DragShadowBuilder(itemView) + @Suppress("DEPRECATION") + itemView.startDrag(null, shadow, item, 0) + // startDragAndDrop is the non-deprecated version, but requires API 24 + true + } else false } else { false } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabViewHolder.kt index 354b79ba1..953511885 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabViewHolder.kt @@ -16,7 +16,6 @@ import mozilla.components.concept.base.images.ImageLoader import org.mozilla.fenix.R import org.mozilla.fenix.databinding.TabTrayGridItemBinding import org.mozilla.fenix.ext.increaseTapArea -import org.mozilla.fenix.utils.Settings import kotlin.math.max import org.mozilla.fenix.selection.SelectionHolder import org.mozilla.fenix.tabstray.TabsTrayStore @@ -33,7 +32,6 @@ sealed class BrowserTabViewHolder(itemView: View) : RecyclerView.ViewHolder(item * @param itemView [View] that displays a "tab". * @param featureName [String] representing the name of the feature displaying tabs. Used in telemetry reporting. */ - @Suppress("LongParameterList") class GridViewHolder( imageLoader: ImageLoader, override val browserTrayInteractor: BrowserTrayInteractor, @@ -41,8 +39,7 @@ sealed class BrowserTabViewHolder(itemView: View) : RecyclerView.ViewHolder(item selectionHolder: SelectionHolder? = null, itemView: View, featureName: String, - settings: Settings? = null - ) : AbstractBrowserTabViewHolder(itemView, imageLoader, store, selectionHolder, featureName, settings) { + ) : AbstractBrowserTabViewHolder(itemView, imageLoader, store, selectionHolder, featureName) { private val closeButton: AppCompatImageButton = itemView.findViewById(R.id.mozac_browser_tabstray_close) @@ -89,16 +86,14 @@ sealed class BrowserTabViewHolder(itemView: View) : RecyclerView.ViewHolder(item * @param itemView [View] that displays a "tab". * @param featureName [String] representing the name of the feature displaying tabs. Used in telemetry reporting. */ - @Suppress("LongParameterList") class ListViewHolder( imageLoader: ImageLoader, override val browserTrayInteractor: BrowserTrayInteractor, store: TabsTrayStore, selectionHolder: SelectionHolder? = null, itemView: View, - featureName: String, - settings: Settings? = null - ) : AbstractBrowserTabViewHolder(itemView, imageLoader, store, selectionHolder, featureName, settings) { + featureName: String + ) : AbstractBrowserTabViewHolder(itemView, imageLoader, store, selectionHolder, featureName) { override val thumbnailSize: Int get() = max( itemView.resources.getDimensionPixelSize(R.dimen.tab_tray_list_item_thumbnail_height), diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt index 2f0fabaa4..9b3962704 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt @@ -68,11 +68,9 @@ class BrowserTabsAdapter( return when (viewType) { ViewType.GRID.layoutRes -> - BrowserTabViewHolder.GridViewHolder(imageLoader, interactor, store, - selectionHolder, view, featureName, context.settings()) + BrowserTabViewHolder.GridViewHolder(imageLoader, interactor, store, selectionHolder, view, featureName) else -> - BrowserTabViewHolder.ListViewHolder(imageLoader, interactor, store, - selectionHolder, view, featureName, context.settings()) + BrowserTabViewHolder.ListViewHolder(imageLoader, interactor, store, selectionHolder, view, featureName) } } From ae79fc327fa7a22ff9734ad1546d217b33c68a11 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Fri, 5 Nov 2021 01:14:15 -0700 Subject: [PATCH 20/38] New UI: Select a tab and then, while holding down, start dragging --- .../browser/AbstractBrowserTabViewHolder.kt | 36 ++++++++++++++----- .../browser/AbstractBrowserTrayList.kt | 21 +++++++++-- 2 files changed, 45 insertions(+), 12 deletions(-) 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 dc169cf36..94340728c 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,6 +4,8 @@ package org.mozilla.fenix.tabstray.browser +import android.annotation.SuppressLint +import android.view.MotionEvent import android.view.View import android.widget.ImageButton import android.widget.ImageView @@ -31,6 +33,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 @@ -201,6 +204,7 @@ abstract class AbstractBrowserTabViewHolder( imageLoader.loadIntoView(thumbnailView, ImageLoadRequest(id, thumbnailSize)) } + @SuppressLint("ClickableViewAccessibility") private fun setSelectionInteractor( item: TabSessionState, holder: SelectionHolder, @@ -222,22 +226,36 @@ abstract class AbstractBrowserTabViewHolder( metrics.track(Event.CollectionTabLongPressed) interactor.select(item) true - } else if (holder.selectedItems.contains(item)) { - val parent = itemView.parent as? AbstractBrowserTrayList - if (parent?.context?.settings()?.searchTermTabGroupsAreEnabled == false) { - val shadow = View.DragShadowBuilder(itemView) - @Suppress("DEPRECATION") - itemView.startDrag(null, shadow, item, 0) - // startDragAndDrop is the non-deprecated version, but requires API 24 - true - } else false } else { false } } + // 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_MOVE -> if (holder.selectedItems.contains(item)) { + val parent = itemView.parent as? AbstractBrowserTrayList + if (parent?.context?.settings()?.searchTermTabGroupsAreEnabled == false) { + for (tabSelected in holder.selectedItems) { + // Exit selection mode by deselecting everything + interactor.deselect(tabSelected) + } + val shadow = View.DragShadowBuilder(itemView) + // startDragAndDrop is the non-deprecated version, but requires API 24 + @Suppress("DEPRECATION") + itemView.startDrag(null, shadow, item, 0) + view.alpha = DRAG_TRANSPARENCY // Make the dragged tab mostly invisible + } + } + else -> view.onTouchEvent(motionEvent) + } + true + } } companion object { + internal const val DRAG_TRANSPARENCY = 0.2f internal const val PLAY_PAUSE_BUTTON_EXTRA_DPS = 24 internal const val GRID_ITEM_CLOSE_BUTTON_EXTRA_DPS = 24 } 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 c9d52f976..e92ac14c5 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 @@ -8,6 +8,7 @@ import android.content.Context import android.graphics.Rect import android.util.AttributeSet import android.view.DragEvent +import android.view.View import androidx.recyclerview.widget.RecyclerView import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.tabstray.TabViewHolder @@ -76,8 +77,6 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( // Find the closest item to the x/y position of the drop. private fun getDropPosition(x: Float, y: Float): String? { if (childCount < 2) return null // If there's 0 or 1 tabs visible, can't reorder - if (layoutManager == null) return null - val lm = layoutManager!! var bestDist = Float.MAX_VALUE var bestId: String? = null for (i in 0 until childCount) { @@ -85,7 +84,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( val targetHolder = findContainingViewHolder(proposedTarget) if (targetHolder is TabViewHolder) { var rect = Rect() // Use layoutManager to get post-animation positioning - lm.getDecoratedBoundsWithMargins(proposedTarget, rect) + getDecoratedBoundsWithMargins(proposedTarget, rect) val targetX = (rect.left + rect.right) / 2 val targetY = (rect.top + rect.bottom) / 2 val xDiff = x - targetX @@ -99,6 +98,16 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( } return bestId } + private fun findSourceView(id: String): View? { + for (i in 0 until childCount) { + val proposed = getChildAt(i) + val targetHolder = findContainingViewHolder(proposed) + if (targetHolder is TabViewHolder && targetHolder.tab?.id == id) { + return proposed + } + } + return null + } private val dragListen = OnDragListener { _, event -> when (event.action) { DragEvent.ACTION_DRAG_STARTED -> { @@ -127,6 +136,12 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( true } DragEvent.ACTION_DRAG_ENDED -> { + // Revert the invisibility + val id = (event.localState as TabSessionState).id + val sourceView = findSourceView(id) + if (sourceView != null) { + sourceView.alpha = 1.0f + } true } else -> { // Unknown action From 9d371f7fd57449f72a309dfb5698a95be9896dc5 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Fri, 5 Nov 2021 03:00:47 -0700 Subject: [PATCH 21/38] Revert to using before/after boolean to accomodate delays Move drag transparency to start of drag --- .../fenix/tabstray/TabsTrayController.kt | 15 ++++--- .../fenix/tabstray/TabsTrayInteractor.kt | 7 +-- .../browser/AbstractBrowserTabViewHolder.kt | 2 - .../browser/AbstractBrowserTrayList.kt | 43 +++++++++++++------ 4 files changed, 42 insertions(+), 25 deletions(-) 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 3e0c3e410..3da2c904b 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt @@ -60,12 +60,13 @@ interface TabsTrayController { fun handleMultipleTabsDeletion(tabs: Collection) /** - * Moves [tabId] to replace position of [targetId] + * 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 replace + * @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?) + fun handleTabsMove(tabId: String, targetId: String?, placeAfter: Boolean) /** * Navigate from TabsTray to Recently Closed section in the History fragment. @@ -181,17 +182,19 @@ class DefaultTabsTrayController( } /** - * Moves [tabId] to replace position of [targetId] + * 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 replace + * @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(tabId, targetId) + tabsUseCases.moveTabs(listOf(tabId), targetId, placeAfter) } } 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 d6db8963d..b78933fba 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt @@ -36,12 +36,12 @@ interface TabsTrayInteractor { fun onInactiveDebugClicked(tabs: Collection) /** - * Invoked when [tabId] should be moved replace [targetId]'s position - * due to a drag-drop operation + * Invoked when [tabId] should be moved to before/after [targetId] from a drag-drop operation */ fun onTabsMove( tabId: String, targetId: String?, + placeAfter: Boolean ) /** @@ -77,8 +77,9 @@ class DefaultTabsTrayInteractor( override fun onTabsMove( tabId: String, targetId: String?, + placeAfter: Boolean ) { - controller.handleTabsMove(tabId, targetId) + controller.handleTabsMove(tabId, targetId, placeAfter) } override fun onInactiveDebugClicked(tabs: Collection) { 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 94340728c..ad74490c2 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 @@ -245,7 +245,6 @@ abstract class AbstractBrowserTabViewHolder( // startDragAndDrop is the non-deprecated version, but requires API 24 @Suppress("DEPRECATION") itemView.startDrag(null, shadow, item, 0) - view.alpha = DRAG_TRANSPARENCY // Make the dragged tab mostly invisible } } else -> view.onTouchEvent(motionEvent) @@ -255,7 +254,6 @@ abstract class AbstractBrowserTabViewHolder( } companion object { - internal const val DRAG_TRANSPARENCY = 0.2f internal const val PLAY_PAUSE_BUTTON_EXTRA_DPS = 24 internal const val GRID_ITEM_CLOSE_BUTTON_EXTRA_DPS = 24 } 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 e92ac14c5..5e5dc3eb3 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 @@ -75,28 +75,31 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( } // Find the closest item to the x/y position of the drop. - private fun getDropPosition(x: Float, y: Float): String? { + private fun getDropPosition(x: Float, y: Float, source: String): Pair? { if (childCount < 2) return null // If there's 0 or 1 tabs visible, can't reorder var bestDist = Float.MAX_VALUE - var bestId: String? = null + var bestOut: Pair? = null + var seenSource = false for (i in 0 until childCount) { val proposedTarget = getChildAt(i) val targetHolder = findContainingViewHolder(proposedTarget) if (targetHolder is TabViewHolder) { - var rect = Rect() // Use layoutManager to get post-animation positioning + var 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) - if (dist < bestDist) { + val id = targetHolder.tab?.id + if (id == source) seenSource = true + if (dist < bestDist && id != null) { bestDist = dist - bestId = targetHolder.tab?.id + bestOut = Pair(id, seenSource) } } } - return bestId + return bestOut } private fun findSourceView(id: String): View? { for (i in 0 until childCount) { @@ -111,16 +114,24 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( private val dragListen = OnDragListener { _, event -> when (event.action) { DragEvent.ACTION_DRAG_STARTED -> { - (event.localState is TabSessionState) + if (event.localState is TabSessionState) { + val id = (event.localState as TabSessionState).id + val sourceView = findSourceView(id) + if (sourceView != null) { + sourceView.alpha = DRAG_TRANSPARENCY + } + true + } else false } DragEvent.ACTION_DRAG_ENTERED -> { true } DragEvent.ACTION_DRAG_LOCATION -> { - val target = getDropPosition(event.x, event.y) + val source = (event.localState as TabSessionState) + val target = getDropPosition(event.x, event.y, source.id) if (target != null) { - val source = (event.localState as TabSessionState) - interactor.onTabsMove(source.id, target) + val (id, placeAfter) = target + interactor.onTabsMove(source.id, id, placeAfter) } true } @@ -128,10 +139,11 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( true } DragEvent.ACTION_DROP -> { - val target = getDropPosition(event.x, event.y) + val source = (event.localState as TabSessionState) + val target = getDropPosition(event.x, event.y, source.id) if (target != null) { - val source = (event.localState as TabSessionState) - interactor.onTabsMove(source.id, target) + val (id, placeAfter) = target + interactor.onTabsMove(source.id, id, placeAfter) } true } @@ -140,7 +152,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( val id = (event.localState as TabSessionState).id val sourceView = findSourceView(id) if (sourceView != null) { - sourceView.alpha = 1.0f + sourceView.alpha = 1f } true } @@ -149,4 +161,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( } } } + companion object { + internal const val DRAG_TRANSPARENCY = 0.2f + } } From 5a715fda9e2e6f23fe187f3b5902130893b71551 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Tue, 9 Nov 2021 00:55:45 -0800 Subject: [PATCH 22/38] Use new BlankDragShadowBuilder and DraggableItemAnimator to handle tab movement --- .../browser/AbstractBrowserTabViewHolder.kt | 9 +- .../browser/AbstractBrowserTrayList.kt | 114 ++++++++++-------- .../browser/BlankDragShadowBuilder.kt | 22 ++++ .../tabstray/browser/DraggableItemAnimator.kt | 26 ++++ 4 files changed, 117 insertions(+), 54 deletions(-) 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 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 ad74490c2..c9f3ffcdc 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 @@ -5,6 +5,7 @@ package org.mozilla.fenix.tabstray.browser import android.annotation.SuppressLint +import android.graphics.PointF import android.view.MotionEvent import android.view.View import android.widget.ImageButton @@ -79,6 +80,7 @@ abstract class AbstractBrowserTabViewHolder( abstract val thumbnailSize: Int override var tab: TabSessionState? = null + internal var beingDragged: Boolean = false /** * Displays the data of the given session and notifies the given observable about events. @@ -91,6 +93,7 @@ abstract class AbstractBrowserTabViewHolder( delegate: TabsTray.Delegate ) { this.tab = tab + beingDragged = false updateTitle(tab) updateUrl(tab) @@ -241,10 +244,12 @@ abstract class AbstractBrowserTabViewHolder( // Exit selection mode by deselecting everything interactor.deselect(tabSelected) } - val shadow = View.DragShadowBuilder(itemView) + val shadow = BlankDragShadowBuilder() + beingDragged = true + val point = PointF(motionEvent.x, motionEvent.y) // startDragAndDrop is the non-deprecated version, but requires API 24 @Suppress("DEPRECATION") - itemView.startDrag(null, shadow, item, 0) + view.startDrag(null, shadow, Pair(item, point), 0) } } else -> view.onTouchEvent(motionEvent) 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 8c9cb32c7..6b8446678 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,6 +5,7 @@ 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 @@ -12,8 +13,6 @@ import android.view.View import androidx.recyclerview.widget.RecyclerView import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.tabstray.TabViewHolder -import mozilla.components.feature.tabs.tabstray.TabsFeature -import org.mozilla.fenix.ext.components import org.mozilla.fenix.tabstray.TabsTrayInteractor import org.mozilla.fenix.tabstray.TabsTrayStore import kotlin.math.abs @@ -41,6 +40,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( adapter?.onAttachedToRecyclerView(this) this.setOnDragListener(dragListen) + itemAnimator = DraggableItemAnimator() } override fun onDetachedFromWindow() { @@ -80,67 +80,77 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( } return bestOut } - private fun findSourceView(id: String): View? { + private fun findSourceViewAndHolder(id: String): Pair? { for (i in 0 until childCount) { val proposed = getChildAt(i) val targetHolder = findContainingViewHolder(proposed) - if (targetHolder is TabViewHolder && targetHolder.tab?.id == id) { - return proposed + if (targetHolder is AbstractBrowserTabViewHolder && targetHolder.tab?.id == id) { + return Pair(proposed, targetHolder) } } return null } private val dragListen = OnDragListener { _, event -> - when (event.action) { - DragEvent.ACTION_DRAG_STARTED -> { - if (event.localState is TabSessionState) { - val id = (event.localState as TabSessionState).id - val sourceView = findSourceView(id) - if (sourceView != null) { - sourceView.alpha = DRAG_TRANSPARENCY + if (event.localState is Pair<*, *>) { + val (tab, dragOffset) = event.localState as Pair<*, *> + if (tab is TabSessionState && dragOffset is PointF) { + 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, _) = sources + sourceView.elevation += DRAGGED_TAB_ELEVATION + } + true + } + DragEvent.ACTION_DRAG_ENTERED -> { + true + } + DragEvent.ACTION_DRAG_LOCATION -> { + val target = getDropPosition(event.x, event.y, tab.id) + if (target != null) { + val (targetId, placeAfter) = target + interactor.onTabsMove(tab.id, targetId, placeAfter) + } + // Move the tab's visual position + if (sources != null) { + val (sourceView, _) = sources + sourceView.x = event.x - dragOffset.x + sourceView.y = event.y - dragOffset.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 + sourceView.elevation -= DRAGGED_TAB_ELEVATION + sourceView.animate() + .translationX(0f).translationY(0f) + .setDuration(itemAnimator?.moveDuration ?: 0) + + sourceViewHolder.beingDragged = false + } + true + } + else -> { // Unknown action + false } - true - } else false - } - DragEvent.ACTION_DRAG_ENTERED -> { - true - } - DragEvent.ACTION_DRAG_LOCATION -> { - val source = (event.localState as TabSessionState) - val target = getDropPosition(event.x, event.y, source.id) - if (target != null) { - val (id, placeAfter) = target - interactor.onTabsMove(source.id, id, placeAfter) - } - true - } - DragEvent.ACTION_DRAG_EXITED -> { - true - } - DragEvent.ACTION_DROP -> { - val source = (event.localState as TabSessionState) - val target = getDropPosition(event.x, event.y, source.id) - if (target != null) { - val (id, placeAfter) = target - interactor.onTabsMove(source.id, id, placeAfter) - } - true - } - DragEvent.ACTION_DRAG_ENDED -> { - // Revert the invisibility - val id = (event.localState as TabSessionState).id - val sourceView = findSourceView(id) - if (sourceView != null) { - sourceView.alpha = 1f } - true - } - else -> { // Unknown action - false - } - } + } else false + } else false } companion object { - internal const val DRAG_TRANSPARENCY = 0.2f + internal const val DRAGGED_TAB_ELEVATION = 10f } } 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 000000000..565da74bd --- /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 000000000..cbd1153d3 --- /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) + } +} From c4964adda5f467e3f794b71289bcfa15cf26c77a Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Tue, 9 Nov 2021 02:24:33 -0800 Subject: [PATCH 23/38] Replace Pair<>s with data classes --- .../browser/AbstractBrowserTabViewHolder.kt | 4 +- .../browser/AbstractBrowserTrayList.kt | 107 +++++++++--------- .../fenix/tabstray/browser/TabDragData.kt | 10 ++ 3 files changed, 64 insertions(+), 57 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/tabstray/browser/TabDragData.kt 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 c9f3ffcdc..31806e821 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 @@ -61,7 +61,7 @@ abstract class AbstractBrowserTabViewHolder( @VisibleForTesting internal val featureName: String, private val store: BrowserStore = itemView.context.components.core.store, - private val metrics: MetricController = itemView.context.components.analytics.metrics, + private val metrics: MetricController = itemView.context.components.analytics.metrics ) : TabViewHolder(itemView) { private val faviconView: ImageView? = @@ -249,7 +249,7 @@ abstract class AbstractBrowserTabViewHolder( val point = PointF(motionEvent.x, motionEvent.y) // startDragAndDrop is the non-deprecated version, but requires API 24 @Suppress("DEPRECATION") - view.startDrag(null, shadow, Pair(item, point), 0) + view.startDrag(null, shadow, TabDragData(item, point), 0) } } else -> view.onTouchEvent(motionEvent) 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 6b8446678..da5417cb1 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,13 +5,11 @@ 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.RecyclerView -import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.tabstray.TabViewHolder import org.mozilla.fenix.tabstray.TabsTrayInteractor import org.mozilla.fenix.tabstray.TabsTrayStore @@ -54,10 +52,11 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( } // Find the closest item to the x/y position of the drop. - private fun getDropPosition(x: Float, y: Float, source: String): Pair? { + private data class DropPositionData(val id: String, val placeAfter: Boolean) + 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: Pair? = null + var bestOut: DropPositionData? = null var seenSource = false for (i in 0 until childCount) { val proposedTarget = getChildAt(i) @@ -74,7 +73,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( if (id == source) seenSource = true if (dist < bestDist && id != null) { bestDist = dist - bestOut = Pair(id, seenSource) + bestOut = DropPositionData(id, seenSource) } } } @@ -91,63 +90,61 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( return null } private val dragListen = OnDragListener { _, event -> - if (event.localState is Pair<*, *>) { - val (tab, dragOffset) = event.localState as Pair<*, *> - if (tab is TabSessionState && dragOffset is PointF) { - val sourceId = tab.id - val sources = findSourceViewAndHolder(sourceId) + if (event.localState is TabDragData) { + val (tab, dragOffset) = 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, _) = sources - sourceView.elevation += DRAGGED_TAB_ELEVATION - } - true + when (event.action) { + DragEvent.ACTION_DRAG_STARTED -> { + // Put the dragged tab on top of all other tabs + if (sources != null) { + val (sourceView, _) = sources + sourceView.elevation += DRAGGED_TAB_ELEVATION } - DragEvent.ACTION_DRAG_ENTERED -> { - true - } - DragEvent.ACTION_DRAG_LOCATION -> { - val target = getDropPosition(event.x, event.y, tab.id) - if (target != null) { - val (targetId, placeAfter) = target - interactor.onTabsMove(tab.id, targetId, placeAfter) - } - // Move the tab's visual position - if (sources != null) { - val (sourceView, _) = sources - sourceView.x = event.x - dragOffset.x - sourceView.y = event.y - dragOffset.y - } - - true - } - DragEvent.ACTION_DRAG_EXITED -> { - true + true + } + DragEvent.ACTION_DRAG_ENTERED -> { + true + } + DragEvent.ACTION_DRAG_LOCATION -> { + val target = getDropPosition(event.x, event.y, tab.id) + if (target != null) { + val (targetId, placeAfter) = target + interactor.onTabsMove(tab.id, targetId, placeAfter) } - DragEvent.ACTION_DROP -> { - true + // Move the tab's visual position + if (sources != null) { + val (sourceView, sourceViewHolder) = sources + sourceView.x = event.x - dragOffset.x + sourceView.y = event.y - dragOffset.y + sourceViewHolder.beingDragged = 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 - sourceView.elevation -= DRAGGED_TAB_ELEVATION - sourceView.animate() - .translationX(0f).translationY(0f) - .setDuration(itemAnimator?.moveDuration ?: 0) + 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 + sourceView.elevation -= DRAGGED_TAB_ELEVATION + sourceView.animate() + .translationX(0f).translationY(0f) + .setDuration(itemAnimator?.moveDuration ?: 0) - sourceViewHolder.beingDragged = false - } - true - } - else -> { // Unknown action - false + sourceViewHolder.beingDragged = false } + true } - } else false + else -> { // Unknown action + false + } + } } else false } companion object { 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 000000000..811118857 --- /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) From 85c5e08474f885f078118b59566c11bf15d968a2 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Tue, 9 Nov 2021 03:03:42 -0800 Subject: [PATCH 24/38] Only drag if exactly 1 tab selected, don't consume drag event if not used --- .../tabstray/browser/AbstractBrowserTabViewHolder.kt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 31806e821..b6c486d11 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 @@ -237,9 +237,11 @@ abstract class AbstractBrowserTabViewHolder( // The ClickableViewAccessibility warning isn't useful itemView.setOnTouchListener { view, motionEvent -> when (motionEvent.actionMasked) { - MotionEvent.ACTION_MOVE -> if (holder.selectedItems.contains(item)) { + MotionEvent.ACTION_MOVE -> { val parent = itemView.parent as? AbstractBrowserTrayList - if (parent?.context?.settings()?.searchTermTabGroupsAreEnabled == false) { + if (parent?.context?.settings()?.searchTermTabGroupsAreEnabled == false && + holder.selectedItems.contains(item) && holder.selectedItems.size == 1 + ) { for (tabSelected in holder.selectedItems) { // Exit selection mode by deselecting everything interactor.deselect(tabSelected) @@ -250,7 +252,7 @@ abstract class AbstractBrowserTabViewHolder( // startDragAndDrop is the non-deprecated version, but requires API 24 @Suppress("DEPRECATION") view.startDrag(null, shadow, TabDragData(item, point), 0) - } + } else view.onTouchEvent(motionEvent) } else -> view.onTouchEvent(motionEvent) } From ca3c8441124c3fcceb2e7cebad9d2dab85284c2f Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Wed, 17 Nov 2021 00:19:20 -0800 Subject: [PATCH 25/38] Auto-scroll tab tray while dragging near top/bottom edge --- .../browser/AbstractBrowserTrayList.kt | 69 ++++++++++++++----- 1 file changed, 53 insertions(+), 16 deletions(-) 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 da5417cb1..1409ec0bc 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,6 +5,7 @@ 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 @@ -27,6 +28,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) } @@ -62,7 +66,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( val proposedTarget = getChildAt(i) val targetHolder = findContainingViewHolder(proposedTarget) if (targetHolder is TabViewHolder) { - var rect = Rect() // Get post-animation positioning + val rect = Rect() // Get post-animation positioning getDecoratedBoundsWithMargins(proposedTarget, rect) val targetX = (rect.left + rect.right) / 2 val targetY = (rect.top + rect.bottom) / 2 @@ -70,6 +74,8 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( 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 @@ -91,7 +97,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( } private val dragListen = OnDragListener { _, event -> if (event.localState is TabDragData) { - val (tab, dragOffset) = event.localState as TabDragData + val (tab, _) = event.localState as TabDragData val sourceId = tab.id val sources = findSourceViewAndHolder(sourceId) @@ -102,24 +108,17 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( val (sourceView, _) = sources 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 -> { - val target = getDropPosition(event.x, event.y, tab.id) - if (target != null) { - val (targetId, placeAfter) = target - interactor.onTabsMove(tab.id, targetId, placeAfter) - } - // Move the tab's visual position - if (sources != null) { - val (sourceView, sourceViewHolder) = sources - sourceView.x = event.x - dragOffset.x - sourceView.y = event.y - dragOffset.y - sourceViewHolder.beingDragged = true - } + lastDragPos = PointF(event.x,event.y) true } DragEvent.ACTION_DRAG_EXITED -> { @@ -134,11 +133,14 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( val (sourceView, sourceViewHolder) = sources sourceView.elevation -= DRAGGED_TAB_ELEVATION sourceView.animate() - .translationX(0f).translationY(0f) - .setDuration(itemAnimator?.moveDuration ?: 0) + .translationX(0f).translationY(0f).duration = + itemAnimator?.moveDuration ?: 0 sourceViewHolder.beingDragged = false } + //This will stop the scroll/update loop + lastDragPos = null + lastDragData = null true } else -> { // Unknown action @@ -147,7 +149,42 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( } } else false } + + private val dragRunnable: Runnable = object: Runnable { + override fun run() { + val pos = lastDragPos + val data = lastDragData + if (pos != null && data != null) { + val (tab, dragOffset) = data + val sourceId = tab.id + val sources = findSourceViewAndHolder(sourceId) + // Move the tab's position in the list + val target = getDropPosition(pos.x, pos.y, tab.id) + if (target != null) { + val (targetId, placeAfter) = target + interactor.onTabsMove(tab.id, targetId, placeAfter) + } + // 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 + } + //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) + + 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 } } From f31706442701fe9f1a048c4edaec92477edc5a91 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Wed, 17 Nov 2021 00:31:50 -0800 Subject: [PATCH 26/38] Remove unexpected scrolling on tab bind (triggered when tab is selected) --- .../fenix/tabstray/viewholders/AbstractBrowserPageViewHolder.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolder.kt index e3a5db8a0..f2b3ebd5d 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolder.kt @@ -53,8 +53,6 @@ abstract class AbstractBrowserPageViewHolder( ) { adapterRef = adapter - scrollToTab(adapter, layoutManager) - trayList.layoutManager = layoutManager trayList.adapter = adapter } From bfb57dda68a9404e4c022e9f25d70d0db5bbda2d Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Wed, 17 Nov 2021 01:31:38 -0800 Subject: [PATCH 27/38] Fix broken scroll behavior during dragging --- .../browser/AbstractBrowserTrayList.kt | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) 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 1409ec0bc..e77d043f2 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 @@ -10,6 +10,7 @@ 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 @@ -56,7 +57,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( } // Find the closest item to the x/y position of the drop. - private data class DropPositionData(val id: String, val placeAfter: Boolean) + 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 @@ -79,7 +80,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( if (id == source) seenSource = true if (dist < bestDist && id != null) { bestDist = dist - bestOut = DropPositionData(id, seenSource) + bestOut = DropPositionData(id, seenSource, proposedTarget) } } } @@ -158,18 +159,25 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( val (tab, dragOffset) = data val sourceId = tab.id val sources = findSourceViewAndHolder(sourceId) - // Move the tab's position in the list - val target = getDropPosition(pos.x, pos.y, tab.id) - if (target != null) { - val (targetId, placeAfter) = target - interactor.onTabsMove(tab.id, targetId, placeAfter) - } // 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 + + // 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 + (layoutManager as? ItemTouchHelper.ViewDropHandler)?.prepareForDrop( + sourceView,targetView, + dragOffset.x.toInt(),dragOffset.y.toInt()) + } + } } //Scroll the tray var scroll = 0 From d36497166b85e8b7d869375a8943ee0d2e2d754d Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Wed, 17 Nov 2021 01:50:17 -0800 Subject: [PATCH 28/38] Cleanup for ktlint/detekt --- .../browser/AbstractBrowserTrayList.kt | 67 +++++++++---------- 1 file changed, 33 insertions(+), 34 deletions(-) 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 e77d043f2..18995d983 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 @@ -109,8 +109,8 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( val (sourceView, _) = sources sourceView.elevation += DRAGGED_TAB_ELEVATION } - //Setup the scrolling/updating loop - lastDragPos = PointF(event.x,event.y) + // Setup the scrolling/updating loop + lastDragPos = PointF(event.x, event.y) lastDragData = event.localState as TabDragData handler.postDelayed(dragRunnable, DRAG_UPDATE_PERIOD_MS) true @@ -119,7 +119,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( true } DragEvent.ACTION_DRAG_LOCATION -> { - lastDragPos = PointF(event.x,event.y) + lastDragPos = PointF(event.x, event.y) true } DragEvent.ACTION_DRAG_EXITED -> { @@ -139,7 +139,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( sourceViewHolder.beingDragged = false } - //This will stop the scroll/update loop + // This will stop the scroll/update loop lastDragPos = null lastDragData = null true @@ -151,42 +151,41 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( } else false } - private val dragRunnable: Runnable = object: Runnable { + private val dragRunnable: Runnable = object : Runnable { override fun run() { val pos = lastDragPos val data = lastDragData - if (pos != null && data != null) { - 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 - - // 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 - (layoutManager as? ItemTouchHelper.ViewDropHandler)?.prepareForDrop( - sourceView,targetView, - dragOffset.x.toInt(),dragOffset.y.toInt()) - } + 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 + + // 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 + (layoutManager as? ItemTouchHelper.ViewDropHandler)?.prepareForDrop( + sourceView, targetView, dragOffset.x.toInt(), dragOffset.y.toInt() + ) } } - //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) - - handler.postDelayed(this, DRAG_UPDATE_PERIOD_MS) } + // 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) + + handler.postDelayed(this, DRAG_UPDATE_PERIOD_MS) } } companion object { From c72675e620d41efb26d76c94415465e0f4933e7d Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Wed, 17 Nov 2021 02:14:25 -0800 Subject: [PATCH 29/38] Constantly set elevation during drag in case of update Clean code at drag start --- .../tabstray/browser/AbstractBrowserTabViewHolder.kt | 10 +++------- .../fenix/tabstray/browser/AbstractBrowserTrayList.kt | 11 ++++++----- 2 files changed, 9 insertions(+), 12 deletions(-) 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 b6c486d11..b608a8bec 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 @@ -242,16 +242,12 @@ abstract class AbstractBrowserTabViewHolder( if (parent?.context?.settings()?.searchTermTabGroupsAreEnabled == false && holder.selectedItems.contains(item) && holder.selectedItems.size == 1 ) { - for (tabSelected in holder.selectedItems) { - // Exit selection mode by deselecting everything - interactor.deselect(tabSelected) - } + interactor.deselect(item) // Exit selection mode val shadow = BlankDragShadowBuilder() - beingDragged = true - val point = PointF(motionEvent.x, motionEvent.y) + val dragOffset = PointF(motionEvent.x, motionEvent.y) // startDragAndDrop is the non-deprecated version, but requires API 24 @Suppress("DEPRECATION") - view.startDrag(null, shadow, TabDragData(item, point), 0) + view.startDrag(null, shadow, TabDragData(item, dragOffset), 0) } else view.onTouchEvent(motionEvent) } else -> view.onTouchEvent(motionEvent) 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 18995d983..c53688101 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 @@ -106,8 +106,9 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( DragEvent.ACTION_DRAG_STARTED -> { // Put the dragged tab on top of all other tabs if (sources != null) { - val (sourceView, _) = sources - sourceView.elevation += DRAGGED_TAB_ELEVATION + val (sourceView, sourceViewHolder) = sources + sourceViewHolder.beingDragged = true + sourceView.elevation = DRAGGED_TAB_ELEVATION } // Setup the scrolling/updating loop lastDragPos = PointF(event.x, event.y) @@ -132,12 +133,11 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( // Move tab to center, set dragging to false, return tab to normal height if (sources != null) { val (sourceView, sourceViewHolder) = sources - sourceView.elevation -= DRAGGED_TAB_ELEVATION + sourceViewHolder.beingDragged = false + sourceView.elevation = 0f sourceView.animate() .translationX(0f).translationY(0f).duration = itemAnimator?.moveDuration ?: 0 - - sourceViewHolder.beingDragged = false } // This will stop the scroll/update loop lastDragPos = null @@ -165,6 +165,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( 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) From 42842cd5de2ea50ae09a1295166023b685fecfea Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Tue, 23 Nov 2021 15:48:52 -0800 Subject: [PATCH 30/38] Add custom drag start behavior --- .../browser/AbstractBrowserTabViewHolder.kt | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) 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 b608a8bec..ca0f9e9bd 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 @@ -82,6 +82,8 @@ 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. */ @@ -214,6 +216,7 @@ abstract class AbstractBrowserTabViewHolder( interactor: BrowserTrayInteractor ) { itemView.setOnClickListener { + touchStartPoint = null val selected = holder.selectedItems when { selected.isEmpty() && trayStore.state.mode.isSelect().not() -> { @@ -226,6 +229,7 @@ abstract class AbstractBrowserTabViewHolder( itemView.setOnLongClickListener { if (holder.selectedItems.isEmpty()) { + touchStartPoint = null metrics.track(Event.CollectionTabLongPressed) interactor.select(item) true @@ -242,17 +246,26 @@ abstract class AbstractBrowserTabViewHolder( if (parent?.context?.settings()?.searchTermTabGroupsAreEnabled == false && holder.selectedItems.contains(item) && holder.selectedItems.size == 1 ) { - interactor.deselect(item) // Exit selection mode - val shadow = BlankDragShadowBuilder() - val dragOffset = PointF(motionEvent.x, motionEvent.y) - // startDragAndDrop is the non-deprecated version, but requires API 24 - @Suppress("DEPRECATION") - view.startDrag(null, shadow, TabDragData(item, dragOffset), 0) - } else view.onTouchEvent(motionEvent) + if (touchStartPoint == null) { + touchStartPoint = PointF(motionEvent.x, motionEvent.y) + } + val motionOffset = PointF( + touchStartPoint!!.x - motionEvent.x, + touchStartPoint!!.y - motionEvent.y + ) + if (PointF.length(motionOffset.x, motionOffset.y) > 10) { + val dragOffset = PointF(motionEvent.x, motionEvent.y) + interactor.deselect(item) // Exit selection mode + val shadow = BlankDragShadowBuilder() + // startDragAndDrop is the non-deprecated version, but requires API 24 + @Suppress("DEPRECATION") + view.startDrag(null, shadow, TabDragData(item, dragOffset), 0) + } + return@setOnTouchListener true + } } - else -> view.onTouchEvent(motionEvent) } - true + view.onTouchEvent(motionEvent) } } From d718241c5a1f5adacd9cf50459f99c7115847cb0 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Tue, 23 Nov 2021 16:29:55 -0800 Subject: [PATCH 31/38] Add drag distance constant, do all touch-drag behavior in OnTouchListener --- .../browser/AbstractBrowserTabViewHolder.kt | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) 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 ca0f9e9bd..2fb1ab4b0 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 @@ -216,7 +216,6 @@ abstract class AbstractBrowserTabViewHolder( interactor: BrowserTrayInteractor ) { itemView.setOnClickListener { - touchStartPoint = null val selected = holder.selectedItems when { selected.isEmpty() && trayStore.state.mode.isSelect().not() -> { @@ -229,7 +228,6 @@ abstract class AbstractBrowserTabViewHolder( itemView.setOnLongClickListener { if (holder.selectedItems.isEmpty()) { - touchStartPoint = null metrics.track(Event.CollectionTabLongPressed) interactor.select(item) true @@ -241,21 +239,30 @@ abstract class AbstractBrowserTabViewHolder( // 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 if (parent?.context?.settings()?.searchTermTabGroupsAreEnabled == false && - holder.selectedItems.contains(item) && holder.selectedItems.size == 1 + holder.selectedItems.contains(item) && holder.selectedItems.size == 1 && + touchStart != null ) { - if (touchStartPoint == null) { - touchStartPoint = PointF(motionEvent.x, motionEvent.y) - } - val motionOffset = PointF( - touchStartPoint!!.x - motionEvent.x, - touchStartPoint!!.y - motionEvent.y + val dist = PointF.length( + touchStart.x - motionEvent.x, + touchStart.y - motionEvent.y ) - if (PointF.length(motionOffset.x, motionOffset.y) > 10) { + if (dist > MINIMUM_DRAG_DISTANCE) { val dragOffset = PointF(motionEvent.x, motionEvent.y) interactor.deselect(item) // Exit selection mode + touchStartPoint = null val shadow = BlankDragShadowBuilder() // startDragAndDrop is the non-deprecated version, but requires API 24 @Suppress("DEPRECATION") @@ -272,5 +279,6 @@ abstract class AbstractBrowserTabViewHolder( companion object { internal const val PLAY_PAUSE_BUTTON_EXTRA_DPS = 24 internal const val GRID_ITEM_CLOSE_BUTTON_EXTRA_DPS = 24 + internal const val MINIMUM_DRAG_DISTANCE = 30 } } From d9408d94973d7978031b4f6cd7162a477dd175ae Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Tue, 23 Nov 2021 18:47:59 -0800 Subject: [PATCH 32/38] Disable parent vertical scrolling on drag start, fix detekt ComplexCondition --- .../tabstray/browser/AbstractBrowserTabViewHolder.kt | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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 2fb1ab4b0..939f6787c 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 @@ -251,18 +251,22 @@ abstract class AbstractBrowserTabViewHolder( MotionEvent.ACTION_MOVE -> { val parent = itemView.parent as? AbstractBrowserTrayList val touchStart = touchStartPoint + val selected = holder.selectedItems + val selectsOnlyThis = (selected.size == 1 && selected.contains(item)) if (parent?.context?.settings()?.searchTermTabGroupsAreEnabled == false && - holder.selectedItems.contains(item) && holder.selectedItems.size == 1 && - touchStart != null + 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 > MINIMUM_DRAG_DISTANCE) { - val dragOffset = PointF(motionEvent.x, motionEvent.y) interactor.deselect(item) // Exit selection mode touchStartPoint = null + val dragOffset = PointF(motionEvent.x, motionEvent.y) val shadow = BlankDragShadowBuilder() // startDragAndDrop is the non-deprecated version, but requires API 24 @Suppress("DEPRECATION") From 4e32e1b0bf64838782f1981685db002c6826cdbb Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Tue, 23 Nov 2021 21:32:23 -0800 Subject: [PATCH 33/38] Minor cleanup/comments --- .../fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt | 2 +- .../mozilla/fenix/tabstray/browser/AbstractBrowserTrayList.kt | 2 ++ .../org/mozilla/fenix/tabstray/browser/BrowserTabViewHolder.kt | 2 +- .../org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt | 1 - 4 files changed, 4 insertions(+), 3 deletions(-) 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 939f6787c..651c76fff 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 @@ -80,8 +80,8 @@ abstract class AbstractBrowserTabViewHolder( abstract val thumbnailSize: Int override var tab: TabSessionState? = null - internal var beingDragged: Boolean = false + internal var beingDragged: Boolean = false private var touchStartPoint: PointF? = null /** 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 c53688101..92483e914 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 @@ -174,6 +174,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( 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, dragOffset.x.toInt(), dragOffset.y.toInt() ) @@ -186,6 +187,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( 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) } } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabViewHolder.kt index 953511885..ca0ae0405 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabViewHolder.kt @@ -38,7 +38,7 @@ sealed class BrowserTabViewHolder(itemView: View) : RecyclerView.ViewHolder(item store: TabsTrayStore, selectionHolder: SelectionHolder? = null, itemView: View, - featureName: String, + featureName: String ) : AbstractBrowserTabViewHolder(itemView, imageLoader, store, selectionHolder, featureName) { private val closeButton: AppCompatImageButton = itemView.findViewById(R.id.mozac_browser_tabstray_close) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt index 9b3962704..6b5d57019 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTabsAdapter.kt @@ -17,7 +17,6 @@ import org.mozilla.fenix.components.Components import org.mozilla.fenix.databinding.TabTrayGridItemBinding import org.mozilla.fenix.databinding.TabTrayItemBinding import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.settings import org.mozilla.fenix.selection.SelectionHolder import org.mozilla.fenix.tabstray.TabsTrayStore From 1d907356483ad3f0cf7c6655cfe6e72c177e09d7 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Wed, 24 Nov 2021 14:42:35 -0800 Subject: [PATCH 34/38] Revert removal of scroll on bind, this was related to something different --- .../fenix/tabstray/viewholders/AbstractBrowserPageViewHolder.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolder.kt index f2b3ebd5d..e3a5db8a0 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/AbstractBrowserPageViewHolder.kt @@ -53,6 +53,8 @@ abstract class AbstractBrowserPageViewHolder( ) { adapterRef = adapter + scrollToTab(adapter, layoutManager) + trayList.layoutManager = layoutManager trayList.adapter = adapter } From b15448563d256ab7d3788c0f03ec5b048a114b4a Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Wed, 24 Nov 2021 15:09:25 -0800 Subject: [PATCH 35/38] Correction to prepareForDrop to match documentation- doesn't seem to have any effect --- .../mozilla/fenix/tabstray/browser/AbstractBrowserTrayList.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 92483e914..542749d77 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 @@ -176,7 +176,7 @@ abstract class AbstractBrowserTrayList @JvmOverloads constructor( // Deal with https://issuetracker.google.com/issues/37018279 // See also https://stackoverflow.com/questions/27992427 (layoutManager as? ItemTouchHelper.ViewDropHandler)?.prepareForDrop( - sourceView, targetView, dragOffset.x.toInt(), dragOffset.y.toInt() + sourceView, targetView, sourceView.left, sourceView.top ) } } From 377df5932d71c42a48bbaac10c88f2e99a09a72d Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Thu, 25 Nov 2021 12:34:28 -0800 Subject: [PATCH 36/38] Simplify via unchecked typecast, use ViewCompat --- .../tabstray/browser/AbstractBrowserTabViewHolder.kt | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) 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 651c76fff..76e5d6e04 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 @@ -14,6 +14,7 @@ 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 @@ -249,11 +250,11 @@ abstract class AbstractBrowserTabViewHolder( touchStartPoint = null } MotionEvent.ACTION_MOVE -> { - val parent = itemView.parent as? AbstractBrowserTrayList + val parent = itemView.parent as AbstractBrowserTrayList val touchStart = touchStartPoint val selected = holder.selectedItems val selectsOnlyThis = (selected.size == 1 && selected.contains(item)) - if (parent?.context?.settings()?.searchTermTabGroupsAreEnabled == false && + if (!parent.context.settings().searchTermTabGroupsAreEnabled && selectsOnlyThis && touchStart != null ) { // Prevent scrolling if the user tries to start drag vertically @@ -268,9 +269,7 @@ abstract class AbstractBrowserTabViewHolder( touchStartPoint = null val dragOffset = PointF(motionEvent.x, motionEvent.y) val shadow = BlankDragShadowBuilder() - // startDragAndDrop is the non-deprecated version, but requires API 24 - @Suppress("DEPRECATION") - view.startDrag(null, shadow, TabDragData(item, dragOffset), 0) + ViewCompat.startDragAndDrop(itemView, null, shadow, TabDragData(item, dragOffset), 0) } return@setOnTouchListener true } From 9c810b97908039a38b6b9c502490d1786f837d8c Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Fri, 26 Nov 2021 15:13:53 -0800 Subject: [PATCH 37/38] Use ViewConfiguration.scaledTouchSlop instead of arbitrary 30px --- .../fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 76e5d6e04..992641631 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 @@ -8,6 +8,7 @@ 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 @@ -264,7 +265,7 @@ abstract class AbstractBrowserTabViewHolder( touchStart.x - motionEvent.x, touchStart.y - motionEvent.y ) - if (dist > MINIMUM_DRAG_DISTANCE) { + if (dist > ViewConfiguration.get(parent.context).scaledTouchSlop) { interactor.deselect(item) // Exit selection mode touchStartPoint = null val dragOffset = PointF(motionEvent.x, motionEvent.y) @@ -282,6 +283,5 @@ abstract class AbstractBrowserTabViewHolder( companion object { internal const val PLAY_PAUSE_BUTTON_EXTRA_DPS = 24 internal const val GRID_ITEM_CLOSE_BUTTON_EXTRA_DPS = 24 - internal const val MINIMUM_DRAG_DISTANCE = 30 } } From bf4670c1bdb56c9720b019638d1acef607b93ad5 Mon Sep 17 00:00:00 2001 From: Steven Knipe Date: Wed, 1 Dec 2021 15:24:55 -0800 Subject: [PATCH 38/38] Added tabReorderingFeature flag, split drag interactor to separate function to satisfy complexity requirement --- .../java/org/mozilla/fenix/FeatureFlags.kt | 5 ++++ .../browser/AbstractBrowserTabViewHolder.kt | 27 ++++++++++--------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt index 9d514cc91..865941a59 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/browser/AbstractBrowserTabViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt index 992641631..0b9e51c4b 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 @@ -29,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 @@ -211,7 +212,6 @@ abstract class AbstractBrowserTabViewHolder( imageLoader.loadIntoView(thumbnailView, ImageLoadRequest(id, thumbnailSize)) } - @SuppressLint("ClickableViewAccessibility") private fun setSelectionInteractor( item: TabSessionState, holder: SelectionHolder, @@ -237,15 +237,21 @@ 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 - ) + touchStartPoint = PointF(motionEvent.x, motionEvent.y) } MotionEvent.ACTION_UP, MotionEvent.ACTION_CANCEL -> { touchStartPoint = null @@ -255,16 +261,13 @@ abstract class AbstractBrowserTabViewHolder( val touchStart = touchStartPoint val selected = holder.selectedItems val selectsOnlyThis = (selected.size == 1 && selected.contains(item)) - if (!parent.context.settings().searchTermTabGroupsAreEnabled && - selectsOnlyThis && touchStart != null - ) { + 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 - ) + 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