Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: exclude items from drag image for large scrollers (#8028) (CP: 24.5) #8053

Merged
merged 1 commit into from
Nov 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions packages/grid/src/vaadin-grid-drag-and-drop-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ export const DragAndDropMixin = (superClass) =>
return ['_dragDropAccessChanged(rowsDraggable, dropMode, dragFilter, dropFilter, loading)'];
}

constructor() {
super();
this.__onDocumentDragStart = this.__onDocumentDragStart.bind(this);
}

/** @protected */
ready() {
super.ready();
Expand All @@ -131,6 +136,22 @@ export const DragAndDropMixin = (superClass) =>
});
}

/** @protected */
connectedCallback() {
super.connectedCallback();
// Chromium based browsers cannot properly generate drag images for elements
// that have children with massive heights. This workaround prevents crashes
// and performance issues by excluding the items from the drag image.
// https://github.com/vaadin/web-components/issues/7985
document.addEventListener('dragstart', this.__onDocumentDragStart, { capture: true });
}

/** @protected */
disconnectedCallback() {
super.disconnectedCallback();
document.removeEventListener('dragstart', this.__onDocumentDragStart, { capture: true });
}

/** @private */
_onDragStart(e) {
if (this.rowsDraggable) {
Expand Down Expand Up @@ -291,6 +312,29 @@ export const DragAndDropMixin = (superClass) =>
}
}

/** @private */
__onDocumentDragStart(e) {
// The dragged element can be the element itself or a parent of the element
if (!e.target.contains(this)) {
return;
}
// The threshold value 20000 provides a buffer to both
// - avoid the crash and the performance issues
// - unnecessarily avoid excluding items from the drag image
if (this.$.items.offsetHeight > 20000) {
const initialItemsMaxHeight = this.$.items.style.maxHeight;
const initialTableOverflow = this.$.table.style.overflow;
// Momentarily hides the items until the browser starts generating the
// drag image.
this.$.items.style.maxHeight = '0';
this.$.table.style.overflow = 'hidden';
requestAnimationFrame(() => {
this.$.items.style.maxHeight = initialItemsMaxHeight;
this.$.table.style.overflow = initialTableOverflow;
});
}
}

/** @private */
__dndAutoScroll(clientY) {
if (this.__dndAutoScrolling) {
Expand Down
60 changes: 60 additions & 0 deletions packages/grid/test/drag-and-drop.common.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { expect } from '@vaadin/chai-plugins';
import { aTimeout, fixtureSync, listenOnce, nextFrame, oneEvent } from '@vaadin/testing-helpers';
import { resetMouse, sendMouse } from '@web/test-runner-commands';
import sinon from 'sinon';
import { hover } from '@vaadin/button/test/visual/helpers.js';
import { flushGrid, getBodyCellContent, getFirstCell, getRowBodyCells, getRows } from './helpers.js';

describe('drag and drop', () => {
Expand Down Expand Up @@ -1083,4 +1085,62 @@ describe('drag and drop', () => {
expect(getFirstCell(grid).getAttribute('part')).to.contain('drag-source-row-cell');
});
});

describe('draggable grid', () => {
let container;
let items;
let table;

beforeEach(async () => {
container = fixtureSync(`
<div style="width: 400px; height: 400px;">
<vaadin-grid draggable="true" style="width: 300px; height: 300px;">
<vaadin-grid-column path="value"></vaadin-grid-column>
</vaadin-grid>
</div>
`);
grid = container.querySelector('vaadin-grid');
document.body.appendChild(container);
flushGrid(grid);
await nextFrame();
items = grid.shadowRoot.querySelector('#items');
table = grid.shadowRoot.querySelector('#table');
});

async function setGridItems(count) {
grid.items = Array.from({ length: count }, (_, i) => ({ value: `Item ${i + 1}` }));
await nextFrame();
}

async function dragElement(element) {
await resetMouse();
await hover(element);
await sendMouse({ type: 'down' });
await sendMouse({ type: 'move', position: [100, 100] });
await sendMouse({ type: 'up' });
}

async function assertDragSucceeds(draggedElement) {
// maxHeight and overflow are temporarily updated in the related fix
const initialItemsMaxHeight = items.style.maxHeight;
const initialTableOverflow = table.style.overflow;
await dragElement(draggedElement);
expect(items.style.maxHeight).to.equal(initialItemsMaxHeight);
expect(table.style.overflow).to.equal(initialTableOverflow);
}

['5000', '50000'].forEach((count) => {
it(`should not crash when dragging a grid with ${count} items`, async () => {
await setGridItems(count);
await assertDragSucceeds(grid);
});

it(`should not crash when dragging a container that has a grid with ${count} items`, async () => {
grid.removeAttribute('draggable');
container.setAttribute('draggable', true);
await setGridItems(count);
await assertDragSucceeds(container);
});
});
});
});
45 changes: 44 additions & 1 deletion packages/virtual-list/src/vaadin-virtual-list-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ export const VirtualListMixin = (superClass) =>
return this.__virtualizer.lastVisibleIndex;
}

