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

Added a setUpWithViewPager() method in BottomBar class #800

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

talhahasanzia
Copy link

@talhahasanzia talhahasanzia commented Jun 21, 2017

  • Created a setUpWithViewPager() method similar to Android's TabLayout
    method.
  • This will couple BottomBar with a ViewPager you implemented in
    your app. And change behaviour of other when one changes.
  • For eg if Tab is selected, ViewPagerwill be updated. If ViewPageris updated Tab will be selected accordingly.
  • It is as simple as a call to setUpWithViewPager() and you have to pass ViewPager instance, no extra implementation required.
  • See ViewPagerActivity for example usage. MainActivity has an added button to navigate to this activity.
  • Code is commented with JavaDoc formatting.
  • Only bad code I see is ViewPager.addOnPageChangedListener() inside my method. I dont know anywhere better to put this implementation.
  • Any problems with ViewPagerwill cause this feature fail to work properly.
  • In original BottomBar class, following code changes are made:
    • ViewPagerprivate member declaration
    • setUpViewPagerMethod() which has listener to OnPageChange, if ViewPagermember is set correctly, this will call selectTabAtPosition(position, true).
    • If tabs are selected, there also viewpager instance is checked and if its not null, viewpager is updated with respect to tab.
    • All code is commented.
  • The setUpWithViewPager() has following description in its docs.
    "Set tab behavior with respect to ViewPager.
    If tab is changed, ViewPagerwill also be updated.
    If ViewPageris changed, tab will be updated.
    "
  • Please review and update (accept/reject) the pull request and or point out any issue or errors.

[UPDATE]

You have not accepted the license agreements of the following SDK components:

  [Solver for ConstraintLayout 1.0.0-alpha8, ConstraintLayout for Android 1.0.0-alpha8].

  Before building your project, you need to accept the license agreements and complete the installation of the missing components using the Android Studio SDK Manager.

  Alternatively, to learn how to transfer the license agreements from one workstation to another, go to http://d.android.com/r/studio-ui/export-licenses.html
  • Removed that library since it is not used, it was added when I created ViewPagerActivity.

  • Moved back to gradle version that original project was using.

  • Cleaned project before commit.

talhahasanzia and others added 2 commits June 21, 2017 02:08
- Created a setUpWithViewPager() method similar to Android's TabLayout
method.
- This will couple Navigation Bar with a ViewPager you implemented in
your app.
- It is as simple as a call to setUpWithViewPager().
- See ViewPagerActivity for example usage.
- Code is commented with JavaDoc formatting.
* Removed ConstraintLayout library since it is not used, it was added
when I created ViewPagerActivity.

* Moved back to gradle version that original project was using.

* Cleaned project before commit.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 62.793% when pulling bfb14f6 on talhahasanzia:master into 711fcaf on roughike:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 62.793% when pulling 0fe5ad8 on talhahasanzia:master into 711fcaf on roughike:master.

@talhahasanzia
Copy link
Author

What is the coverage threshold? Does this mean PR will fail the integration?

@talhahasanzia
Copy link
Author

I will be writing tests in next commit to increase coverage.

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.

2 participants