From 55ad49d6b94d45e0360f3d3ca021210eb54f310d Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Wed, 3 Apr 2019 17:03:27 -0700 Subject: [PATCH] Fix: On Thumbnails initial render, scroll item into view (#975) --- src/lib/VirtualScroller.js | 9 ++++++-- src/lib/__tests__/VirtualScroller-test.js | 9 ++++++-- .../document/Thumbnails.e2e.test.js | 22 +++++++++++++++++++ 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/lib/VirtualScroller.js b/src/lib/VirtualScroller.js index 8605adf51..fc4926e7c 100644 --- a/src/lib/VirtualScroller.js +++ b/src/lib/VirtualScroller.js @@ -109,8 +109,12 @@ class VirtualScroller { this.anchorEl.appendChild(this.scrollingEl); this.resize(this.containerHeight); + + const initialRowIndex = config.initialRowIndex || 0; // If initialRowIndex is < the first window into the list, then just render from the first item - this.renderItems(config.initialRowIndex < this.maxRenderedItems ? 0 : config.initialRowIndex); + this.renderItems(initialRowIndex < this.maxRenderedItems ? 0 : initialRowIndex); + + this.scrollIntoView(initialRowIndex); this.bindDOMListeners(); @@ -441,7 +445,8 @@ class VirtualScroller { const { scrollTop } = this.scrollingEl; const { offsetTop } = listItemEl; - return scrollTop <= offsetTop && offsetTop <= scrollTop + this.containerHeight; + // Ensure that the offsetTop and entire height of the listItemEl are inside the visible window + return scrollTop <= offsetTop && offsetTop + this.itemHeight <= scrollTop + this.containerHeight; } /** diff --git a/src/lib/__tests__/VirtualScroller-test.js b/src/lib/__tests__/VirtualScroller-test.js index 01c97edf0..e124e3ced 100644 --- a/src/lib/__tests__/VirtualScroller-test.js +++ b/src/lib/__tests__/VirtualScroller-test.js @@ -555,6 +555,7 @@ describe('VirtualScroller', () => { const scrollingEl = { scrollTop: 100, remove: () => {} }; virtualScroller.scrollingEl = scrollingEl; virtualScroller.containerHeight = 100; + virtualScroller.itemHeight = 20; }); it('should return false if scrollingEl is falsy', () => { @@ -575,8 +576,12 @@ describe('VirtualScroller', () => { expect(virtualScroller.isVisible({ offsetTop: 201 })).to.be.false; }); - it('should return true if the offsetTop of listItemEl is >= scrollTop && <= scrollTop + containerHeight', () => { - expect(virtualScroller.isVisible({ offsetTop: 101 })).to.be.true; + it('should return true if the offsetTop + itemHeight of listItemEl is fully within the containerHeight', () => { + expect(virtualScroller.isVisible({ offsetTop: 120 })).to.be.true; + }); + + it('should return false if the offsetTop + itemHeight of listItemEl is not fully within the containerHeight', () => { + expect(virtualScroller.isVisible({ offsetTop: 190 })).to.be.false; }); }); diff --git a/test/integration/document/Thumbnails.e2e.test.js b/test/integration/document/Thumbnails.e2e.test.js index 549003c2a..ae18e715f 100644 --- a/test/integration/document/Thumbnails.e2e.test.js +++ b/test/integration/document/Thumbnails.e2e.test.js @@ -242,4 +242,26 @@ describe('Preview Document Thumbnails', () => { cy.getByTestId('thumbnails-sidebar').should('not.be.visible'); }); + + it('Should scroll previewed page into view', () => { + showDocumentPreview({ enableThumbnailsSidebar: true }); + cy.getByTestId('thumbnails-sidebar').should('be.visible'); + + cy.getByTitle('Click to enter page number').click(); + cy + .getByTestId('page-num-input') + .should('be.visible') + .type('50') + .blur(); + + getThumbnailWithRenderedImage(50).should('have.class', THUMBNAIL_SELECTED_CLASS); + + cy.reload(); + + cy.getByTestId('thumbnails-sidebar').should('be.visible'); + getThumbnailWithRenderedImage(50).should('have.class', THUMBNAIL_SELECTED_CLASS); + cy.getByTestId('thumbnails-sidebar').find('.bp-vs').then(($virtualScrollerEl) => { + expect($virtualScrollerEl[0].scrollTop).to.not.equal(0); + }); + }); });