WebUI Tab Strip: Update drag image scale and positioning

The drag image was previously misaligned because of added padding and
because ChromeOS automatically scales and offsets drag images. This CL
fixes the alignment issue and forces the drag image to scale to 110%
instead as per UX request.

The offsets are calculated by determining the pixels the drag started
from the center of the element, and then mapping them onto the bounds
of the drag image.

https://i.imgur.com/yLSJQ47.png

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

chrome/browser/resources/tab_strip/drag_manager.js

@@ -297,9 +297,28 @@ class DragSession {
     event.dataTransfer.effectAllowed = 'move';
     const draggedItemRect = this.element_.getBoundingClientRect();
     this.element_.setDragging(true);
+
+    const dragImage = this.element_.getDragImage();
+    const dragImageRect = dragImage.getBoundingClientRect();
+
+    let scaleFactor = 1;
+    let verticalOffset = 0;
+
+    // <if expr="chromeos">
+    // Touch on ChromeOS automatically scales drag images by 1.2 and adds a
+    // vertical offset of 25px. See //ash/drag_drop/drag_drop_controller.cc.
+    scaleFactor = 1.2;
+    verticalOffset = 25;
+    // </if>
+
+    const xDiffFromCenter =
+        event.clientX - draggedItemRect.left - (draggedItemRect.width / 2);
+    const yDiffFromCenter = event.clientY - draggedItemRect.top -
+        verticalOffset - (draggedItemRect.height / 2);
+
     event.dataTransfer.setDragImage(
-        this.element_.getDragImage(), event.clientX - draggedItemRect.left,
-        event.clientY - draggedItemRect.top);
+        dragImage, (dragImageRect.width / 2 + xDiffFromCenter / scaleFactor),
+        (dragImageRect.height / 2 + yDiffFromCenter / scaleFactor));
 
     if (isTabElement(this.element_)) {
       event.dataTransfer.setData(

chrome/browser/resources/tab_strip/tab.html

@@ -2,6 +2,7 @@
   :host {
     --tabstrip-tab-transition-duration: 250ms;
 
+    display: block;
     height: var(--tabstrip-tab-height);
     padding-inline-end: var(--tabstrip-tab-spacing);
     position: relative;
@@ -288,12 +289,17 @@
   /* When being dragged, the contents of the drag image needs to be off-screen
    * with nothing else on top or below obscuring it. */
   :host([dragging_]) #dragImage {
+    /* Enough padding to not crop the box shadow set on #tab below. */
+    --drag-image-padding: 25px;
     align-items: center;
     display: flex;
     height: 100%;
     justify-content: center;
-    /* Enough padding to not crop the box shadow set on #tab below. */
-    padding: 25px;
+    padding-block-end: var(--drag-image-padding);
+    padding-block-start: var(--drag-image-padding);
+    padding-inline-end: calc(
+        var(--tabstrip-tab-spacing) + var(--drag-image-padding));
+    padding-inline-start: var(--drag-image-padding);
     position: absolute;
     top: 100vh;
     width: 100%;
@@ -301,6 +307,7 @@
 
   :host([dragging_]) #tab {
     box-shadow: var(--tabstrip-tab-dragging-shadow);
+    transform: scale(var(--tabstrip-tab-drag-image-scale));
   }
 </style>
 

chrome/browser/resources/tab_strip/tab_group.html

@@ -105,6 +105,7 @@
   box-shadow: var(--tabstrip-tab-dragging-shadow);
   height: var(--tabstrip-tab-height);
   overflow: hidden;
+  transform: scale(var(--tabstrip-tab-drag-image-scale));
 }
 
 :host([getting-drag-image_]) ::slotted(tabstrip-tab) {

chrome/browser/resources/tab_strip/tab_list.html

@@ -8,6 +8,15 @@
         0 4px 4px 0 rgba(var(--google-grey-800-rgb), .3),
         0 8px 12px 6px rgba(var(--google-grey-800-rgb), .15);
 
+<if expr="not chromeos">
+    --tabstrip-tab-drag-image-scale: 1.1;
+</if>
+<if expr="chromeos">
+    /* ChromeOS scales drag images by 1.2, so this variable multiplied by
+     * 1.2 should be around 1.1. */
+    --tabstrip-tab-drag-image-scale: calc(1.1 / 1.2);
+</if>
+
     background: var(--tabstrip-background-color);
     box-sizing: border-box;
     display: flex;

chrome/browser/resources/tab_strip/tab_strip_resources.grd

@@ -38,7 +38,8 @@
           file="${root_gen_dir}/chrome/browser/resources/tab_strip/tab_list.js"
           use_base_dir="false"
           type="chrome_html"
-          compress="gzip"/>
+          compress="gzip"
+          preprocess="true"/>
       <structure
           name="IDR_TAB_STRIP_TAB_JS"
           file="${root_gen_dir}/chrome/browser/resources/tab_strip/tab.js"
@@ -76,7 +77,8 @@
           name="IDR_TAB_STRIP_DRAG_MANAGER_JS"
           file="drag_manager.js"
           type="chrome_html"
-          compress="gzip"/>
+          compress="gzip"
+          preprocess="true"/>
     </structures>
 
     <includes>

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

 // Copyright 2020 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
+import {isChromeOS} from 'chrome://resources/js/cr.m.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {DragManager, PLACEHOLDER_GROUP_ID, PLACEHOLDER_TAB_ID} from 'chrome://tab-strip/drag_manager.js';
 import {TabElement} from 'chrome://tab-strip/tab.js';
@@ -128,26 +129,56 @@ suite('DragManager', () => {
     });
     dragManager = new DragManager(delegate);
     dragManager.startObserving();
+
+    document.body.style.margin = 0;
+    document.body.appendChild(delegate);
   });
 
   test('DragStartSetsDragImage', () => {
-    const draggedTab = delegate.children[0];
+    const draggedElement = delegate.children[0];
+    const dragImage = draggedElement.getDragImage();
+
+    // Mock the dimensions and position of the element and the drag image.
+    const draggedElementRect = {top: 20, left: 30, width: 200, height: 150};
+    draggedElement.getBoundingClientRect = () => draggedElementRect;
+    const dragImageRect = {top: 20, left: 30, width: 200, height: 150};
+    dragImage.getBoundingClientRect = () => dragImageRect;
+
+    const eventClientX = 100;
+    const eventClientY = 50;
     const mockDataTransfer = new MockDataTransfer();
     const dragStartEvent = new DragEvent('dragstart', {
       bubbles: true,
       composed: true,
-      clientX: 100,
-      clientY: 150,
+      clientX: eventClientX,
+      clientY: eventClientY,
       dataTransfer: mockDataTransfer,
     });
-    draggedTab.dispatchEvent(dragStartEvent);
+    draggedElement.dispatchEvent(dragStartEvent);
     assertEquals(dragStartEvent.dataTransfer.effectAllowed, 'move');
     assertEquals(
-        mockDataTransfer.dragImageData.image, draggedTab.getDragImage());
-    assertEquals(
-        mockDataTransfer.dragImageData.offsetX, 100 - draggedTab.offsetLeft);
-    assertEquals(
-        mockDataTransfer.dragImageData.offsetY, 150 - draggedTab.offsetTop);
+        mockDataTransfer.dragImageData.image, draggedElement.getDragImage());
+
+    const xDiffFromCenter =
+        eventClientX - draggedElementRect.left - draggedElementRect.width / 2;
+    const yDiffFromCenter =
+        eventClientY - draggedElementRect.top - draggedElementRect.height / 2;
+
+    if (isChromeOS) {
+      assertEquals(
+          dragImageRect.width / 2 + xDiffFromCenter / 1.2,
+          mockDataTransfer.dragImageData.offsetX);
+      assertEquals(
+          dragImageRect.height / 2 + (yDiffFromCenter - 25) / 1.2,
+          mockDataTransfer.dragImageData.offsetY);
+    } else {
+      assertEquals(
+          dragImageRect.width / 2 + xDiffFromCenter,
+          mockDataTransfer.dragImageData.offsetX);
+      assertEquals(
+          dragImageRect.height / 2 + yDiffFromCenter,
+          mockDataTransfer.dragImageData.offsetY);
+    }
   });
 
   test('DragOverMovesTabs', async () => {

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

@@ -22,6 +22,7 @@ suite('Tab', function() {
     alertStates: [],
     id: 1001,
     networkState: TabNetworkState.NONE,
+    pinned: false,
     title: 'My title',
   };
 
@@ -38,6 +39,7 @@ suite('Tab', function() {
     document.body.innerHTML = '';
 
     // Set CSS variable for animations
+    document.body.style.setProperty('--tabstrip-tab-height', '100px');
     document.body.style.setProperty('--tabstrip-tab-width', '280px');
     document.body.style.setProperty('--tabstrip-tab-spacing', '20px');
 
@@ -60,7 +62,7 @@ suite('Tab', function() {
     const animationPromise = tabElement.slideIn();
     // Before animation completes.
     assertEquals('20px', tabElementStyle.paddingRight);
-    assertEquals('none', tabElementStyle.maxWidth);
+    assertEquals('280px', tabElementStyle.maxWidth);
     assertEquals('matrix(0, 0, 0, 0, 0, 0)', tabElementStyle.transform);
     await animationPromise;
     // After animation completes.
@@ -432,4 +434,19 @@ suite('Tab', function() {
     assertEquals(await testTabsApiProxy.whenCalled('activateTab'), tab.id);
     testTabsApiProxy.reset();
   });
+
+  test('DragImagePreservesAspectRatio', () => {
+    const originalBoundingBox = tabElement.$('#tab').getBoundingClientRect();
+    const originalAspectRatio =
+        originalBoundingBox.width / originalBoundingBox.height;
+    tabElement.setDragging(true);
+    const dragImageBoundingBox =
+        tabElement.getDragImage().querySelector('#tab').getBoundingClientRect();
+    const dragImageAspectRatio =
+        dragImageBoundingBox.width / dragImageBoundingBox.height;
+    // Check the Math.floor values of these values to prevent possible
+    // flakiness caused by comparing float values.
+    assertEquals(
+        Math.floor(originalAspectRatio), Math.floor(dragImageAspectRatio));
+  });
 });