Commit ee657fa3 authored by John Lee's avatar John Lee Committed by Commit Bot

WebUI Tab Strip: Show context menu also on drop

Previously, with CL 2446353, the context menu was moved to be shown on
dragend and not drop. However, if the user moves their finger by a few
pixels but not enough for a index change, a drop event can occur before
a dragend event which finishes out the DragSession and therefore never
calls the dragend code that shows the context menu [1]. This CL
duplicates the context menu call to also happen on drop for these cases.

[1] https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/resources/tab_strip/drag_manager.js;l=574;drc=31eb3f4376c66b42eb4fc269025be4a8ee13e357;bpv=1

Bug: 1142608
Change-Id: Ic6daf3dfc38c945e45cf366baea7901ca645ff68
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500482
Commit-Queue: John Lee <johntlee@chromium.org>
Reviewed-by: default avatardpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821387}
parent c385712b
......@@ -252,12 +252,8 @@ class DragSession {
this.element_.setDragging(false);
this.element_.setDraggedOut(false);
if (event.type === 'dragend' && isTabElement(this.element_) &&
!this.hasMoved_) {
// If the user was dragging a tab and the tab has not ever been moved,
// show a context menu instead.
this.tabStripEmbedderProxy_.showTabContextMenu(
this.element_.tab.id, this.lastPoint_.x, this.lastPoint_.y);
if (event.type === 'dragend') {
this.maybeShowTabContextMenu_();
}
}
......@@ -311,6 +307,20 @@ class DragSession {
this.element_.setDragging(false);
this.element_.setDraggedOut(false);
this.maybeShowTabContextMenu_();
}
/** @private */
maybeShowTabContextMenu_() {
if (!isTabElement(this.element_) || this.hasMoved_) {
return;
}
// If the user was dragging a tab and the tab has not ever been moved,
// show a context menu instead.
this.tabStripEmbedderProxy_.showTabContextMenu(
this.element_.tab.id, this.lastPoint_.x, this.lastPoint_.y);
}
/**
......
......@@ -690,6 +690,27 @@ suite('DragManager', () => {
0, testTabStripEmbedderProxy.getCallCount('showTabContextMenu'));
});
test('DropWithoutMovingShowsContextMenu', async () => {
const draggedTab = delegate.children[0];
const dragDetails = {
bubbles: true,
composed: true,
clientX: 100,
clientY: 150,
dataTransfer: new MockDataTransfer(),
};
draggedTab.dispatchEvent(new DragEvent('dragstart', dragDetails));
draggedTab.dispatchEvent(new DragEvent('drop', dragDetails));
assertEquals(
1, testTabStripEmbedderProxy.getCallCount('showTabContextMenu'));
const [tabId, clientX, clientY] =
await testTabStripEmbedderProxy.whenCalled('showTabContextMenu');
assertEquals(draggedTab.tab.id, tabId);
assertEquals(dragDetails.clientX, clientX);
assertEquals(dragDetails.clientY, clientY);
});
test('DragEndWithDropEffectMoveDoesNotRemoveDraggedOutAttribute', () => {
const draggedTab = delegate.children[0];
const dataTransfer = new MockDataTransfer();
......
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