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

When the layoutProvider is changed, it keeps moving to the top. #731

Open
jameszamface opened this issue Jul 17, 2022 · 7 comments
Open

Comments

@jameszamface
Copy link

jameszamface commented Jul 17, 2022

When the layoutprovider is changed, if the size of the content is the same, componentDidUpdate is continuously executed in the ViewRenderer.

If the size of the content is the same when the layoutprovider is changed, it is suspected that the _forceSizeUpdate function is executed while componentDidUpdate is continuously called in the ViewRenderer.

This problem occurs from version 3.2.0-beta.2. (commit 049c297)

@kevin-kp
Copy link
Contributor

I'm seeing similar behavior when using 3.0.5 with a horizontal recyclerlistview. Haven't debugged the library yet, but when the layoutProvider changes, there is a small jump back at the offset when there is a new layoutProvider.
When just adding new items to the recyclerlistview without updating the layoutProvider, I'm not having this issue.

@naqvitalha
Copy link
Collaborator

There's a flag in layout provider call shouldRefreshWithAnchoring. You can set it to false to avoid the issue

@kevin-kp
Copy link
Contributor

Hi @naqvitalha
Thank you for your quick response, that did fix my issue, but as it seemed that I missed a couple of updates, I tried updating the library to the latest version. And I think I'm seeing the same issue as @jameszamface is seeing.
Starting of 3.2.0-beta-2, there seems to be an infinite loop.

I started printing out the index and type in the getLayoutTypeForIndex callback of LayoutProvider and it keeps printing the log statements infinitely.
I tried circumventing the issue, but as jameszamface already mentioned, the issue is probably in the componentDidUpdate that keeps getting called.

@kevin-kp
Copy link
Contributor

Hi @naqvitalha

After some further debugging, it seems that recyclerlistview is in a cycle due to the following line:

this results in the following behavior:

  1. componentDidUpdate is called in RecyclerListView because the state has changed.
  2. _checkAndChangeLayouts is called in componentDidUpdate
  3. The check this._relayoutReqIndex >= 0 results in true which results in a call to _refreshViewability
  4. _refreshViewability calls the method _queueStateRefresh
  5. _queueStateRefresh calls the mentioned line again triggering this cycle again.

I'm not sure if this is the same behavior that @jameszamface is seeing, but if I comment the componentDidUpdate in ViewRenderer, this entire cycle does not occur.

kevin-kp added a commit to kevin-kp/recyclerlistview that referenced this issue Jul 29, 2022
…ndering is used. Otherwise a layout cycle occurs.

- Add extra check before calling overrideLayout in RecyclerListView
naqvitalha added a commit that referenced this issue Aug 3, 2022
…is used. Otherwise a layout cycle occurs. (#734)

- Add extra check before calling overrideLayout in RecyclerListView

Co-authored-by: Kevin Pittevils <kevin.pittevils@androme.be>
Co-authored-by: Talha Naqvi <naqvitalha@gmail.com>
@naqvitalha
Copy link
Collaborator

@kevin-kp Is this also fixed with your change? If yes, we can close this one too

@kevin-kp
Copy link
Contributor

kevin-kp commented Aug 3, 2022

Normally, this issue is also fixed; @jameszamface could you confirm this with version 4.1.3?

@MichalKrakow
Copy link

MichalKrakow commented Nov 23, 2022

shouldRefreshWithAnchoring

This seems required for any deterministic layout provider that updates along with updating dataProvider. But docs and examples lack any mention of this. Anyway:

const getLayoutProvider = (dataProvider) => {
    const lp = new LayoutProvider(
        (index) => {
            return dataProvider.getDataForIndex(index).title.length > 38 ? 2 : 1 // this will get smarter, just for testing
        },
        (type, dim) => {
            dim.width = width
            dim.height = type == 1 ? 225 : 250
        }
    )
    lp.shouldRefreshWithAnchoring = false
    return lp;
}

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

4 participants