WebUI Tab Strip: Support dropping a group into place

When a drag enters with tab group data from another window, the tab
strip will create a placeholder TabGroupElement and use it to move
elements around.

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

chrome/browser/resources/tab_strip/drag_manager.js

@@ -14,6 +14,9 @@ import {TabData, TabNetworkState, TabsApiProxy} from './tabs_api_proxy.js';
 /** @const {number} */
 export const PLACEHOLDER_TAB_ID = -1;
 
+/** @const {string} */
+export const PLACEHOLDER_GROUP_ID = 'placeholder';
+
 /**
  * The data type key for pinned state of a tab. Since drag events only expose
  * whether or not a data type exists (not the actual value), presence of this
@@ -78,9 +81,6 @@ export class DragManagerDelegate {
    * @param {number} index
    */
   placeTabGroupElement(element, index) {}
-
-  /** @param {!Element} element */
-  showDropPlaceholder(element) {}
 }
 
 /** @typedef {!DragManagerDelegate|!HTMLElement} */
@@ -149,6 +149,15 @@ class DragSession {
       return DragSession.createFromElement(delegate, placeholderTabElement);
     }
 
+    if (event.dataTransfer.types.includes(getGroupIdDataType())) {
+      const placeholderGroupElement = /** @type {!TabGroupElement} */
+          (document.createElement('tabstrip-tab-group'));
+      placeholderGroupElement.dataset.groupId = PLACEHOLDER_GROUP_ID;
+      placeholderGroupElement.setDragging(true);
+      delegate.placeTabGroupElement(placeholderGroupElement, -1);
+      return DragSession.createFromElement(delegate, placeholderGroupElement);
+    }
+
     return null;
   }
 
@@ -169,6 +178,26 @@ class DragSession {
           /** @type {!TabElement} */ (this.element_));
     }
 
+    if (this.element_.children.length === 0) {
+      // If this group element has no children, it was a placeholder element
+      // being dragged. Find out the destination index by finding the index of
+      // the tab closest to it and incrementing it by 1.
+      const previousElement = this.element_.previousElementSibling;
+      if (!previousElement) {
+        return 0;
+      }
+      if (isTabElement(previousElement)) {
+        return this.delegate_.getIndexOfTab(
+                   /** @private {!TabElement} */ (previousElement)) +
+            1;
+      }
+
+      assert(isTabGroupElement(previousElement));
+      return this.delegate_.getIndexOfTab(/** @private {!TabElement} */ (
+                 previousElement.lastElementChild)) +
+          1;
+    }
+
     // If a tab group is moving backwards (to the front of the tab strip), the
     // new index is the index of the first tab in that group. If a tab group is
     // moving forwards (to the end of the tab strip), the new index is the index
