Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various reflowable EPUB columns shift issues #166

Closed
mickael-menu opened this issue Jun 29, 2020 · 12 comments · Fixed by readium/r2-navigator-kotlin#178
Closed

Various reflowable EPUB columns shift issues #166

mickael-menu opened this issue Jun 29, 2020 · 12 comments · Fixed by readium/r2-navigator-kotlin#178
Labels
bug Something isn't working

Comments

@mickael-menu
Copy link
Member

On some books, when swiping through the content, the EPUB columns are shifting and are not aligned anymore. This reverts by tapping on the edges.

I could reproduce this on oryel_1984.zip from this issue: #167

Screenshot_20200629-101320

@mickael-menu mickael-menu added the bug Something isn't working label Jun 29, 2020
@mickael-menu
Copy link
Member Author

Another related issue, the columns are slightly shifting in long resources, such that the last page is not centered. For example with Gatsby:

Screenshot_20200818-111928

@mickael-menu mickael-menu changed the title EPUB columns shift when swiping through a resource Various reflowable EPUB columns shift issues Aug 19, 2020
@mickael-menu
Copy link
Member Author

Another issue, I can move the offset of a page by pinching it, then panning it in a single gesture.
Screenshot_20200819-162838

@johanpoirier
Copy link
Contributor

I also have issues with going to a locator with an anchor fragment (i.e. Chapitre02.html#int02-s3).
The columns are shifted as well. Loading the webView with an URL containing an anchor is problematic. Is it CSS related?

@nidhinprathap
Copy link

@johanpoirier there's a method that's javascript getting called but the definition is missing. Please check the below-mentioned ticket.
readium/r2-testapp-kotlin#351

For now, you can change tat and call scrollToTag method in utility.js

@johanpoirier
Copy link
Contributor

For now, you can change tat and call scrollToTag method in utility.js

Yes I did see that and I used scrollToId method instead. But the behaviour is sometimes odd. When the URL has an anchor, the page displayed is the one before so I'm trying to debug the issue right now.

@nidhinprathap
Copy link

nidhinprathap commented Sep 7, 2020

ah for that can u try this

var scrollAnchor = function(id) {
        var element = document.getElementById(id);
        var elementOffset = element.scrollLeft // element.getBoundingClientRect().left works for Gutenbergs books
        if (elementOffset == 0) {
            elementOffset = element.getBoundingClientRect().left;
        }
        var offset = Math.round(window.scrollX + elementOffset);

        document.scrollingElement.scrollLeft = snapOffset(offset);
 
};

@johanpoirier can you try this. Add this method to utils.js. And call scrollAnchor from Kotlin like this

val id = url.substring(url.indexOf('#')+1); // this is to ignore hash from start
webView.loadUrl("javascript: scrollAnchor('$id');")

@johanpoirier
Copy link
Contributor

johanpoirier commented Oct 27, 2020

Another issue, I can move the offset of a page by pinching it, then panning it in a single gesture.

This is caused by those lines of code: https://github.com/readium/r2-navigator-kotlin/blob/develop/r2-navigator/src/main/java/org/readium/r2/navigator/R2WebView.kt#L735-L738

The mLastMotionX is set by the second finger down, so the xDiff is almost never greater than mTouchSlop: https://github.com/readium/r2-navigator-kotlin/blob/develop/r2-navigator/src/main/java/org/readium/r2/navigator/R2WebView.kt#L677

My question is: why are we mixing touch pointers values into mLastMotionX?

@mickael-menu
Copy link
Member Author

It looks like the offset issues are caused by some rendering bug in Android's Web View, but we have a potential workaround enabled by Readium CSS: readium/readium-css#97

@mickael-menu
Copy link
Member Author

I pushed a PR which addresses the offset issue at least in my use cases, using the latest version of Readium CSS. I'm not 100% sure it fixes all of the reported offset issues.

readium/r2-navigator-kotlin#178

Note that utils.js is completely rewritten, to align with the Swift version. I didn't see any regression with RTL (which didn't work anyway in my tests) but something to keep in mind in case some other issues pop up.

@johanpoirier

I also have issues with going to a locator with an anchor fragment (i.e. Chapitre02.html#int02-s3).
The columns are shifted as well.

This should be fixed as well, would you mind double-checking with this PR?

@johanpoirier
Copy link
Contributor

johanpoirier commented Nov 13, 2020

thanks @mickael-menu , I'll test that next monday.

@johanpoirier
Copy link
Contributor

This should be fixed as well, would you mind double-checking with this PR?

I just tested it on my own app and the r2-testapp but the offset issue is still present in this case.
I fixed it on my app a few weeks ago, I'll try to put the fix in a PR tomorrow.

@mickael-menu
Copy link
Member Author

I fixed it on my app a few weeks ago, I'll try to put the fix in a PR tomorrow.

Sounds good, might be that we're using different test cases. Thanks for taking a look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants