WebUI Tab Strip: Animate pinned tabs moving

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

chrome/browser/resources/tab_strip/tab_list.html

@@ -41,11 +41,12 @@
      * tab. 20px is subtracted from the height of an unpinned tab as there
      * are two 10px gaps to separate each of the 3 pinned tabs. */
     --tabstrip-pinned-tab-size: calc((var(--tabstrip-tab-height) - 20px) / 3);
+    --tabstrip-tab-spacing: 10px;
 
     display: grid;
     grid-auto-columns: var(--tabstrip-pinned-tab-size);
     grid-auto-flow: column;
-    grid-gap: 10px;
+    grid-gap: var(--tabstrip-tab-spacing);
     grid-template-rows: repeat(3, var(--tabstrip-pinned-tab-size));
     padding-block-end: var(--tabstrip-tab-list-vertical-padding);
     padding-block-start: var(--tabstrip-tab-list-vertical-padding);

chrome/browser/resources/tab_strip/tab_list.js

@@ -15,7 +15,7 @@ import {isRTL} from 'chrome://resources/js/util.m.js';
 
 import {CustomElement} from './custom_element.js';
 import {DragManager, DragManagerDelegate} from './drag_manager.js';
-import {TabElement} from './tab.js';
+import {isTabElement, TabElement} from './tab.js';
 import {isTabGroupElement, TabGroupElement} from './tab_group.js';
 import {TabStripEmbedderProxy, TabStripEmbedderProxyImpl} from './tab_strip_embedder_proxy.js';
 import {tabStripOptions} from './tab_strip_options.js';
