Smarter thumbnail scrolling #854
Conversation
|
Tried it out and it works well. Do you think we could pre-render a few more pages than what are visible so there's a little less lag? |
| @@ -119,6 +118,7 @@ | |||
|
|
|||
| <div id="loading">Loading... 0%</div> | |||
| <div id="viewer"></div> | |||
|
|
|||
| <script type="text/javascript" src="viewer.js"></script> | |||
There was a problem hiding this comment.
I don't think it's a good practice to move the "global" page scripts here.
You can move the initialization sidebarScrollView.addEventListener('scroll', updateThumbViewArea, true); into some event when page loaded, e.g. https://developer.mozilla.org/en/Gecko-Specific_DOM_Events#DOMContentLoaded
There was a problem hiding this comment.
there is also https://github.com/mozilla/pdf.js/blob/master/web/viewer.js#L630
or
or you can provide the comment why the script load moved here, to avoid other developer moving it back in the header
There was a problem hiding this comment.
I don't think it's a good practice to move the "global" page scripts here.
AFAIK there are more pros than cons in keeping scripts at the bottom (progressive rendering, parallel downloads, etc http://developer.yahoo.com/blogs/ydn/posts/2007/07/high_performanc_5/). In our case the additional advantage is that we don't have to bind to DOC-ready or -load.
So I moved the whole thing down.
|
Since we rendering pages to display thumbs, shall we create full size pages and place them into secondary cache while setting thumbnails, and then re-use those cached images during display? |
That sounds like a great idea, but I think it's beyond the scope of the pull. |
|
I'm not comfortable with moving the external script references to the bottom of the html page, that's not where I would expect them. Let's see if other people okay with that. |
I'm reversing the change, and moving the event binding to 'load'. This is not an ideal solution ( I've listed the advantages of having the script at the bottom; it's also the recommended practice from just about everywhere I know. Next time please list the actual reasons why you feel uncomfortable with a change. |
|
I like to have the For people coming to the project, they might look at the viewer.html file to get a basic understanding on how things are working. Therefore, I think the viewer.html file should be as simple as possible to understand. @arturadib, we could mabye add the script tags at the bottom during builds (extension, ...)? As this pull request is about thumbnail rendering, there should be another pull request for doing this script tag change if we agree on doing it. |
|
I had already reversed the changes (cf above), the scripts are already at the head. I'm moving on. |
|
Beautiful. Thank you. |
Smarter thumbnail scrolling
OK hopefully this time the conflicts are fixed... sorry about the repeated requests!
Currently we render all thumbs in series starting from #1, with a 500ms delay, which is pretty much useless for a large document.
This new code renders the thumbs that correspond to the current scroll position in the thumbnail viewer, after the scrolling has paused for some milliseconds. This allows users to scroll to page #1000 and see the thumb almost instantly, without hiccups in the UI.
Also the thumbnail viewer now works in WebKit.
PS: I've noticed that we start throwing some blank pages and errors with intelisa, and it just won't work at all with pdf.pdf. I think these issues are specific to the thumbnail code.