airbnb / epoxy

Epoxy is an Android library for building complex screens in a RecyclerView
https://goo.gl/eIK82p
Apache License 2.0
8.46k stars 730 forks source link

StickyHeaderLinearLayoutManager's scrollToPosition function calculates wrong offset for horizontal orientation #1305

Open ardmn opened 1 year ago

ardmn commented 1 year ago

Here you can find a fix of my teammate https://github.com/airbnb/epoxy/pull/1304

I have simple demo project for bug reprodusing https://github.com/ardmn/EpoxyHorizontalStickyHeaderBugSample

It has next view configuration(epoxy 4.6.3 but bug reproduces on epoxy's master branch too ):

    implementation "com.airbnb.android:epoxy:4.6.3"
    kapt "com.airbnb.android:epoxy-processor:4.6.3"

Main's activity layout:

<?xml version="1.0" encoding="utf-8"?>
<!-- NestedScrollView is a trick for RecyclerView's  wrap_content height -->
<androidx.core.widget.NestedScrollView xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
    android:fillViewport="true">

    <LinearLayout
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:orientation="vertical"
        tools:context=".MainActivity">

        <com.airbnb.epoxy.EpoxyRecyclerView
            android:id="@+id/recycler_view_wrong"
            android:layout_width="match_parent"
            android:layout_height="wrap_content"
            tools:listitem="@layout/epoxy_item" />

         <Button
                android:id="@+id/button"
                android:layout_width="wrap_content"
                android:layout_height="wrap_content"
                android:layout_margin="10dp"
                android:text="Scroll to" />

 </LinearLayout>
</androidx.core.widget.NestedScrollView>

And next code in MainActivity:

class MainActivity : AppCompatActivity() {

    private lateinit var recyclerViewWrong: EpoxyRecyclerView
    private var scrollTo = 10

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_main)

        recyclerViewWrong = findViewById<EpoxyRecyclerView>(R.id.recycler_view_wrong).apply {
            layoutManager = StickyHeaderLinearLayoutManager(this@MainActivity,LinearLayoutManager.HORIZONTAL)
            setController(StickyHeaderController())
            requestModelBuild()
        }

        findViewById<Button>(R.id.button).setOnClickListener{
            recyclerViewWrong.scrollToPosition(scrollTo)
            scrollTo++
        }
    }
}



In demo project I added a little more to demonstrate a problem. My main Activity looks like this image:

bug_img_start
App consist of two epoxy's RecyclerViews: upper - with actual behavior and second with expected behavior. Main screen also has a button which click event lead to recyclerView.scrollToPosition() call. First scrollToPosition 10 then each click increments next position to scroll.

Steps to reproduce: 1) Click on button i.e. call to recyclerView.scrollToPosition(10).

Actual result: In upper view I don't see View Holder 10 fully. Expected result: RecyclerView shows View Holder 10 fully as in second lower RecyclerView.

bug_img_demo

Next You can see it in a view bellow:

https://user-images.githubusercontent.com/7188636/178377365-446f84c4-a032-4805-a937-a84bbb363875.mov


The problem in implementation of StickyHeaderLinearLayoutManager.scrollToPositionWithOffset method which doesn't take into account an orientation of LinearLayoutManager and always adds to adjustedOffset a stickyHeader!!.height value but for horizontal orientation should add stickyHeader!!.width.