Commit 10e2eac7 authored by John Lee's avatar John Lee Committed by Commit Bot

WebUI Tab Strip: Update drag image logic

This CL aims to make the drag image while dragging tabs and tab groups
more consistent and more inline to each element's dimensions and DOM
structure by:

1) Initially basing offsets on the draggable target (the element with
the draggable attribute) instead of the element. TabGroupElements'
drag targets are their chips, which will result in different
calculations than if using the entire TabGroupElement's dimensions
since tab groups can have many tabs within them.

2) Using percentages instead of exact pixel values as the dimensions
of the drag image can change before and after dragstart.

3) Taking into consideration any margin or padding that a drag image
may have by using the dimensions of a drag image's "center" element.

Change-Id: I83c22a220023e6686ebea053a70c61f16c6239f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2300988
Commit-Queue: John Lee <johntlee@chromium.org>
Reviewed-by: default avatardpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790028}
parent 3296e9bb
...@@ -297,7 +297,7 @@ class DragSession { ...@@ -297,7 +297,7 @@ class DragSession {
/** @param {!DragEvent} event */ /** @param {!DragEvent} event */
start(event) { start(event) {
event.dataTransfer.effectAllowed = 'move'; event.dataTransfer.effectAllowed = 'move';
const draggedItemRect = this.element_.getBoundingClientRect(); const draggedItemRect = event.composedPath()[0].getBoundingClientRect();
this.element_.setDragging(true); this.element_.setDragging(true);
const dragImage = this.element_.getDragImage(); const dragImage = this.element_.getDragImage();
...@@ -313,14 +313,26 @@ class DragSession { ...@@ -313,14 +313,26 @@ class DragSession {
verticalOffset = 25; verticalOffset = 25;
// </if> // </if>
const xDiffFromCenter = const eventXPercentage =
event.clientX - draggedItemRect.left - (draggedItemRect.width / 2); (event.clientX - draggedItemRect.left) / draggedItemRect.width;
const yDiffFromCenter = event.clientY - draggedItemRect.top - const eventYPercentage =
verticalOffset - (draggedItemRect.height / 2); (event.clientY - draggedItemRect.top) / draggedItemRect.height;
event.dataTransfer.setDragImage( // First, align the top-left corner of the drag image's center element
dragImage, (dragImageRect.width / 2 + xDiffFromCenter / scaleFactor), // to the event's coordinates.
(dragImageRect.height / 2 + yDiffFromCenter / scaleFactor)); const dragImageCenterRect =
this.element_.getDragImageCenter().getBoundingClientRect();
let xOffset = (dragImageCenterRect.left - dragImageRect.left) * scaleFactor;
let yOffset = (dragImageCenterRect.top - dragImageRect.top) * scaleFactor;
// Then, offset the drag image again by using the event's coordinates
// within the dragged item itself so that the drag image appears positioned
// as closely as its state before dragging.
xOffset += dragImageCenterRect.width * eventXPercentage;
yOffset += dragImageCenterRect.height * eventYPercentage;
yOffset -= verticalOffset;
event.dataTransfer.setDragImage(dragImage, xOffset, yOffset);
if (isTabElement(this.element_)) { if (isTabElement(this.element_)) {
event.dataTransfer.setData( event.dataTransfer.setData(
......
...@@ -295,15 +295,8 @@ ...@@ -295,15 +295,8 @@
:host([dragging_]) #dragImage { :host([dragging_]) #dragImage {
/* Enough padding to not crop the box shadow set on #tab below. */ /* Enough padding to not crop the box shadow set on #tab below. */
--drag-image-padding: 25px; --drag-image-padding: 25px;
align-items: center;
display: flex;
height: 100%; height: 100%;
justify-content: center; padding: var(--drag-image-padding);
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; position: absolute;
top: 100vh; top: 100vh;
width: 100%; width: 100%;
......
...@@ -200,6 +200,13 @@ export class TabElement extends CustomElement { ...@@ -200,6 +200,13 @@ export class TabElement extends CustomElement {
return this.dragImageEl_; return this.dragImageEl_;
} }
/** @return {!HTMLElement} */
getDragImageCenter() {
// dragImageEl_ has padding, so the drag image should be centered relative
// to tabEl_, the element within the padding.
return this.tabEl_;
}
/** /**
* @param {string} imgData * @param {string} imgData
*/ */
......
...@@ -48,6 +48,13 @@ export class TabGroupElement extends CustomElement { ...@@ -48,6 +48,13 @@ export class TabGroupElement extends CustomElement {
return /** @type {!HTMLElement} */ (this.$('#dragImage')); return /** @type {!HTMLElement} */ (this.$('#dragImage'));
} }
/** @return {!HTMLElement} */
getDragImageCenter() {
// Since the drag handle is #chip, the drag image should be centered
// relatively to it.
return /** @type {!HTMLElement} */ (this.$('#chip'));
}
/** @private */ /** @private */
onClickChip_() { onClickChip_() {
if (!this.dataset.groupId) { if (!this.dataset.groupId) {
......
...@@ -150,12 +150,15 @@ suite('DragManager', () => { ...@@ -150,12 +150,15 @@ suite('DragManager', () => {
test('DragStartSetsDragImage', () => { test('DragStartSetsDragImage', () => {
const draggedElement = delegate.children[0]; const draggedElement = delegate.children[0];
const dragImage = draggedElement.getDragImage(); const dragImage = draggedElement.getDragImage();
const dragImageCenter = draggedElement.getDragImageCenter();
// Mock the dimensions and position of the element and the drag image. // Mock the dimensions and position of the element and the drag image.
const draggedElementRect = {top: 20, left: 30, width: 200, height: 150}; const draggedElementRect = {top: 20, left: 30, width: 200, height: 150};
draggedElement.getBoundingClientRect = () => draggedElementRect; draggedElement.getBoundingClientRect = () => draggedElementRect;
const dragImageRect = {top: 20, left: 30, width: 200, height: 150}; const dragImageRect = {top: 20, left: 30, width: 200, height: 150};
dragImage.getBoundingClientRect = () => dragImageRect; dragImage.getBoundingClientRect = () => dragImageRect;
const dragImageCenterRect = {top: 25, left: 25, width: 100, height: 120};
dragImageCenter.getBoundingClientRect = () => dragImageCenterRect;
const eventClientX = 100; const eventClientX = 100;
const eventClientY = 50; const eventClientY = 50;
...@@ -172,26 +175,40 @@ suite('DragManager', () => { ...@@ -172,26 +175,40 @@ suite('DragManager', () => {
assertEquals( assertEquals(
mockDataTransfer.dragImageData.image, draggedElement.getDragImage()); mockDataTransfer.dragImageData.image, draggedElement.getDragImage());
const xDiffFromCenter = const eventXPercentage =
eventClientX - draggedElementRect.left - draggedElementRect.width / 2; (eventClientX - draggedElementRect.left) / draggedElementRect.width;
const yDiffFromCenter = const eventYPercentage =
eventClientY - draggedElementRect.top - draggedElementRect.height / 2; (eventClientY - draggedElementRect.top) / draggedElementRect.height;
// Offset should account for any margins or padding between the
// dragImageCenter and the dragImage.
let dragImageCenterLeftMargin =
dragImageCenterRect.left - dragImageRect.left;
let dragImageCenterTopMargin = dragImageCenterRect.top - dragImageRect.top;
if (isChromeOS) { if (isChromeOS) {
assertEquals( // Dimensions are scaled on ChromeOS so the margins and paddings are also
dragImageRect.width / 2 + xDiffFromCenter / 1.2, // scaled.
mockDataTransfer.dragImageData.offsetX); dragImageCenterLeftMargin *= 1.2;
assertEquals( dragImageCenterTopMargin *= 1.2;
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);
} }
// Offset should map event's coordinates to within the dimensions of the
// dragImageCenter.
const eventXWithinDragImageCenter =
eventXPercentage * dragImageCenterRect.width;
const eventYWithinDragImageCenter =
eventYPercentage * dragImageCenterRect.height;
let expectedOffsetX =
dragImageCenterLeftMargin + eventXWithinDragImageCenter;
let expectedOffsetY =
dragImageCenterTopMargin + eventYWithinDragImageCenter;
if (isChromeOS) {
expectedOffsetY -= 25;
}
assertEquals(expectedOffsetX, mockDataTransfer.dragImageData.offsetX);
assertEquals(expectedOffsetY, mockDataTransfer.dragImageData.offsetY);
}); });
test('DragOverMovesTabs', async () => { test('DragOverMovesTabs', async () => {
......
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