@@ -54,55 +54,77 @@ const LayoutVariable = {
  * @param {number} newIndex
  */
 function animateElementMoved(movedElement, prevIndex, newIndex) {
-  // Direction is -1 for moving the movedElement towards the start of the tab
-  // strip, and +1 for moving towards the end of the tab strip.
-  const direction = Math.sign(prevIndex - newIndex);
-
-  // The horizontal direction is flipped depending on RTL vs. LTR.
-  const horizontalDirection = (isRTL() ? -1 : 1) * direction;
+  // Direction is -1 for moving towards a lower index, +1 for moving
+  // towards a higher index. If moving towards a lower index, the TabList needs
+  // to animate everything from the movedElement's current index to its prev
+  // index by traversing the nextElementSibling of each element because the
+  // movedElement is now at a preceding position from all the elements it has
+  // slid across. If moving towards a higher index, the TabList needs to
+  // traverse the previousElementSiblings.
+  const direction = Math.sign(newIndex - prevIndex);
 
   /**
    * @param {!Element} element
    * @return {?Element}
    */
   function getSiblingToAnimate(element) {
-    return direction > -1 ? element.nextElementSibling :
+    return direction === -1 ? element.nextElementSibling :
                               element.previousElementSibling;
   }
   let elementToAnimate = getSiblingToAnimate(movedElement);
-  for (let i = prevIndex; i !== newIndex && elementToAnimate; i -= direction) {
-    slideElement(elementToAnimate, -1 * horizontalDirection);
+  for (let i = newIndex; i !== prevIndex && elementToAnimate; i -= direction) {
+    const elementToAnimatePrevIndex = i;
+    const elementToAnimateNewIndex = i - direction;
+    slideElement(
+        elementToAnimate, elementToAnimatePrevIndex, elementToAnimateNewIndex);
     elementToAnimate = getSiblingToAnimate(elementToAnimate);
   }
 
-  // Animate the moved TabElement itself the total number of tabs that it
-  // has been moved across.
-  slideElement(
-      movedElement, horizontalDirection * Math.abs(newIndex - prevIndex));
+  slideElement(movedElement, prevIndex, newIndex);
 }
 
 /**
- * Animates the slide of an element across the tab strip.
+ * Animates the slide of an element across the tab strip (both vertically and
+ * horizontally for pinned tabs, and horizontally for other tabs and groups).
  * @param {!Element} element
- * @param {number} horizontalScale
+ * @param {number} prevIndex
+ * @param {number} newIndex
  */
-function slideElement(element, horizontalScale) {
+function slideElement(element, prevIndex, newIndex) {
+  let horizontalMovement = newIndex - prevIndex;
+  let verticalMovement = 0;
+
+  if (isTabElement(element) && element.tab.pinned) {
+    const pinnedTabsPerColumn = 3;
+    const columnChange = Math.floor(newIndex / pinnedTabsPerColumn) -
+        Math.floor(prevIndex / pinnedTabsPerColumn);
+    horizontalMovement = columnChange;
+    verticalMovement =
+        (newIndex - prevIndex) - (columnChange * pinnedTabsPerColumn);
+  }
+
+  horizontalMovement *= isRTL() ? -1 : 1;
+
+  const translateX = `calc(${horizontalMovement * -1} * ` +
+      '(var(--tabstrip-tab-width) + var(--tabstrip-tab-spacing)))';
+  const translateY = `calc(${verticalMovement * -1} * ` +
+      '(var(--tabstrip-tab-height) + var(--tabstrip-tab-spacing)))';
+
   element.isValidDragOverTarget = false;
   const animation = element.animate(
       [
-        {
-          transform: 'translateX(calc(' + horizontalScale + ' ' +
-              '* (var(--tabstrip-tab-width) + var(--tabstrip-tab-spacing))))',
-        },
-        {transform: 'translateX(0)'},
+        {transform: `translate(${translateX}, ${translateY})`},
+        {transform: 'translate(0, 0)'},
       ],
       {
         duration: 120,
         easing: 'ease-out',
       });
-  animation.onfinish = () => {
+  function onComplete() {
     element.isValidDragOverTarget = true;
-  };
+  }
+  animation.oncancel = onComplete;
+  animation.onfinish = onComplete;
 }
 
 /** @implements {DragManagerDelegate} */

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

@@ -269,9 +269,11 @@ suite('TabList', () => {
 
   /**
    * @param {!Element} element
-   * @param {number} scale
+   * @param {number} horizontalScale
+   * @param {number} verticalScale
    */
-  function testPlaceElementAnimationParams(element, scale) {
+  function testPlaceElementAnimationParams(
+      element, horizontalScale, verticalScale) {
     const animations = element.getAnimations();
 
     // TODO(crbug.com/1090645): Remove logging once the test no longer flakes.
@@ -289,13 +291,17 @@ suite('TabList', () => {
     assertEquals('ease-out', animations[0].effect.getTiming().easing);
 
     const keyframes = animations[0].effect.getKeyframes();
-    const tabSpacingVars =
+    const horizontalTabSpacingVars =
         '(var(--tabstrip-tab-width) + var(--tabstrip-tab-spacing))';
+    const verticalTabSpacingVars =
+        '(var(--tabstrip-tab-height) + var(--tabstrip-tab-spacing))';
     assertEquals(2, keyframes.length);
     assertEquals(
-        `translateX(calc(${scale} * ${tabSpacingVars}))`,
+        `translate(calc(${horizontalScale} * ${
+            horizontalTabSpacingVars}), calc(${verticalScale} * ${
+            verticalTabSpacingVars}))`,
         keyframes[0].transform);
-    assertEquals('translateX(0px)', keyframes[1].transform);
+    assertEquals('translate(0px, 0px)', keyframes[1].transform);
   }
 
   /**
@@ -314,13 +320,13 @@ suite('TabList', () => {
     const movedTab = unpinnedTabs[indexToMove];
     tabList.placeTabElement(movedTab, newIndex, false, undefined);
     testPlaceElementAnimationParams(
-        movedTab, -1 * direction * Math.abs(newIndex - indexToMove));
+        movedTab, -1 * direction * Math.abs(newIndex - indexToMove), 0);
 
     Array.from(unpinnedTabs)
         .filter(tabElement => tabElement !== movedTab)
         .forEach(
             tabElement =>
-                testPlaceElementAnimationParams(tabElement, direction));
+                testPlaceElementAnimationParams(tabElement, direction, 0));
   }
 
   test('PlaceTabElementAnimatesTabMovedTowardsStart', () => {
@@ -341,6 +347,75 @@ suite('TabList', () => {
     return testPlaceTabElementAnimation(0, tabs.length - 1, -1);
   });
 
+  test('PlacePinnedTabElementAnimatesTabsWithinSameColumn', async () => {
+    tabs.forEach(pinTabAt);
+    await tabList.animationPromises;
+
+    // Test moving a tab within the same column. If a tab is moved from index 0
+    // to index 2, it should move vertically down 2 places. Tabs at index 1 and
+    // index 2 should move up 1 space.
+    const pinnedTabs = getPinnedTabs();
+    tabList.placeTabElement(pinnedTabs[0], 2, /*pinned=*/ true);
+    await Promise.all([
+      testPlaceElementAnimationParams(pinnedTabs[0], 0, -2),
+      testPlaceElementAnimationParams(pinnedTabs[1], 0, 1),
+      testPlaceElementAnimationParams(pinnedTabs[2], 0, 1),
+    ]);
+  });
+
+  test(
+      'PlacePinnedTabElementAnimatesTabsAcrossColumnsToHigherIndex',
+      async () => {
+        tabs.forEach(pinTabAt);
+        for (let i = 0; i < 4; i++) {
+          webUIListenerCallback('tab-created', {
+            active: false,
+            alertStates: [],
+            id: tabs.length + i,
+            index: tabs.length + i,
+            pinned: true,
+            title: 'Pinned tab',
+          });
+        }
+        await tabList.animationPromises;
+
+        const pinnedTabs = getPinnedTabs();
+        tabList.placeTabElement(pinnedTabs[2], 6, /*pinned=*/ true);
+        await Promise.all([
+          testPlaceElementAnimationParams(pinnedTabs[2], -2, 2),
+          testPlaceElementAnimationParams(pinnedTabs[3], 1, -2),
+          testPlaceElementAnimationParams(pinnedTabs[4], 0, 1),
+          testPlaceElementAnimationParams(pinnedTabs[5], 0, 1),
+          testPlaceElementAnimationParams(pinnedTabs[6], 1, -2),
+        ]);
+      });
+
+  test(
+      'PlacePinnedTabElementAnimatesTabsAcrossColumnsToLowerIndex',
+      async () => {
+        tabs.forEach(pinTabAt);
+        for (let i = 0; i < 4; i++) {
+          webUIListenerCallback('tab-created', {
+            active: false,
+            alertStates: [],
+            id: tabs.length + i,
+            index: tabs.length + i,
+            pinned: true,
+            title: 'Pinned tab',
+          });
+        }
+        await tabList.animationPromises;
+
+        const pinnedTabs = getPinnedTabs();
+        tabList.placeTabElement(pinnedTabs[3], 0, /*pinned=*/ true);
+        await Promise.all([
+          testPlaceElementAnimationParams(pinnedTabs[3], 1, 0),
+          testPlaceElementAnimationParams(pinnedTabs[2], -1, 2),
+          testPlaceElementAnimationParams(pinnedTabs[1], 0, -1),
+          testPlaceElementAnimationParams(pinnedTabs[0], 0, -1),
+        ]);
+      });
+
   test('PlacesTabGroupElement', () => {
     const tabGroupElement = /** @type {!TabGroupElement} */ (
         document.createElement('tabstrip-tab-group'));
@@ -373,14 +448,14 @@ suite('TabList', () => {
         /** @type {!TabGroupElement} */ (tabToGroup.parentElement);
     tabList.placeTabGroupElement(groupElement, newIndex);
     testPlaceElementAnimationParams(
-        groupElement, -1 * direction * Math.abs(newIndex - indexToGroup));
+        groupElement, -1 * direction * Math.abs(newIndex - indexToGroup), 0);
 
     // Test animations on all the other tabs.
     Array.from(getUnpinnedTabs())
         .filter(tabElement => tabElement.parentElement !== groupElement)
         .forEach(
             tabElement =>
-                testPlaceElementAnimationParams(tabElement, direction));
+                testPlaceElementAnimationParams(tabElement, direction, 0));
   }
 
   test('PlaceTabGroupElementAnimatesTabGroupMovedTowardsStart', () => {
@@ -416,8 +491,8 @@ suite('TabList', () => {
     tabList.placeTabGroupElement(tabGroup, 0);
 
     // Both the TabElement and TabGroupElement should move by a scale of 1.
-    testPlaceElementAnimationParams(tabGroup, 1);
-    testPlaceElementAnimationParams(ungroupedTab, -1);
+    testPlaceElementAnimationParams(tabGroup, 1, 0);
+    testPlaceElementAnimationParams(ungroupedTab, -1, 0);
   });
 
   test('AddNewTabGroup', () => {