[TIMOB-13392] Android: Implement Scrollable Tabs
GitHub Issue | n/a |
---|---|
Type | New Feature |
Priority | High |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2014-12-03T01:14:58.000+0000 |
Affected Version/s | n/a |
Fix Version/s | Release 4.0.0 |
Components | Android |
Labels | notable |
Reporter | Igor Santos |
Assignee | Hieu Pham |
Created | 2012-12-12T09:10:24.000+0000 |
Updated | 2015-03-18T21:52:30.000+0000 |
Description
As seen in the [documentation](http://developer.android.com/design/patterns/actionbar.html) [(another place)](http://developer.android.com/design/building-blocks/tabs.html), there's a new subtype of tab bar in the Android, called Scrollable Tabs.
With 3.3.0 using ActionBar AppCompat I think it's appropriate for Titanium to implement scrollable tabs: http://developer.android.com/training/implementing-navigation/lateral.html This should be the standard tab setup on Android. Virtually every user I've seen tries to swipe between tabs, of course currently to no avail.... A good example is Google Play Store app. The big advantage of scrollable tabs is that it allows the developer to implement many side by side tabs (currently 8 tabs in the Play Store app).
See pull request: https://github.com/appcelerator/titanium_mobile/pull/5651 The app below has been tested on Android 2.3.6 and 4.1.2 Test app: index.xml:
index.js:
Try adding more tabs to the test program and watch them scroll :)
[~mokesmokes] We are greatly appreciative of the work you've done here. Our feature freeze was two weeks ago in order to for us to meet our release date in June. We had a discussion, but it's a bit too large of a change to squeeze in at this point. However, we will be reviewing the PR at a priority so interested parties can use it before the official next release.
It's up to you guys.... but quite honestly it maintains the current API to a tee, even all the events are exactly as before, except that two events and two methods are added (doesn't affect existing apps). The only break I see is with the map module, as noted in the PR comments.
I will also add that the problem with delaying the merge is that (from prior experience) as time passes there is high probability that it not not be mergeable, thus requiring additional work that is not required now. That's understandable in most cases, but in this case we're talking about not merging what is really core functionality in the Android UI, and a user expectation whenever they see tabs in Android.
Now that the master is at 3.4.0, can this please be merged? I'm asking since I would like to avoid unnecessary work if the codebase diverges. The PR is well tested, with all the features that I wrote into it working and in production. Again - the only break is with the Map module being used inside the tab group, and that in any case needed a more robust solution if the system is to use fragments (e.g. for lightweight windows).
[~mokesmokes] yes, I just spoke with Vishal about this yesterday. We have a LOT going on, but this is very high on our list.
+1 :) I hope to merge this feature ASAP!
I just created this feature by building it myself, it's almost working the same as the native Android feature, but it's also working for iOS. Check this out: http://developer.appcelerator.com/question/175688/scrollable-tabs--draggable-windows-example-titanium-alloy
[~Serfenia] Great. Are you interested in filing a pull request? Have others who are interested in this feature tried out this version?
It's build with the current available features in the Titanium SDK (3.2.3) and Alloy (1.3), so it's just an example in how to implement this feature with current available features. Currently I am working on a mobile application project for my company so I don't really have time to study and modify the SDK.
@Ingo, Patrick's example is 100% Javascript, so it's not really relevant for people who use the TabGroup APIs. Regarding the pull request for Android - it's been working for me merged into 3.3.0 for a very long time now, works perfectly with zero issues. As noted, the API is entirely backwards compatible with the standard Tab and TabGroup APIs, so for the vast majority of apps it will work with zero changes in Javascript. It's really too bad the Ti community needs to wait for 3.4.0 for this. I know of other Titanium developers who merged it into their private trees as well, so it really is well tested on 3.3.0. The only break I know of is with the Map module inside a TabGroup, with the method that was used to find the Map fragment. That should be easily fixed, and would have been broken anyway when Titanium will use Fragments more extensively (e.g. as lightweight window replacement).
The only thing Mark, that this feature would be nice to have for iOS as well. I know it's not native, but if you're trying to create the exact same application for both platforms, it would be nice to have it for both as well. In case it's not coming for iOS, I will stick to my own solution.
@Patrick, the difference is that scrollable tabs are native to Android, and expected of all tabbed applications (I have seen many Android users try to swipe tabs and get frustrated when it doesn't work - but not on iOS), while this is not the case for iOS. There are lots of differences between the platforms, and a complex app will never look and feel identical (e.g. ActionBar and menu usage on Android, no equivalent on iOS, etc). Plus in iOS swipes are often used to open item options in ListViews (native behavior), and this cannot live side-by-side with scrollable tabs without confusing the user. A good example is the Facebook app: while on iOS and Android that app has very similar structure, the UI elements are native and behave differently. So tabs are scrollable in Facebook Android, but not on iOS. Your solution may be OK for a simple app (haven't tried it since it's irrelevant to me), but it's not scalable to encompass the overall Android and iOS UI functionality. The platforms are different, having a common Javascript API will never change that, and that is not the point of Titanium. Titanium is about developing great Android and iOS native apps, not identical apps. If you want identical apps then you should use HTML5 and maybe Phonegap.
I understand your point, but if the wish from the cliënt is to create identical applications we as a company can't ignore that wish. Even though we advise them to choose the native features that both platforms deliver, they always have their own choice of interaction design. And the cliënt is 'king' in that case. And you may be right on creating the application with Phonegap (or even Sencha), but the MVC framework of Alloy works great for us so we will be using Titanium.
Patrick, your app requirements are your responsibility of course. But try to see my point of view here: I have worked hard on adding this native functionality to Titanium Android, and while the Javascript you wrote is OK in some limited circumstances, it really has nothing to do with providing the required native functionality. So it should not have been reported here, and it's distracting from the real issue. And as to your clients, people have all kinds of misconceptions. You can show them the Facebook app and they will see how they are different. And again, in many cases on iOS what you wrote will just not work (e.g. ListViews with deletable items).
Now that 3.3.0.GA is out, can you guys please merge this PR? https://github.com/appcelerator/titanium_mobile/pull/5651 Thanks.
[~hpham] Can we get this merged in?
Any update? Thanks.
[~ingo], [~hpham] - this PR has been outstanding for 3.5 months now, works great... can you please finally merge this? Thanks
From my current understanding, this PR introduces several side effects, some of which we will need to correct ASAP? Do we have tickets for those issues? I believe we have two choices:
Merge the PR and fix the regressions
Update the PR with the changes necessary
My preference is that we merge this onto a new feature branch and once all the related issues have been solved, we can then proceed from there.And regarding https://github.com/appcelerator/titanium_mobile/pull/5836 : The issue is unchanged in my PR, and the fix will most likely be the same as whatever will end up being the fix for that issue. The only concern here is that https://github.com/appcelerator/titanium_mobile/pull/5836 may make my PR unable to be merged automatically. The work to fix that underlying issue could just as easily be done on top of my PR as on the original code (which will then require more additional work....). Just trying to save time and effort here....
Hi Mark, The PR looks great. I think the only outstanding things that needs to be fixed are the following: - Fix TabGroups for API Level 10 with a non-AppCompat theme: You removed the tabhost implementation, and if someone is using a non-appcompat theme for API level 10 or below, this will not work for this. We probably want to keep that implementation there just in case. - We plan to merge https://github.com/appcelerator/titanium_mobile/pull/5836, so it would be great if you could touch up your PR after that has been merged.
Future to do: this version of the Android tab group uses [FragmentPagerAdapter](http://developer.android.com/reference/android/support/v4/app/FragmentPagerAdapter.html), which is optimized for small tab groups, keeps all the tabs in memory and thus switches fast. Android also provides a [FragmentStatePagerAdapter](http://developer.android.com/reference/android/support/v4/app/FragmentStatePagerAdapter.html) , which is optimized for lean memory, and flushes fragments outside the view. This would enable a large tab group, and we can have an proxy property to pick a largeTabGroup if we choose. The APIs are similar but not identical, so I had no time to implement it now. Note that for a tab group of 3 tabs, when the middle tab is selected both these implementations would keep all fragments in memory. SO the difference only comes to light in big tab groups.
Updated the PR with comments (no functional changes), please look over and merge. Thanks.
A good addition would be to add a flag to make or not the tabs swipeable. In my case I have an app that populates the window in the tab only when focused in one case and with a small delay in other case. This leads to a "not-so-smooth" scroll effect that makes the app look slow. The scroll stales for a split second in a state between windows then goes on. This happens on a powerful device (OnePlus). I suppose the issue is more visible on slower devices. I'll try to provide a testcase soon.
There is a flag "swipeable" - try to set it true/false. May I ask why you're populating the tabs in that fashion?
Amazing, and it works :D Why? well, very old codebase (2.5 years) very heavy app, trying to improve the perceived loading time :(
Spoke too soon, the swipeable flag set to false indeed disables the gesture itself, but the clicking on tabs the window change it's still animated.
[~rborn] my apologies but you will need to rebuild.... again :D You're right, a tab click should transition without a scroll. Fixed! Just get the latest update from the PR. Regarding the heavy app, see my previous comment. In a next phase (probably not soon, relax Appcelerator guys ;) ) I will let the dev pick between a FragmentPagerAdapter (as today - all tabs in memory together), or a [FragmentStatePagerAdapter](http://developer.android.com/reference/android/support/v4/app/FragmentStatePagerAdapter.html) which does exactly what you want.
I looked into it a bit more, some apps (e.g. Facebook) do a smooth scroll on tab clicks as well. So I decided to add a TabGroup property "smoothScrollOnTabClick" which defaults to true, to give the developer flexibility. Already tested and pushed to the PR. That's it! No more changes :D
Thank you sir! Set both - swipeable & smoothScrollOnTabClick - to false and works as advertised. Hope this will be merged in the master branch so we have it ready for 3.5.0 :) [~ingo] any chance?
Here is a test program that covers about everything:
Hi All, I appreciate greatly the effort the work that went into this PR in. However, as much as we would like to immediately accept this for the next release, we can't pull this into 3.4.0 for a few reasons:
3.4.0 is a release specifically targeted for iOS 8. If we take a new Android feature, we divert attention from iOS testing. Ordinarily, the split would be even. We will do greater testing on Android when L is released.
We're past code freeze for 3.4.0. An iOS update release is different than others in that we have a hard and fast deadline with little insight into the actual due date other than guessing what Apple will announce. That means we have to be ready to release early (if necessary), and we have likely no flexibility to move the release date back if necessary because a new feature caused an unexpected regression.
We am able to consider major bugs that remain as a result of TIMOB-17016, but we can't take a new feature in to replace that. Small, targeted fixes are welcome. I appreciate your understanding. [~mokesmokes] For the changes you have in your last PR, should I reopen this ticket as they were bugs in the initial implementation, or should we have a separate ticket to encapsulate the new things that were added?Reminder: This PR: https://github.com/appcelerator/titanium_mobile/pull/6008 fixes and improves the original merged PR. Thanks.
[~ingo] [~hpham] I just spent time rebasing https://github.com/appcelerator/titanium_mobile/pull/6008 to current master, can we please merge this and avoid more future work? It's already reviewed, and fixes a number of issues with my previous PR for this ticket. Thanks!
Reopening to address new comments.
[~ingo] the latest PR is bugfixes to the existing one, so it really belongs in 3.5.0 in my opinion, plus it's already reviewed and tested by [~hpham]
Hi [~mokesmokes] 3.6.0 is the new 3.5.0. Same release date--we just changed versions to indicate the 64-bit builds of iOS is a bigger jump than a minor patch.
[~ingo] thanks for the update. So will there be a 3.4.2 release prior as well? - i.e. is 3_4_X still being updated? What is 3_5_X based on? I noticed that Material is not in it, for example. Just trying to understand the content and schedule of your upcoming releases. Thanks :)
I've created a new PR to resolve conflicts with current master: https://github.com/appcelerator/titanium_mobile/pull/6414
Thanks [~hpham] :)
Verified the implementation of scrollable tabs. Closing. Environment: Appc Studio : 3.5.1.201412091616 Ti SDK : 4.0.0.v20150313181810 CLI : 3.4.2 Alloy : 1.5.1 MAC Yosemite : 10.10.2 Nexus 5 - Android 5.0 Samsung Galaxy S3 - Android 4.0.4