From bb9b3b77c7ed4c847e407e2083d81170a3077e87 Mon Sep 17 00:00:00 2001 From: Vincent TE Date: Wed, 30 Oct 2024 12:38:25 +0100 Subject: [PATCH] chore: Code review --- .../ui/components/SwissTransferTopAppBar.kt | 26 ++++--------------- .../ui/screen/newtransfer/FilesSize.kt | 24 +++++++---------- .../screen/newtransfer/NewTransferNavHost.kt | 5 ++-- .../importfiles/FilesDetailsScreen.kt | 13 +++++----- .../importfiles/FilesDetailsViewModel.kt | 10 +++++-- .../ui/utils/HumanReadableSizeUtils.kt | 2 +- 6 files changed, 33 insertions(+), 47 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/components/SwissTransferTopAppBar.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/components/SwissTransferTopAppBar.kt index d671ba617..52c8aa736 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/components/SwissTransferTopAppBar.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/components/SwissTransferTopAppBar.kt @@ -33,42 +33,26 @@ import com.infomaniak.swisstransfer.ui.utils.PreviewAllWindows @Composable fun SwissTransferTopAppBar( - title: String? = null, @StringRes titleRes: Int, navigationMenu: TopAppBarButton? = null, vararg actionMenus: TopAppBarButton, ) { - if (title != null) { - SwissTransferTopAppBar(title = title, navigationMenu = navigationMenu, actionMenus = actionMenus) - } else { - SwissTransferTopAppBar(titleRes = titleRes, navigationMenu = navigationMenu, actionMenus = actionMenus) - } + SwissTransferTopAppBarContent(title = stringResource(titleRes), navigationMenu, *actionMenus) } @Composable -@OptIn(ExperimentalMaterial3Api::class) fun SwissTransferTopAppBar( title: String, navigationMenu: TopAppBarButton? = null, vararg actionMenus: TopAppBarButton, ) { - TopAppBar( - colors = TopAppBarDefaults.topAppBarColors( - containerColor = SwissTransferTheme.materialColors.tertiary, - titleContentColor = SwissTransferTheme.colors.toolbarTextColor, - actionIconContentColor = SwissTransferTheme.colors.toolbarIconColor, - navigationIconContentColor = SwissTransferTheme.colors.toolbarIconColor, - ), - title = { Text(text = title, style = SwissTransferTheme.typography.h2) }, - navigationIcon = { navigationMenu?.let { MenuButton(navigationMenu) } }, - actions = { actionMenus.forEach { actionMenu -> MenuButton(actionMenu) } }, - ) + SwissTransferTopAppBarContent(title, navigationMenu, *actionMenus) } @Composable @OptIn(ExperimentalMaterial3Api::class) -fun SwissTransferTopAppBar( - @StringRes titleRes: Int, +private fun SwissTransferTopAppBarContent( + title: String, navigationMenu: TopAppBarButton? = null, vararg actionMenus: TopAppBarButton, ) { @@ -79,7 +63,7 @@ fun SwissTransferTopAppBar( actionIconContentColor = SwissTransferTheme.colors.toolbarIconColor, navigationIconContentColor = SwissTransferTheme.colors.toolbarIconColor, ), - title = { Text(text = stringResource(id = titleRes), style = SwissTransferTheme.typography.h2) }, + title = { Text(text = title, style = SwissTransferTheme.typography.h2) }, navigationIcon = { navigationMenu?.let { MenuButton(navigationMenu) } }, actions = { actionMenus.forEach { actionMenu -> MenuButton(actionMenu) } }, ) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/FilesSize.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/FilesSize.kt index f52d32b40..9f99dbff0 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/FilesSize.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/FilesSize.kt @@ -35,22 +35,19 @@ import com.infomaniak.swisstransfer.ui.previewparameter.FileUiListPreviewParamet import com.infomaniak.swisstransfer.ui.theme.Margin import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme import com.infomaniak.swisstransfer.ui.utils.HumanReadableSizeUtils.formatSpaceLeft -import com.infomaniak.swisstransfer.ui.utils.HumanReadableSizeUtils.getFilesSize +import com.infomaniak.swisstransfer.ui.utils.HumanReadableSizeUtils.getSpaceUsed import com.infomaniak.swisstransfer.ui.utils.HumanReadableSizeUtils.getSpaceLeft @Composable -fun FilesSize(files: List, withSpaceLeft: Boolean) { - Row( - modifier = Modifier - .padding(vertical = Margin.Medium), - ) { - val filesCount = files.count() - val filesSize = LocalContext.current.getFilesSize(files) +fun FilesSize(modifier: Modifier, files: () -> List, withSpaceLeft: Boolean) { + Row(modifier = modifier) { + val files = files() + val filesCount = files().count() + val filesSize = LocalContext.current.getSpaceUsed(files) val filesDetail = "${pluralStringResource(R.plurals.filesCount, filesCount, filesCount)} • $filesSize" Text( filesDetail, - modifier = Modifier - .padding(start = Margin.Medium), + modifier = Modifier.padding(start = Margin.Medium), color = SwissTransferTheme.colors.secondaryTextColor, style = SwissTransferTheme.typography.bodySmallRegular, ) @@ -69,15 +66,12 @@ fun FilesSize(files: List, withSpaceLeft: Boolean) { } @Preview(name = "Light") -@Preview( - name = "Dark", - uiMode = Configuration.UI_MODE_NIGHT_YES or Configuration.UI_MODE_TYPE_NORMAL, -) +@Preview(name = "Dark", uiMode = Configuration.UI_MODE_NIGHT_YES or Configuration.UI_MODE_TYPE_NORMAL) @Composable fun FileSizePreview(@PreviewParameter(FileUiListPreviewParameter::class) files: List) { SwissTransferTheme { Surface { - FilesSize(files, withSpaceLeft = true) + FilesSize(Modifier.padding(Margin.Medium), { files }, withSpaceLeft = true) } } } diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferNavHost.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferNavHost.kt index d9dc7b3c6..f45e4d55f 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferNavHost.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferNavHost.kt @@ -61,9 +61,10 @@ fun NewTransferNavHost(navController: NavHostController, closeActivity: () -> Un navController.navigate(FilesDetailsDestination(fileId)) }, fileId = filesDetailsDestination.fileId, - withSpaceLeft = false, onCloseClicked = {}, + withSpaceLeft = true, + onCloseClicked = {}, onFileRemoved = {}, - navigateBack = { navController.popBackStack() } + navigateBack = { navController.popBackStack() }, ) } } diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/FilesDetailsScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/FilesDetailsScreen.kt index af5b9a17d..a8d4aa493 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/FilesDetailsScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/FilesDetailsScreen.kt @@ -21,9 +21,10 @@ import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.padding import androidx.compose.material3.Surface import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue import androidx.compose.ui.Modifier import androidx.hilt.navigation.compose.hiltViewModel -import com.infomaniak.swisstransfer.R +import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.infomaniak.swisstransfer.ui.components.FileItemList import com.infomaniak.swisstransfer.ui.components.FileUi import com.infomaniak.swisstransfer.ui.components.SwissTransferTopAppBar @@ -43,12 +44,14 @@ fun FilesDetailsScreen( onCloseClicked: (() -> Unit), navigateBack: (() -> Unit), ) { + val files by filesDetailsViewModel.getFilesFromUUID(fileId).collectAsStateWithLifecycle(emptyList()) + FilesDetailsScreen( title = { //TODO Get fileId detail to get the title "Some Folder" }, - files = { filesDetailsViewModel.getFilesFromUUID(fileId) }, + files = { files ?: emptyList() }, navigateToDetails = navigateToDetails, withSpaceLeft = withSpaceLeft, onFileRemoved = onFileRemoved, @@ -67,19 +70,17 @@ private fun FilesDetailsScreen( onCloseClicked: (() -> Unit), navigateBack: (() -> Unit), ) { - val filesToDisplay = files() Column { SwissTransferTopAppBar( title = title(), - titleRes = R.string.importFilesScreenTitle, navigationMenu = TopAppBarButton.backButton { navigateBack() }, TopAppBarButton.closeButton { onCloseClicked() }, ) - FilesSize(filesToDisplay, withSpaceLeft) + FilesSize(Modifier.padding(vertical = Margin.Medium), files, withSpaceLeft) FileItemList( modifier = Modifier.padding(horizontal = Margin.Medium), - files = filesToDisplay, + files = files(), isRemoveButtonVisible = onFileRemoved != null, isCheckboxVisible = false, isUidChecked = { false }, diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/FilesDetailsViewModel.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/FilesDetailsViewModel.kt index a90fefba7..6e78ca73f 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/FilesDetailsViewModel.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/FilesDetailsViewModel.kt @@ -18,16 +18,21 @@ package com.infomaniak.swisstransfer.ui.screen.newtransfer.importfiles import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope import com.infomaniak.swisstransfer.ui.components.FileUi import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.stateIn import javax.inject.Inject @HiltViewModel class FilesDetailsViewModel @Inject constructor() : ViewModel() { - fun getFilesFromUUID(uuid: String): List { + fun getFilesFromUUID(uuid: String): Flow?> { // TODO Call the right manager to fetch files from Realm - return listOf( + val fileUiList = listOf( FileUi( fileName = "How to not get fired.pdf", uid = "How to not get fired.pdf", @@ -50,5 +55,6 @@ class FilesDetailsViewModel @Inject constructor() : ViewModel() { uri = "", ), ) + return flow { emit(fileUiList) }.stateIn(viewModelScope, SharingStarted.Eagerly, null) } } diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/HumanReadableSizeUtils.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/HumanReadableSizeUtils.kt index b7feedad5..bcc39e663 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/HumanReadableSizeUtils.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/HumanReadableSizeUtils.kt @@ -33,7 +33,7 @@ object HumanReadableSizeUtils { private fun getFilesSizeInBytes(files: List) = files.sumOf { it.fileSizeInBytes } - fun Context.getFilesSize(files: List): String = getHumanReadableSize(this, getFilesSizeInBytes(files)) + fun Context.getSpaceUsed(files: List): String = getHumanReadableSize(this, getFilesSizeInBytes(files)) fun Context.getSpaceLeft(files: List): String { val spaceLeft = (TOTAL_FILE_SIZE - getFilesSizeInBytes(files)).coerceAtLeast(0)