naver / android-pull-to-refresh

Pull To Refresh Views for Android! v3.2.3
Apache License 2.0
310 stars 82 forks source link

OnItemClickListener - wrong position parameter #6

Open eunjae-lee opened 10 years ago

eunjae-lee commented 10 years ago

I'm using 3.2.0 version. When I use setOnItemClickListener method on PullToRefreshListView, the callback method delivers wrong position parameter value. position becomes +1. I think it includes the hidden loading indicator view.

ncoolz commented 10 years ago

Ok, I will find the problem and fix it.

ps. If your app using this pull to refresh library is on google market, let me know your app if you do not mind. I want to add it into Apps page. Thanks.

eunjae-lee commented 10 years ago

I'm just trying this library in my sample project. Thanks.

ncoolz commented 10 years ago

I'm trying to resolve the problem. But it is hard to fix positions correct.

PullToRefreshListView adds a loading view on a header of a ListView as following code. After that, the ListView computes a position by including a number of headers.

// Create Loading Views ready for use later
FrameLayout frame = new FrameLayout(getContext());
mHeaderLoadingView = createLoadingLayout(getContext(), Mode.PULL_FROM_START, a);
mHeaderLoadingView.setVisibility(View.GONE);
frame.addView(mHeaderLoadingView, lp);
mRefreshableView.addHeaderView(frame, null, false);

You can see a related issue in the link below. ListView addHeaderView causes position to increase by one?

Of course, this issue can be resolved by overriding methods using a position parameter in ListView. But there are too many methods to be overrided. Also, I cannot defend side effects caused by overriding it.

So I recommend you to follow my suggestions listed below,

  1. Use the ListView.getItemAtPosition(position) instead of ListAdapter.getItem(position)
  2. If you have to get a correct position, create a decorator for OnItemClickListener like below.
class CorrectPositionOnItemClickListener implements OnItemClickListener {
    OnItemClickListener delegate;
    ListView listView;

    public CorrectPositionOnItemClickListener(OnItemClickListener delegate, ListView listView) {
        this.delegate = delegate;
        this.listView = listView;
    }

    @Override
    public void onItemClick(AdapterView<?> adapterView, View view,
                    int position, long id) {
        // Convert a position to a index of a clicked item
        position -= listView.getHeaderViewsCount();
        delegate.onItemClick(adapterView, view, position, id);
    }};
}

...

mPullRefreshListView.setOnItemClickListener(new CorrectPositionOnItemClickListener(new OnItemClickListener() { 
    @Override
    public void onItemClick(AdapterView<?> adapterView, View view,
                    int position, long id) {
        // Do something
    }

}, mPullRefreshListView));

...

NOTE : Don't use 1 and 2 together at once. If you use 1, you don't need to use 2.

I'm so confused to handle positions of items in ListView. It seems there is no consistent policy of that in it.

I'm sorry for not giving you a clearer way. I will give you if I come up with new idea.

peacepassion commented 9 years ago

I use the last parameter long id in public void onItemClick(AdapterView<?> adapterView, View view, int position, long id) as the position. It is OK so far.