constructor() {
super();
this.__onDragStart = this.__onDragStart.bind(this);
}

/** @protected */
ready() {
super.ready();
Expand All @@ -75,13 +80,28 @@ export const VirtualListMixin = (superClass) =>
scrollContainer: this.shadowRoot.querySelector('#items'),
reorderElements: true,
});

this.__overflowController = new OverflowController(this);
this.addController(this.__overflowController);

processTemplates(this);
}

/** @protected */
connectedCallback() {
super.connectedCallback();
// Chromium based browsers cannot properly generate drag images for elements
// that have children with massive heights. This workaround prevents crashes
// and performance issues by excluding the items from the drag image.
// https://github.com/vaadin/web-components/issues/7985
document.addEventListener('dragstart', this.__onDragStart, { capture: true });
}

/** @protected */
disconnectedCallback() {
super.disconnectedCallback();
document.removeEventListener('dragstart', this.__onDragStart, { capture: true });
}

/**
* Scroll to a specific index in the virtual list.
*
Expand Down Expand Up @@ -133,6 +153,29 @@ export const VirtualListMixin = (superClass) =>
}
}

/** @private */
__onDragStart(e) {
// The dragged element can be the element itself or a parent of the element
if (!e.target.contains(this)) {
return;
}
// The threshold value 20000 provides a buffer to both
// - avoid the crash and the performance issues
// - unnecessarily avoid excluding items from the drag image
if (this.$.items.offsetHeight > 20000) {
const initialItemsMaxHeight = this.$.items.style.maxHeight;
const initialVirtualListOverflow = this.style.overflow;
// Momentarily hides the items until the browser starts generating the
// drag image.
this.$.items.style.maxHeight = '0';
this.style.overflow = 'hidden';
requestAnimationFrame(() => {
this.$.items.style.maxHeight = initialItemsMaxHeight;
this.style.overflow = initialVirtualListOverflow;
});
}
}

/**
* Requests an update for the content of the rows.
* While performing the update, it invokes the renderer passed in the `renderer` property for each visible row.
Expand Down
2 changes: 2 additions & 0 deletions packages/virtual-list/test/drag-and-drop-lit.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import '../vaadin-lit-virtual-list.js';
import './drag-and-drop.common.js';
2 changes: 2 additions & 0 deletions packages/virtual-list/test/drag-and-drop-polymer.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import '../vaadin-virtual-list.js';
import './drag-and-drop.common.js';
63 changes: 63 additions & 0 deletions packages/virtual-list/test/drag-and-drop.common.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { expect } from '@vaadin/chai-plugins';
import { fixtureSync, nextFrame } from '@vaadin/testing-helpers';
import { resetMouse, sendMouse } from '@web/test-runner-commands';
import { hover } from '@vaadin/button/test/visual/helpers.js';

describe('drag and drop', () => {
let virtualList;
let container;
let items;

beforeEach(async () => {
container = fixtureSync(`
<div style="width: 300px; height: 300px;">
<vaadin-virtual-list draggable="true"></vaadin-virtual-list>
</div>
`);
virtualList = container.querySelector('vaadin-virtual-list');
virtualList.renderer = (root, _, { item }) => {
root.innerHTML = `<div>${item.label}</div>`;
};
document.body.appendChild(container);
await nextFrame();
items = virtualList.shadowRoot.querySelector('#items');
});

async function setVirtualListItems(count) {
virtualList.items = Array.from({ length: count }).map((_, i) => {
return { label: `Item ${i}` };
});
await nextFrame();
}

async function dragElement(element) {
await resetMouse();
await hover(element);
await sendMouse({ type: 'down' });
await sendMouse({ type: 'move', position: [100, 100] });
await sendMouse({ type: 'up' });
}

async function assertDragSucceeds(draggedElement) {
// maxHeight and overflow are temporarily updated in the related fix
const initialItemsMaxHeight = items.style.maxHeight;
const initialVirtualListOverflow = virtualList.style.overflow;
await dragElement(draggedElement);
expect(items.style.maxHeight).to.equal(initialItemsMaxHeight);
expect(virtualList.style.overflow).to.equal(initialVirtualListOverflow);
}

['5000', '50000'].forEach((count) => {
it(`should not crash when dragging a virtual list with ${count} items`, async () => {
await setVirtualListItems(count);
await assertDragSucceeds(virtualList);
});

it(`should not crash when dragging a container that has a virtual list with ${count} items`, async () => {
virtualList.removeAttribute('draggable');
container.setAttribute('draggable', true);
await setVirtualListItems(count);
await assertDragSucceeds(container);
});
});
});