From 6f2200a44606bdebca1478a3c50a81e7462f4587 Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Tue, 26 Mar 2019 12:55:26 -0700 Subject: [PATCH] Update: Wider thumbnails (#963) --- src/lib/ThumbnailsSidebar.js | 4 +- src/lib/__tests__/ThumbnailsSidebar-test.js | 11 +++--- src/lib/viewers/doc/_docBase.scss | 9 ++--- .../document/Thumbnails.e2e.test.js | 37 +++++++++++-------- 4 files changed, 33 insertions(+), 28 deletions(-) diff --git a/src/lib/ThumbnailsSidebar.js b/src/lib/ThumbnailsSidebar.js index e13b886a9..3fa959e79 100644 --- a/src/lib/ThumbnailsSidebar.js +++ b/src/lib/ThumbnailsSidebar.js @@ -8,9 +8,9 @@ const CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE = 'bp-thumbnail-image'; const CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE_LOADED = 'bp-thumbnail-image-loaded'; const CLASS_BOX_PREVIEW_THUMBNAIL_IS_SELECTED = 'bp-thumbnail-is-selected'; const CLASS_BOX_PREVIEW_THUMBNAIL_PAGE_NUMBER = 'bp-thumbnail-page-number'; -const DEFAULT_THUMBNAILS_SIDEBAR_WIDTH = 127; // 200px sidebar width - 25px margin right, - 40px for page number - 8px for border +export const DEFAULT_THUMBNAILS_SIDEBAR_WIDTH = 154; // 225px sidebar width - 25px margin right, - 40px for page number - 6px for border const THUMBNAIL_MARGIN = 15; -const BORDER_WIDTH = 8; +const BORDER_WIDTH = 6; class ThumbnailsSidebar { /** @property {HTMLElement} - The anchor element for this ThumbnailsSidebar */ diff --git a/src/lib/__tests__/ThumbnailsSidebar-test.js b/src/lib/__tests__/ThumbnailsSidebar-test.js index 684b3d652..7d151ef7e 100644 --- a/src/lib/__tests__/ThumbnailsSidebar-test.js +++ b/src/lib/__tests__/ThumbnailsSidebar-test.js @@ -1,8 +1,9 @@ /* eslint-disable no-unused-expressions */ -import ThumbnailsSidebar from '../ThumbnailsSidebar'; +import ThumbnailsSidebar, { DEFAULT_THUMBNAILS_SIDEBAR_WIDTH } from '../ThumbnailsSidebar'; import VirtualScroller from '../VirtualScroller'; const sandbox = sinon.sandbox.create(); +const TEST_SCALE = DEFAULT_THUMBNAILS_SIDEBAR_WIDTH / 10; describe('ThumbnailsSidebar', () => { let thumbnailsSidebar; @@ -102,7 +103,7 @@ describe('ThumbnailsSidebar', () => { return pagePromise.then(() => { expect(stubs.getViewport).to.be.called; - expect(thumbnailsSidebar.scale).to.be.equal(12.7); // DEFAULT_THUMBNAILS_SIDEBAR_WIDTH / width + expect(thumbnailsSidebar.scale).to.be.equal(TEST_SCALE); // DEFAULT_THUMBNAILS_SIDEBAR_WIDTH / width expect(thumbnailsSidebar.pageRatio).to.be.equal(1); expect(stubs.vsInit).to.be.called; }); @@ -209,6 +210,7 @@ describe('ThumbnailsSidebar', () => { .returns(createImagePromise); stubs.appendChild = sandbox.stub(); stubs.addClass = sandbox.stub(); + stubs.renderNextThumbnailImage = sandbox.stub(thumbnailsSidebar, 'renderNextThumbnailImage'); const thumbnailEl = { lastChild: { appendChild: stubs.appendChild }, @@ -281,7 +283,7 @@ describe('ThumbnailsSidebar', () => { stubs.getViewport.withArgs(1).returns({ width: 10, height: 10 }); stubs.render.returns(Promise.resolve()); - const expScale = 12.7; // Should be DEFAULT_THUMBNAILS_SIDEBAR_WIDTH / 10 + const expScale = TEST_SCALE; // Should be DEFAULT_THUMBNAILS_SIDEBAR_WIDTH / 10 return thumbnailsSidebar.getThumbnailDataURL(1).then(() => { expect(stubs.getPage).to.be.called; @@ -298,8 +300,7 @@ describe('ThumbnailsSidebar', () => { stubs.getViewport.withArgs(1).returns({ width: 10, height: 20 }); stubs.render.returns(Promise.resolve()); - const expScale = 6.3; // Should be 6.3 instead of 12.7 because the viewport ratio above is 0.5 instead of 1 - // and because canvas width is integer, so 63.5 becomes 63 + const expScale = TEST_SCALE / 2; // Should be DEFAULT_THUMBNAILS_SIDEBAR_WIDTH / 10 / 2 return thumbnailsSidebar.getThumbnailDataURL(0).then(() => { expect(stubs.getPage).to.be.called; diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss index 8d5a03b21..a3e48c6fb 100644 --- a/src/lib/viewers/doc/_docBase.scss +++ b/src/lib/viewers/doc/_docBase.scss @@ -1,8 +1,8 @@ @import '../../boxuiVariables'; -$thumbnail-border-radius: 4px; -// Accounts for the 1px border on the right so the remaining width is actually 200 -$thumbnail-sidebar-width: 201px; +$thumbnail-border-radius: 3px; +// Accounts for the 1px border on the right so the remaining width is actually 225 +$thumbnail-sidebar-width: 226px; .bp { .bp-thumbnails-container { @@ -52,10 +52,9 @@ $thumbnail-sidebar-width: 201px; border: $thumbnail-border-radius solid $ffive; border-radius: $thumbnail-border-radius; cursor: pointer; - flex: 1 0 auto; + flex: 1 0 154px; overflow: hidden; padding: 0; - width: 134px; } .bp-thumbnail-image { diff --git a/test/integration/document/Thumbnails.e2e.test.js b/test/integration/document/Thumbnails.e2e.test.js index 820075c4f..b7f63ced1 100644 --- a/test/integration/document/Thumbnails.e2e.test.js +++ b/test/integration/document/Thumbnails.e2e.test.js @@ -3,6 +3,7 @@ describe('Preview Document Thumbnails', () => { const token = Cypress.env('ACCESS_TOKEN'); const fileId = Cypress.env('FILE_ID_DOC_LARGE'); const THUMBNAIL_SELECTED_CLASS = 'bp-thumbnail-is-selected'; + const THUMBNAILS_OPEN = 'bp-thumbnails-open'; /** * Gets the thumbnail with the specified page number @@ -52,19 +53,15 @@ describe('Preview Document Thumbnails', () => { /** * Asserts that the thumbnails sidebar object is visible - * @param {Element} $thumbnailsSidebar - The thumbnails sidebar subject - * @return {Assertion} Chai Assertion + * @return {Element} Preview element */ - const beVisible = ($thumbnailsSidebar) => - expect($thumbnailsSidebar).to.have.css('transform', 'matrix(1, 0, 0, 1, 0, 0)'); // translateX(0) + const verifyThumbnailsVisible = () => cy.getByTestId('bp').should('have.class', THUMBNAILS_OPEN); /** * Asserts that the thumbnails sidebar object is not visible - * @param {Element} $thumbnailsSidebar - The thumbnails sidebar subject - * @return {Assertion} Chai Assertion + * @return {Element} Preview element */ - const notBeVisible = ($thumbnailsSidebar) => - expect($thumbnailsSidebar).to.have.css('transform', 'matrix(1, 0, 0, 1, -201, 0)'); // translateX(-201px) + const verifyThumbnailsNotVisible = () => cy.getByTestId('bp').should('not.have.class', THUMBNAILS_OPEN); beforeEach(() => { cy.visit('/'); @@ -87,9 +84,11 @@ describe('Preview Document Thumbnails', () => { it('Should render thumbnails when toggled', () => { showDocumentPreview({ enableThumbnailsSidebar: true }); - toggleThumbnails().should(beVisible); + toggleThumbnails(); + verifyThumbnailsVisible(); - toggleThumbnails().should(notBeVisible); + toggleThumbnails(); + verifyThumbnailsNotVisible(); }); it('Should be able to change page by clicking on the thumbnail', () => { @@ -102,7 +101,8 @@ describe('Preview Document Thumbnails', () => { .invoke('text') .should('equal', '1'); - toggleThumbnails().should(beVisible); + toggleThumbnails(); + verifyThumbnailsVisible(); // Verify which thumbnail is selected getThumbnailWithRenderedImage(1).should('have.class', THUMBNAIL_SELECTED_CLASS); @@ -124,7 +124,8 @@ describe('Preview Document Thumbnails', () => { .invoke('text') .should('equal', '1'); - toggleThumbnails().should(beVisible); + toggleThumbnails(); + verifyThumbnailsVisible(); getThumbnailWithRenderedImage(1).should('have.class', THUMBNAIL_SELECTED_CLASS); @@ -146,7 +147,8 @@ describe('Preview Document Thumbnails', () => { .invoke('text') .should('equal', '1'); - toggleThumbnails().should(beVisible); + toggleThumbnails(); + verifyThumbnailsVisible(); getThumbnailWithRenderedImage(1).should('have.class', THUMBNAIL_SELECTED_CLASS); @@ -175,7 +177,8 @@ describe('Preview Document Thumbnails', () => { .invoke('text') .should('equal', '1'); - toggleThumbnails().should(beVisible); + toggleThumbnails(); + verifyThumbnailsVisible(); getThumbnailWithRenderedImage(1).should('have.class', THUMBNAIL_SELECTED_CLASS); @@ -190,7 +193,8 @@ describe('Preview Document Thumbnails', () => { cy.getPreviewPage(200).should('be.visible'); getThumbnailWithRenderedImage(200).should('have.class', THUMBNAIL_SELECTED_CLASS); - toggleThumbnails().should(notBeVisible); + toggleThumbnails(); + verifyThumbnailsNotVisible(); cy.getByTitle('Click to enter page number').click(); cy @@ -201,7 +205,8 @@ describe('Preview Document Thumbnails', () => { cy.getPreviewPage(1).should('be.visible'); - toggleThumbnails().should(beVisible); + toggleThumbnails(); + verifyThumbnailsVisible(); getThumbnailWithRenderedImage(1).should('have.class', THUMBNAIL_SELECTED_CLASS); });