Switching underlying PDF viewer

[mkoo21]

Switching the pdf renderer from an old deprecated library to a newer, better maintained library per #34.

Using the newer library will resolve some pixelation issues when zooming in android 6. It also lets you use the Apache License 2.0 rather than GPL.

[andrejunges]

@mkoo21 is it ready to be used/merged?

[mkoo21]

I have been using it for a while now without noticing any issues (android only, since react-native-quick-look takes care of document rendering in ios very well)

[andrejunges]

I've tested it - and now the app isn't crashing, but nor its rendering the pdf (this behavior only happens for one of the pdf files, but I can open it in other pdf apps or ios).
Im trying now to open it from the URI

[mkoo21]

did the old lib work better for you?

[andrejunges]

nop, I got a crash in that specific file in the old lib (PDF is corrupted..)

[andrejunges]

any idea/suggestion?

[mkoo21]

Haven't run into the issue myself so not really sure what to say. Does it look similar to this issue? If there really isn't anything special about your file maybe ask there and see if you get a better response

[andrejunges]

Indeed its similar.
I noticed now that the problem is related to the pdf's size.
Files with over 400kb are not opening in here

[andrejunges]

@mkoo21 I finally found the problem.
Actually it was in my code. I passed the file path to my componente before the file creation completes - and once the component's state got that path it renders the PdfViewer..
So it was trying to read the file when it was still creating it (before being closed).
classic IO problem
thanks for the help anyway 👍

[mkoo21]

Sure, glad you got it resolved and sorry I wasn't able to help more

[andrejunges]

Any idea why I get this error when I rerender an pdf on Android?

[gyzerok]

@mkoo21 @andrejunges any updates about the PR?

[mkoo21]

Issue #39 has nothing to do with this PR (you'll get it with the old lib as well), so it's been the same as always

[gyzerok]

@mkoo21 can you rebase your fork on top of current master so I can check if it works? And them we can probably ask to merge it.

[mkoo21]

done

[gyzerok]

@mkoo21 thank you! Will try it next week and report my results.

[aeirola]

Nice work, the new PDF rendering library seems to work better. Found a couple of issues though:

1. The page numbering of the new library seems to be zero-indexed, meaning that a PDF always opens on the second page. Passing the prop pageNumber={0} doesn't help, since there is a check for non-positive page numbers at https://github.com/cnjon/react-native-pdf-view/blob/master/android/src/main/java/com/keyee/pdfview/PDFViewManager.java#L106

2. The old dependency is still present in the gradle file, causing unneccessary libraries to be loaded in the application. Would be nice if this could be removed.

[gyzerok]

@mkoo21 sorry for long response. Here my coworker tried your branch and reported some issues.

[mkoo21]

Thanks for the feedback, will take a look when I get home

[gyzerok]

Hello @mkoo21! Do you have any updates?

[mkoo21]

Yes, sorry, I added some minor changes as requested but haven't really had a chance to test them yet. You are welcome to try these 'unstable' changes if you would like, otherwise I will hopefully get back to you by the end of the week

[aeirola]

Thanks for the changes, seem to work nicely. I think we'll take these changes in use in our application. Hopefully they will soon be merged into the main repository too.

[gyzerok]

Would it be clearer to do pageNum >= 0?

[mkoo21]

Sure, I can do that if it helps

[gyzerok]

@sibelius it seems like we haven't had any issues while using version in this PR. Should we maybe merge it then?

[sibelius]

@gyzerok could u please upgrade the example project to the latest version of React Native?

So I can test against your branch and merge it

[gyzerok]

@sibelius oh, it's not my PR.

@mkoo21 could you, please, do what @sibelius is asking for?

[mkoo21]

@sibelius updated example

[sibelius]

it does look good, thanks for the awesome work