jviereck / pdfListView

A simple list view to render a PDF document using PDF.JS
Apache License 2.0
17 stars 6 forks source link

These are the changes I made #10

Closed PabloK closed 10 years ago

PabloK commented 10 years ago

I made the following changes to my local copy of the ListViewer Im not sure if they are accurate but the things I was trying to do started working as expected.

PabloK commented 10 years ago

The following function is the one that caused me trouble. It was used to scroll in either direction upon detecting touch movement or basic scroll events.

var scrollpdf = function(delta) {
            if (self.pdfListView === null) {
                return;
            }
            var newPos = self.pdfListView.getPdfPosition();
            if (newPos.offset) {
                newPos.offset.top += delta;
                self.pdfListView.setPdfPosition(newPos);
            }
        };
jviereck commented 10 years ago

@PabloK Thanks for making this PR.

The following function is the one that caused me trouble. It was used to scroll in either direction upon detecting touch movement or basic scroll events.

I don't understand your usecase and the code you provided here. Can you show a small example how the code is used?

jviereck commented 10 years ago

I console logged both this.canvas.offsetTop and this.dom.offsetTop while scrolling down on a large pdf and noticed that suddenly this.canvas.offsetTop increased dramatically from having been zero all the time it moved to 12000.

This sounds very wired. Are you sure this is not a browser bug or something else that is going on in your app? Without a working example / document the current code fails I don't feel convinced to merge the change about:

- left: this.canvas.offsetLeft + this.dom.offsetLeft,
- top: this.canvas.offsetTop + this.dom.offsetTop
+ left: this.dom.offsetLeft,
+ top: this.dom.offsetTop

But I am more than happy to accept your other patch. Can you please update the PR to include only the second change, such that I can pull it in?

PabloK commented 10 years ago

Yes!

On Sat, Aug 9, 2014 at 3:54 PM, Julian Viereck notifications@github.com wrote:

I console logged both this.canvas.offsetTop and this.dom.offsetTop while scrolling down on a large pdf and noticed that suddenly this.canvas.offsetTop increased dramatically from having been zero all the time it moved to 12000.

This sounds very wired. Are you sure this is not a browser bug or something else that is going on in your app? Without a working example / document the current code fails I don't feel convinced to merge the change about:

  • left: this.canvas.offsetLeft + this.dom.offsetLeft,- top: this.canvas.offsetTop + this.dom.offsetTop+ left: this.dom.offsetLeft,+ top: this.dom.offsetTop

But I am more than happy to accept your other patch. Can you please update the PR to include only the second change, such that I can pull it in?

— Reply to this email directly or view it on GitHub https://github.com/jviereck/pdfListView/pull/10#issuecomment-51687181.

Med vänliga hälsningar Pablo Karlsson