Remove duplicated code regarding user (room member and user profile) screens #3700
Conversation
bmarty
changed the title
|
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
| private val factory: UserProfilePresenter.Factory, | ||
| ) : UserProfilePresenterFactory { | ||
| override fun create(userId: UserId): Presenter<UserProfileState> = factory.create(userId) | ||
| } |
There was a problem hiding this comment.
This double factory level is a bit weird, there is maybe a better way to handle this. Any ideas @ganfra @jmartinesp ?
There was a problem hiding this comment.
I can't see any, sorry... I was going to suggest trying to reuse UserProfilePresenter.Factory but that's not possible since the AssistedFactory needs to return a concrete type.
The best I could get was making UserProfilePresenter.Factory implement UserProfilePresenterFactory, then use it in DefaultUserProfilePresenterFactory like this:
@ContributesBinding(SessionScope::class)
class DefaultUserProfilePresenterFactory @Inject constructor(
private val factory: UserProfilePresenter.Factory,
) : UserProfilePresenterFactory by factoryThere was a problem hiding this comment.
Thanks for looking into it. Let's keep it like this then!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3700 +/- ##
===========================================
- Coverage 82.79% 82.77% -0.02%
===========================================
Files 1752 1751 -1
Lines 41967 41926 -41
Branches 5125 5120 -5
===========================================
- Hits 34746 34705 -41
- Misses 5398 5401 +3
+ Partials 1823 1820 -3 ☔ View full report in Codecov by Sentry. |
| @@ -55,6 +55,7 @@ class UserProfilePresenter @AssistedInject constructor( | |||
| @Composable | |||
| override fun present(): UserProfileState { | |||
| val coroutineScope = rememberCoroutineScope() | |||
| val isCurrentUser = remember { client.isMe(userId) } | |||
| client = client, | ||
| ) | ||
| @Composable | ||
| fun getDmRoomId(): State<RoomId?> { |
There was a problem hiding this comment.
Yes, done in the latest commit: e46503a
| private val factory: UserProfilePresenter.Factory, | ||
| ) : UserProfilePresenterFactory { | ||
| override fun create(userId: UserId): Presenter<UserProfileState> = factory.create(userId) | ||
| } |
There was a problem hiding this comment.
I can't see any, sorry... I was going to suggest trying to reuse UserProfilePresenter.Factory but that's not possible since the AssistedFactory needs to return a concrete type.
The best I could get was making UserProfilePresenter.Factory implement UserProfilePresenterFactory, then use it in DefaultUserProfilePresenterFactory like this:
@ContributesBinding(SessionScope::class)
class DefaultUserProfilePresenterFactory @Inject constructor(
private val factory: UserProfilePresenter.Factory,
) : UserProfilePresenterFactory by factory
bmarty
added
the
Run-Maestro
github-actions
bot
removed
the
Run-Maestro
|
Content
Let
RoomMemberDetailsPresenterdelegates the computation of profile state toUserProfilePresenter, which allow to reduce code duplication.Motivation and context
Less code duplication (production and test).
Preparatory work for #3684
Screenshots / GIFs
Tests
There should be no change.
Tested devices
Checklist