iskugor / Ti.SwipeRefreshLayout

Titanium module for Android's SwipeRefreshLayout.
56 stars 19 forks source link

listView disappears if used in a scrollableView with 3 or more views #5

Open arifje opened 9 years ago

arifje commented 9 years ago

I have a scrollableView with 3 views in it. Each view contains a listView. If the scrollableView contains 3 (or probably more) views with the swiperefresh enabled, the listViews disappear / go blank.

It works with a maximum of 2 views in the scrollableView though. Both ListViews keep working properly.

I use the the scrollableTab addon (https://marketplace.appcelerator.com/apps/10618), but I also tested it with a clean/custom scrollableView in a new window.

Screencast of the problem: https://www.youtube.com/watch?v=WoSHP4SygvE&feature=youtu.be

iskugor commented 9 years ago

Hi.

Could you provide test case?

Thanks.

arifje commented 9 years ago

Sure, I created a clean project, you can download it here: http://www.filedropper.com/ptrtestcase

arifje commented 9 years ago

New link: http://content.skoften.net/ptr_testcase.zip

mikefogg commented 9 years ago

@skoften, ah, funny I just noticed this same thing :) At first I thought it was something to do with the cacheSize of scrollableViews but I think (after checking the docs) those are only for iOS. I'll take a look too and see if I can see why this is happening, though I'm curious if it has something to do with http://stackoverflow.com/questions/11736953/viewpager-does-not-redraw-content-remains-turns-blank ...

mikefogg commented 9 years ago

@iskugor @skoften just doing a little research here and here's a thought (not sure if it's warranted or not, I will try modifying the titanium source and testing it if we think this may be it)...

It seems as though ScrollableView is really an Android ViewPager. ViewPager has a method called setOffscreenPageLimit that "Set the number of pages that should be retained to either side of the current page in the view hierarchy in an idle state. Pages beyond this limit will be recreated from the adapter when needed." ( copied from http://developer.android.com/reference/android/support/v4/view/ViewPager.html#setOffscreenPageLimit(int) ).

That value defaults to 1, which would mean that at 2 views it would be fine! And even in your test this seems consistant, if I scroll from the first to second page, all is good! If I scroll from the second to the third, the first page is immediately destroyed (tested by adding logging statements on onMeasure and onLayout).Scrolling back to the first page is when the third page is destroyed.

Checking the titanium code at https://github.com/appcelerator/titanium_mobile/blob/master/android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUIScrollableView.java I see no mention of this method at all... which leads me to believe that it's set to the default of 1 at all times (and the iOS cacheSize property isn't implemented yet).

SO... after all that... it seems like 2 things may be happening or need to happen:

1) For some reason... the view is not being redrawn. It will take some more research to figure out why this may be happening (and/or how it works to begin with).

2) Titanium should implement cacheSize in android using this method :)

mikefogg commented 9 years ago

I think I'm sold on it being this issue:

setOffscreenPageLimit being 1 is causing it to redraw the 1st page after scrolling to the third. For some reason, when that is redrawn, the SwipeRefresh view is not being redrawn along with it.

@iskugor, any thoughts on why this may be happening (or if this is even the reason)?

arifje commented 9 years ago

Hi Mike, nice research!

I think your last conclusion is the correct one; the SwipeRefresh view isn't redrawn with the view in the scrollableView. I tested it with 3 listViews -witthout- the swiperefresh view, which does work and gets redrawn perfectly...so I hope there will be a fix for the module soon :-)

iskugor commented 9 years ago

Hi. Thanks for your info, I was going in that direction, although I'm not yet sure how to fix it.

mikefogg commented 9 years ago

@iskugor Ugh, I know I spent a little time trying to figure out why it wasn't happening in the first place but couldn't figure it out. I will report back if I find something.

mikefogg commented 9 years ago

@skoften @iskugor... well... I have some good news and some bad news :)

The good news is I managed to get it to work! The bad news is that I had to modify Titanium's source code... which is NEVER ideal. This honestly should be implemented by the core team in a more "official" way. This way is kind of a dirty way as I'm sure they would want getters and setters as well instead of JUST when the scrollview is initialized (and some property variables etc.).

Either way, if you need a quick fix, this will work!

// TiUIScrollableView.java#270

@Override
public void processProperties(KrollDict d)
  {

    // Leave the existing code in place
    ...

    //
    // This is the new code!
    //

    if (d.containsKey("cacheSize")) {
        int cacheSize = TiConvert.toInt(d.get("cacheSize"));
        mPager.setOffscreenPageLimit(cacheSize);
    }

    //
    //
    //

    super.processProperties(d);
}

Then you can just add the parameter:

// Alloy
<ScrollableView id="scroller" cacheSize="2">

// Classic
var scrollableView = Ti.UI.createScrollableView({
    height: Ti.UI.FILL,
    width: Ti.UI.FILL,
    views: [view1, view2],
    cacheSize: 2
});

cacheSize can be set to the total number of views minus one (because one is visible) if you want to cache all of them.

This is not an ideal solution, but, it may be better than nothing.

iskugor commented 9 years ago

Well, I would not modify Titanium source code (and setting bigger cache size is just a workaround, not proper solution). :)

The problem should be fixable in Java code, I need to check out how some other modules were implemented, I guess this module misses something (just need to have some free time).

Ivan

jjcantillon commented 9 years ago

Hi Ivan,

Have you had any luck finding the issue causing this or a workaround?

Also experiencing the same issue and I want to avoid editing Titanium source code.

iskugor commented 9 years ago

Unfortunately, no.

PatrickJongmans commented 9 years ago

Another workaround is to remove the RefreshControl on 'scrollend' from the non-active views and add it on the active view. It's not the best option, but it is a workaround that you can use without adjusting the Ti core.

iskugor commented 8 years ago

Could you all check latest 0.6 module version? Thanks