wdullaer/SwipeActionAdapter

List item height not adjusted after removing element

MatthewDailey opened this issue · 15 comments

If I remove an element from the list, the element that is now at the element's position inherits the height of the removed element.

This is because SwipeActionAdapter.getView() is too aggressive when it recycles it's backgrounds and does not resize the background to fit the new foreground.

I think the right way to do this is to check if contentView's height has changed and if so re-inflate the background.

A work around is to wrap SwipeActionAdapter with a NonRecyclingAdapter such as:

    private static class NonRecyclingAdapter extends DecoratorAdapter {
        public NonRecyclingAdapter(BaseAdapter baseAdapter) {
            super(baseAdapter);
        }

        @Override
        public View getView(final int position, final View convertView, final ViewGroup parent) {
            return super.getView(position, null, parent);
        }
    }

To follow up, that work around I proposed above gave me jerky animations while the list item is removed. I went with a workaround of making every list element equal height.

This issue is similar to #26

Recycling views is very important to retain good performance. It is no accident that google made this pattern mandatory in their new recyclerview.

The proper solution is probably to inspect the new item in getView and adjust the backgrounds if necessary, either by reinflating or redrawing them with new parameters.
I'll need to experiment with this to see how all of it works together.

I've been trying to reproduce this behaviour for quite some time now in the demo app, but I can't.

When I explicitly set the height in getView everything works out just fine.
For safety I would give your backgrounds a height of match_parent and do a requestLayout() on your output view if you changed its height.

I have set height to match_parent and adding requestLayout() didn't fix it.

I have worked around by keeping my variable text field at 1 line and adding dialog but it would be great if I could get this working so I'm happy to help debug.

Gist with with relevant code: https://gist.github.com/MatthewDailey/8ac2b26fee9a23ac46f9

Video of the error: https://www.youtube.com/watch?v=FBj4lFtOtWA

The behaviour in the video is indeed due to the recycling of the row Views.

I went through your gist but couldn't quite see how you got the first item to be higher than the others.

In any case, if you want different items to have different heights, I think you'll have to set the height in getView using convertView.getLayoutParameters().height.
The idea is that your ListView rows are all very similar, any difference between them (such as the content of the fields or, in this case, the height of the row) you should account for in getView.

Ok thanks! I'll give it a shot in a bit.

Did you resolve your issue?

Nope the work around I had is sufficient for now.

Hello, i'm trying to use your library to delete some items from my listview.
It works perfect appart from the height resizing of the items.
I'm using the same layout in each getView(), but with height set as wrap_content. I need my rows to adapt to the textview content so i can't solve it with by fixing the height.

Could you help me on that ?

I never got this to work reliably in the past.

Basically you'll have to remeasure and invalidate the row view each time you bind new data in it.

It should be doable but I haven't figured out the right incantation for it.

I don't think changed to the actual library will be required, but let me know if you need any help there.

Well, i'm new to android so it took me 5mn to integrate the lib and making it work, and the whole day trying to find a way to remeasure the views after the animation is done.
No success so far. If you could give me a code snippet of what you have in mind you would save my day!

Basically what i do is this :

        @Override
        public void onSwipe(int[] positionList, int[] directionList) {
            for (int i = 0; i < positionList.length; i++) {
                int direction = directionList[i];
                int position = positionList[i];
                if (direction == SwipeDirections.DIRECTION_FAR_LEFT 
                ||  direction == SwipeDirections.DIRECTION_NORMAL_LEFT) {
                        Card card = wrappedAdapter.getItem(position);
                        wrappedAdapter.remove(card);
                        swipeAdapter.notifyDataSetChanged();
                }
            }
        }

even if i try to add getListView().invalidateViews(); there, it doesn't make any difference. Could it be because this is called before the animation start and not when it's actually done?
I guess the problems come from this part of the lib :

                ViewGroup.LayoutParams lp;
                for (PendingDismissData pendingDismiss : mPendingDismisses) {
                    // Reset view presentation
                    pendingDismiss.view.setAlpha(1f);
                    pendingDismiss.view.setTranslationX(0);
                    lp = pendingDismiss.view.getLayoutParams();
                    lp.height = originalHeight;
                    pendingDismiss.view.setLayoutParams(lp);
                }

Do you think there's any way to set the lp.height to the new layout's height taking it's place?

(Offtopic : By the way is it possible to disable the right swipe?)

Oh.. Turns out it was pretty simple :
When the animation is done just set it back to WRAP_CONTENT instead of "originalHeight"
Only fix I could find, though i had to modify the lib.

pendingDismiss.view.setLayoutParams(new AbsListView.LayoutParams(AbsListView.LayoutParams.MATCH_PARENT, AbsListView.LayoutParams.WRAP_CONTENT));

Nice work!

I'll try this out when I have some time and add the change to the library.

Disabling swipes by direction is an often requested feature. I have an idea on how to do it, but need to find the time really.

I did a pull request with those two change (row heights bug + swipe direction enabling settings)
None of the swipe directions are enabled by default - enables on addBackground or addDirection (for use with no background).