-
Notifications
You must be signed in to change notification settings - Fork 741
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
Feature/fga/improve room detail start #4065
Conversation
|
||
class LifecycleAwareLazy<out T>( | ||
private val owner: LifecycleOwner, | ||
initializer: () -> T |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not private val?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, just 2 comments. I guess you have check the the emoji completion and the Emoji screen are still working well.
Also do you have some measurement of the improvement to display a room?
return getOrInflate(inflateIfNeeded, roomDetailBinding?.inviteViewStub, this::inviteView) | ||
} | ||
|
||
private inline fun <reified T : View> getOrInflate(inflateIfNeeded: Boolean, stub: ViewStub?, reference: KMutableProperty0<T?>): T? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be extracted to a more generic place
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will see if we reuse this mechanism somewhere else I guess, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but if it is hidden here, there is less chance. Anyway, ok
@@ -43,13 +44,13 @@ import kotlin.math.abs | |||
* TODO: Performances |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this TODO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't really improve performance on this screen, so not really
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
Yes I tested the different screens using emojis, all ok. |
Try to improve performance on RoomDetail screen by: