-
Notifications
You must be signed in to change notification settings - Fork 116
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.
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
Conversation
tonyjin
commented
Aug 30, 2017
- Add transparent overlay over document Instant Preview to better indicate that file is still loading
- Replace document loading icon GIF with CSS
- Add transparent overlay over document Instant Preview to better indicate that file is still loading - Replace document loading icon GIF with CSS
height: 30px; | ||
transform: translate(-15px, -15px) scale(.15) translate(15px, 15px); | ||
width: 30px; | ||
} |
There was a problem hiding this comment.
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
resetFunc.bind(this)(args); | ||
this.loadingIconDiv.classList.add(CLASS_SPINNER); | ||
this.loadingIconDiv.innerHTML = '<div></div>'; | ||
}; |
There was a problem hiding this comment.
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
this.wrapperEl.classList.add(CLASS_IS_TRANSPARENT); | ||
|
||
// Cleanup preload DOM after fade out | ||
this.wrapperEl.addEventListener('transitionend', this.cleanupPreload); |
There was a problem hiding this comment.
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.
// Only show page loading indicator when viewer has loaded | ||
.bp-loaded & .loadingIcon { | ||
background: url('loadingIcon.gif') center no-repeat; | ||
margin: auto; |
There was a problem hiding this comment.
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/
margin: auto; | ||
position: absolute; | ||
right: 0; | ||
top: 0; |
There was a problem hiding this comment.
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/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We no longer need loadingIcon.gif since this patch replaces it with a CSS loading indicator