Skip to content

Commit

Permalink
Fix: Cleanup the thumbnail sidebar when preview errors
Browse files Browse the repository at this point in the history
  • Loading branch information
Conrad Chan committed Apr 4, 2019
1 parent 3b09559 commit 4aa48d4
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 1 deletion.
6 changes: 6 additions & 0 deletions src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,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;
}
Expand Down
18 changes: 17 additions & 1 deletion src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()', () => {
Expand Down Expand Up @@ -2399,7 +2414,8 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
classList: {
add: stubs.classListAdd,
remove: stubs.classListRemove
}
},
remove: sandbox.stub()
};

docBase.thumbnailsSidebarEl = thumbnailsSidebarEl;
Expand Down
8 changes: 8 additions & 0 deletions test/integration/document/Thumbnails.e2e.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -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');
});
});
1 change: 1 addition & 0 deletions test/support/constants.js
Original file line number Diff line number Diff line change
@@ -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',
Expand Down

0 comments on commit 4aa48d4

Please sign in to comment.