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

shouldRecycle=false breaks UI when scrolling the elements fast. Issue#296 #299

Merged
merged 3 commits into from
Feb 1, 2022

Conversation

ahamedalthaf
Copy link
Contributor

Commit for the below issue.

  • shouldRecycle=false breaks UI when scrolling the elements fast.

Issue#296

Reference: #296

…#296

shouldRecycle=false breaks UI when scrolling the elements fast.

Issue#296
@RobbieTheWagner
Copy link
Member

@pzuraq I'm not familiar with the internals. Does this work?

@pzuraq
Copy link
Contributor

pzuraq commented Sep 24, 2019

it's been a long time, I'd have to reboot a lot of context and unfortunately I'm stretched a bit too thin to do that at the moment 😕 I'll try to get back to this when I have time

@RobbieTheWagner
Copy link
Member

I don't know enough about VC to review this unfortunately.

@ahamedalthaf
Copy link
Contributor Author

@pzuraq Can you please check this once.
Root-cause:
Some components not appended properly when we scroll fast. So we are getting negative values in offset and assertion error is thrown.
Solution:
Have created an array 'appendComponentPool' and appending the items after components are rendered. Already we are following the same approach in 'prependComponent' method.

flexyford added a commit to flexyford/vertical-collection that referenced this pull request Feb 4, 2020
Should render the appendComponentPool only if ‘shouldRecycle’ is false

We can stop using this fork when the following Issue/PR is resolved:

html-next#296
html-next#299
lan0 pushed a commit to lan0/vertical-collection that referenced this pull request Jun 8, 2020
Should render the appendComponentPool only if ‘shouldRecycle’ is false

We can stop using this fork when the following Issue/PR is resolved:

html-next#296
html-next#299
@knownasilya
Copy link
Contributor

Also see this, get nextSibling errors in the glimmer vm

@RobbieTheWagner
Copy link
Member

Anyone interested in helping maintain this repo? I don't know enough about the internals myself

ro0gr pushed a commit to ro0gr/vertical-collection that referenced this pull request Feb 25, 2021
@pablomaribondo
Copy link

@rwwagner90 @pzuraq @runspired Can you guys give this another try ? this error is still happening

Copy link
Contributor

@runspired runspired left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this change seems good.

I suspect there's a more efficient way to do this in keeping with the original rendering model, as in the original rendering model what this PR brings is exactly how the order of operations would have occurred, but we likely have deviated from that with time.

@RobbieTheWagner RobbieTheWagner merged commit b76c7e6 into html-next:master Feb 1, 2022
@mixonic
Copy link
Contributor

mixonic commented Mar 1, 2022

In 3.0.0-1

mixonic added a commit that referenced this pull request Apr 4, 2022
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

Successfully merging this pull request may close these issues.

7 participants