search

readium / swift-toolkit

A toolkit for ebooks, audiobooks and comics written in Swift
https://readium.org/mobile/
BSD 3-Clause "New" or "Revised" License
222 stars 96 forks source link

`Navigator.go` results in misaligned view #449

Open domkm opened 4 weeks ago

domkm commented 4 weeks ago

Describe the bug

I'm synchronizing EPUB location and decoration with audiobook narration and finding that using go on the EPUB navigator can result in a view that is not center aligned and is therefore cut off. As one navigates through the cut off view, the view can quickly swap back and forth between the two sides of the cut page. However, swiping causes the pages to transition correctly. I'm attaching a screen recording below showing the correct swiping behavior followed by the cut off go behavior.

https://github.com/readium/swift-toolkit/assets/1292853/e92fa7ff-308b-4810-a02d-e6b66e3477be

How to reproduce?

  1. Open an EPUB
  2. Use go to navigate to locators currently displaying or about to display on the next page

Though not the same EPUB as in the video, I also noticed the issue using this one (zipped because GitHub won't permit EPUB upload): 2 B R 0 2 B.epub.zip

Readium version

develop 8fad4d5a

OS version

iOS 17.5.1

Testing device

iPhone 11 (also noticing it in simulator)

Environment

macOS: 14.5
platform: arm64
Xcode 15.4
Build version 15F31d

Additional context

No response

mickael-menu commented 4 weeks ago

Do you have a way to reproduce this problem in the Test App, maybe after applying some code patch? go() is already used with the TTS in the Test App and don't seem to show this issue.

domkm commented 4 weeks ago

I'm having trouble reproducing this in the Test App. Would it be okay to zip up my project and send it to you privately?

mickael-menu commented 4 weeks ago

Unfortunately I can't spend time debugging individual projects.

In your screencast the navigator is not spanning the whole screen, I wonder if that could have an impact on this bug?

domkm commented 3 weeks ago

It's totally understanding that you can't debug individual projects. I just assumed that this was a bug in swift-toolkit, not specifically in my implementation. The code I'm using is pretty much directly pulled from this documentation and I am not doing anything else to limit the width of the display, just the height.

If it is a bug in swift-toolkit, I believe the reason I am unable to replicate it is due to:

  1. I am using develop but had to switch to main to attempt to reproduce because Test App fails to compile on develop. Perhaps the issue was introduced in the transition from 2 to 3.
  2. My project uses SwiftUI and Test App uses UIKit, which I am not familiar with beyond the bare minimum I used to wrap EPUBNavigationViewController. As such, I am not able to copy the relevant parts of my UI into Test App.

One thing which does work around this issue is setting scroll: true in EPUBPreferences. However, that causes go to scroll such that the target locator is at the very top of the screen. Is there any way to make to go scroll such that the target locator is vertically centered?

mickael-menu commented 3 weeks ago

I am not doing anything else to limit the width of the display, just the height.

We can see left and right margins around the navigator in your video. I wonder if you have the same issue if you remove them.

I am using develop but had to switch to main to attempt to reproduce because Test App fails to compile on develop. Perhaps the issue was introduced in the transition from 2 to 3.

Did you try to compile it after running make dev? (make spm/make carthage/make cocoapods only work properly from a released version). What errors did you get? If the GitHub checks pass, the Test App should not have build issues.

One thing which does work around this issue is setting scroll: true in EPUBPreferences. However, that causes go to scroll such that the target locator is at the very top of the screen. Is there any way to make to go scroll such that the target locator is vertically centered?

Not at the moment, but that could be an EPUBNavigatorViewController.Configuration setting if you're interested in contributing it?

domkm commented 3 weeks ago

We can see left and right margins around the navigator in your video. I wonder if you have the same issue if you remove them.

You're right! I didn't realize that I had done that. And you're also right about that causing this issue. Thanks! Is there an easy way for me to add horizontal padding in Test App to attempt to reproduce this?

Did you try to compile it after running make dev? (make spm/make carthage/make cocoapods only work properly from a released version). What errors did you get? If the GitHub checks pass, the Test App should not have build issues.

I did not and that fixed it. Thanks. Looks like I didn't read the README very thoroughly. My mistake.

Not at the moment, but that could be an EPUBNavigatorViewController.Configuration setting if you're interested in contributing it?

Even though this is no longer my top priority, I would be interested in this at some future point. If you point me in the right direction as to how to implement this, I will try to get to it eventually.

mickael-menu commented 3 weeks ago

Is there an easy way for me to add horizontal padding in Test App to attempt to reproduce this?

You might be able to do it here: https://github.com/readium/swift-toolkit/blob/024fdc0f5f794dce388528f913e584be9bba74cb/TestApp/Sources/Reader/Common/VisualReaderViewController.swift#L60-L61

I did not and that fixed it. Thanks. Looks like I didn't read the README very thoroughly. My mistake.

You're not the only one, I'll see if we can't have a guard in the makefile to check this 😄

Even though this is no longer my top priority, I would be interested in this at some future point. If you point me in the right direction as to how to implement this, I will try to get to it eventually.

That would be in the JavaScript layer, every time scrollTop is set, for example here: https://github.com/readium/swift-toolkit/blob/024fdc0f5f794dce388528f913e584be9bba74cb/Sources/Navigator/EPUB/Scripts/src/utils.js#L163

Take a look at this guide if you want to modify the JavaScript files: https://github.com/readium/swift-toolkit/blob/develop/CONTRIBUTING.md#modifying-the-epub-navigators-javascript-layer

domkm commented 3 weeks ago

You might be able to do it here:

https://github.com/readium/swift-toolkit/blob/024fdc0f5f794dce388528f913e584be9bba74cb/TestApp/Sources/Reader/Common/VisualReaderViewController.swift#L60-L61

That's it! To reproduce the issue, change the code like this:

- navigator.view.frame = view.bounds
+ let padding: CGFloat = 25
+ navigator.view.frame = CGRect(x: view.bounds.origin.x + padding, y: view.bounds.origin.y, width: view.bounds.width - 2 * padding, height: view.bounds.height)

Then add some bookmarks at various places and navigate directly to them. It seems like bookmarks on the first page of a chapter will display correctly while others will be cut off.

domkm commented 2 weeks ago

Actually, this is possible to replicate without modifying the Test App. If you open it in an iPad or Mac you can resize the window and that will cause navigating to arbitrary bookmarks to break. I just reproduced this on unmodified develop (91bc1592).