Commit f0518a82 authored by Yue Zhang's avatar Yue Zhang Committed by Commit Bot

Enabling Dragging for TabGridDialog and TabGridSheet

This CL enables dragging to reorganize tabs in TabGridDialog and
TabGridSheet. It is now behind the flag TabGroupsAndroidUiImprovements.

Bug: 961433
Change-Id: Ib12025fdd1635962cb785399b7f7558d11ce6e58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1611305
Commit-Queue: Yue Zhang <yuezhanggg@google.com>
Reviewed-by: default avatarYusuf Ozuysal <yusufo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659949}
parent d3f22423
......@@ -12,6 +12,7 @@ import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.TabModel;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tabmodel.TabModelSelectorTabObserver;
import org.chromium.chrome.browser.tasks.tabgroup.TabGroupModelFilter;
......@@ -131,4 +132,20 @@ public class TabGroupUtils {
return selector.getCurrentModel().indexOf(tabs.get(tabs.size() - 1));
}
/**
* This method judges whether the current move from {@code curIndex} to {@code newIndex} in
* {@code tabModel} is a move within one TabGroup or not.
* @param tabModel The TabModel that owns the moving tab.
* @param curIndex The current index of the moving tab.
* @param newIndex The new index of the moving tab.
* @return Whether is move happens within one TabGroup or not.
*/
public static boolean isMoveInSameGroup(TabModel tabModel, int curIndex, int newIndex) {
int rootId = tabModel.getTabAt(curIndex).getRootId();
for (int i = Math.min(newIndex, curIndex); i <= Math.max(newIndex, curIndex); i++) {
if (tabModel.getTabAt(i).getRootId() != rootId) return false;
}
return true;
}
}
......@@ -20,6 +20,7 @@ import android.view.View;
import org.chromium.base.Callback;
import org.chromium.base.ContextUtils;
import org.chromium.base.Log;
import org.chromium.base.VisibleForTesting;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.browser.native_page.NativePageFactory;
......@@ -429,13 +430,25 @@ class TabListMediator {
// Handle move without groups enabled.
if (mTabModelSelector.getTabModelFilterProvider().getCurrentTabModelFilter()
instanceof EmptyTabModelFilter) {
if (mModel.indexFromId(tab.getId()) == TabModel.INVALID_TAB_INDEX) return;
if (newIndex >= mModel.size() || curIndex >= mModel.size()) return;
if (!isValidMovePosition(curIndex) || !isValidMovePosition(newIndex)) return;
mModel.move(curIndex, newIndex);
return;
}
// Handle move with groups enabled.
// When move within one group.
if (TabGroupUtils.isMoveInSameGroup(tabModel, curIndex, newIndex)) {
int curPosition = mModel.indexFromId(tab.getId());
if (!isValidMovePosition(curPosition)) return;
Tab destinationTab =
tabModel.getTabAt(newIndex > curIndex ? newIndex - 1 : newIndex + 1);
int newPosition = mModel.indexFromId(destinationTab.getId());
if (!isValidMovePosition(newPosition)) return;
mModel.move(curPosition, newPosition);
return;
}
// When move between groups.
// Only proceed when all members of the group are moved to target index in TabModel.
// Regardless of the size of tab group, one tab(group) only proceeds once here per
// move.
......@@ -446,7 +459,7 @@ class TabListMediator {
Tab currentGroupSelectedTab =
TabGroupUtils.getSelectedTabInGroupForTab(mTabModelSelector, tab);
int curPosition = mModel.indexFromId(currentGroupSelectedTab.getId());
if (curPosition == TabModel.INVALID_TAB_INDEX || curPosition > mModel.size()) return;
if (!isValidMovePosition(curPosition)) return;
// Find the tab which was in the destination index before this move. Use that tab to
// figure out the new position.
......@@ -456,11 +469,15 @@ class TabListMediator {
Tab destinationGroupSelectedTab =
TabGroupUtils.getSelectedTabInGroupForTab(mTabModelSelector, destinationTab);
int newPosition = mModel.indexFromId(destinationGroupSelectedTab.getId());
if (newPosition == TabModel.INVALID_TAB_INDEX || newPosition > mModel.size()) return;
if (!isValidMovePosition(newPosition)) return;
mModel.move(curPosition, newPosition);
}
private boolean isValidMovePosition(int position) {
return position != TabModel.INVALID_TAB_INDEX && position < mModel.size();
}
/**
* Initialize the component with a list of tabs to show in a grid.
* @param tabs The list of tabs to be shown.
......@@ -507,10 +524,16 @@ class TabListMediator {
toViewHolder.getAdapterPosition() - fromViewHolder.getAdapterPosition();
TabModelFilter filter =
mTabModelSelector.getTabModelFilterProvider().getCurrentTabModelFilter();
TabModel tabModel = mTabModelSelector.getCurrentModel();
if (filter instanceof EmptyTabModelFilter) {
mTabModelSelector.getCurrentModel().moveTab(currentTabId,
tabModel.moveTab(currentTabId,
mModel.indexFromId(currentTabId)
+ (distance > 0 ? distance + 1 : distance));
} else if (!mCloseAllRelatedTabs) {
int destinationIndex =
tabModel.indexOf(mTabModelSelector.getTabById(destinationTabId));
tabModel.moveTab(
currentTabId, distance > 0 ? destinationIndex + 1 : destinationIndex);
} else {
List<Tab> destinationTabGroup = getRelatedTabsForId(destinationTabId);
int newIndex = distance >= 0 ? TabGroupUtils.getLastTabModelIndexForList(
......@@ -571,6 +594,11 @@ class TabListMediator {
ContextUtils.getApplicationContext().registerComponentCallbacks(mComponentCallbacks);
}
@VisibleForTesting
void setCloseAllRelatedTabs(boolean flag) {
mCloseAllRelatedTabs = flag;
}
/**
* Destroy any members that needs clean up.
*/
......
......@@ -232,6 +232,7 @@ public class TabListMediatorUnitTest {
public void sendsMoveTabSignalCorrectlyWithGroup() {
initAndAssertAllProperties();
mMediator.setCloseAllRelatedTabs(true);
doReturn(mTabGroupModelFilter).when(mTabModelFilterProvider).getCurrentTabModelFilter();
mMediator.getItemTouchHelperCallback(0f).onMove(mRecyclerView, mViewHolder1, mViewHolder2);
......@@ -239,6 +240,17 @@ public class TabListMediatorUnitTest {
verify(mTabGroupModelFilter).moveRelatedTabs(eq(TAB1_ID), eq(2));
}
@Test
public void sendsMoveTabSignalCorrectlyWithinGroup() {
initAndAssertAllProperties();
doReturn(mTabGroupModelFilter).when(mTabModelFilterProvider).getCurrentTabModelFilter();
mMediator.getItemTouchHelperCallback(0f).onMove(mRecyclerView, mViewHolder1, mViewHolder2);
verify(mTabModel).moveTab(eq(TAB1_ID), eq(2));
}
@Test
public void tabClosure() {
initAndAssertAllProperties();
......@@ -458,6 +470,50 @@ public class TabListMediatorUnitTest {
assertThat(mModel.get(0).get(TabProperties.TITLE), equalTo(TAB2_TITLE));
}
@Test
public void tabMovementWithinGroup_Forward() {
initAndAssertAllProperties();
// Assume that moveTab in TabModel is finished.
doReturn(mTab1).when(mTabModel).getTabAt(POSITION2);
doReturn(mTab2).when(mTabModel).getTabAt(POSITION1);
doReturn(mTabGroupModelFilter).when(mTabModelFilterProvider).getCurrentTabModelFilter();
doReturn(TAB1_ID).when(mTab1).getRootId();
doReturn(TAB1_ID).when(mTab2).getRootId();
assertThat(mModel.size(), equalTo(2));
assertThat(mModel.get(1).get(TabProperties.TAB_ID), equalTo(TAB2_ID));
assertThat(mModel.get(1).get(TabProperties.TITLE), equalTo(TAB2_TITLE));
mTabModelObserverCaptor.getValue().didMoveTab(mTab2, POSITION1, POSITION2);
assertThat(mModel.size(), equalTo(2));
assertThat(mModel.get(0).get(TabProperties.TAB_ID), equalTo(TAB2_ID));
assertThat(mModel.get(0).get(TabProperties.TITLE), equalTo(TAB2_TITLE));
}
@Test
public void tabMovementWithinGroup_Backward() {
initAndAssertAllProperties();
// Assume that moveTab in TabModel is finished.
doReturn(mTab1).when(mTabModel).getTabAt(POSITION2);
doReturn(mTab2).when(mTabModel).getTabAt(POSITION1);
doReturn(mTabGroupModelFilter).when(mTabModelFilterProvider).getCurrentTabModelFilter();
doReturn(TAB1_ID).when(mTab1).getRootId();
doReturn(TAB1_ID).when(mTab2).getRootId();
assertThat(mModel.size(), equalTo(2));
assertThat(mModel.get(1).get(TabProperties.TAB_ID), equalTo(TAB2_ID));
assertThat(mModel.get(1).get(TabProperties.TITLE), equalTo(TAB2_TITLE));
mTabModelObserverCaptor.getValue().didMoveTab(mTab1, POSITION2, POSITION1);
assertThat(mModel.size(), equalTo(2));
assertThat(mModel.get(0).get(TabProperties.TAB_ID), equalTo(TAB2_ID));
assertThat(mModel.get(0).get(TabProperties.TITLE), equalTo(TAB2_TITLE));
}
private void initAndAssertAllProperties() {
List<Tab> tabs = new ArrayList<>();
for (int i = 0; i < mTabModel.getCount(); i++) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment