Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OnItemClickListener & OnItemLongClickListener #22

Closed
jonathan-caryl opened this issue Mar 19, 2013 · 6 comments
Closed

OnItemClickListener & OnItemLongClickListener #22

jonathan-caryl opened this issue Mar 19, 2013 · 6 comments

Comments

@jonathan-caryl
Copy link

Our ListView uses OnItemClickListener to switch to a detail view, and OnItemLongClickListener to reveal a button in the ListView item. I've switched to a PullToRefreshListView, and I find I still get these listeners firing when I'd expect the view to be consuming the touch events as part of the pull-to-refresh functionality. E.g. press and slowly move down, we start to reveal the 'pull to refresh' row at the top of the list, but we then get our OnItemLongClickListener called. Or pull down swiftly for 1/2 a row, again we see the 'pull to refresh' start to be displayed, but if you then release the touch we get the OnItemClickListener called.

This feels wrong, I'd expect to have these Listeners called only if we're not doing any pull to refresh action. Is this how it's meant to work? Or is there some way I can configure it to behave the way we need?

@erikwt
Copy link
Owner

erikwt commented Mar 21, 2013

You're right, this is not the way it's supposed to work.

What revision are you using? The latest 'master' or are you on the 'development' branch?

@jonathan-caryl
Copy link
Author

I'm using 'master'.

I've not completely figured out how the code works, but I see in PTROnItemClickListener and PTROnItemLongClickListener it checks that the state is PULL_TO_REFRESH before it fires the listener, but if you're just at the point of dragging the list down to reveal the 'pull to refresh' bit at the top, then that's what the state will be. Perhaps there needs to be another state, which indicates the user's dragging, but we're still showing 'pull to refresh'?

@vibu
Copy link
Contributor

vibu commented Jun 3, 2013

I also had the problem with Click and LongClick listeners with PullToRefreshListView. I have made some modifications and now it works fine. Hope it helps. Might also be helpful for issue #25 :

file: PullToRefreshListView.java
...

float mScrollStartY;

    @Override
    public boolean onTouchEvent(MotionEvent event){
        if(lockScrollWhileRefreshing
                && (state == State.REFRESHING || getAnimation() != null && !getAnimation().hasEnded())){
            return true;
        }

        switch(event.getAction()){
            case MotionEvent.ACTION_DOWN:
                if(getFirstVisiblePosition() == 0) {
                    previousY = event.getY();                   
                }
                else previousY = -1;
                //+++
                mScrollStartY = event.getY();
                //+++
                break;

            case MotionEvent.ACTION_UP:
                if(previousY != -1 && (state == State.RELEASE_TO_REFRESH || getFirstVisiblePosition() == 0)){
                    switch(state){
                        case RELEASE_TO_REFRESH:
                            setState(State.REFRESHING);
                            bounceBackHeader();

                            break;

                        case PULL_TO_REFRESH:
                            resetHeader();
                            break;
                    }
                }
                break;

            case MotionEvent.ACTION_MOVE:
                if(previousY != -1 /*+++*/&& Math.abs(mScrollStartY-event.getY()) > 5 /*+++*/){
                    float y = event.getY();
                    float diff = y - previousY;
                    if(diff > 0) diff /= PULL_RESISTANCE;
                    previousY = y;

                    int newHeaderPadding = Math.max(Math.round(headerPadding + diff), -header.getHeight());

                    if(newHeaderPadding != headerPadding && state != State.REFRESHING){
                        setHeaderPadding(newHeaderPadding);

                        if(state == State.PULL_TO_REFRESH && headerPadding > 0){
                            setState(State.RELEASE_TO_REFRESH);

                            image.clearAnimation();
                            image.startAnimation(flipAnimation);
                        }else if(state == State.RELEASE_TO_REFRESH && headerPadding < 0){
                            setState(State.PULL_TO_REFRESH);

                            image.clearAnimation();
                            image.startAnimation(reverseFlipAnimation);
                        }    
                        // +++ return true;
                    }         
                    //+++                               
                    return super.onTouchEvent(event);
                    //+++
                }

                break;
        }

        return super.onTouchEvent(event);
    }

...

private class PTROnItemLongClickListener implements OnItemLongClickListener{

        @Override
        public boolean onItemLongClick(AdapterView<?> adapterView, View view, int position, long id){
            hasResetHeader = false;

            //+++if(onItemLongClickListener != null && state == State.PULL_TO_REFRESH){
                //+++ Passing up onItemLongClick. Correct position with the number of header views
                return onItemLongClickListener.onItemLongClick(adapterView, view, position - getHeaderViewsCount(), id);
            //+++}

            //+++return false;
        }
    }

@erikwt
Copy link
Owner

erikwt commented Jun 3, 2013

Hi @vibu,

That looks promising. Would you make it a commit & pull request so I can test it out and possible merge it in?

Thanks in advance,
Erik

@lechon
Copy link

lechon commented Jun 5, 2013

This code seems to work fine for me, I'm happy using it.
Thanks.
Jose M¬

@erikwt
Copy link
Owner

erikwt commented Jun 6, 2013

I tested the code in development and master, merged in both. Thanks @vibu!

@erikwt erikwt closed this as completed Jun 6, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants