From 464f58372ce4157b8118576bca597f9cd43a858b Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Tue, 26 Mar 2019 13:50:02 -0700 Subject: [PATCH 1/2] Update: Add hover state to thumbnails --- src/lib/ThumbnailsSidebar.js | 18 ++++++++++---- src/lib/__tests__/ThumbnailsSidebar-test.js | 10 ++++++-- src/lib/viewers/doc/_docBase.scss | 27 +++++++++++++++++---- 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/lib/ThumbnailsSidebar.js b/src/lib/ThumbnailsSidebar.js index 3fa959e79..5c2e5e41d 100644 --- a/src/lib/ThumbnailsSidebar.js +++ b/src/lib/ThumbnailsSidebar.js @@ -3,6 +3,7 @@ import VirtualScroller from './VirtualScroller'; import BoundedCache from './BoundedCache'; const CLASS_BOX_PREVIEW_THUMBNAIL = 'bp-thumbnail'; +const CLASS_BOX_PREVIEW_THUMBNAIL_BORDER = 'bp-thumbnail-border'; const CLASS_BOX_PREVIEW_THUMBNAIL_NAV = 'bp-thumbnail-nav'; const CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE = 'bp-thumbnail-image'; const CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE_LOADED = 'bp-thumbnail-image-loaded'; @@ -75,7 +76,7 @@ class ThumbnailsSidebar { // The image and page number have pointer-events: none so // any click should be the thumbnail element itself. if (target.classList.contains(CLASS_BOX_PREVIEW_THUMBNAIL_NAV)) { - const thumbnailEl = target.parentNode; + const thumbnailEl = target.parentNode.parentNode; // Get the page number const { bpPageNum: pageNumStr } = thumbnailEl.dataset; const pageNum = parseInt(pageNumStr, 10); @@ -208,8 +209,14 @@ class ThumbnailsSidebar { thumbnailEl.className = CLASS_BOX_PREVIEW_THUMBNAIL; thumbnailEl.dataset.bpPageNum = pageNum; thumbnailEl.appendChild(this.createPageNumber(pageNum)); + + const thumbnailBorderEl = document.createElement('div'); + thumbnailBorderEl.className = CLASS_BOX_PREVIEW_THUMBNAIL_BORDER; + const thumbnailNav = this.createThumbnailNav(); - thumbnailEl.appendChild(thumbnailNav); + + thumbnailBorderEl.appendChild(thumbnailNav); + thumbnailEl.appendChild(thumbnailBorderEl); if (pageNum === this.currentPage) { thumbnailEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IS_SELECTED); @@ -233,6 +240,7 @@ class ThumbnailsSidebar { createThumbnailNav() { const thumbnailNav = document.createElement('a'); thumbnailNav.className = CLASS_BOX_PREVIEW_THUMBNAIL_NAV; + thumbnailNav.tabIndex = '0'; return thumbnailNav; } @@ -240,7 +248,7 @@ class ThumbnailsSidebar { * Request the thumbnail image to be made * * @param {number} itemIndex - the item index in the overall list (0 indexed) - * @param {HTMLElement} thumbnailEl - the thumbnail button element + * @param {HTMLElement} thumbnailEl - the thumbnail element * @return {void} */ requestThumbnailImage(itemIndex, thumbnailEl) { @@ -248,8 +256,8 @@ class ThumbnailsSidebar { this.createThumbnailImage(itemIndex).then((imageEl) => { // Promise will resolve with null if create image request was already in progress if (imageEl) { - // Appends to the lastChild which should be the thumbnail nav element - thumbnailEl.lastChild.appendChild(imageEl); + // Appends to the thumbnail nav element + thumbnailEl.lastChild.firstChild.appendChild(imageEl); thumbnailEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE_LOADED); } diff --git a/src/lib/__tests__/ThumbnailsSidebar-test.js b/src/lib/__tests__/ThumbnailsSidebar-test.js index 7d151ef7e..5c90a3879 100644 --- a/src/lib/__tests__/ThumbnailsSidebar-test.js +++ b/src/lib/__tests__/ThumbnailsSidebar-test.js @@ -213,7 +213,7 @@ describe('ThumbnailsSidebar', () => { stubs.renderNextThumbnailImage = sandbox.stub(thumbnailsSidebar, 'renderNextThumbnailImage'); const thumbnailEl = { - lastChild: { appendChild: stubs.appendChild }, + lastChild: { firstChild: { appendChild: stubs.appendChild } }, classList: { add: stubs.addClass } }; @@ -311,6 +311,7 @@ describe('ThumbnailsSidebar', () => { describe('thumbnailClickHandler()', () => { let targetEl; + let borderEl; let parentEl; let evt; @@ -325,7 +326,12 @@ describe('ThumbnailsSidebar', () => { targetEl = document.createElement('div'); targetEl.classList.add('bp-thumbnail-nav'); - parentEl.appendChild(targetEl); + borderEl = document.createElement('div'); + borderEl.classList.add('bp-thumbnail-border'); + + borderEl.appendChild(targetEl); + + parentEl.appendChild(borderEl); evt = { target: targetEl, diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss index a3e48c6fb..d030c1973 100644 --- a/src/lib/viewers/doc/_docBase.scss +++ b/src/lib/viewers/doc/_docBase.scss @@ -1,6 +1,5 @@ @import '../../boxuiVariables'; -$thumbnail-border-radius: 3px; // Accounts for the 1px border on the right so the remaining width is actually 225 $thumbnail-sidebar-width: 226px; @@ -47,16 +46,31 @@ $thumbnail-sidebar-width: 226px; width: 34px; } + .bp-thumbnail-border { + border: 2px solid transparent; + border-radius: 3px; + display: flex; + flex: 1 0 auto; + padding: 0; + } + .bp-thumbnail-nav { background-color: $d-eight; - border: $thumbnail-border-radius solid $ffive; - border-radius: $thumbnail-border-radius; + border: 1px solid $ffive; cursor: pointer; flex: 1 0 154px; + outline: none; overflow: hidden; padding: 0; } + .bp-thumbnail:not(.bp-thumbnail-is-selected) { + .bp-thumbnail-nav:focus, + .bp-thumbnail-nav:hover { + border-color: $seesee; + } + } + .bp-thumbnail-image { background-position: center; background-repeat: no-repeat; @@ -64,8 +78,11 @@ $thumbnail-sidebar-width: 226px; pointer-events: none; } - .bp-thumbnail.bp-thumbnail-is-selected .bp-thumbnail-nav { - border-color: $fours; + .bp-thumbnail.bp-thumbnail-is-selected { + .bp-thumbnail-nav, + .bp-thumbnail-border { + border-color: $fours; + } } .bp-thumbnails-container--dark { From debe003f7632acb672523a1b4519ed1e9053f8ab Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Tue, 26 Mar 2019 14:25:46 -0700 Subject: [PATCH 2/2] Chore: removing extra div --- src/lib/ThumbnailsSidebar.js | 12 +++-------- src/lib/__tests__/ThumbnailsSidebar-test.js | 10 ++-------- src/lib/viewers/doc/_docBase.scss | 22 +++++++-------------- 3 files changed, 12 insertions(+), 32 deletions(-) diff --git a/src/lib/ThumbnailsSidebar.js b/src/lib/ThumbnailsSidebar.js index 5c2e5e41d..db9cd905e 100644 --- a/src/lib/ThumbnailsSidebar.js +++ b/src/lib/ThumbnailsSidebar.js @@ -3,7 +3,6 @@ import VirtualScroller from './VirtualScroller'; import BoundedCache from './BoundedCache'; const CLASS_BOX_PREVIEW_THUMBNAIL = 'bp-thumbnail'; -const CLASS_BOX_PREVIEW_THUMBNAIL_BORDER = 'bp-thumbnail-border'; const CLASS_BOX_PREVIEW_THUMBNAIL_NAV = 'bp-thumbnail-nav'; const CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE = 'bp-thumbnail-image'; const CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE_LOADED = 'bp-thumbnail-image-loaded'; @@ -76,7 +75,7 @@ class ThumbnailsSidebar { // The image and page number have pointer-events: none so // any click should be the thumbnail element itself. if (target.classList.contains(CLASS_BOX_PREVIEW_THUMBNAIL_NAV)) { - const thumbnailEl = target.parentNode.parentNode; + const thumbnailEl = target.parentNode; // Get the page number const { bpPageNum: pageNumStr } = thumbnailEl.dataset; const pageNum = parseInt(pageNumStr, 10); @@ -210,13 +209,8 @@ class ThumbnailsSidebar { thumbnailEl.dataset.bpPageNum = pageNum; thumbnailEl.appendChild(this.createPageNumber(pageNum)); - const thumbnailBorderEl = document.createElement('div'); - thumbnailBorderEl.className = CLASS_BOX_PREVIEW_THUMBNAIL_BORDER; - const thumbnailNav = this.createThumbnailNav(); - - thumbnailBorderEl.appendChild(thumbnailNav); - thumbnailEl.appendChild(thumbnailBorderEl); + thumbnailEl.appendChild(thumbnailNav); if (pageNum === this.currentPage) { thumbnailEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IS_SELECTED); @@ -257,7 +251,7 @@ class ThumbnailsSidebar { // Promise will resolve with null if create image request was already in progress if (imageEl) { // Appends to the thumbnail nav element - thumbnailEl.lastChild.firstChild.appendChild(imageEl); + thumbnailEl.lastChild.appendChild(imageEl); thumbnailEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE_LOADED); } diff --git a/src/lib/__tests__/ThumbnailsSidebar-test.js b/src/lib/__tests__/ThumbnailsSidebar-test.js index 5c90a3879..7d151ef7e 100644 --- a/src/lib/__tests__/ThumbnailsSidebar-test.js +++ b/src/lib/__tests__/ThumbnailsSidebar-test.js @@ -213,7 +213,7 @@ describe('ThumbnailsSidebar', () => { stubs.renderNextThumbnailImage = sandbox.stub(thumbnailsSidebar, 'renderNextThumbnailImage'); const thumbnailEl = { - lastChild: { firstChild: { appendChild: stubs.appendChild } }, + lastChild: { appendChild: stubs.appendChild }, classList: { add: stubs.addClass } }; @@ -311,7 +311,6 @@ describe('ThumbnailsSidebar', () => { describe('thumbnailClickHandler()', () => { let targetEl; - let borderEl; let parentEl; let evt; @@ -326,12 +325,7 @@ describe('ThumbnailsSidebar', () => { targetEl = document.createElement('div'); targetEl.classList.add('bp-thumbnail-nav'); - borderEl = document.createElement('div'); - borderEl.classList.add('bp-thumbnail-border'); - - borderEl.appendChild(targetEl); - - parentEl.appendChild(borderEl); + parentEl.appendChild(targetEl); evt = { target: targetEl, diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss index d030c1973..1fc22ff5e 100644 --- a/src/lib/viewers/doc/_docBase.scss +++ b/src/lib/viewers/doc/_docBase.scss @@ -1,5 +1,6 @@ @import '../../boxuiVariables'; +$thumbnail-border-radius: 3px; // Accounts for the 1px border on the right so the remaining width is actually 225 $thumbnail-sidebar-width: 226px; @@ -46,17 +47,10 @@ $thumbnail-sidebar-width: 226px; width: 34px; } - .bp-thumbnail-border { - border: 2px solid transparent; - border-radius: 3px; - display: flex; - flex: 1 0 auto; - padding: 0; - } - .bp-thumbnail-nav { background-color: $d-eight; - border: 1px solid $ffive; + border: $thumbnail-border-radius solid $ffive; + border-radius: $thumbnail-border-radius; cursor: pointer; flex: 1 0 154px; outline: none; @@ -67,7 +61,8 @@ $thumbnail-sidebar-width: 226px; .bp-thumbnail:not(.bp-thumbnail-is-selected) { .bp-thumbnail-nav:focus, .bp-thumbnail-nav:hover { - border-color: $seesee; + border: 1px solid $seesee; + margin: 2px; } } @@ -78,11 +73,8 @@ $thumbnail-sidebar-width: 226px; pointer-events: none; } - .bp-thumbnail.bp-thumbnail-is-selected { - .bp-thumbnail-nav, - .bp-thumbnail-border { - border-color: $fours; - } + .bp-thumbnail.bp-thumbnail-is-selected .bp-thumbnail-nav { + border-color: $fours; } .bp-thumbnails-container--dark {