diff --git a/packages/grid/src/vaadin-grid-drag-and-drop-mixin.js b/packages/grid/src/vaadin-grid-drag-and-drop-mixin.js index 512d7fec04..56a4fce6dc 100644 --- a/packages/grid/src/vaadin-grid-drag-and-drop-mixin.js +++ b/packages/grid/src/vaadin-grid-drag-and-drop-mixin.js @@ -139,10 +139,6 @@ 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 }); } @@ -312,25 +308,24 @@ export const DragAndDropMixin = (superClass) => } } - /** @private */ + /** + * Webkit-based browsers have issues with generating drag images + * for elements that have children with massive heights. Chromium + * browsers crash, while Safari experiences significant performance + * issues. To mitigate these issues, we hide the scroller element + * when drag starts to remove it from the drag image. + * + * Related issues: + * - https://github.com/vaadin/web-components/issues/7985 + * - https://issues.chromium.org/issues/383356871 + * + * @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'; + if (e.target.contains(this) && this.$.table.scrollHeight > 20000) { + this.$.scroller.style.display = 'none'; requestAnimationFrame(() => { - this.$.items.style.maxHeight = initialItemsMaxHeight; - this.$.table.style.overflow = initialTableOverflow; + this.$.scroller.style.display = ''; }); } } diff --git a/packages/grid/test/drag-and-drop.common.js b/packages/grid/test/drag-and-drop.common.js index 32e68fe1ec..a60ebba8d9 100644 --- a/packages/grid/test/drag-and-drop.common.js +++ b/packages/grid/test/drag-and-drop.common.js @@ -1088,8 +1088,6 @@ describe('drag and drop', () => { describe('draggable grid', () => { let container; - let items; - let table; beforeEach(async () => { container = fixtureSync(` @@ -1103,8 +1101,6 @@ describe('drag and drop', () => { document.body.appendChild(container); flushGrid(grid); await nextFrame(); - items = grid.shadowRoot.querySelector('#items'); - table = grid.shadowRoot.querySelector('#table'); }); async function setGridItems(count) { @@ -1121,12 +1117,8 @@ describe('drag and drop', () => { } 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); + expect(grid.$.scroller.style.display).to.equal(''); } ['5000', '50000'].forEach((count) => { diff --git a/packages/virtual-list/src/vaadin-virtual-list-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-mixin.js index e5b3d427e2..1dcd83c373 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-mixin.js @@ -65,7 +65,7 @@ export const VirtualListMixin = (superClass) => constructor() { super(); - this.__onDragStart = this.__onDragStart.bind(this); + this.__onDocumentDragStart = this.__onDocumentDragStart.bind(this); } /** @protected */ @@ -89,17 +89,13 @@ export const VirtualListMixin = (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.__onDragStart, { capture: true }); + document.addEventListener('dragstart', this.__onDocumentDragStart, { capture: true }); } /** @protected */ disconnectedCallback() { super.disconnectedCallback(); - document.removeEventListener('dragstart', this.__onDragStart, { capture: true }); + document.removeEventListener('dragstart', this.__onDocumentDragStart, { capture: true }); } /** @@ -153,25 +149,24 @@ 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'; + /** + * Webkit-based browsers have issues with generating drag images + * for elements that have children with massive heights. Chromium + * browsers crash, while Safari experiences significant performance + * issues. To mitigate these issues, we hide the items container + * when drag starts to remove it from the drag image. + * + * Related issues: + * - https://github.com/vaadin/web-components/issues/7985 + * - https://issues.chromium.org/issues/383356871 + * + * @private + */ + __onDocumentDragStart(e) { + if (e.target.contains(this) && this.scrollHeight > 20000) { + this.$.items.style.display = 'none'; requestAnimationFrame(() => { - this.$.items.style.maxHeight = initialItemsMaxHeight; - this.style.overflow = initialVirtualListOverflow; + this.$.items.style.display = ''; }); } } diff --git a/packages/virtual-list/test/drag-and-drop.common.js b/packages/virtual-list/test/drag-and-drop.common.js index cd844bf877..4c443504ab 100644 --- a/packages/virtual-list/test/drag-and-drop.common.js +++ b/packages/virtual-list/test/drag-and-drop.common.js @@ -6,7 +6,6 @@ import { hover } from '@vaadin/button/test/visual/helpers.js'; describe('drag and drop', () => { let virtualList; let container; - let items; beforeEach(async () => { container = fixtureSync(` @@ -20,7 +19,6 @@ describe('drag and drop', () => { }; document.body.appendChild(container); await nextFrame(); - items = virtualList.shadowRoot.querySelector('#items'); }); async function setVirtualListItems(count) { @@ -39,12 +37,8 @@ describe('drag and drop', () => { } 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); + expect(virtualList.$.items.style.display).to.equal(''); } ['5000', '50000'].forEach((count) => {