From 574e6199fee76f521f307b2da8e6c131b173da8c Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 15 Sep 2023 18:20:44 +0200 Subject: [PATCH] Cleanup and compact code. Also prefer usage of DayNightPreview. --- .../editprofile/EditUserProfilePresenter.kt | 6 +- .../user/editprofile/EditUserProfileView.kt | 36 +++---- .../EditUserProfilePresenterTest.kt | 95 +++---------------- .../impl/edit/RoomDetailsEditView.kt | 2 +- .../components/LabelledOutlinedTextField.kt | 17 +--- libraries/matrix/impl/build.gradle.kts | 2 +- 6 files changed, 36 insertions(+), 122 deletions(-) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenter.kt index edbb2467f2..5d3821931e 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenter.kt @@ -95,8 +95,8 @@ class EditUserProfilePresenter @AssistedInject constructor( } val canSave = remember(userDisplayName, userAvatarUri) { - val hasProfileChanged = hasDisplayNameChanged(userDisplayName, matrixUser) - || hasAvatarUrlChanged(userAvatarUri, matrixUser) + val hasProfileChanged = hasDisplayNameChanged(userDisplayName, matrixUser) || + hasAvatarUrlChanged(userAvatarUri, matrixUser) !userDisplayName.isNullOrBlank() && hasProfileChanged } @@ -139,6 +139,6 @@ class EditUserProfilePresenter @AssistedInject constructor( } else { matrixClient.removeAvatar().getOrThrow() } - }.onFailure { it.printStackTrace() } + }.onFailure { Timber.e(it, "Unable to update avatar") } } } diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfileView.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfileView.kt index 02423470a9..5b921c047c 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfileView.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfileView.kt @@ -40,7 +40,6 @@ import androidx.compose.ui.input.pointer.pointerInput import androidx.compose.ui.platform.LocalFocusManager import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextAlign -import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.dp import io.element.android.features.preferences.impl.R @@ -50,8 +49,8 @@ import io.element.android.libraries.designsystem.components.ProgressDialog import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.designsystem.components.button.BackButton import io.element.android.libraries.designsystem.components.dialogs.ErrorDialog -import io.element.android.libraries.designsystem.preview.ElementPreviewDark -import io.element.android.libraries.designsystem.preview.ElementPreviewLight +import io.element.android.libraries.designsystem.preview.DayNightPreviews +import io.element.android.libraries.designsystem.preview.ElementPreview import io.element.android.libraries.designsystem.theme.aliasScreenTitle import io.element.android.libraries.designsystem.theme.components.Scaffold import io.element.android.libraries.designsystem.theme.components.Text @@ -135,7 +134,6 @@ fun EditUserProfileView( ) } Spacer(modifier = Modifier.height(40.dp)) - LabelledOutlinedTextField( label = stringResource(R.string.screen_edit_profile_display_name), value = state.displayName, @@ -155,7 +153,6 @@ fun EditUserProfileView( is Async.Loading -> { ProgressDialog(text = stringResource(R.string.screen_edit_profile_updating_details)) } - is Async.Failure -> { ErrorDialog( title = stringResource(R.string.screen_edit_profile_error_title), @@ -163,40 +160,31 @@ fun EditUserProfileView( onDismiss = { state.eventSink(EditUserProfileEvents.CancelSaveChanges) }, ) } - is Async.Success -> { LaunchedEffect(state.saveAction) { onProfileEdited() } } - else -> Unit } } } private fun Modifier.clearFocusOnTap(focusManager: FocusManager): Modifier = - this.pointerInput(Unit) { + pointerInput(Unit) { detectTapGestures(onTap = { focusManager.clearFocus() }) } -@Preview +@DayNightPreviews @Composable -fun EditUserProfileViewLightPreview(@PreviewParameter(EditUserProfileStateProvider::class) state: EditUserProfileState) = - ElementPreviewLight { ContentToPreview(state) } +internal fun EditUserProfileViewPreview(@PreviewParameter(EditUserProfileStateProvider::class) state: EditUserProfileState) = + ElementPreview { + EditUserProfileView( + onBackPressed = {}, + onProfileEdited = {}, + state = state, + ) + } -@Preview -@Composable -fun EditUserProfileViewDarkPreview(@PreviewParameter(EditUserProfileStateProvider::class) state: EditUserProfileState) = - ElementPreviewDark { ContentToPreview(state) } - -@Composable -private fun ContentToPreview(state: EditUserProfileState) { - EditUserProfileView( - onBackPressed = {}, - onProfileEdited = {}, - state = state, - ) -} diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenterTest.kt index c377265689..d3580a82b1 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenterTest.kt @@ -74,7 +74,7 @@ class EditUserProfilePresenterTest { unmockkAll() } - private fun anEditUserProfilePresenter( + private fun createEditUserProfilePresenter( matrixClient: MatrixClient = FakeMatrixClient(), matrixUser: MatrixUser = aMatrixUser(), ): EditUserProfilePresenter { @@ -89,8 +89,7 @@ class EditUserProfilePresenterTest { @Test fun `present - initial state is created from room info`() = runTest { val user = aMatrixUser(avatarUrl = AN_AVATAR_URL) - val presenter = anEditUserProfilePresenter(matrixUser = user) - + val presenter = createEditUserProfilePresenter(matrixUser = user) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -111,27 +110,23 @@ class EditUserProfilePresenterTest { @Test fun `present - updates state in response to changes`() = runTest { val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) - val presenter = anEditUserProfilePresenter(matrixUser = user) - + val presenter = createEditUserProfilePresenter(matrixUser = user) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() assertThat(initialState.displayName).isEqualTo("Name") assertThat(initialState.userAvatarUrl).isEqualTo(userAvatarUri) - initialState.eventSink(EditUserProfileEvents.UpdateDisplayName("Name II")) awaitItem().apply { assertThat(displayName).isEqualTo("Name II") assertThat(userAvatarUrl).isEqualTo(userAvatarUri) } - initialState.eventSink(EditUserProfileEvents.UpdateDisplayName("Name III")) awaitItem().apply { assertThat(displayName).isEqualTo("Name III") assertThat(userAvatarUrl).isEqualTo(userAvatarUri) } - initialState.eventSink(EditUserProfileEvents.HandleAvatarAction(AvatarAction.Remove)) awaitItem().apply { assertThat(displayName).isEqualTo("Name III") @@ -143,17 +138,13 @@ class EditUserProfilePresenterTest { @Test fun `present - obtains avatar uris from gallery`() = runTest { val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) - fakePickerProvider.givenResult(anotherAvatarUri) - - val presenter = anEditUserProfilePresenter(matrixUser = user) - + val presenter = createEditUserProfilePresenter(matrixUser = user) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() assertThat(initialState.userAvatarUrl).isEqualTo(userAvatarUri) - initialState.eventSink(EditUserProfileEvents.HandleAvatarAction(AvatarAction.ChoosePhoto)) awaitItem().apply { assertThat(userAvatarUrl).isEqualTo(anotherAvatarUri) @@ -164,17 +155,13 @@ class EditUserProfilePresenterTest { @Test fun `present - obtains avatar uris from camera`() = runTest { val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) - fakePickerProvider.givenResult(anotherAvatarUri) - - val presenter = anEditUserProfilePresenter(matrixUser = user) - + val presenter = createEditUserProfilePresenter(matrixUser = user) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() assertThat(initialState.userAvatarUrl).isEqualTo(userAvatarUri) - initialState.eventSink(EditUserProfileEvents.HandleAvatarAction(AvatarAction.TakePhoto)) awaitItem().apply { assertThat(userAvatarUrl).isEqualTo(anotherAvatarUri) @@ -185,35 +172,28 @@ class EditUserProfilePresenterTest { @Test fun `present - updates save button state`() = runTest { val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) - fakePickerProvider.givenResult(userAvatarUri) - - val presenter = anEditUserProfilePresenter(matrixUser = user) - + val presenter = createEditUserProfilePresenter(matrixUser = user) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() assertThat(initialState.saveButtonEnabled).isEqualTo(false) - // Once a change is made, the save button is enabled initialState.eventSink(EditUserProfileEvents.UpdateDisplayName("Name II")) awaitItem().apply { assertThat(saveButtonEnabled).isEqualTo(true) } - // If it's reverted then the save disables again initialState.eventSink(EditUserProfileEvents.UpdateDisplayName("Name")) awaitItem().apply { assertThat(saveButtonEnabled).isEqualTo(false) } - // Make a change... initialState.eventSink(EditUserProfileEvents.HandleAvatarAction(AvatarAction.Remove)) awaitItem().apply { assertThat(saveButtonEnabled).isEqualTo(true) } - // Revert it... initialState.eventSink(EditUserProfileEvents.HandleAvatarAction(AvatarAction.ChoosePhoto)) awaitItem().apply { @@ -225,35 +205,28 @@ class EditUserProfilePresenterTest { @Test fun `present - updates save button state when initial values are null`() = runTest { val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = null) - fakePickerProvider.givenResult(userAvatarUri) - - val presenter = anEditUserProfilePresenter(matrixUser = user) - + val presenter = createEditUserProfilePresenter(matrixUser = user) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() assertThat(initialState.saveButtonEnabled).isEqualTo(false) - // Once a change is made, the save button is enabled initialState.eventSink(EditUserProfileEvents.UpdateDisplayName("Name II")) awaitItem().apply { assertThat(saveButtonEnabled).isEqualTo(true) } - // If it's reverted then the save disables again initialState.eventSink(EditUserProfileEvents.UpdateDisplayName("fallback")) awaitItem().apply { assertThat(saveButtonEnabled).isEqualTo(false) } - // Make a change... initialState.eventSink(EditUserProfileEvents.HandleAvatarAction(AvatarAction.ChoosePhoto)) awaitItem().apply { assertThat(saveButtonEnabled).isEqualTo(true) } - // Revert it... initialState.eventSink(EditUserProfileEvents.HandleAvatarAction(AvatarAction.Remove)) awaitItem().apply { @@ -266,26 +239,21 @@ class EditUserProfilePresenterTest { fun `present - save changes room details if different`() = runTest { val matrixClient = FakeMatrixClient() val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) - - val presenter = anEditUserProfilePresenter( + val presenter = createEditUserProfilePresenter( matrixClient = matrixClient, matrixUser = user ) - moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() - initialState.eventSink(EditUserProfileEvents.UpdateDisplayName("New name")) initialState.eventSink(EditUserProfileEvents.HandleAvatarAction(AvatarAction.Remove)) initialState.eventSink(EditUserProfileEvents.Save) skipItems(5) - assertThat(matrixClient.setDisplayNameCalled).isTrue() assertThat(matrixClient.removeAvatarCalled).isTrue() assertThat(matrixClient.uploadAvatarCalled).isFalse() - cancelAndIgnoreRemainingEvents() } } @@ -294,24 +262,19 @@ class EditUserProfilePresenterTest { fun `present - save doesn't change room details if they're the same trimmed`() = runTest { val matrixClient = FakeMatrixClient() val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) - - val presenter = anEditUserProfilePresenter( + val presenter = createEditUserProfilePresenter( matrixClient = matrixClient, matrixUser = user ) - moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() - initialState.eventSink(EditUserProfileEvents.UpdateDisplayName(" Name ")) initialState.eventSink(EditUserProfileEvents.Save) - assertThat(matrixClient.setDisplayNameCalled).isTrue() assertThat(matrixClient.uploadAvatarCalled).isFalse() assertThat(matrixClient.removeAvatarCalled).isFalse() - cancelAndIgnoreRemainingEvents() } } @@ -320,24 +283,19 @@ class EditUserProfilePresenterTest { fun `present - save doesn't change name if it's now empty`() = runTest { val matrixClient = FakeMatrixClient() val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) - - val presenter = anEditUserProfilePresenter( + val presenter = createEditUserProfilePresenter( matrixClient = matrixClient, matrixUser = user ) - moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() - initialState.eventSink(EditUserProfileEvents.UpdateDisplayName("")) initialState.eventSink(EditUserProfileEvents.Save) - assertThat(matrixClient.setDisplayNameCalled).isFalse() assertThat(matrixClient.uploadAvatarCalled).isFalse() assertThat(matrixClient.removeAvatarCalled).isFalse() - cancelAndIgnoreRemainingEvents() } } @@ -346,23 +304,18 @@ class EditUserProfilePresenterTest { fun `present - save processes and sets avatar when processor returns successfully`() = runTest { val matrixClient = FakeMatrixClient() val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) - givenPickerReturnsFile() - - val presenter = anEditUserProfilePresenter( + val presenter = createEditUserProfilePresenter( matrixClient = matrixClient, matrixUser = user ) - moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() - initialState.eventSink(EditUserProfileEvents.HandleAvatarAction(AvatarAction.ChoosePhoto)) initialState.eventSink(EditUserProfileEvents.Save) skipItems(2) - assertThat(matrixClient.uploadAvatarCalled).isTrue() } } @@ -371,26 +324,20 @@ class EditUserProfilePresenterTest { fun `present - save does not set avatar data if processor fails`() = runTest { val matrixClient = FakeMatrixClient() val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) - - val presenter = anEditUserProfilePresenter( + val presenter = createEditUserProfilePresenter( matrixClient = matrixClient, matrixUser = user ) - fakePickerProvider.givenResult(anotherAvatarUri) fakeMediaPreProcessor.givenResult(Result.failure(Throwable("Oh no"))) - moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() - initialState.eventSink(EditUserProfileEvents.HandleAvatarAction(AvatarAction.ChoosePhoto)) initialState.eventSink(EditUserProfileEvents.Save) skipItems(2) - assertThat(matrixClient.uploadAvatarCalled).isFalse() - assertThat(awaitItem().saveAction).isInstanceOf(Async.Failure::class.java) } } @@ -401,7 +348,6 @@ class EditUserProfilePresenterTest { val matrixClient = FakeMatrixClient().apply { givenSetDisplayNameResult(Result.failure(Throwable("!"))) } - saveAndAssertFailure(user, matrixClient, EditUserProfileEvents.UpdateDisplayName("New name")) } @@ -411,61 +357,49 @@ class EditUserProfilePresenterTest { val matrixClient = FakeMatrixClient().apply { givenRemoveAvatarResult(Result.failure(Throwable("!"))) } - saveAndAssertFailure(user, matrixClient, EditUserProfileEvents.HandleAvatarAction(AvatarAction.Remove)) } @Test fun `present - sets save action to failure if setting avatar fails`() = runTest { givenPickerReturnsFile() - val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) val matrixClient = FakeMatrixClient().apply { givenUploadAvatarResult(Result.failure(Throwable("!"))) } - saveAndAssertFailure(user, matrixClient, EditUserProfileEvents.HandleAvatarAction(AvatarAction.ChoosePhoto)) } @Test fun `present - CancelSaveChanges resets save action state`() = runTest { givenPickerReturnsFile() - val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) val matrixClient = FakeMatrixClient().apply { givenSetDisplayNameResult(Result.failure(Throwable("!"))) } - - val presenter = anEditUserProfilePresenter(matrixUser = user) - + val presenter = createEditUserProfilePresenter(matrixUser = user) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() - initialState.eventSink(EditUserProfileEvents.UpdateDisplayName("foo")) initialState.eventSink(EditUserProfileEvents.Save) skipItems(2) - assertThat(awaitItem().saveAction).isInstanceOf(Async.Failure::class.java) - initialState.eventSink(EditUserProfileEvents.CancelSaveChanges) assertThat(awaitItem().saveAction).isInstanceOf(Async.Uninitialized::class.java) } } private suspend fun saveAndAssertFailure(matrixUser: MatrixUser, matrixClient: MatrixClient, event: EditUserProfileEvents) { - val presenter = anEditUserProfilePresenter(matrixUser = matrixUser, matrixClient = matrixClient) - + val presenter = createEditUserProfilePresenter(matrixUser = matrixUser, matrixClient = matrixClient) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() - initialState.eventSink(event) initialState.eventSink(EditUserProfileEvents.Save) skipItems(1) - assertThat(awaitItem().saveAction).isInstanceOf(Async.Loading::class.java) assertThat(awaitItem().saveAction).isInstanceOf(Async.Failure::class.java) } @@ -476,7 +410,6 @@ class EditUserProfilePresenterTest { val processedFile: File = mockk { every { readBytes() } returns fakeFileContents } - fakePickerProvider.givenResult(anotherAvatarUri) fakeMediaPreProcessor.givenResult( Result.success( diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditView.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditView.kt index 4553161d73..ac9853a387 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditView.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditView.kt @@ -223,7 +223,7 @@ private fun LabelledReadOnlyField( } private fun Modifier.clearFocusOnTap(focusManager: FocusManager): Modifier = - this.pointerInput(Unit) { + pointerInput(Unit) { detectTapGestures(onTap = { focusManager.clearFocus() }) diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/LabelledOutlinedTextField.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/LabelledOutlinedTextField.kt index a273acf25c..4939cac38d 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/LabelledOutlinedTextField.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/LabelledOutlinedTextField.kt @@ -24,10 +24,9 @@ import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier -import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp -import io.element.android.libraries.designsystem.preview.ElementPreviewDark -import io.element.android.libraries.designsystem.preview.ElementPreviewLight +import io.element.android.libraries.designsystem.preview.DayNightPreviews +import io.element.android.libraries.designsystem.preview.ElementPreview import io.element.android.libraries.designsystem.theme.components.OutlinedTextField import io.element.android.libraries.designsystem.theme.components.Text import io.element.android.libraries.theme.ElementTheme @@ -66,16 +65,9 @@ fun LabelledOutlinedTextField( } } -@Preview +@DayNightPreviews @Composable -internal fun LabelledOutlinedTextFieldLightPreview() = ElementPreviewLight { ContentToPreview() } - -@Preview -@Composable -internal fun LabelledOutlinedTextFieldDarkPreview() = ElementPreviewDark { ContentToPreview() } - -@Composable -private fun ContentToPreview() { +internal fun LabelledOutlinedTextFieldPreview() = ElementPreview { Column { LabelledOutlinedTextField( label = "Room name", @@ -89,3 +81,4 @@ private fun ContentToPreview() { ) } } + diff --git a/libraries/matrix/impl/build.gradle.kts b/libraries/matrix/impl/build.gradle.kts index 88cc929e79..a2b616f989 100644 --- a/libraries/matrix/impl/build.gradle.kts +++ b/libraries/matrix/impl/build.gradle.kts @@ -29,7 +29,7 @@ anvil { } dependencies { -// implementation(projects.libraries.rustsdk) + // implementation(projects.libraries.rustsdk) implementation(libs.matrix.sdk) implementation(projects.libraries.di) implementation(projects.libraries.androidutils)