Skip to content

Commit eda141f

Browse files
authored
fix: hide scroller completely when dragging large grids (#8351)
1 parent 23271ea commit eda141f

File tree

4 files changed

+38
-62
lines changed

4 files changed

+38
-62
lines changed

packages/grid/src/vaadin-grid-drag-and-drop-mixin.js

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,6 @@ export const DragAndDropMixin = (superClass) =>
139139
/** @protected */
140140
connectedCallback() {
141141
super.connectedCallback();
142-
// Chromium based browsers cannot properly generate drag images for elements
143-
// that have children with massive heights. This workaround prevents crashes
144-
// and performance issues by excluding the items from the drag image.
145-
// https://github.com/vaadin/web-components/issues/7985
146142
document.addEventListener('dragstart', this.__onDocumentDragStart, { capture: true });
147143
}
148144

@@ -312,25 +308,24 @@ export const DragAndDropMixin = (superClass) =>
312308
}
313309
}
314310

315-
/** @private */
311+
/**
312+
* Webkit-based browsers have issues with generating drag images
313+
* for elements that have children with massive heights. Chromium
314+
* browsers crash, while Safari experiences significant performance
315+
* issues. To mitigate these issues, we hide the scroller element
316+
* when drag starts to remove it from the drag image.
317+
*
318+
* Related issues:
319+
* - https://github.com/vaadin/web-components/issues/7985
320+
* - https://issues.chromium.org/issues/383356871
321+
*
322+
* @private
323+
*/
316324
__onDocumentDragStart(e) {
317-
// The dragged element can be the element itself or a parent of the element
318-
if (!e.target.contains(this)) {
319-
return;
320-
}
321-
// The threshold value 20000 provides a buffer to both
322-
// - avoid the crash and the performance issues
323-
// - unnecessarily avoid excluding items from the drag image
324-
if (this.$.items.offsetHeight > 20000) {
325-
const initialItemsMaxHeight = this.$.items.style.maxHeight;
326-
const initialTableOverflow = this.$.table.style.overflow;
327-
// Momentarily hides the items until the browser starts generating the
328-
// drag image.
329-
this.$.items.style.maxHeight = '0';
330-
this.$.table.style.overflow = 'hidden';
325+
if (e.target.contains(this) && this.$.table.scrollHeight > 20000) {
326+
this.$.scroller.style.display = 'none';
331327
requestAnimationFrame(() => {
332-
this.$.items.style.maxHeight = initialItemsMaxHeight;
333-
this.$.table.style.overflow = initialTableOverflow;
328+
this.$.scroller.style.display = '';
334329
});
335330
}
336331
}

packages/grid/test/drag-and-drop.common.js

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,8 +1088,6 @@ describe('drag and drop', () => {
10881088

10891089
describe('draggable grid', () => {
10901090
let container;
1091-
let items;
1092-
let table;
10931091

10941092
beforeEach(async () => {
10951093
container = fixtureSync(`
@@ -1103,8 +1101,6 @@ describe('drag and drop', () => {
11031101
document.body.appendChild(container);
11041102
flushGrid(grid);
11051103
await nextFrame();
1106-
items = grid.shadowRoot.querySelector('#items');
1107-
table = grid.shadowRoot.querySelector('#table');
11081104
});
11091105

11101106
async function setGridItems(count) {
@@ -1121,12 +1117,8 @@ describe('drag and drop', () => {
11211117
}
11221118

11231119
async function assertDragSucceeds(draggedElement) {
1124-
// maxHeight and overflow are temporarily updated in the related fix
1125-
const initialItemsMaxHeight = items.style.maxHeight;
1126-
const initialTableOverflow = table.style.overflow;
11271120
await dragElement(draggedElement);
1128-
expect(items.style.maxHeight).to.equal(initialItemsMaxHeight);
1129-
expect(table.style.overflow).to.equal(initialTableOverflow);
1121+
expect(grid.$.scroller.style.display).to.equal('');
11301122
}
11311123

11321124
['5000', '50000'].forEach((count) => {

packages/virtual-list/src/vaadin-virtual-list-mixin.js

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export const VirtualListMixin = (superClass) =>
7777

7878
constructor() {
7979
super();
80-
this.__onDragStart = this.__onDragStart.bind(this);
80+
this.__onDocumentDragStart = this.__onDocumentDragStart.bind(this);
8181
}
8282

8383
/** @protected */
@@ -102,17 +102,13 @@ export const VirtualListMixin = (superClass) =>
102102
/** @protected */
103103
connectedCallback() {
104104
super.connectedCallback();
105-
// Chromium based browsers cannot properly generate drag images for elements
106-
// that have children with massive heights. This workaround prevents crashes
107-
// and performance issues by excluding the items from the drag image.
108-
// https://github.com/vaadin/web-components/issues/7985
109-
document.addEventListener('dragstart', this.__onDragStart, { capture: true });
105+
document.addEventListener('dragstart', this.__onDocumentDragStart, { capture: true });
110106
}
111107

112108
/** @protected */
113109
disconnectedCallback() {
114110
super.disconnectedCallback();
115-
document.removeEventListener('dragstart', this.__onDragStart, { capture: true });
111+
document.removeEventListener('dragstart', this.__onDocumentDragStart, { capture: true });
116112
}
117113

118114
/**
@@ -182,25 +178,24 @@ export const VirtualListMixin = (superClass) =>
182178
}
183179
}
184180

185-
/** @private */
186-
__onDragStart(e) {
187-
// The dragged element can be the element itself or a parent of the element
188-
if (!e.target.contains(this)) {
189-
return;
190-
}
191-
// The threshold value 20000 provides a buffer to both
192-
// - avoid the crash and the performance issues
193-
// - unnecessarily avoid excluding items from the drag image
194-
if (this.$.items.offsetHeight > 20000) {
195-
const initialItemsMaxHeight = this.$.items.style.maxHeight;
196-
const initialVirtualListOverflow = this.style.overflow;
197-
// Momentarily hides the items until the browser starts generating the
198-
// drag image.
199-
this.$.items.style.maxHeight = '0';
200-
this.style.overflow = 'hidden';
181+
/**
182+
* Webkit-based browsers have issues with generating drag images
183+
* for elements that have children with massive heights. Chromium
184+
* browsers crash, while Safari experiences significant performance
185+
* issues. To mitigate these issues, we hide the items container
186+
* when drag starts to remove it from the drag image.
187+
*
188+
* Related issues:
189+
* - https://github.com/vaadin/web-components/issues/7985
190+
* - https://issues.chromium.org/issues/383356871
191+
*
192+
* @private
193+
*/
194+
__onDocumentDragStart(e) {
195+
if (e.target.contains(this) && this.scrollHeight > 20000) {
196+
this.$.items.style.display = 'none';
201197
requestAnimationFrame(() => {
202-
this.$.items.style.maxHeight = initialItemsMaxHeight;
203-
this.style.overflow = initialVirtualListOverflow;
198+
this.$.items.style.display = '';
204199
});
205200
}
206201
}

packages/virtual-list/test/drag-and-drop.common.js

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { hover } from '@vaadin/button/test/visual/helpers.js';
66
describe('drag and drop', () => {
77
let virtualList;
88
let container;
9-
let items;
109

1110
beforeEach(async () => {
1211
container = fixtureSync(`
@@ -20,7 +19,6 @@ describe('drag and drop', () => {
2019
};
2120
document.body.appendChild(container);
2221
await nextFrame();
23-
items = virtualList.shadowRoot.querySelector('#items');
2422
});
2523

2624
async function setVirtualListItems(count) {
@@ -39,12 +37,8 @@ describe('drag and drop', () => {
3937
}
4038

4139
async function assertDragSucceeds(draggedElement) {
42-
// maxHeight and overflow are temporarily updated in the related fix
43-
const initialItemsMaxHeight = items.style.maxHeight;
44-
const initialVirtualListOverflow = virtualList.style.overflow;
4540
await dragElement(draggedElement);
46-
expect(items.style.maxHeight).to.equal(initialItemsMaxHeight);
47-
expect(virtualList.style.overflow).to.equal(initialVirtualListOverflow);
41+
expect(virtualList.$.items.style.display).to.equal('');
4842
}
4943

5044
['5000', '50000'].forEach((count) => {

0 commit comments

Comments
 (0)