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 <[email protected]>
Reviewed-by: dpapad <[email protected]>
Cr-Commit-Position: refs/heads/master@{#780391}
diff --git a/chrome/browser/resources/tab_strip/drag_manager.js b/chrome/browser/resources/tab_strip/drag_manager.js
index e39900f1..b5073320 100644
--- a/chrome/browser/resources/tab_strip/drag_manager.js
+++ b/chrome/browser/resources/tab_strip/drag_manager.js
@@ -359,7 +359,8 @@
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 @@
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 @@
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;
diff --git a/chrome/browser/resources/tab_strip/tab_group.js b/chrome/browser/resources/tab_strip/tab_group.js
index 46d6da5..cfd47a53 100644
--- a/chrome/browser/resources/tab_strip/tab_group.js
+++ b/chrome/browser/resources/tab_strip/tab_group.js
@@ -24,6 +24,23 @@
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} */
diff --git a/chrome/browser/resources/tab_strip/tab_list.js b/chrome/browser/resources/tab_strip/tab_list.js
index 7b19dc4..3aa01dac 100644
--- a/chrome/browser/resources/tab_strip/tab_list.js
+++ b/chrome/browser/resources/tab_strip/tab_list.js
@@ -711,8 +711,8 @@
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 @@
* @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 @@
}
this.unpinnedTabsElement_.insertBefore(element, elementAtIndex);
+ animateElementMoved(element, initialDomPrevSibling, initialDomNextSibling);
}
/** @private */
