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

Return SizeOnlyPlaceholder when the placeholder queue is empty #1733

Merged

Conversation

jingwei99
Copy link

@jingwei99 jingwei99 commented Dec 7, 2023

Return a SizeOnlyPlaceholder when the placeholder queue is empty, which could happen when:

(placeholderPoolSize * height of a single placeholder) < height of the list in view

Follow up:

  • implement createPlaceholder for iOS and Android compose emoji search

issues:

Comment on lines -57 to -63

val visibleItemCount = localLastVisibleItemIndex - localFirstVisibleItemIndex
val proposedPlaceholderPoolSize = visibleItemCount + visibleItemCount / 2
// We only ever want to increase the pool size.
if (placeholderPoolSize < proposedPlaceholderPoolSize) {
placeholderPoolSize = proposedPlaceholderPoolSize
}
Copy link
Author

Choose a reason for hiding this comment

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

no longer needed

Comment on lines -108 to -114

val visibleItemCount = localLastVisibleItemIndex - localFirstVisibleItemIndex
val proposedPlaceholderPoolSize = visibleItemCount + visibleItemCount / 2
// We only ever want to increase the pool size.
if (placeholderPoolSize < proposedPlaceholderPoolSize) {
placeholderPoolSize = proposedPlaceholderPoolSize
}
Copy link
Author

Choose a reason for hiding this comment

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

no longer needed

Comment on lines +75 to +81
override fun createPlaceholder(original: View): View {
return object : View(value.context) {
override fun onMeasure(widthMeasureSpec: Int, heightMeasureSpec: Int) {
setMeasuredDimension(original.width, original.height)
}
}
}
Copy link
Author

Choose a reason for hiding this comment

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

needs to be implemented on emoji search compose and iOS

Comment on lines +107 to +109
override fun uncaughtException(exception: Throwable) {
Log.e("Treehouse", "uncaughtException", exception)
}
Copy link
Author

Choose a reason for hiding this comment

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

not directly related, added it to make debugging easier

@jingwei99 jingwei99 requested review from swankjesse and veyndan and removed request for swankjesse December 7, 2023 22:00
Copy link
Contributor

@veyndan veyndan left a comment

Choose a reason for hiding this comment

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

I noticed that you referenced #1571. Are you implying that this PR would fix the issue?

@jingwei99
Copy link
Author

jingwei99 commented Dec 8, 2023

I noticed that you referenced #1571. Are you implying that this PR would fix the issue?

This PR would return a SizeOnlyPlaceholder when there's no place holder in the queue anymore. #1571 is referenced here because during slack discussion, it's mentioned that this crash (root of the crash) is related to #1571

@jingwei99 jingwei99 merged commit 63ddcf0 into trunk Dec 8, 2023
8 checks passed
@jingwei99 jingwei99 deleted the jingwei.return_SizeOnlyPlaceholder_for_empty_queue branch December 8, 2023 20:19
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.

3 participants