Skip to content

Commit

Permalink
Update: Wider thumbnails (#963)
Browse files Browse the repository at this point in the history
  • Loading branch information
Conrad Chan authored Mar 26, 2019
1 parent d8cd848 commit 6f2200a
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 28 deletions.
4 changes: 2 additions & 2 deletions src/lib/ThumbnailsSidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
11 changes: 6 additions & 5 deletions src/lib/__tests__/ThumbnailsSidebar-test.js
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
});
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
9 changes: 4 additions & 5 deletions src/lib/viewers/doc/_docBase.scss
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
37 changes: 21 additions & 16 deletions test/integration/document/Thumbnails.e2e.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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('/');
Expand All @@ -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', () => {
Expand All @@ -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);
Expand All @@ -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);

Expand All @@ -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);

Expand Down Expand Up @@ -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);

Expand All @@ -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
Expand All @@ -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);
});
Expand Down

0 comments on commit 6f2200a

Please sign in to comment.