davideas / FlexibleAdapter

Fast and versatile Adapter for RecyclerView which regroups several features into one library to considerably improve the user experience :-)
Apache License 2.0
3.55k stars 552 forks source link

drag support not correct. (The drag view is hide another view while dragging) #519

Closed quangson91 closed 6 years ago

quangson91 commented 6 years ago

There is a problem when use drag view. I made a simple demo use flexbile adapter to handle swipe/drag. While testing, I found the dragging has a problem. The dragging view is bellowing another. Like this one: drag_error

We have 3 item on the list. The item with text "2" is dragging, but it hides bellow another view.

The sample code: https://github.com/quangson91/demoflexibleadapter

Step to reproduce:

  1. Drag view in 2nd position to Bottom.
  2. Drag view in 2nd position to Top.

Repeat (1) & (2) then we will see the problems (about 5 steps - very easy to reproduce).

I guess in this method eu.davidea.flexibleadapter.helpers.ItemTouchHelperCallback#onChildDraw we pass the front-view: getDefaultUIUtil().onDraw(c, recyclerView, frontView, dX, dY, actionState, isCurrentlyActive);

But in android.support.v7.widget.helper.ItemTouchUIUtilImpl.Api21Impl#onDraw they use the view to find MaxElevation (but we passing wrong view - the front view -> the elevation may be wrong) android.support.v7.widget.helper.ItemTouchUIUtilImpl.Api21Impl#findMaxElevation

davideas commented 6 years ago

@quangson91, we need more investigation, the onDraw in that point is valid only for actionState == ItemTouchHelper.ACTION_STATE_SWIPE.

quangson91 commented 6 years ago

But you can see this method: (From: android.support.v7.widget.helper.ItemTouchUIUtilImpl.Api21Impl)

        @Override
        public void onDraw(Canvas c, RecyclerView recyclerView, View view,
                float dX, float dY, int actionState, boolean isCurrentlyActive) {
            if (isCurrentlyActive) {
                Object originalElevation = view.getTag(R.id.item_touch_helper_previous_elevation);
                if (originalElevation == null) {
                    originalElevation = ViewCompat.getElevation(view);
                    float newElevation = 1f + findMaxElevation(recyclerView, view);
                    ViewCompat.setElevation(view, newElevation);
                    view.setTag(R.id.item_touch_helper_previous_elevation, originalElevation);
                }
            }
            super.onDraw(c, recyclerView, view, dX, dY, actionState, isCurrentlyActive);
        }

        private float findMaxElevation(RecyclerView recyclerView, View itemView) {
            final int childCount = recyclerView.getChildCount();
            float max = 0;
            for (int i = 0; i < childCount; i++) {
                final View child = recyclerView.getChildAt(i);
                if (child == itemView) {
                    continue;
                }
                final float elevation = ViewCompat.getElevation(child);
                if (elevation > max) {
                    max = elevation;
                }
            }
            return max;
        }

When we pass frontView -> method findMaxElevation will be wrong. (I think so). At least this condition will never happen.

if (child == itemView) {
    continue;
}
quangson91 commented 6 years ago

But actually, I can't reproduce this problem when using the sample. I thought the difference is you use handle drag (by set handle view). So I try to implement the sample with handle view. It is better. (I need take more time drag/drop to reproduce the problem).

quangson91 commented 6 years ago

Also, the problem will never happen if we don't set rearLeft/Right & frontView.

davideas commented 6 years ago

Thanks, for the info. I will check when more free.

quangson91 commented 6 years ago

thank you so much.

ghost commented 6 years ago

Any updates on this issue ? I have the same problem :(

davideas commented 6 years ago

In the demoApp, it might be that when start dragging, I activate the view, doing that I also rise the elevation. Therefore it passes over the others.