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

fix: infinite loop on category swipe #168

Merged

Conversation

AnMiZa
Copy link
Contributor

@AnMiZa AnMiZa commented Feb 4, 2024

This PR fixes infinite loop on quick emoji list scroll behavior reported in issue #159

Changes:

  • moved logic for scrolling emoji categories from useEffect hook to separate method
  • passed said method into handleScrollToCategory method in Categories component and into SearchBar component to handle EmojiCategory list scrolling from children

In the previous implementation, after scrolling between EmojiCategories using gesture, when setActiveCategory was called inside onScrollEnd method, useEffect callback with scrollToIndex was fired with activeCategoryIndex that the list was already at. Quick scrolling back and forth with gesture was causing infinite state updates between current and previous activeCategoryIndexes. To fix that behavior while leaving ability to browse EmojiCategories using CategoryItem press scrollToIndex is now called directly inside CategoryItem

@efstathiosntonas
Copy link

@AnMiZa when you tap the search bar, does the list scroll to the search at the end? in my case it won't scroll after applying this pr.

@AnMiZa
Copy link
Contributor Author

AnMiZa commented Feb 6, 2024

Hey, @efstathiosntonas great catch! I was not aware of that functionality so I did not refactor it. I also had this loop bug when I was checking it out on master just now. Thanks for pointing that out, I will fix it tomorrow. I'll convert this PR to draft until then @jakex7.

@AnMiZa AnMiZa marked this pull request as draft February 6, 2024 21:55
@AnMiZa AnMiZa marked this pull request as ready for review February 7, 2024 08:32
@AnMiZa
Copy link
Contributor Author

AnMiZa commented Feb 7, 2024

@efstathiosntonas everything should work as intended now. Check it out.

@efstathiosntonas
Copy link

efstathiosntonas commented Feb 7, 2024

@AnMiZa works as a charm. Thank you for jumping so fast in this and for this PR overall!

Copy link
Member

@danielmark0116 danielmark0116 left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for your contribution! let's land it in the next release 🚀

@danielmark0116 danielmark0116 merged commit 4cbe3e2 into TheWidlarzGroup:master Mar 25, 2024
@focux
Copy link

focux commented Mar 27, 2024

I'm also experiencing this and I'm glad that is already fixed, thank you so much 🙌

When is the next version going to be released?

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.

None yet

4 participants