Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: Add Instant Preview loading overlay #350

Merged
merged 1 commit into from
Aug 30, 2017
Merged

Update: Add Instant Preview loading overlay #350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 47 additions & 1 deletion src/lib/_loading.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
font-size: 12px;
}

// This crawler wrapper is used duing preview loading
// This crawler wrapper is used during preview loading
.bp-crawler-wrapper {
position: absolute;
top: 72px;
Expand Down Expand Up @@ -99,3 +99,49 @@
margin-left: 2px;
}
}

@keyframes bp-spinner {
0% {
transform: translate(-50%, -50%) rotate(0deg);
}

100% {
transform: translate(-50%, -50%) rotate(360deg);
}
}

// Center spinner horizontally and vertically within container
.bp-spinner {
bottom: 0;
left: 0;
margin: auto;
position: absolute;
right: 0;
top: 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using this absolute centering strategy: https://www.smashingmagazine.com/2013/08/absolute-horizontal-vertical-centering-css/

}

.bp-spinner div,
.bp-spinner div::after {
border: 25px solid #0061d5;
border-radius: 50%;
border-top-color: transparent;
height: 170px;
position: absolute;
width: 170px;
}

.bp-spinner div {
animation: bp-spinner 1s linear infinite;
left: 100px;
top: 100px;
}

.bp-spinner div::after {
transform: rotate(90deg);
}

