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

Replace ViewPager with RecyclerView + SnapHandler #150

Closed
ErikHellman opened this issue Dec 7, 2018 · 6 comments
Closed

Replace ViewPager with RecyclerView + SnapHandler #150

ErikHellman opened this issue Dec 7, 2018 · 6 comments
Labels

Comments

@ErikHellman
Copy link
Contributor

The ViewPager solution currently used relies on fragments, which makes the navigator component a bit heavy on fragment use. A better approach would be to use a RecyclerView with a SnapHandler. This would also make the nested scrolling happening today much simpler, as RecyclerView supports this in a better way.

@stevenzeck
Copy link
Contributor

Another option would be to migrate to viewpager2 from androidx. That would replace the current viewpager and a bunch of extra code supporting RTL. It wouldn't replace fragments though.

@stevenzeck
Copy link
Contributor

@ErikHellman @aferditamuriqi I started a very basic replacement of the custom ViewPager (R2RTLViewPager and R2ViewPager) with ViewPager2. https://github.com/stevenzeck/r2-navigator-kotlin/tree/viewpager2 and https://github.com/stevenzeck/r2-testapp-kotlin/tree/viewpager2.

I'm still trying to understand the architecture here with submodules, but it "works", as in pages load and I can go from page to page. I'm not sure if RTL works however, as I don't know how to test that. And a couple of parts I'm unsure of that I just commented out. Something worth pursuing or go with another approach?

@aferditamuriqi
Copy link
Member

@stevenzeck This is great, thank you, I will definitely take a look into your solution with the viewpager2 as I have planned to replace the current custom implementation with it as well. I will test your solution and give you some feedback. I am sure we can tweak what we need to replace the custom implementation.

@stevenzeck
Copy link
Contributor

Things to note/do:

  • Determine what code can be safely removed or commented out. This is mainly concerning the RTL custom code, which viewpager2 supports out of the box
  • There are a lot of places where it gets the current fragment via the pager adapter. With the new adapter, FragmentStateAdapter, the fragments array is private. Have to either use the supportFragmentManager or do something else
  • On that note, continue using fragments or move to RecyclerView? If that is the case, RecyclerView.Adapter would be the adapter to use

@stevenzeck
Copy link
Contributor

stevenzeck commented Jul 22, 2019

@aferditamuriqi was there something special you had to do so that viewpager goes to the next page instead of the next chapter/fragment? With viewpager2 the way it is now, swiping goes between each fragment instead of pages and can't figure out why the original viewpager doesn't do the same.

Edit: So R2WebView onTouchEvent is receiving ACTION_DOWN, ACTION_MOVE and ACTION_CANCEL. ACTION_UP isn't registering. Is there a better way to break down the chapters into pages so that the ViewPager changes pages and doesn't rely on scrolling via onTouchEvent?

@mickael-menu mickael-menu transferred this issue from readium/r2-navigator-kotlin Jul 29, 2022
@mickael-menu
Copy link
Member

We can follow-up in this thread: #51 (reply in thread)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants