WebUI Tab Strip: Animate grouped tabs and tab groups on drag

This CL adds animations to grouped tabs and entire groups being dragged.
It reuses most of the already implemented methods of animating tabs
because tab groups are shrunken to the width of one TabElement when
being dragged.

The CL also removes a previous |.remove()| call when placing a
TabGroupElement as the removal and inserting of the TabGroupElement
while animating caused some performance issues.

Bug: 1082344
Change-Id: I04c757b4d2d7f919a3021491e559e020f7e1010d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2252036
Commit-Queue: John Lee <johntlee@chromium.org>
Reviewed-by: dpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780391}

chrome/browser/resources/tab_strip/drag_manager.js

@@ -359,7 +359,8 @@ class DragSession {
 
     const dragOverTabElement =
         /** @type {!TabElement|undefined} */ (composedPath.find(isTabElement));
-    if (dragOverTabElement && !dragOverTabElement.tab.pinned) {
+    if (dragOverTabElement && !dragOverTabElement.tab.pinned &&
+        dragOverTabElement.isValidDragOverTarget) {
       let dragOverIndex = this.delegate_.getIndexOfTab(dragOverTabElement);
       dragOverIndex +=
           this.shouldOffsetIndexForGroup_(dragOverTabElement) ? 1 : 0;
@@ -369,7 +370,7 @@ class DragSession {
 
     const dragOverGroupElement = /** @type {!TabGroupElement|undefined} */ (
         composedPath.find(isTabGroupElement));
-    if (dragOverGroupElement) {
+    if (dragOverGroupElement && dragOverGroupElement.isValidDragOverTarget) {
       let dragOverIndex = this.delegate_.getIndexOfTab(
           /** @type {!TabElement} */ (dragOverGroupElement.firstElementChild));
       dragOverIndex +=
@@ -402,7 +403,8 @@ class DragSession {
     const dragOverTabGroup =
         /** @type {?TabGroupElement} */ (composedPath.find(isTabGroupElement));
     if (dragOverTabGroup &&
-        dragOverTabGroup.dataset.groupId !== previousGroupId) {
+        dragOverTabGroup.dataset.groupId !== previousGroupId &&
+        dragOverTabGroup.isValidDragOverTarget) {
       this.delegate_.placeTabElement(
           tabElement, this.dstIndex, false, dragOverTabGroup.dataset.groupId);
       return;

chrome/browser/resources/tab_strip/tab_group.js

@@ -24,6 +24,23 @@ export class TabGroupElement extends CustomElement {
     this.chip_.addEventListener('click', () => this.onClickChip_());
     this.chip_.addEventListener(
         'keydown', e => this.onKeydownChip_(/** @type {!KeyboardEvent} */ (e)));
+
+    /**
+     * Flag indicating if this element can accept dragover events. This flag
+     * is updated by TabListElement while animating.
+     * @private {boolean}
+     */
+    this.isValidDragOverTarget_ = true;
+  }
+
+  /** @return {boolean} */
+  get isValidDragOverTarget() {
+    return !this.hasAttribute('dragging_') && this.isValidDragOverTarget_;
+  }
+
+  /** @param {boolean} isValid */
+  set isValidDragOverTarget(isValid) {
+    this.isValidDragOverTarget_ = isValid;
   }
 
   /** @return {!HTMLElement} */

chrome/browser/resources/tab_strip/tab_list.js

@@ -711,8 +711,8 @@ export class TabListElement extends CustomElement {
   placeTabElement(element, index, pinned, groupId) {
     const isInserting = !element.isConnected;
 
-    // TODO(johntlee): Animate pinned tabs and grouped tabs.
-    const shouldAnimate = !pinned && !groupId && !isInserting;
+    // TODO(johntlee): Animate pinned tabs.
+    const shouldAnimate = !pinned && !isInserting;
 
     // Cache the previous and next element siblings as these will be needed
     // after the placement to determine which tabs to animate.
@@ -739,7 +739,16 @@ export class TabListElement extends CustomElement {
    * @param {number} index
    */
   placeTabGroupElement(element, index) {
-    element.remove();
+    if (element.isConnected && element.childElementCount &&
+        this.getIndexOfTab(
+            /** @type {!TabElement} */ (element.firstElementChild)) < index) {
+      // If moving after its original position, the index value needs to be
+      // offset by 1 to consider itself already attached to the DOM.
+      index++;
+    }
+
+    const initialDomPrevSibling = element.previousElementSibling;
+    const initialDomNextSibling = element.nextElementSibling;
 
     let elementAtIndex = this.$all('tabstrip-tab')[index];
     if (elementAtIndex && elementAtIndex.parentElement &&
@@ -748,6 +757,7 @@ export class TabListElement extends CustomElement {
     }
 
     this.unpinnedTabsElement_.insertBefore(element, elementAtIndex);
+    animateElementMoved(element, initialDomPrevSibling, initialDomNextSibling);
   }
 
   /** @private */

chrome/test/data/webui/tab_strip/tab_list_test.js

@@ -271,7 +271,7 @@ suite('TabList', () => {
    * @param {!Element} element
    * @param {number} scale
    */
-  function testPlaceTabElementAnimationParams(element, scale) {
+  function testPlaceElementAnimationParams(element, scale) {
     const animations = element.getAnimations();
 
     // TODO(crbug.com/1090645): Remove logging once the test no longer flakes.
@@ -313,32 +313,32 @@ suite('TabList', () => {
 
     const movedTab = unpinnedTabs[indexToMove];
     tabList.placeTabElement(movedTab, newIndex, false, undefined);
-    testPlaceTabElementAnimationParams(
-        movedTab, -1 * direction * (unpinnedTabs.length - 1));
+    testPlaceElementAnimationParams(
+        movedTab, -1 * direction * Math.abs(newIndex - indexToMove));
 
     Array.from(unpinnedTabs)
         .filter(tabElement => tabElement !== movedTab)
         .forEach(
             tabElement =>
-                testPlaceTabElementAnimationParams(tabElement, direction));
+                testPlaceElementAnimationParams(tabElement, direction));
   }
 
-  test('PlaceTabElementAnimatesTabMovedTowardsStart', async () => {
-    await testPlaceTabElementAnimation(tabs.length - 1, 0, -1);
+  test('PlaceTabElementAnimatesTabMovedTowardsStart', () => {
+    return testPlaceTabElementAnimation(tabs.length - 1, 0, -1);
   });
 
-  test('PlaceTabElementAnimatesTabMovedTowardsStartRTL', async () => {
+  test('PlaceTabElementAnimatesTabMovedTowardsStartRTL', () => {
     document.documentElement.dir = 'rtl';
-    await testPlaceTabElementAnimation(tabs.length - 1, 0, 1);
+    return testPlaceTabElementAnimation(tabs.length - 1, 0, 1);
   });
 
-  test('PlaceTabElementAnimatesTabMovedTowardsEnd', async () => {
-    await testPlaceTabElementAnimation(0, tabs.length - 1, 1);
+  test('PlaceTabElementAnimatesTabMovedTowardsEnd', () => {
+    return testPlaceTabElementAnimation(0, tabs.length - 1, 1);
   });
 
-  test('PlaceTabElementAnimatesTabMovedTowardsEndRTL', async () => {
+  test('PlaceTabElementAnimatesTabMovedTowardsEndRTL', () => {
     document.documentElement.dir = 'rtl';
-    await testPlaceTabElementAnimation(0, tabs.length - 1, -1);
+    return testPlaceTabElementAnimation(0, tabs.length - 1, -1);
   });
 
   test('PlacesTabGroupElement', () => {
@@ -354,6 +354,72 @@ suite('TabList', () => {
     assertEquals(getUnpinnedTabs()[1], tabGroupElement.previousElementSibling);
   });
 
+  /**
+   * @param {number} indexToGroup
+   * @param {number} newIndex
+   * @param {number} direction
+   */
+  async function testPlaceTabGroupElementAnimation(
+      indexToGroup, newIndex, direction) {
+    await tabList.animationPromises;
+
+    // Group the tab at indexToGroup.
+    const unpinnedTabs = getUnpinnedTabs();
+    const tabToGroup = unpinnedTabs[indexToGroup];
+    webUIListenerCallback(
+        'tab-group-state-changed', tabToGroup.tab.id, indexToGroup, 'group0');
+
+    const groupElement =
+        /** @type {!TabGroupElement} */ (tabToGroup.parentElement);
+    tabList.placeTabGroupElement(groupElement, newIndex);
+    testPlaceElementAnimationParams(
+        groupElement, -1 * direction * Math.abs(newIndex - indexToGroup));
+
+    // Test animations on all the other tabs.
+    Array.from(getUnpinnedTabs())
+        .filter(tabElement => tabElement.parentElement !== groupElement)
+        .forEach(
+            tabElement =>
+                testPlaceElementAnimationParams(tabElement, direction));
+  }
+
+  test('PlaceTabGroupElementAnimatesTabGroupMovedTowardsStart', () => {
+    return testPlaceTabGroupElementAnimation(tabs.length - 1, 0, -1);
+  });
+
+  test('PlaceTabGroupElementAnimatesTabGroupMovedTowardsStartRTL', () => {
+    document.documentElement.dir = 'rtl';
+    return testPlaceTabGroupElementAnimation(tabs.length - 1, 0, 1);
+  });
+
+  test('PlaceTabGroupElementAnimatesTabGroupMovedTowardsEnd', () => {
+    return testPlaceTabGroupElementAnimation(0, tabs.length - 1, 1);
+  });
+
+  test('PlaceTabGroupElementAnimatesTabGroupMovedTowardsEndRTL', () => {
+    document.documentElement.dir = 'rtl';
+    return testPlaceTabGroupElementAnimation(0, tabs.length - 1, -1);
+  });
+
+  test('PlaceTabGroupElementAnimationWithMultipleTabs', async () => {
+    await tabList.animationPromises;
+
+    // Group all tabs except for the first one.
+    const ungroupedTab = getUnpinnedTabs()[0];
+    tabs.slice(1).forEach(tab => {
+      webUIListenerCallback(
+          'tab-group-state-changed', tab.id, tab.index, 'group0');
+    });
+
+    // Move the group to index 0.
+    const tabGroup = getTabGroups()[0];
+    tabList.placeTabGroupElement(tabGroup, 0);
+
+    // Both the TabElement and TabGroupElement should move by a scale of 1.
+    testPlaceElementAnimationParams(tabGroup, 1);
+    testPlaceElementAnimationParams(ungroupedTab, -1);
+  });
+
   test('AddNewTabGroup', () => {
     const appendedTab = {
       active: false,