Skip to content

Conversation

@mickael-menu
Copy link
Member

Fixed

Navigator

  • Fixed turning pages of an EPUB reflowable resource with an odd number of columns. A virtual blank trailing column is appended to the resource when displayed as two columns.

For an EPUB reflowable resource, having an odd number of columns when displaying two columns per screen causes snapping and page turning issues. To fix this, we insert a blank virtual column at the end of the resource.

For an EPUB reflowable resource, having an odd number of columns when displaying two columns per screen causes snapping and page turning issues. To fix this, we insert a blank virtual column at the end of the resource.
mickael-menu added a commit to demarque/readium-kotlin-toolkit that referenced this pull request Oct 12, 2021
@mickael-menu
Copy link
Member Author

@aferditamuriqi I'm going to merge this PR by the end of the week unless you report any problem with this. We had this fix in production for about two weeks now. If an issue pops up, we can always revert these changes later on.

@aferditamuriqi
Copy link
Member

@aferditamuriqi I'm going to merge this PR by the end of the week unless you report any problem with this. We had this fix in production for about two weeks now. If an issue pops up, we can always revert these changes later on.

Hey @mickael-menu I will be trying to test this before the end of the week, if i can't test it within a current implementation this week, for sure next week. i really would like to see if the last reading position stays accurate between scroll mode and paginated mode when adding an additional column.

@mickael-menu
Copy link
Member Author

Alright, I'll wait one more week then

@aferditamuriqi
Copy link
Member

@mickael-menu i did test this in terms of last reading position and it does falsify it, but i think it's a bit more complicated than that. we can maybe discuss this in more detail tomorrow on the call or when you have some times.

@aferditamuriqi
Copy link
Member

@mickael-menu there was actually a bigger issue with odd numbers on two column screens, the tap to go to the next page was stuck when there was an odd number. i do think that this PR fixes majority of the issues around page numbers, snapping and swiping. the only thing i am concerned about that it falsifies the progression. maybe we need to change how the progression is calculated and base it on the column numbers instead of screen numbers. that could i think fix all the issues. but more to discuss i think :)

@aferditamuriqi aferditamuriqi self-requested a review November 2, 2021 13:53
@mickael-menu
Copy link
Member Author

@aferditamuriqi Yeah we can raise the subject during next call. I think it would be interesting to save the text context as well, now that we have a way to jump to a given text. This would be less fragile than the progression and might work better if the publication content changes.

@mickael-menu mickael-menu merged commit d087f3a into develop Nov 2, 2021
@mickael-menu mickael-menu deleted the fix/two-columns branch November 2, 2021 16:45
@aferditamuriqi
Copy link
Member

@aferditamuriqi Yeah we can raise the subject during next call. I think it would be interesting to save the text context as well, now that we have a way to jump to a given text. This would be less fragile than the progression and might work better if the publication content changes.

I agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants