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

List item height not adjusted after removing element #27

Closed
MatthewDailey opened this issue Jul 23, 2015 · 15 comments
Closed

List item height not adjusted after removing element #27

MatthewDailey opened this issue Jul 23, 2015 · 15 comments

Comments

@MatthewDailey
Copy link

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);
        }
    }
@MatthewDailey
Copy link
Author

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.

@wdullaer
Copy link
Owner

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.

@wdullaer
Copy link
Owner

wdullaer commented Aug 1, 2015

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.

@MatthewDailey
Copy link
Author

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

@wdullaer
Copy link
Owner

wdullaer commented Aug 1, 2015

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.

@MatthewDailey
Copy link
Author

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

@wdullaer
Copy link
Owner

Did you resolve your issue?

@MatthewDailey
Copy link
Author

Nope the work around I had is sufficient for now.

@graphee-gabriel
Copy link
Contributor

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 ?

@wdullaer
Copy link
Owner

wdullaer commented Nov 6, 2015

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.

@graphee-gabriel
Copy link
Contributor

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!

@graphee-gabriel
Copy link
Contributor

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?)

@graphee-gabriel
Copy link
Contributor

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));

@wdullaer
Copy link
Owner

wdullaer commented Nov 9, 2015

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.

@graphee-gabriel
Copy link
Contributor

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).

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

3 participants