@@ -201,15 +230,36 @@ class DragSession {
 
   /** @return {boolean} */
   isDraggingPlaceholder() {
+    return this.isDraggingPlaceholderTab_() ||
+        this.isDraggingPlaceholderGroup_();
+  }
+
+  /**
+   * @return {boolean}
+   * @private
+   */
+  isDraggingPlaceholderTab_() {
     return isTabElement(this.element_) &&
-        this.element_.tab.id == PLACEHOLDER_TAB_ID;
+        this.element_.tab.id === PLACEHOLDER_TAB_ID;
+  }
+
+  /**
+   * @return {boolean}
+   * @private
+   */
+  isDraggingPlaceholderGroup_() {
+    return isTabGroupElement(this.element_) &&
+        this.element_.dataset.groupId === PLACEHOLDER_GROUP_ID;
   }
 
   /** @param {!DragEvent} event */
   finish(event) {
-    if (this.isDraggingPlaceholder()) {
+    if (this.isDraggingPlaceholderTab_()) {
       const id = Number(event.dataTransfer.getData(getTabIdDataType()));
       this.element_.tab = Object.assign({}, this.element_.tab, {id});
+    } else if (this.isDraggingPlaceholderGroup_()) {
+      this.element_.dataset.groupId =
+          event.dataTransfer.getData(getGroupIdDataType());
     }
 
     const dstIndex = this.dstIndex;
@@ -229,6 +279,19 @@ class DragSession {
     this.element_.setDragging(false);
   }
 
+  /**
+   * @param {!TabElement|!TabGroupElement} dragOverElement
+   * @return {boolean}
+   */
+  shouldOffsetIndexForGroup_(dragOverElement) {
+    // Since TabGroupElements do not have any TabElements, they need to offset
+    // the index for any elements that come after it as if there is at least
+    // one element inside of it.
+    return this.isDraggingPlaceholder() &&
+        !!(dragOverElement.compareDocumentPosition(this.element_) &
+           Node.DOCUMENT_POSITION_PRECEDING);
+  }
+
   /** @param {!DragEvent} event */
   start(event) {
     event.dataTransfer.effectAllowed = 'move';
@@ -278,15 +341,20 @@ class DragSession {
     const dragOverTabElement =
         /** @type {!TabElement|undefined} */ (composedPath.find(isTabElement));
     if (dragOverTabElement && !dragOverTabElement.tab.pinned) {
-      const dragOverIndex = this.delegate_.getIndexOfTab(dragOverTabElement);
+      let dragOverIndex = this.delegate_.getIndexOfTab(dragOverTabElement);
+      dragOverIndex +=
+          this.shouldOffsetIndexForGroup_(dragOverTabElement) ? 1 : 0;
       this.delegate_.placeTabGroupElement(tabGroupElement, dragOverIndex);
       return;
     }
 
-    const dragOverGroupElement = composedPath.find(isTabGroupElement);
+    const dragOverGroupElement = /** @type {!TabGroupElement|undefined} */ (
+        composedPath.find(isTabGroupElement));
     if (dragOverGroupElement) {
-      const dragOverIndex = this.delegate_.getIndexOfTab(
+      let dragOverIndex = this.delegate_.getIndexOfTab(
           /** @type {!TabElement} */ (dragOverGroupElement.firstElementChild));
+      dragOverIndex +=
+          this.shouldOffsetIndexForGroup_(dragOverGroupElement) ? 1 : 0;
       this.delegate_.placeTabGroupElement(tabGroupElement, dragOverIndex);
     }
   }
@@ -345,34 +413,24 @@ export class DragManager {
     /** @type {?DragSession} */
     this.dragSession_ = null;
 
-    /** @type {!Element} */
-    this.dropPlaceholder_ = document.createElement('div');
-    this.dropPlaceholder_.id = 'dropPlaceholder';
-
     /** @private {!TabsApiProxy} */
     this.tabsProxy_ = TabsApiProxy.getInstance();
   }
 
   /** @private */
   onDragLeave_() {
-    if (this.dragSession_ && this.dragSession_.isDraggingPlaceholder()) {
-      this.dragSession_.cancel();
-      this.dragSession_ = null;
+    if (this.dragSession_ && !this.dragSession_.isDraggingPlaceholder()) {
+      return;
     }
 
-    // TODO(johntlee): Handle drag and drop groups from other windows with
-    // DragSession.
-    this.dropPlaceholder_.remove();
+    this.dragSession_.cancel();
+    this.dragSession_ = null;
   }
 
   /** @param {!DragEvent} event */
   onDragOver_(event) {
     event.preventDefault();
-
     if (!this.dragSession_) {
-      // TODO(johntlee): Handle drag and drop groups from other windows with
-      // DragSession.
-      this.delegate_.showDropPlaceholder(this.dropPlaceholder_);
       return;
     }
 
@@ -418,19 +476,12 @@ export class DragManager {
    * @param {!DragEvent} event
    */
   onDrop_(event) {
-    if (this.dragSession_) {
-      this.dragSession_.finish(event);
-      this.dragSession_ = null;
+    if (!this.dragSession_) {
       return;
     }
 
-    // TODO(johntlee): Handle drag and drop groups from other windows with
-    // DragSession.
-    this.dropPlaceholder_.remove();
-    if (event.dataTransfer.types.includes(getGroupIdDataType())) {
-      const groupId = event.dataTransfer.getData(getGroupIdDataType());
-      this.tabsProxy_.moveGroup(groupId, -1);
-    }
+    this.dragSession_.finish(event);
+    this.dragSession_ = null;
   }
 
   startObserving() {

chrome/browser/resources/tab_strip/tab_group.html

@@ -4,7 +4,7 @@
   --tabstrip-tab-group-title-margin: var(--tabstrip-tab-spacing);
 }
 
-:host(:empty) {
+:host(:empty:not([dragging])) {
   /* A tab group can temporarily become empty as a tab is being dragged out. */
   display: none;
 }

chrome/browser/resources/tab_strip/tab_list.js

@@ -748,12 +748,6 @@ class TabListElement extends CustomElement {
     this.animateScrollPosition_(scrollBy);
   }
 
-  /** @param {!Element} element */
-  showDropPlaceholder(element) {
-    this.unpinnedTabsElement_.appendChild(element);
-    this.animateScrollPosition_(element.offsetLeft);
-  }
-
   /**
    * @param {number} tabId
    * @param {string} imgData

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

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
-import {DragManager, PLACEHOLDER_TAB_ID} from 'chrome://tab-strip/drag_manager.js';
+import {DragManager, PLACEHOLDER_GROUP_ID, PLACEHOLDER_TAB_ID} from 'chrome://tab-strip/drag_manager.js';
 import {TabElement} from 'chrome://tab-strip/tab.js';
 import {TabGroupElement} from 'chrome://tab-strip/tab_group.js';
 import {TabsApiProxy} from 'chrome://tab-strip/tabs_api_proxy.js';
@@ -96,6 +96,7 @@ suite('DragManager', () => {
   ];
 
   const strings = {
+    tabGroupIdDataType: 'application/group-id',
     tabIdDataType: 'application/tab-id',
   };
 
@@ -395,6 +396,85 @@ suite('DragManager', () => {
     assertEquals('group0', groupId);
   });
 
+  test('DragExternalTabGroupOverTab', async () => {
+    const externalGroupId = 'external-group';
+    const mockDataTransfer = new MockDataTransfer();
+    mockDataTransfer.setData(strings.tabGroupIdDataType, `${externalGroupId}`);
+    const dragEnterEvent = new DragEvent('dragenter', {
+      bubbles: true,
+      composed: true,
+      dataTransfer: mockDataTransfer,
+    });
+    delegate.dispatchEvent(dragEnterEvent);
+
+    // Test that a placeholder group was created.
+    const placeholderGroupElement = delegate.lastElementChild;
+    assertEquals(PLACEHOLDER_GROUP_ID, placeholderGroupElement.dataset.groupId);
+
+    function dragOverTabAt(dragOverIndex) {
+      const dragOverTab = delegate.children[dragOverIndex];
+      const dragOverEvent = new DragEvent('dragover', {
+        bubbles: true,
+        composed: true,
+        dataTransfer: mockDataTransfer,
+      });
+      dragOverTab.dispatchEvent(dragOverEvent);
+      assertEquals(placeholderGroupElement, delegate.children[dragOverIndex]);
+    }
+
+    // Test moving forwards and backwards in the tab strip.
+    dragOverTabAt(0);
+    dragOverTabAt(1);
+    dragOverTabAt(2);
+    dragOverTabAt(0);
+
+    const dropEvent = new DragEvent('drop', {
+      bubbles: true,
+      composed: true,
+      dataTransfer: mockDataTransfer,
+    });
+    placeholderGroupElement.dispatchEvent(dropEvent);
+    assertEquals(externalGroupId, placeholderGroupElement.dataset.groupId);
+    const [groupId, index] = await testTabsApiProxy.whenCalled('moveGroup');
+    assertEquals(externalGroupId, groupId);
+    assertEquals(0, index);
+  });
+
+  test('DragExternalTabGroupOverTabGroup', async () => {
+    const externalGroupId = 'external-group';
+    const mockDataTransfer = new MockDataTransfer();
+    mockDataTransfer.setData(strings.tabGroupIdDataType, `${externalGroupId}`);
+    const dragEnterEvent = new DragEvent('dragenter', {
+      bubbles: true,
+      composed: true,
+      dataTransfer: mockDataTransfer,
+    });
+    delegate.dispatchEvent(dragEnterEvent);
+    const placeholderGroupElement = delegate.lastElementChild;
+
+    const dragOverGroupIndex = 0;
+    const dragOverGroup =
+        groupTab(delegate.children[dragOverGroupIndex], 'group0');
+    const dragOverEvent = new DragEvent('dragover', {
+      bubbles: true,
+      composed: true,
+      dataTransfer: mockDataTransfer,
+    });
+    dragOverGroup.dispatchEvent(dragOverEvent);
+    assertEquals(
+        placeholderGroupElement, delegate.children[dragOverGroupIndex]);
+
+    const dropEvent = new DragEvent('drop', {
+      bubbles: true,
+      composed: true,
+      dataTransfer: mockDataTransfer,
+    });
+    placeholderGroupElement.dispatchEvent(dropEvent);
+    const [groupId, index] = await testTabsApiProxy.whenCalled('moveGroup');
+    assertEquals(externalGroupId, groupId);
+    assertEquals(dragOverGroupIndex, index);
+  });
+
   test('CancelDragResetsPosition', () => {
     const draggedIndex = 0;
     const draggedTab = delegate.children[draggedIndex];