TIMOB-13392: Android scrollable tabs #5651
Conversation
|
See test app in JIRA: https://jira.appcelerator.org/browse/TIMOB-13392 |
|
Note this PR will break https://jira.appcelerator.org/browse/TIMOB-13864 since we lose the capability to setId on the tab fragment when using a ViewPager as the fragment container. But we have no choice - we want swipe tabs! :) ...... In any case, the fix should be rather simple: The FragmentPagerAdapter in the implementation can return a unique ID per fragment which can be stored somewhere and checked, similar to the current solution of 13864. This however is not part of this PR. |
mokesmokes
changed the title
|
Changed PR name to "scrollable tabs" to indicate this functionality exists. You can scroll tabs offscreen if you have many tabs and/or long tab titles :) |
| @@ -247,6 +241,14 @@ void close(boolean activityIsFinishing) { | |||
|
|
|||
| void onSelectionChanged(boolean selected) | |||
| { | |||
| if (!selected) { | |||
There was a problem hiding this comment.
Åny reason why you move this from onFocusChanged to onSelectionChanged?
There was a problem hiding this comment.
Yup, there's a good reason: as it turns out, in Android the Tab focus/blur events fire in many cases, not just when the tab is selected and unselected. For example - when the screen is locked/unlocked blur/focus will be fired (this is unlike iOS, BTW). So for clarity and also backwards compatibility, I did not touch the focus/blur stuff at all, but I did move relevant selection/unselection logic into the relevant events.
That's also why I added the selected/unselected tab events for Android. Again, the focus/blur events work exactly as before, so anything relying on them is not broken.
|
I did functional on KitchenSink. Fantastic work, everything seems to work! My only concern is that this is a breaking change that will probably affect all modules extending the current framework of tabgroup. |
|
@hieupham007 I don't see what else it can break besides those two issues. I suggest you merge this, and then we tackle these two specific issues ASAP. Makes no sense to fix #5836 before this merge, and it's also possible that this issue will affect the map module issue in any case (both deal with fragment handling). BTW - there is a flag to disable the swiping, so old behavior can be maintained by just setting this flag. |
|
In any case I see you wrote #5836 (comment) that it's not fixed. |
| private boolean tabsDisabled = false; | ||
| private Fragment savedFragment = null; | ||
| private boolean savedSwipeable; | ||
| public boolean swipeable = true; |
There was a problem hiding this comment.
It's better to use a getter and setter to get access to it.
There was a problem hiding this comment.
@pingwang2011 this is internal to the implementation and not accessed from Javascript. In TabGroupProxy.java it is indeed a getter/setter - here it is of no consequence in my opinion.
mokesmokes
force-pushed
the
TIMOB-13392
branch
from
719f68f
to
b608241
Compare
|
TIMOB-17016 is now fixed for swipe tabs, fixed the regression for a map inside a tab, and fixed up remoteTab. The merge conflict is due to #5836 , the fix for TIMOB-17016 for the non-swipe tabs, the merge should just be to accept my version of the files and that's it. |
|
@pingwang2011 @hieupham007 I made the change to propertyAccessors as requested, please let me know if any more issues, and let's close this one. Thanks! |
|
Merging. Thank you so much for this! |
TIMOB-13392: Android scrollable tabs
Android tab group can now by default be navigated by swipes (e.g. see Play Store app). The functionality can be toggled with the new
TabGroup.swipeableproperty. In addition, tab navigation can be disabled (or re-enabled) by callingdisableTabNavigation(boolean). Plus added tabselectedandunselectedevents (currently these events exist in Android only. In iOS the tab focus/blur events do the same - I suggest they be renamed to selected/unselected). Note that all additional events are fired exactly as before.