From e8d8a46dc2547c4d21c15242a8ea813dc2c87d25 Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Wed, 27 Mar 2019 11:11:43 -0700 Subject: [PATCH 1/5] Fix: Hide thumbnails in mobile portrait --- .stylelintrc | 2 +- src/lib/viewers/doc/_docBase.scss | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/.stylelintrc b/.stylelintrc index ba7b81433..581ab9d3c 100644 --- a/.stylelintrc +++ b/.stylelintrc @@ -7,7 +7,7 @@ "rules": { "indentation": 4, "at-rule-no-vendor-prefix": true, - "at-rule-no-unknown": [true, { "ignoreAtRules": ["include", "mixin"] }], + "at-rule-no-unknown": [true, { "ignoreAtRules": ["include", "mixin", "content"] }], "media-feature-name-no-vendor-prefix": true, "property-no-vendor-prefix": true, "selector-no-vendor-prefix": true, diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss index 1fc22ff5e..f7d60cf33 100644 --- a/src/lib/viewers/doc/_docBase.scss +++ b/src/lib/viewers/doc/_docBase.scss @@ -4,6 +4,10 @@ $thumbnail-border-radius: 3px; // Accounts for the 1px border on the right so the remaining width is actually 225 $thumbnail-sidebar-width: 226px; +@mixin for-phone-only { + @media (max-width: 599px) { @content; } +} + .bp { .bp-thumbnails-container { border-right: solid 1px $seesee; @@ -16,6 +20,10 @@ $thumbnail-sidebar-width: 226px; transform: translateX(-$thumbnail-sidebar-width); transition: transform 300ms cubic-bezier(.4, 0, .2, 1); width: $thumbnail-sidebar-width; + + @include for-phone-only { + display: none; + } } &.bp-thumbnails-open { @@ -25,6 +33,10 @@ $thumbnail-sidebar-width: 226px; .bp-content { left: $thumbnail-sidebar-width; + + @include for-phone-only { + left: 0; + } } } @@ -217,3 +229,9 @@ $thumbnail-sidebar-width: 226px; .bp-content.bp-is-fullscreen .bp-toggle-thumbnails-icon { display: none; } + +.bp-controls-btn.bp-toggle-thumbnails-icon { + @include for-phone-only { + display: none; + } +} From cf83152171a518de7b6cf84c49e75988908ede7e Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Wed, 27 Mar 2019 13:15:39 -0700 Subject: [PATCH 2/5] Chore: address PR comments --- src/lib/viewers/doc/_docBase.scss | 34 ++++++++++++++++++------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss index f7d60cf33..71b30caeb 100644 --- a/src/lib/viewers/doc/_docBase.scss +++ b/src/lib/viewers/doc/_docBase.scss @@ -20,10 +20,6 @@ $thumbnail-sidebar-width: 226px; transform: translateX(-$thumbnail-sidebar-width); transition: transform 300ms cubic-bezier(.4, 0, .2, 1); width: $thumbnail-sidebar-width; - - @include for-phone-only { - display: none; - } } &.bp-thumbnails-open { @@ -33,10 +29,6 @@ $thumbnail-sidebar-width: 226px; .bp-content { left: $thumbnail-sidebar-width; - - @include for-phone-only { - left: 0; - } } } @@ -105,6 +97,26 @@ $thumbnail-sidebar-width: 226px; border-color: $black; } } + + @include for-phone-only { + .bp-thumbnails-container { + display: none; + } + + &.bp-thumbnails-open { + .bp-content { + left: 0; + } + + .bp-thumbnails-container { + transform: none; + } + } + + .bp-controls-btn.bp-toggle-thumbnails-icon { + display: none; + } + } } .bp-theme-dark { @@ -229,9 +241,3 @@ $thumbnail-sidebar-width: 226px; .bp-content.bp-is-fullscreen .bp-toggle-thumbnails-icon { display: none; } - -.bp-controls-btn.bp-toggle-thumbnails-icon { - @include for-phone-only { - display: none; - } -} From 927fe5fdf8d5e232781c0503834c005ab3e7d9e1 Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Wed, 27 Mar 2019 14:00:51 -0700 Subject: [PATCH 3/5] Chore: adding e2e tests --- .../document/Thumbnails.e2e.test.js | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/integration/document/Thumbnails.e2e.test.js b/test/integration/document/Thumbnails.e2e.test.js index b7f63ced1..92f7666ee 100644 --- a/test/integration/document/Thumbnails.e2e.test.js +++ b/test/integration/document/Thumbnails.e2e.test.js @@ -210,4 +210,31 @@ describe('Preview Document Thumbnails', () => { getThumbnailWithRenderedImage(1).should('have.class', THUMBNAIL_SELECTED_CLASS); }); + + it('Should not show the toggle thumbnails button on a small viewport', () => { + cy.viewport(375, 667); + + showDocumentPreview({ enableThumbnailsSidebar: true }); + + cy.showDocumentControls(); + cy.getByTitle('Toggle thumbnails').should('not.be.visible'); + }); + + it('Should hide the thumbnails when changing to small viewport', () => { + showDocumentPreview({ enableThumbnailsSidebar: true }); + + cy.showDocumentControls(); + cy.getByTitle('Toggle thumbnails').should('be.visible'); + + toggleThumbnails(); + verifyThumbnailsVisible(); + + // Change to small viewport while thumbnails sidebar is open + cy.viewport(375, 667); + + cy.showDocumentControls(); + cy.getByTitle('Toggle thumbnails').should('not.be.visible'); + // Not using `verifyThumbnailsNotVisible because that checks for the presence of the CSS class + cy.getByTestId('thumbnails-sidebar').should('not.be.visible'); + }); }); From 3ea55e61646ffeb72b96b38f68f4d51aa00543af Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Wed, 27 Mar 2019 14:01:56 -0700 Subject: [PATCH 4/5] Chore: mobile first CSS --- src/lib/viewers/doc/_docBase.scss | 34 +++++++++++++------------------ 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss index 71b30caeb..2e169bd80 100644 --- a/src/lib/viewers/doc/_docBase.scss +++ b/src/lib/viewers/doc/_docBase.scss @@ -4,34 +4,26 @@ $thumbnail-border-radius: 3px; // Accounts for the 1px border on the right so the remaining width is actually 225 $thumbnail-sidebar-width: 226px; -@mixin for-phone-only { - @media (max-width: 599px) { @content; } +@mixin breakpoint-medium { + @media (min-width: 600px) { @content; } } .bp { + .bp-toggle-thumbnails-icon { + display: none; + } + .bp-thumbnails-container { border-right: solid 1px $seesee; bottom: 0; - display: flex; + display: none; flex: 0 0 auto; left: 0; position: absolute; top: 0; - transform: translateX(-$thumbnail-sidebar-width); - transition: transform 300ms cubic-bezier(.4, 0, .2, 1); width: $thumbnail-sidebar-width; } - &.bp-thumbnails-open { - .bp-thumbnails-container { - transform: translateX(0); - } - - .bp-content { - left: $thumbnail-sidebar-width; - } - } - .bp-thumbnail { display: flex; flex: 1 0 auto; @@ -98,23 +90,25 @@ $thumbnail-sidebar-width: 226px; } } - @include for-phone-only { + @include breakpoint-medium { .bp-thumbnails-container { - display: none; + display: flex; + transform: translateX(-$thumbnail-sidebar-width); + transition: transform 300ms cubic-bezier(.4, 0, .2, 1); } &.bp-thumbnails-open { .bp-content { - left: 0; + left: $thumbnail-sidebar-width; } .bp-thumbnails-container { - transform: none; + transform: translateX(0); } } .bp-controls-btn.bp-toggle-thumbnails-icon { - display: none; + display: block; } } } From 0fa7c1197c43c1a4771dd8ad6832abe92f1f020e Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Wed, 27 Mar 2019 14:31:55 -0700 Subject: [PATCH 5/5] Chore: using viewport present in e2e tests --- test/integration/document/Thumbnails.e2e.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/document/Thumbnails.e2e.test.js b/test/integration/document/Thumbnails.e2e.test.js index 92f7666ee..6e4f4aee4 100644 --- a/test/integration/document/Thumbnails.e2e.test.js +++ b/test/integration/document/Thumbnails.e2e.test.js @@ -212,7 +212,7 @@ describe('Preview Document Thumbnails', () => { }); it('Should not show the toggle thumbnails button on a small viewport', () => { - cy.viewport(375, 667); + cy.viewport('iphone-6'); showDocumentPreview({ enableThumbnailsSidebar: true }); @@ -230,7 +230,7 @@ describe('Preview Document Thumbnails', () => { verifyThumbnailsVisible(); // Change to small viewport while thumbnails sidebar is open - cy.viewport(375, 667); + cy.viewport('iphone-6'); cy.showDocumentControls(); cy.getByTitle('Toggle thumbnails').should('not.be.visible');