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

#260: bottom navigation demo #565

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

sdelaysam
Copy link

Another attempt to make a demo for #260
Also check #316 and #474

@fonix232
Copy link

fonix232 commented Sep 27, 2019

May I recommend a more elegant, generic solution that is completely self-contained? It's in Kotlin though:

https://gist.github.com/fonix232/14294caea86c4161478f5263a41fc50b

I've used the generic ViewPager, and have not prepared to swipes, but it should be relatively straightforward to use your NonScrollableViewPager implementation.

@sdelaysam
Copy link
Author

@fonix232 any good reason not to use the RouterPagerAdapter provided by Conductor?

@fonix232
Copy link

@sdelaysam Using Kotlin I've simplified the logic of the RouterPagerAdapter. It's basically a verbatim copy, with some Kotlin-specific sugar added on top. No other reason whatsoever, in fact if you want me to base one off of RouterPagerAdapter, I can probably do it in a few minutes.

@sdelaysam
Copy link
Author

@fonix232 The whole project including demos using Java so I'm not sure about pushing Kotlin code. And it seems our PRs won't be merged anytime soon here 😅
Anyway, thanks for sharing Kotlin version!

@fonix232
Copy link

Honestly, apart from the handful Java purist crybabies, I don't see any valid reason why Conductor couldn't and shouldn't transition to Kotlin soon. The codebase would be cut in about half, would be nicer to use and read, and so on.

@vincent-paing

This comment has been minimized.

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.

3 participants