5-stones / react-native-readium

📚 📖 React Native ebook reader for iOS, Android, & Web via Readium
MIT License
89 stars 25 forks source link

Location doesn't work on Android and iOS #16

Closed M-HARIS-97 closed 1 year ago

M-HARIS-97 commented 1 year ago

Location doesn't work on Android. Always opens the first page. Locator props works fine.

<ReadiumView file={{url: filePath}} settings={settings} location={{ href: '/EPUB/text/ch002.xhtml', locations: { position: 5, progression: 0, totalProgression: 0.024390243902439025, }, type: 'application/xhtml+xml', }} onLocationChange={locator => locationChanged(locator)} />

Error: lateinit property navigator has not been initialized

jspizziri commented 1 year ago

@M-HARIS-97 interesting, this was working before. I'll try to take a look at it today. A full stacktrace from the android side would be helpful.

jspizziri commented 1 year ago

@M-HARIS-97 , I just tested this in the example app and I'm unable to reproduce it.

Can you please pull the latest version of the project and see if you can reproduce it in the example app? I've added instructions on how to run the example app.

You can test it by selecting one of the chapters from the table of contents. You can see that the navigator correctly navigates to the chapter. I'm going to close this until you figure out a repro in the example.

M-HARIS-97 commented 1 year ago

@jspizziri Please check the following image. This is the problem I faced

image

jspizziri commented 1 year ago

@M-HARIS-97 you need to describe to me how to reproduce the error in the example app. Have you tried that?

M-HARIS-97 commented 1 year ago

@jspizziri My application has 2 screens. First is an epub file list screen and second is a readium reading screen. I select an epub file, flick through some pages and store the last page locator value in my local db (sqlite), then return to the listing page. I select the same book again, that time I retrieve the locator value from the local DB and pass the value to the location property. That time I got an error message which I shared earlier.

jspizziri commented 1 year ago

@M-HARIS-97 , I don't want to sound like a jerk. I'll help you solve this, but you need to create a reproduction in the example app. The problem could be many things, including your ebook. If I can't see a reproduction I won't be able to fix it or help you.

Do you have questions on how to set up a repro with the example?

M-HARIS-97 commented 1 year ago

@jspizziri I was able to reproduce the problem using the example app. I made some changes to the example app. Please refer to the following to reproduce the issue; 1 - Install sqlite db 2 - Store the locator value in DB 3 - Close the app 4 - Then, open the app again 5 - Retrieve the locator value from DB 6 - Pass that value to the ReadiumView location parameter

jspizziri commented 1 year ago

@M-HARIS-97 thanks for working on the repro. We need to take it a bit further.

I want to eliminate all possible external variables from the equation, such as there being a problem in the way you're saving or fetching data to/from sqlite. Therefore, please create a reproducible example by just storing a locator value in JS and seeing if that causes the same problem (i.e remove sqlite from the example). I really need to know what the book and the locator actually look like at the time you're attempting to navigate.

Additionally, if you're needing to make changes to the code to reproduce it, please fork the library, make the actual changes on your fork, and then send me a link to your fork with your changes in a comment.

M-HARIS-97 commented 1 year ago

@jspizziri Please go through the following steps;

1 - Open the book. Flick some pages and console the locator value.

Screenshot 2022-12-08 at 5 16 37 PM

2 - Copy that specific locator value from console. 3 - Then apply that locator value directly to the location props

Screenshot 2022-12-08 at 5 17 38 PM

4 - Close the app 5 - Reopen the app 6 - Expected 6.1 - Opens the book and navigates to the specified location But What I'm Seeing 6.2 - The book opens and shows its first page

After updating to the latest version 1.0.4 its not working on iOS too.

M-HARIS-97 commented 1 year ago

@jspizziri I'm still facing this problem on iOS and android. Could you pleas verify this? Please check my last comment.

jspizziri commented 1 year ago

@M-HARIS-97 upon opening the App you should be using initialLocation which is part of the File object. The location attribute should really only be used during navigation once the reader view is booted. The reason for the duplicate property is to avoid a race condition. In order to properly build the view at the correct initial location the location needs to be guaranteed to be present at the time the view is built. Because of the asynchronous nature of the react-native bridge layer we can't guarantee that the location property will arrive at the native layer before or at the same time that the file property does. Therefore they need to be passed as the same object.

In short, location was never intended to be used for boot position, and it never should've been working on iOS or Android unless it was by pure luck that when you happened to run it, the async calls randomly happened to resolve in the correct order to make it work.

With all that said, I could see a potential need for a feature that reports the state of the views initialization process. Something like the following to help determine if you're allowed to start passing locations, etc:

onStateChange={(state: 'loading' | 'initialized' | 'errored' ) => { ... }}
M-HARIS-97 commented 1 year ago

@jspizziri Thanks for your detailed explanation. Currently it works well on iOS and android. Previously I was not using the initialLocation property. I think that's why I faced problems in my project. Once again Thank you @jspizziri for your unconditional help

jspizziri commented 1 year ago

@M-HARIS-97 thanks for the bug reports! I should probably add something to the README.md to make it more clear to folks what that property is for.

jspizziri commented 1 year ago

I've updated the readme for better clarity here https://github.com/5-stones/react-native-readium/commit/dc92e1e38c4c9f37aa8e7e078efba8501e2bdc91

M-HARIS-97 commented 1 year ago

@jspizziri Ok Noted