Skip to content

Evaluate replacing R2RTLViewPager and R2ViewPager with ViewPager2 #148

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

Closed
stevenzeck opened this issue Aug 4, 2020 · 4 comments
Closed

Comments

@stevenzeck
Copy link
Contributor

I don't know the history of the usage of R2RTLViewPager, was it used just for RTL support? ViewPager2 supports RTL natively.

Migration guide: https://developer.android.com/training/animation/vp2-migration
Jetpack library reference: https://developer.android.com/jetpack/androidx/releases/viewpager2
Class docs: https://developer.android.com/reference/androidx/viewpager2/widget/ViewPager2

I had looked at this before but had personal things come up, so had to stop. Once readium/r2-navigator-kotlin#148 is merged I can take another look.

@mickael-menu
Copy link
Member

Thanks, such contribution is most welcome! And yes, as far as I know R2RTLViewPager was here for the RTL support.

Right now, I'm focused on releasing the 2.0.0-alpha.1, but once this is behind us, I'll take a look at readium/r2-navigator-kotlin#148 first thing.

@stevenzeck
Copy link
Contributor Author

stevenzeck commented Aug 10, 2020

Initial investigation

  • ViewPager2 would eliminate the need for R2RTLViewPager and R2ViewPager
  • ViewPager2 is final so can't be extended
  • addOnPageChangeListener is replaced by registerOnPageChangeCallback
  • VIewPager2 has a layoutDirection attribute that accepts a View.LAYOUT_DIRECTION_ int. Thinking of adding these to the ReadingProgression enum class
  • FragmentStatePagerAdapter is replaced by FragmentStateAdapter

I replaced all of these locally and am trying to get through errors and how best to replace some of the old ViewPager code (like setPrimaryItem). Right now when I swipe to page, it goes from chapter to chapter instead of page to page. Clicking the edge pages properly.

@mickael-menu
Copy link
Member

@stevenzeck This sounds great, thanks for taking a look into this.

VIewPager2 has a layoutDirection attribute that accepts a View.LAYOUT_DIRECTION_ int. Thinking of adding these to the ReadingProgression enum class

For ReadingProgression we're limited to what is specified in RWPM, so: left to right, right to left, top to bottom, bottom to top and auto. So what do you need to add? I think that we won't use LAYOUT_DIRECTION_INHERIT and LAYOUT_DIRECTION_LOCALE.

@stevenzeck
Copy link
Contributor Author

stevenzeck commented Aug 12, 2020

I was hesitant at first to put the logic that determines which one to use in EpubNavigatorFragment. But now I don't think it's horrible to just put it there.

I pushed a first round to the testapp and navigator repositories.

@mickael-menu mickael-menu transferred this issue from readium/r2-navigator-kotlin Jul 29, 2022
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 a pull request may close this issue.

2 participants