swift-toolkit icon indicating copy to clipboard operation
swift-toolkit copied to clipboard

`Navigator.go` results in misaligned view

Open domkm opened this issue 1 year ago • 9 comments

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

domkm avatar Jun 04 '24 20:06 domkm

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.

mickael-menu avatar Jun 05 '24 11:06 mickael-menu

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?

domkm avatar Jun 06 '24 02:06 domkm

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?

mickael-menu avatar Jun 06 '24 11:06 mickael-menu

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?

domkm avatar Jun 06 '24 19:06 domkm

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?

mickael-menu avatar Jun 07 '24 07:06 mickael-menu

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.

domkm avatar Jun 07 '24 18:06 domkm

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

mickael-menu avatar Jun 08 '24 07:06 mickael-menu

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 avatar Jun 08 '24 17:06 domkm

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).

domkm avatar Jun 15 '24 22:06 domkm