Fix: On Thumbnails initial render, scroll item into view (#975)

box · Apr 4, 2019 · 55ad49d · 55ad49d
1 parent a1ea622
commit 55ad49d
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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);
+        });
+    });
 });