.bp-spinner {
height: 30px;
transform: translate(-15px, -15px) scale(.15) translate(15px, 15px);
width: 30px;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what it looks like:
https://codepen.io/box-platform/pen/225158cd7ec46b0bd504620808e59748

4 changes: 4 additions & 0 deletions src/lib/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export const CLASS_BOX_PREVIEW_OVERLAY = 'bp-overlay';
export const CLASS_BOX_PREVIEW_OVERLAY_WRAPPER = 'bp-overlay-wrapper';
export const CLASS_BOX_PREVIEW_PRELOAD = 'bp-preload';
export const CLASS_BOX_PREVIEW_PRELOAD_CONTENT = 'bp-preload-content';
export const CLASS_BOX_PREVIEW_PRELOAD_OVERLAY = 'bp-preload-overlay';
export const CLASS_BOX_PREVIEW_PRELOAD_WRAPPER_DOCUMENT = 'bp-document-preload-wrapper';
export const CLASS_BOX_PREVIEW_PRELOAD_WRAPPER_PRESENTATION = 'bp-presentation-preload-wrapper';
export const CLASS_BOX_PREVIEW_PROGRESS_BAR = 'bp-progress-bar';
Expand All @@ -31,11 +32,14 @@ export const CLASS_ELEM_KEYBOARD_FOCUS = 'bp-has-keyboard-focus';
export const CLASS_FULLSCREEN = 'bp-is-fullscreen';
export const CLASS_FULLSCREEN_UNSUPPORTED = 'bp-fullscreen-unsupported';
export const CLASS_INVISIBLE = 'bp-is-invisible';
export const CLASS_IS_TRANSPARENT = 'bp-is-transparent';
export const CLASS_IS_VISIBLE = 'bp-is-visible';
export const CLASS_IS_SCROLLABLE = 'bp-is-scrollable';
export const CLASS_IS_SELECTABLE = 'bp-is-selectable';
export const CLASS_IS_BUFFERING = 'bp-is-buffering';
export const CLASS_DARK = 'bp-dark';
export const CLASS_CRAWLER = 'bp-crawler';
export const CLASS_SPINNER = 'bp-spinner';

export const SELECTOR_BOX_PREVIEW_CONTAINER = `.${CLASS_BOX_PREVIEW_CONTAINER}`;
export const SELECTOR_BOX_PREVIEW = `.${CLASS_BOX_PREVIEW}`;
Expand Down
13 changes: 12 additions & 1 deletion src/lib/viewers/doc/DocBaseViewer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import Popup from '../../Popup';
import RepStatus from '../../RepStatus';
import {
CLASS_BOX_PREVIEW_FIND_BAR,
CLASS_CRAWLER,
CLASS_HIDDEN,
CLASS_IS_SCROLLABLE,
CLASS_SPINNER,
DOC_STATIC_ASSETS_VERSION,
PERMISSION_DOWNLOAD,
PRELOAD_REP_NAME,
Expand Down Expand Up @@ -577,6 +579,15 @@ class DocBaseViewer extends BaseViewer {
// Do not disable create object URL in IE11 or iOS Chrome - pdf.js issues #3977 and #8081 are
// not applicable to Box's use case and disabling causes performance issues
PDFJS.disableCreateObjectURL = false;

// Customize pdf.js loading icon. We modify the prototype of PDFPageView to get around directly modifying
// pdf_viewer.js
const resetFunc = PDFJS.PDFPageView.prototype.reset;
PDFJS.PDFPageView.prototype.reset = function reset(...args) {
resetFunc.bind(this)(args);
this.loadingIconDiv.classList.add(CLASS_SPINNER);
this.loadingIconDiv.innerHTML = '<div></div>';
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so we don't have to go into pdf.js and modify their code after each upgrade.

These are the relevant lines in pdf.js: https://github.com/mozilla/pdf.js/blob/1419b7ffe7fb34aa48002fd388d7a3848c668709/web/pdf_page_view.js#L183

}

/**
Expand All @@ -593,7 +604,7 @@ class DocBaseViewer extends BaseViewer {
printCheckmark.innerHTML = ICON_PRINT_CHECKMARK.trim();

const loadingIndicator = document.createElement('div');
loadingIndicator.classList.add('bp-crawler');
loadingIndicator.classList.add(CLASS_CRAWLER);
loadingIndicator.innerHTML = `
<div></div>
<div></div>
Expand Down
66 changes: 60 additions & 6 deletions src/lib/viewers/doc/DocPreloader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ import EventEmitter from 'events';
import {
CLASS_BOX_PREVIEW_PRELOAD,
CLASS_BOX_PREVIEW_PRELOAD_CONTENT,
CLASS_BOX_PREVIEW_PRELOAD_OVERLAY,
CLASS_BOX_PREVIEW_PRELOAD_WRAPPER_DOCUMENT,
CLASS_INVISIBLE,
CLASS_PREVIEW_LOADED
CLASS_IS_TRANSPARENT,
CLASS_PREVIEW_LOADED,
CLASS_SPINNER
} from '../../constants';
import { get, setDimensions } from '../../util';

Expand All @@ -21,7 +24,33 @@ const NUM_PAGES_MAX = 500; // Don't show more than 500 placeholder pages

const ACCEPTABLE_RATIO_DIFFERENCE = 0.025; // Acceptable difference in ratio of PDF dimensions to image dimensions

const SPINNER_HTML = `<div class="${CLASS_SPINNER}"><div></div></div>`;

class DocPreloader extends EventEmitter {
/** @property {HTMLElement} - Viewer container */
containerEl;

/** @property {HTMLElement} - Preload image element */
imageEl;

/** @property {HTMLElement} - Preload overlay element */
overlayEl;

/** @property {HTMLElement} - Preload container element */
preloadEl;

/** @property {PreviewUI} - Preview's UI instance */
previewUI;

/** @property {string} - Preload representation content URL */
srcUrl;

/** @property {string} - Class name for preload wrapper */
wrapperClassName;

/** @property {HTMLElement} - Preload wrapper element */
wrapperEl;

/**
* [constructor]
*
Expand Down Expand Up @@ -59,12 +88,16 @@ class DocPreloader extends EventEmitter {
this.wrapperEl.innerHTML = `
<div class="${CLASS_BOX_PREVIEW_PRELOAD} ${CLASS_INVISIBLE}">
<img class="${CLASS_BOX_PREVIEW_PRELOAD_CONTENT}" src="${this.srcUrl}" />
<div class="${CLASS_BOX_PREVIEW_PRELOAD_CONTENT} ${CLASS_BOX_PREVIEW_PRELOAD_OVERLAY}">
${SPINNER_HTML}
</div>
</div>
`.trim();

this.containerEl.appendChild(this.wrapperEl);
this.preloadEl = this.wrapperEl.querySelector(`.${CLASS_BOX_PREVIEW_PRELOAD}`);
this.imageEl = this.preloadEl.querySelector(`img.${CLASS_BOX_PREVIEW_PRELOAD_CONTENT}`);
this.overlayEl = this.preloadEl.querySelector(`.${CLASS_BOX_PREVIEW_PRELOAD_OVERLAY}`);
this.bindDOMListeners();
});
}
Expand All @@ -82,13 +115,15 @@ class DocPreloader extends EventEmitter {
return;
}

// Set image dimensions
// Set image and overlay dimensions
setDimensions(this.imageEl, scaledWidth, scaledHeight);
setDimensions(this.overlayEl, scaledWidth, scaledHeight);

// Add and scale correct number of placeholder elements
for (let i = 0; i < numPages - 1; i++) {
const placeholderEl = document.createElement('div');
placeholderEl.className = CLASS_BOX_PREVIEW_PRELOAD_CONTENT;
placeholderEl.innerHTML = SPINNER_HTML;
setDimensions(placeholderEl, scaledWidth, scaledHeight);
this.preloadEl.appendChild(placeholderEl);
}
Expand All @@ -113,18 +148,37 @@ class DocPreloader extends EventEmitter {
return;
}

this.restoreScrollPosition();
this.unbindDOMListeners();
this.restoreScrollPosition();
this.wrapperEl.classList.add(CLASS_IS_TRANSPARENT);

// Cleanup preload DOM after fade out
this.wrapperEl.addEventListener('transitionend', this.cleanupPreload);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we fade out the preload image into the real page and then remove the preload elements. This seems to be a better experience than swapping out the preload instantaneously.


// Cleanup preload DOM immediately if user scrolls after the document is ready since we don't want half-faded
// out preload content to be on top of real document content while scrolling
this.wrapperEl.addEventListener('scroll', this.cleanupPreload);
}

/**
* Cleans up preload DOM.
*
* @private
* @return {void}
*/
cleanupPreload = () => {
if (this.wrapperEl) {
this.wrapperEl.parentNode.removeChild(this.wrapperEl);
this.wrapperEl = undefined;
}

this.wrapperEl.parentNode.removeChild(this.wrapperEl);
this.wrapperEl = undefined;
this.preloadEl = undefined;
this.imageEl = undefined;

if (this.srcUrl) {
URL.revokeObjectURL(this.srcUrl);
}
}
};

/**
* Binds event listeners for preload
Expand Down
21 changes: 19 additions & 2 deletions src/lib/viewers/doc/Document.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,19 @@
bottom: 0;
left: 0;
margin: 0;
opacity: 1;
overflow-y: scroll;
position: absolute;
right: 0;
top: 0;
transition: opacity .5s;

&.bp-is-invisible {
visibility: visible;
visibility: hidden;
}

&.bp-is-transparent {
opacity: 0;
}

.bp-preload {
Expand All @@ -30,8 +36,19 @@

// Position preload content as if they were blank loading pages
.bp-preload-content {
background: $white url('loadingIcon.gif') center no-repeat;
background-color: $white;
display: block;
margin: 15px auto 30px;
padding-left: 1px; // Slight padding to help image text align with real text
position: relative;

&.bp-preload-overlay {
background-color: rgba(255, 255, 255, .4);
bottom: 0;
left: 0;
position: absolute;
right: 0;
top: 0;
}
}
}
15 changes: 12 additions & 3 deletions src/lib/viewers/doc/Presentation.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,14 @@
position: absolute;
right: 0;
top: 0;
transition: opacity .5s;

&.bp-is-invisible {
visibility: visible;
visibility: hidden;
}

&.bp-is-transparent {
opacity: 0;
}

.bp-preload {
Expand All @@ -50,13 +55,17 @@

// Absolutely center preload content
.bp-preload-content {
background: $white url('loadingIcon.gif') center no-repeat;
background-color: $white;
bottom: 0;
display: block;
left: 0;
left: 1px; // Slight padding to help image text align with real text
margin: auto;
position: absolute;
right: 0;
top: 0;

&.bp-preload-overlay {
background-color: rgba(255, 255, 255, .4);
}
}
}
10 changes: 10 additions & 0 deletions src/lib/viewers/doc/PresentationPreloader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,15 @@ import { CLASS_INVISIBLE, CLASS_BOX_PREVIEW_PRELOAD_WRAPPER_PRESENTATION } from
import { setDimensions } from '../../util';

class PresentationPreloader extends DocPreloader {
/** @property {HTMLElement} - Preload container element */
preloadEl;

/** @property {PreviewUI} - Preview's UI instance */
previewUI;

/** @property {string} - Class name for preload wrapper */
wrapperClassName;

/**
* [constructor]
*
Expand All @@ -29,6 +38,7 @@ class PresentationPreloader extends DocPreloader {
}

setDimensions(this.imageEl, scaledWidth, scaledHeight);
setDimensions(this.overlayEl, scaledWidth, scaledHeight);

// Hide the preview-level loading indicator
this.previewUI.hideLoadingIndicator();
Expand Down
6 changes: 6 additions & 0 deletions src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -945,6 +945,12 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
docBase.setupPdfjs();
expect(PDFJS.disableCreateObjectURL).to.equal(false);
});

it('should override pdf.js PDFPageView reset with custom loading indicator logic', () => {
const resetFunc = PDFJS.PDFPageView.prototype.reset;
docBase.setupPdfjs();
expect(resetFunc).to.not.equal(PDFJS.PDFPageView.prototype.reset);
});
});

describe('initPrint()', () => {
Expand Down
Loading