search

ncapdevi / FragNav

An Android library for managing multiple stacks of fragments
1.5k stars 220 forks source link

Changed max number of tabs to 20 instead of 5. (issue #63) #80

Closed Thomas-Vos closed 7 years ago

Thomas-Vos commented 7 years ago

Changed the maximum number of tabs to 20. This should be enough for most people. This pull request solves issue #63.

ncapdevi commented 7 years ago

@Thomas-Vos Thanks so much for the PR (I somehow missed it until today). Was it really this minimal to change things? I need to test it (will do so either today or tomorrow) but if everything looks good, I'll merge and post a library update. I don't think this is the longest term solution as I'd love a way to put zero limit on the number of tabs and essentially send in a list, but this certainly is a huge improvement for the time being and should alleviate some headaches for users. Thanks again for the PR

Thomas-Vos commented 7 years ago

@ncapdevi No problem. I don't think anyone needs more than 20 "tabs", so this should be enough for now. Your library is really useful!

ncapdevi commented 7 years ago

@Thomas-Vos Agreed, but people come up with crazy ideas and I think it's ideal when a library only puts in useful constraints, not limiting ones. But yes, this is a huge improvement and gives some breathing room for the problem, so your contribution is very much appreciated. I had a chance to give everything a quick scan in context and I'm hopefully going to merge tonight because it all looks correct to me. The one thing that could help would be if there was a test using the 20 tabs. The quickest/easiest way to do that would be do modify the NavDrawerActivity to have more (even 10 would be great) and it could just duplicated the already existing fragment types just multiple times. If you have time to add that to this PR, it'd be amazing, if not. I'll do it myself ASAP. Thanks again.

Thomas-Vos commented 7 years ago

@ncapdevi Sorry, I don't have time to modify the sample.

We could remove the 20 tabs limit, but then I'm not sure what to do with the index constants.

ncapdevi commented 7 years ago

Merged into branch 2.1.0