From 05fa0a27253005860921fba8c427e2d27ab3c590 Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Thu, 4 Apr 2019 14:33:39 -0700 Subject: [PATCH] Fix: Cleanup the thumbnail sidebar when preview errors (#976) --- src/lib/viewers/doc/DocBaseViewer.js | 6 ++++++ .../doc/__tests__/DocBaseViewer-test.js | 18 +++++++++++++++++- .../document/Thumbnails.e2e.test.js | 8 ++++++++ test/support/constants.js | 1 + 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index adc0a1f4f..d81551bfd 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -201,6 +201,12 @@ class DocBaseViewer extends BaseViewer { if (this.thumbnailsSidebar) { this.thumbnailsSidebar.destroy(); + } + + if (this.thumbnailsSidebarEl) { + // Since we are cleaning up make sure the thumbnails open class is + // removed so that the content div shifts back left + this.rootEl.classList.remove(CLASS_BOX_PREVIEW_THUMBNAILS_OPEN); this.thumbnailsSidebarEl.remove(); this.thumbnailsSidebarEl = null; } diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index 714dbe100..4c2e9af16 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -305,6 +305,21 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { expect(docBase.pdfViewer.cleanup).to.be.called; expect(docBase.pdfViewer.pdfDocument.destroy).to.be.called; }); + + it('should clean up the thumbnails sidebar instance and DOM element', () => { + docBase.thumbnailsSidebar = { + destroy: sandbox.stub() + }; + const thumbnailsSidebarEl = { + remove: sandbox.stub() + }; + docBase.thumbnailsSidebarEl = thumbnailsSidebarEl; + + docBase.destroy(); + expect(docBase.thumbnailsSidebar.destroy).to.be.called; + expect(thumbnailsSidebarEl.remove).to.be.called; + expect(stubs.classListRemove).to.be.called; + }); }); describe('prefetch()', () => { @@ -2399,7 +2414,8 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { classList: { add: stubs.classListAdd, remove: stubs.classListRemove - } + }, + remove: sandbox.stub() }; docBase.thumbnailsSidebarEl = thumbnailsSidebarEl; diff --git a/test/integration/document/Thumbnails.e2e.test.js b/test/integration/document/Thumbnails.e2e.test.js index ae18e715f..b83c9c23b 100644 --- a/test/integration/document/Thumbnails.e2e.test.js +++ b/test/integration/document/Thumbnails.e2e.test.js @@ -2,6 +2,7 @@ describe('Preview Document Thumbnails', () => { const token = Cypress.env('ACCESS_TOKEN'); const fileId = Cypress.env('FILE_ID_DOC_LARGE'); + const badFileId = Cypress.env('FILE_ID_BAD'); const THUMBNAIL_SELECTED_CLASS = 'bp-thumbnail-is-selected'; /** @@ -264,4 +265,11 @@ describe('Preview Document Thumbnails', () => { expect($virtualScrollerEl[0].scrollTop).to.not.equal(0); }); }); + + it('Should not show the thumbnails sidebar when a document preview errors', () => { + cy.showPreview(token, badFileId, { enableThumbnailsSidebar: true }); + + cy.contains('We\'re sorry the preview didn\'t load. This file could not be converted.'); + cy.getByTestId('thumbnails-sidebar').should('not.exist'); + }); }); diff --git a/test/support/constants.js b/test/support/constants.js index 1c665f226..3b6a7a926 100644 --- a/test/support/constants.js +++ b/test/support/constants.js @@ -1,5 +1,6 @@ Cypress.env({ ACCESS_TOKEN: 'S8wjvjOL9GEK5VtXsQNVMOwSrx1g55oC', + FILE_ID_BAD: '433514141824', FILE_ID_DOC: '415542803939', FILE_ID_DOC_LARGE: '420985736453', FILE_ID_HTML: '420872247534',