brnunes / SwipeableRecyclerView

Implementation of an Android CardView list in a RecyclerView that allows dismissing/deleting elements by swiping them to the left or right.
Apache License 2.0
180 stars 43 forks source link

Cursor Recyclerview Item flash after swipe #8

Closed lampione closed 9 years ago

lampione commented 9 years ago

Hi! I'm trying to implement swipeable item for my recycler view. I have a custom Recycler Adapter which is populated from a Cursor.

When I perform the swipe, I call a simple method swipeRemove(int position); which perform the deleting (instead of mItem.remove(position);) at the end of which I do call getLoaderManager().restartLoader(MAIN_LOADER_ID, null, mCallbacks);.

However, the swiped item (after I do swipe) re-appear in a very fast way and depending on my recyclerView.setItemAnimator(); it fade away slowly (with DefaultItemAnimator) or disappear instantly (if null).

I think this is due to my swipeRemove method which is not performed instantly but it makes a call to a database from which I retrieve a cursor, then I filter this cursor and get my column index and finally I perform my delete.

Is there any other way I could approach this? Like maybe trigger the method when animation is not yet at 100%.

Sorry for my english and for my syntax and thank you in advance.

benhylau commented 9 years ago

It seems that the flash is due to that transient state where the item is removed according the UI, but the removal hasn't yet been reflected in the data model, and the two only sync up after the change has been written to persistent storage. The way to prevent the flash is to update the in-memory data model backing the list synchronously in the onDismissedBySwipeLeft() and onDismissedBySwipeRight() and call notifyItemRemoved(position) and notifyDataSetChanged(). This should be ok in your case, if you are reloading from database to sync up your object model whenever the data changes, so you don't need to worry about the UI and persistent data going out of sync for too long right?

My implementation currently has the same flashing issue and I am also thinking to remove the item synchronously in the onDismissedBySwipeLeft(), then schedule some pending removal operation to keep the UI in sync in case the removal operation fails.

I hope that helps.

lampione commented 9 years ago

The way you suggested is not working for me, I'm loading the data from a local stored database with a LoaderManager. If I do not restart the loader, the UI get stuck there with the swiped item still visible. If I call notifyItemRemoved() and notifyDataSetChanged(), nothing happens.

In my opinion, this is not a matter of UI and actual data sync. Rather it could be a matter of listener efficiency, in fact, if you have the time to test this listener with the sample provided by the original author, so with a simple ArrayList (if I'm not mistaken), it works perfectly without any real flashing.

My concern is about the way the listener handle the remove animation, it seems like there is a lack of code in which the swiped view should be somehow cached, then the real .remove() or .delete() or whatever you want get fired and then, only then, the cached view get removed from the UI, so, in my opinion, the delete should be called lets say at 80% of the animation.

I know, this wouldn't be very user-friendly.

I've been also playing all the afternoon with the visibility/height of the child, but without success. I'll let you know if I get somewhere with this.

benhylau commented 9 years ago

I think we are talking about the same thing. The example works because the adapter is backed by an in-memory ArrayList, where removal is synchronous. In your implementation, is there an in-memory object that you can synchronously remove the item from? This will serve as the 'cache' you described, except it's not that view that's cached.

lampione commented 9 years ago

That's not my case I think... there is a sqlite database class, in which I create and get a filtered Cursor, I call this cursor from my custom adapter, in my OnCreateLoader I call my mainLoader class which extends CursorLoader and I call in the loadInBackground as:

@Override
public Cursor loadInBackground() {

    mDatabase = new Database(getContext());
    mDatabase.open();

    Cursor cursor = mDatabase.fetchVisible(filter);
    cursor.setNotificationUri(getContext().getContentResolver(), uri);

    return cursor;
}

The uri is taken from a ContentProvider class.

All I do in my main activity is setting the adapter to get data directly from the cursor and then I do just mRecycler.setAdapter(mAdapter);

Whenever I make an operation to my DB, I do it always directly so calling e.g. mDatabase.deleteItem(position) and immediatly getLoaderManager,restartLoader(loader_id, null, mCallbacks).

So, no. the only way I could remove the data from is making a call to my database, and calling the void I need to call.

lampione commented 9 years ago

I think I've found a cool solution for this issue. Problem is that my Recycler Adapter is structured as CursorRecyclerAdapter extends RecyclerView.Adapter .

There is a nice discussion on this http://stackoverflow.com/questions/15468100/cursoradapter-backed-listview-delete-animation-flickers-on-delete SO question.

Some answer may be the solution, like this guy which provide a cool BaseSwipeableCursorAdapter at this link: https://gist.github.com/q1p/0b95633ab9367fb86785

My problem is that I don't really know how to implement this in my RecyclerAdapter, I'm working on this.

brnunes commented 9 years ago

It should be simple to add a new method such as onBeginDismiss(int position) to the SwipeListener interface, and call it at the beginning of the dismiss animation. However that does not guarantee that the operation on the DB will be done in time, and may cause other inconsistencies.

If I know that the list will be of a reasonable size at most, I usually load everything to memory and synchronize with the database manually. If the number of elements can be very large, swapping cursors sounds like a good solution.

Let us know how you made it work.

benhylau commented 9 years ago

@lampione That is exactly the problem I was trying to explain. I guess I must have worded it extremely poorly :-1:

It seems that the flash is due to that transient state where the item is removed according the UI, but the removal hasn't yet been reflected in the data model, and the two only sync up after the change has been written to persistent storage.

I agree with @brnunes that starting the animation earlier is not a solution. There seem to be two reasonable solutions to the flash:

  1. Back the adapter with an in-memory list, which you remove synchronously in onDismissedBySwipe...(), and sync with the database once the removal operation completes. This is what @brnunes suggested and is also what I am doing.
  2. Use a dummy empty view in the gist that @lampione posted.

The former solution requires no change in the library, and should be sufficient unless you have a REALLY long list.

lampione commented 9 years ago

@benhylau It was not poorly worder, it's my not-so-good english :smile: . After a little struggling, I think I've finally found the simplest solution possible, plus the less expansive.

I took the CursorWrapper from this SO question previously linked provided by both Federico Ponzi and Emanuel Moecklin.

What I do now: In my onDismissedBySwipeLeft I create a SwipeToDeleteCursorWrapper cursorWrapper and pass it a cursor mAdapter.getCursor(); and the item position.

The wrapper tricks the user by hiding the effective row, then I set this wrapper to my view: mRecyclerAdapter.swapCursor(cursorWrapper);.

Finally, in my onDismissedBySwipeLeft I make a call to an AsyncTask class which does the real removing, and then I simply call getLoaderManager.restartLoader(); from onPostExecute.

The original cursor is setted automatically now, without any flickering, Plus my UI isn't slowed down, plus this works even if I swipe multiple item at once, so pretty user friendly.

Let me know if this sounds nice to you, I'm honestly happy with this workaround.