alexvasilkov / GestureViews

ImageView and FrameLayout with gestures control and position animation
Apache License 2.0
2.37k stars 384 forks source link

FromRecyclerViewListener incorrect center scroll #167

Closed ParticleCore closed 3 years ago

ParticleCore commented 3 years ago

I noticed that this piece of code which is responsible for centering an item in RecyclerView is missing some more calculations for when the clip to padding is set to false.

https://github.com/alexvasilkov/GestureViews/blob/f16372e55a49e8070369ed52461eb6aa8d4b7a7f/library/src/main/java/com/alexvasilkov/gestures/transition/internal/FromRecyclerViewListener.java#L63-L65

I'd suggest to verify the flag and if it is set to false update the offset with the paddings while taking into account the reverse flag as well because we don't want to vertically offset using the top value if the list is set to scroll from bottom-up, for example.

            int offset = isHorizontal
                    ? (list.getWidth() - list.getPaddingLeft() - list.getPaddingRight()) / 2
                    : (list.getHeight() - list.getPaddingTop() - list.getPaddingBottom()) / 2;

            if (!list.getClipToPadding()) {
                offset += isHorizontal
                    ? manager.getReverseLayout() ? list.getPaddingRight() : list.getPaddingLeft()
                    : manager.getReverseLayout() ? list.getPaddingBottom() : list.getPaddingTop();
            }

I am not sure if stackFromEnd also plays any influence here on top of the reverseLayout, but I'll let you decide how to handle that best.

alexvasilkov commented 3 years ago

I experimented with checking clipToPadding and using padding but didn't found a good option that will work universally well in all cases. I think I'd prefer to keep current implementation as-is.

But, I found another issue that affected the centering: the item view may no be ready and then we won't correctly offset the position by half of its height (or width), the item then will appear lower than it should. The fix is part of v2.8.0, please check it out.

If you still believe we need to take clipToPadding into account then please provide specific example(s), ideally with screenshots demonstrating invalid centring issue. Closing this ticket for now.