From cd7d5bf2ab2fc4faefc162dd9409f44d6ec2e4b5 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Mon, 20 Dec 2021 19:19:35 +0200 Subject: [PATCH] fix: disable row drag and drop while loading (#2957) (#2198) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: disable row drag and drop while loading (#2957) * chore: fix lint issue * test: create new instance for each test * fix: workaround an IE11 bug * test: remove a skipped flaky test * Revert "test: create new instance for each test" This reverts commit 6039fb6778cc9eb6b078731641d228b3ba203585. * fix drag and drop test setup Co-authored-by: Sascha Ißbrücker --- src/vaadin-grid-drag-and-drop-mixin.html | 33 ++--- test/data-provider.html | 17 --- test/drag-and-drop.html | 146 ++++++++++++++++------- 3 files changed, 114 insertions(+), 82 deletions(-) diff --git a/src/vaadin-grid-drag-and-drop-mixin.html b/src/vaadin-grid-drag-and-drop-mixin.html index aa1b81b81..02ca8e32a 100644 --- a/src/vaadin-grid-drag-and-drop-mixin.html +++ b/src/vaadin-grid-drag-and-drop-mixin.html @@ -93,7 +93,7 @@ static get observers() { return [ - '_dragDropAccessChanged(rowsDraggable, dropMode, dragFilter, dropFilter)' + '_dragDropAccessChanged(rowsDraggable, dropMode, dragFilter, dropFilter, loading)' ]; } @@ -305,25 +305,15 @@ __getViewportRows() { // Workaround an IE11 bug where the `getBoundingClientRect` for header/footer // might return incorrect values - if (this._ie) { - this.$.header.style.outline = '0px solid transparent'; - const scrollerRect = this.$.scroller.getBoundingClientRect(); - const headerBottom = Math.max(this.$.header.getBoundingClientRect().bottom, scrollerRect.top); - const footerTop = Math.min(this.$.footer.getBoundingClientRect().top, scrollerRect.bottom); + this.$.header.style.outline = '0px solid transparent'; + const scrollerRect = this.$.scroller.getBoundingClientRect(); + const headerBottom = Math.max(this.$.header.getBoundingClientRect().bottom, scrollerRect.top); + const footerTop = Math.min(this.$.footer.getBoundingClientRect().top, scrollerRect.bottom); - return Array.from(this.$.items.children).filter((row) => { - const rowRect = row.getBoundingClientRect(); - return rowRect.bottom > headerBottom && rowRect.top < footerTop; - }); - } - - const headerBottom = this.$.header.getBoundingClientRect().bottom; - const footerTop = this.$.footer.getBoundingClientRect().top; - return Array.from(this.$.items.children) - .filter(row => { - const rowRect = row.getBoundingClientRect(); - return rowRect.bottom > headerBottom && rowRect.top < footerTop; - }); + return Array.from(this.$.items.children).filter((row) => { + const rowRect = row.getBoundingClientRect(); + return rowRect.bottom > headerBottom && rowRect.top < footerTop; + }); } /** @protected */ @@ -402,8 +392,9 @@ * @protected */ _filterDragAndDrop(row, model) { - const dragDisabled = !this.rowsDraggable || (this.dragFilter && !this.dragFilter(model)); - const dropDisabled = !this.dropMode || (this.dropFilter && !this.dropFilter(model)); + const loading = this.loading || row.hasAttribute('loading'); + const dragDisabled = !this.rowsDraggable || loading || (this.dragFilter && !this.dragFilter(model)); + const dropDisabled = !this.dropMode || loading || (this.dropFilter && !this.dropFilter(model)); const draggableElements = window.ShadyDOM ? [row] diff --git a/test/data-provider.html b/test/data-provider.html index 64fc1e81b..3e37dc812 100644 --- a/test/data-provider.html +++ b/test/data-provider.html @@ -671,23 +671,6 @@ }, loadDebounceTime); }); - // FIXME: flaky - it.skip('should call dataprovider multiple times to load all items', done => { - container.dataProvider.reset(); - grid.style.fontSize = '12px'; - grid.pageSize = 10; - flushGrid(grid); - - setTimeout(() => { - // assuming grid has about 30 items - expect(container.dataProvider.callCount).to.be.above(2); - for (var i = 0; i < container.dataProvider.callCount; i++) { - expect(container.dataProvider.getCall(i).args[0].page).to.eql(i); - } - done(); - }, loadDebounceTime); - }); - it('should always load visible items', done => { grid.pageSize = 10; diff --git a/test/drag-and-drop.html b/test/drag-and-drop.html index 4809794d8..96313a3d3 100644 --- a/test/drag-and-drop.html +++ b/test/drag-and-drop.html @@ -30,6 +30,11 @@ describe('drag and drop', () => { let grid; + const getTestItems = () => [ + {first: 'foo', last: 'bar'}, + {first: 'baz', last: 'qux'} + ]; + const getDraggable = (grid, rowIndex = 0) => { const row = Array.from(grid.$.items.children).filter(row => row.index === rowIndex)[0]; const cellContent = row.querySelector('slot').assignedNodes()[0]; @@ -119,43 +124,18 @@ return event; }; - before(done => { - grid = fixture('default'); - grid.hidden = true; - requestAnimationFrame(() => { - // Wait for the appear animation to finish - listenOnce(grid, 'animationend', () => { - flush(() => done()); - }); - requestAnimationFrame(() => grid.hidden = false); - }); - }); - beforeEach(done => { + grid = fixture('default'); + grid.items = getTestItems(); dragData = {}; - if (!grid.parentElement) { - document.body.appendChild(grid); - } - grid.items = [ - {first: 'foo', last: 'bar'}, - {first: 'baz', last: 'qux'} - ]; flushGrid(grid); - if (grid._safari) { - setTimeout(() => done()); - } else { - done(); - } - - }); - - afterEach(() => { - fireDragEnd(grid.$.table); - fireDrop(grid.$.table); - grid.rowsDraggable = false; - grid.dropMode = null; - grid.style.height = ''; - grid.selectedItems = []; + requestAnimationFrame(() => { + if (grid._safari) { + setTimeout(() => done()); + } else { + done(); + } + }); }); it('should not be draggable by default', () => { @@ -183,14 +163,11 @@ let dragStartSpy; let dropSpy; - before(() => { + beforeEach(() => { dragStartSpy = sinon.spy(); dropSpy = sinon.spy(); grid.addEventListener('grid-dragstart', dragStartSpy); grid.addEventListener('grid-drop', dropSpy); - }); - - beforeEach(() => { grid.rowsDraggable = true; grid.dropMode = 'on-top'; dragStartSpy.reset(); @@ -224,8 +201,7 @@ expect(spy.called).to.be.true; }); - // TODO: flaky in IE, timeout might not be appropriate - it.skip('should only use visible items for the row count state attribute', done => { + it('should only use visible items for the row count state attribute', done => { grid.style.height = '80px'; flushGrid(grid); grid.notifyResize(); @@ -759,6 +735,44 @@ expect(row.getAttribute('dragstart')).to.equal(''); }); + describe('filtering row drag - lazy loading', () => { + let finishLoadingItems; + + beforeEach(() => { + grid.dataProvider = (_params, callback) => { + finishLoadingItems = (items) => callback(items || getTestItems()); + }; + }); + + it('should disable row drag while loading items', () => { + expect(getDraggable(grid, 1)).not.to.be.ok; + expect(grid.$.items.children[1].hasAttribute('drag-disabled')).to.be.true; + }); + + it('should enable row drag once loading has finished', () => { + finishLoadingItems(); + expect(getDraggable(grid, 1)).to.be.ok; + expect(grid.$.items.children[1].hasAttribute('drag-disabled')).to.be.false; + }); + + it('should not run drag filter while loading items', () => { + grid.dragFilter = sinon.spy(); + expect(grid.dragFilter.called).to.be.false; + }); + + it('should disable row drag for rows without an item', () => { + finishLoadingItems([getTestItems()[0], undefined]); + expect(getDraggable(grid, 1)).not.to.be.ok; + expect(grid.$.items.children[1].hasAttribute('drag-disabled')).to.be.true; + }); + + it('should enable row drag once items are available', () => { + finishLoadingItems([getTestItems()[0], undefined]); + finishLoadingItems(); + expect(getDraggable(grid, 1)).to.be.ok; + expect(grid.$.items.children[1].hasAttribute('drag-disabled')).to.be.false; + }); + }); }); describe('filtering row drop', () => { @@ -817,6 +831,52 @@ expect(e.defaultPrevented).to.be.false; }); + describe('filtering row drop - lazy loading', () => { + let finishLoadingItems; + + beforeEach(() => { + grid.dataProvider = (_params, callback) => { + finishLoadingItems = (items) => callback(items || getTestItems()); + }; + }); + + it('should disable drop on row while loading items', () => { + const row = grid.$.items.children[1]; + fireDragOver(row, 'above'); + expect(row.hasAttribute('dragover')).to.be.false; + expect(row.hasAttribute('drop-disabled')).to.be.true; + }); + + it('should enable drop on row once loading has finished', () => { + finishLoadingItems(); + const row = grid.$.items.children[1]; + fireDragOver(row, 'above'); + expect(row.hasAttribute('dragover')).to.be.true; + expect(row.hasAttribute('drop-disabled')).to.be.false; + }); + + it('should not run drop filter while loading items', () => { + grid.dropFilter = sinon.spy(); + expect(grid.dropFilter.called).to.be.false; + }); + + it('should disable drop on row for rows without an item', () => { + finishLoadingItems([getTestItems()[0], undefined]); + const row = grid.$.items.children[1]; + fireDragOver(row, 'above'); + expect(row.hasAttribute('dragover')).to.be.false; + expect(row.hasAttribute('drop-disabled')).to.be.true; + }); + + it('should enable drop on row once items are available', () => { + finishLoadingItems([getTestItems()[0], undefined]); + finishLoadingItems(); + const row = grid.$.items.children[1]; + fireDragOver(row, 'above'); + expect(row.hasAttribute('dragover')).to.be.true; + expect(row.hasAttribute('drop-disabled')).to.be.false; + }); + }); }); describe('auto scroll', () => { @@ -842,16 +902,14 @@ expect(grid.$.table.scrollTop).to.be.within(scrollTop + 100, scrollTop + 200); }); - // TODO: flaky in Edge, magic numbers are not reliable, - it.skip('should auto scroll up', () => { + it('should auto scroll up', () => { grid._scrollToIndex(50); const scrollTop = grid.$.table.scrollTop; fireDragOver(grid.__getViewportRows()[0], 'below'); expect(grid.$.table.scrollTop).to.be.within(scrollTop - 100, scrollTop - 20); }); - // TODO: flaky in Edge, magic numbers are not reliable, - it.skip('should auto scroll up fast', () => { + it('should auto scroll up fast', () => { grid._scrollToIndex(50); const scrollTop = grid.$.table.scrollTop; fireDragOver(grid.__getViewportRows()[0], 'above');