Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.wire.android.di.KaliumCoreLogic
import com.wire.kalium.cells.CellsScope
import com.wire.kalium.cells.domain.CellUploadManager
import com.wire.kalium.cells.domain.usecase.AddAttachmentDraftUseCase
import com.wire.kalium.cells.domain.usecase.CreateFolderUseCase
import com.wire.kalium.cells.domain.usecase.DeleteCellAssetUseCase
import com.wire.kalium.cells.domain.usecase.DownloadCellFileUseCase
import com.wire.kalium.cells.domain.usecase.GetNodesUseCase
Expand Down Expand Up @@ -120,4 +121,8 @@ class CellsModule {
@ViewModelScoped
@Provides
fun provideRetryAttachmentUploadUseCase(cellsScope: CellsScope): RetryAttachmentUploadUseCase = cellsScope.retryAttachmentUpload

@ViewModelScoped
@Provides
fun provideCreateFolderUseCase(cellsScope: CellsScope): CreateFolderUseCase = cellsScope.createFolderUseCase
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ internal fun NavController.navigateToItem(command: NavigationCommand) {
BackStackMode.NONE -> {
}
}
launchSingleTop = true
launchSingleTop = command.launchSingleTop
restoreState = true
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package com.wire.android.navigation
import com.ramcosta.composedestinations.spec.DestinationSpec
import com.ramcosta.composedestinations.spec.NavGraphSpec
import com.wire.android.feature.cells.ui.destinations.ConversationFilesScreenDestination
import com.wire.android.feature.cells.ui.destinations.ConversationFilesWithSlideInTransitionScreenDestination
import com.wire.android.feature.cells.ui.destinations.CreateFolderScreenDestination
import com.wire.android.feature.cells.ui.destinations.PublicLinkScreenDestination
import com.wire.android.feature.sketch.destinations.DrawingCanvasScreenDestination
Expand All @@ -32,6 +33,7 @@ object WireMainNavGraph : NavGraphSpec {
.plus(DrawingCanvasScreenDestination)
.plus(PublicLinkScreenDestination)
.plus(ConversationFilesScreenDestination)
.plus(ConversationFilesWithSlideInTransitionScreenDestination)
.plus(CreateFolderScreenDestination)
override val destinationsByRoute = destinations.associateBy { it.route }
override val nestedNavGraphs = NavGraphs.root.nestedNavGraphs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ data class NavigationCommand(
* Whether we want to clear the previously added screens on the backstack, only until the current one, or none of them.
*/
val backStackMode: BackStackMode = BackStackMode.NONE,

/**
* Whether we want to clear the backstack of the current graph.
*/
val launchSingleTop: Boolean = true
)

enum class BackStackMode {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,11 @@ fun AllFilesScreen(
actionsFlow = viewModel.actions,
pagingListItems = pagingListItems,
sendIntent = { viewModel.sendIntent(it) },
onFolderClick = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to use existing API for communication between View and ViewModel. You can define another CellViewIntent type OnFolderClick and use existing sendIntent method to send it to ViewModel.
Or change existing intent OnFileClick to OnNodeClick and choose the handler in the ViewModel depending on the clicked node type (file / folder).

Copy link
Member Author

Choose a reason for hiding this comment

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

We will not need the viewModel here as we will navigate to a different screen

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe right now we do not have any extra logic, but we could have some checks in future (e.g. permissions) which would be better to do in the view model.
I guess the only real reason to handle it via the view model is to make this flow testable via view model unit tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's keep it simple for now—no need to over-engineer prematurely. If we need to add permission checks or more complex logic later, we can refactor and move that into the ViewModel when it makes sense.

// TODO: Handle folder click later
},
downloadFileState = viewModel.downloadFileSheet,
fileMenuState = viewModel.menu,
menuState = viewModel.menu,
isAllFiles = true,
isSearchResult = viewModel.hasSearchQuery(),
showPublicLinkScreen = { assetId, fileName, linkId ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,7 @@
*/
package com.wire.android.feature.cells.ui

data class CellFilesNavArgs(val conversationId: String? = null)
data class CellFilesNavArgs(
val conversationId: String? = null,
val screenTitle: String? = null
)
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ internal fun CellListItem(
modifier = Modifier
.fillMaxWidth()
.align(Alignment.BottomStart),
progress = { cell.downloadProgress },
progress = { it },
color = colorsScheme().primary,
trackColor = Color.Transparent,
)
Expand Down Expand Up @@ -254,12 +254,12 @@ private fun PreviewCellListItem() {
cell = CellNodeUi.File(
uuid = "",
name = "file name",
downloadProgress = 0.75f,
assetType = AttachmentFileType.IMAGE,
assetSize = 123214,
localPath = null,
mimeType = "image/jpg",
publicLinkId = "",
downloadProgress = 0.75f,
userName = "Test User",
conversationName = "Test Conversation",
modifiedTime = null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ internal fun CellScreenContent(
actionsFlow: Flow<CellViewAction>,
pagingListItems: LazyPagingItems<CellNodeUi>,
sendIntent: (CellViewIntent) -> Unit,
onFolderClick: (CellNodeUi.Folder) -> Unit,
downloadFileState: StateFlow<CellNodeUi.File?>,
fileMenuState: Flow<MenuOptions?>,
menuState: Flow<MenuOptions?>,
showPublicLinkScreen: (String, String, String?) -> Unit,
isAllFiles: Boolean,
isSearchResult: Boolean = false,
Expand All @@ -88,7 +89,12 @@ internal fun CellScreenContent(
else ->
CellFilesScreen(
cellNodes = pagingListItems,
onItemClick = { sendIntent(CellViewIntent.OnItemClick(it)) },
onItemClick = {
when (it) {
is CellNodeUi.File -> sendIntent(CellViewIntent.OnFileClick(it))
is CellNodeUi.Folder -> onFolderClick(it)
}
},
onItemMenuClick = { sendIntent(CellViewIntent.OnItemMenuClick(it)) },
// onRefresh = {
// viewModel.loadFiles(pullToRefresh = true)
Expand Down Expand Up @@ -161,7 +167,7 @@ internal fun CellScreenContent(

LaunchedEffect(Unit) {
lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) {
fileMenuState.collect { showMenu ->
menuState.collect { showMenu ->
menu = showMenu
}
}
Expand Down Expand Up @@ -253,7 +259,7 @@ private fun EmptyScreen(
.weight(1f)
)

if (!isSearchResult) {
if (!isSearchResult && isAllFiles) {
WirePrimaryButton(
text = stringResource(R.string.reload),
onClick = onRetry
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,22 @@ class CellViewModel @Inject constructor(
}
.map {
when (it) {
is Node.File -> it.toUiModel().copy(
downloadProgress = downloadData[it.uuid]?.progress,
localPath = downloadData[it.uuid]?.localPath?.toString()
)

is Node.Folder -> it.toUiModel()
is Node.File -> {
it.toUiModel().copy(
downloadProgress = downloadData[it.uuid]?.progress,
localPath = downloadData[it.uuid]?.localPath?.toString()
)
}
}
}
}
}

internal fun currentNodeUuid(): String? = navArgs.conversationId

internal fun screenTitle(): String? = navArgs.screenTitle

internal fun onSearchQueryUpdated(text: String) = viewModelScope.launch {
searchQueryFlow.emit(text)
}
Expand All @@ -135,7 +140,7 @@ class CellViewModel @Inject constructor(

internal fun sendIntent(intent: CellViewIntent) {
when (intent) {
is CellViewIntent.OnItemClick -> onItemClick(intent.cellNode)
is CellViewIntent.OnFileClick -> onFileClick(intent.file)
is CellViewIntent.OnItemMenuClick -> onItemMenuClick(intent.cellNode)
is CellViewIntent.OnMenuFileActionSelected -> onMenuFileAction(intent.file, intent.action)
is CellViewIntent.OnMenuFolderActionSelected -> onMenuFolderAction(intent.folder, intent.action)
Expand All @@ -145,15 +150,11 @@ class CellViewModel @Inject constructor(
}
}

private fun onItemClick(cellNode: CellNodeUi) {
if (cellNode is CellNodeUi.File) {
when {
cellNode.localFileAvailable() -> openLocalFile(cellNode)
cellNode.canOpenWithUrl() -> openFileContentUrl(cellNode)
else -> viewModelScope.launch { _downloadFileSheet.emit(cellNode) }
}
} else {
// TODO: Open folder
private fun onFileClick(cellNode: CellNodeUi.File) {
when {
cellNode.localFileAvailable() -> openLocalFile(cellNode)
cellNode.canOpenWithUrl() -> openFileContentUrl(cellNode)
else -> viewModelScope.launch { _downloadFileSheet.emit(cellNode) }
}
}

Expand Down Expand Up @@ -339,8 +340,8 @@ class CellViewModel @Inject constructor(
}
}

internal sealed interface CellViewIntent {
data class OnItemClick(val cellNode: CellNodeUi) : CellViewIntent
sealed interface CellViewIntent {
data class OnFileClick(val file: CellNodeUi.File) : CellViewIntent
data class OnItemMenuClick(val cellNode: CellNodeUi) : CellViewIntent
data class OnMenuFileActionSelected(val file: CellNodeUi.File, val action: BottomSheetAction.File) : CellViewIntent
data class OnMenuFolderActionSelected(val folder: CellNodeUi.Folder, val action: BottomSheetAction.Folder) : CellViewIntent
Expand All @@ -349,7 +350,7 @@ internal sealed interface CellViewIntent {
data object OnDownloadMenuClosed : CellViewIntent
}

internal sealed interface CellViewAction
sealed interface CellViewAction
internal data class ShowDeleteConfirmation(val file: CellNodeUi.File) : CellViewAction
internal data class ShowError(val error: CellError) : CellViewAction
internal data class ShowPublicLinkScreen(val file: CellNodeUi.File) : CellViewAction
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,16 @@ import androidx.compose.ui.layout.ContentScale
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.res.stringResource
import androidx.hilt.navigation.compose.hiltViewModel
import androidx.paging.compose.LazyPagingItems
import androidx.paging.compose.collectAsLazyPagingItems
import com.ramcosta.composedestinations.annotation.Destination
import com.wire.android.feature.cells.R
import com.wire.android.feature.cells.ui.destinations.ConversationFilesWithSlideInTransitionScreenDestination
import com.wire.android.feature.cells.ui.destinations.CreateFolderScreenDestination
import com.wire.android.feature.cells.ui.destinations.PublicLinkScreenDestination
import com.wire.android.feature.cells.ui.dialog.CellsNewActionsBottomSheet
import com.wire.android.feature.cells.ui.model.CellNodeUi
import com.wire.android.navigation.BackStackMode
import com.wire.android.navigation.NavigationCommand
import com.wire.android.navigation.WireNavigator
import com.wire.android.navigation.style.PopUpNavigationAnimation
Expand All @@ -48,6 +52,9 @@ import com.wire.android.ui.common.dimensions
import com.wire.android.ui.common.scaffold.WireScaffold
import com.wire.android.ui.common.topappbar.NavigationIconType
import com.wire.android.ui.common.topappbar.WireCenterAlignedTopAppBar
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.flow.StateFlow

/**
* Show files in one conversation.
Expand All @@ -60,10 +67,33 @@ import com.wire.android.ui.common.topappbar.WireCenterAlignedTopAppBar
@Composable
fun ConversationFilesScreen(
navigator: WireNavigator,
viewModel: CellViewModel = hiltViewModel(),
) {

ConversationFilesScreenContent(
navigator = navigator,
currentNodeUuid = viewModel.currentNodeUuid(),
actions = viewModel.actions,
pagingListItems = viewModel.nodesFlow.collectAsLazyPagingItems(),
downloadFileSheet = viewModel.downloadFileSheet,
menu = viewModel.menu,
sendIntent = { viewModel.sendIntent(it) },
)
}

@Composable
fun ConversationFilesScreenContent(
navigator: WireNavigator,
currentNodeUuid: String?,
actions: Flow<CellViewAction>,
pagingListItems: LazyPagingItems<CellNodeUi>,
downloadFileSheet: StateFlow<CellNodeUi.File?>,
menu: SharedFlow<MenuOptions>,
sendIntent: (CellViewIntent) -> Unit,
modifier: Modifier = Modifier,
viewModel: CellViewModel = hiltViewModel()
screenTitle: String? = null,
navigationIconType: NavigationIconType = NavigationIconType.Close()
) {
val pagingListItems = viewModel.nodesFlow.collectAsLazyPagingItems()
val sheetState = rememberWireModalSheetState<Unit>()

val isFabVisible = when {
Expand All @@ -78,16 +108,16 @@ fun ConversationFilesScreen(
sheetState.hide()
},
onCreateFolder = {
navigator.navigate(NavigationCommand(CreateFolderScreenDestination()))
navigator.navigate(NavigationCommand(CreateFolderScreenDestination(currentNodeUuid)))
}
)
WireScaffold(
modifier = modifier,
topBar = {
WireCenterAlignedTopAppBar(
onNavigationPressed = { navigator.navigateBack() },
title = stringResource(R.string.conversation_files_title),
navigationIconType = NavigationIconType.Close(),
title = screenTitle ?: stringResource(R.string.conversation_files_title),
navigationIconType = navigationIconType,
elevation = dimensions().spacing0x
)
},
Expand Down Expand Up @@ -122,12 +152,25 @@ fun ConversationFilesScreen(
) { innerPadding ->
Box(modifier = Modifier.padding(innerPadding)) {
CellScreenContent(
actionsFlow = viewModel.actions,
actionsFlow = actions,
pagingListItems = pagingListItems,
sendIntent = { viewModel.sendIntent(it) },
downloadFileState = viewModel.downloadFileSheet,
fileMenuState = viewModel.menu,
sendIntent = sendIntent,
downloadFileState = downloadFileSheet,
menuState = menu,
isAllFiles = false,
onFolderClick = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to use the intent - action API and send this to a ViewModel via sendIntent, prepare the right destination and send new OpenFolder action back to the view for navigation.
This way we will have same approach for all user actions handling, move a small piece of logic (building the destination name) to the ViewModel and can add test case for it.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's more extra work to do, pass intent from view to viewModel back to view again.
And ViewModels should be UI-agnostic, they should not know about navigation, screens..

Copy link
Contributor

Choose a reason for hiding this comment

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

By passing it via viewmodel

  • we will make this action testable
  • user action handling logic will be in the viewmodel, not in the view

ViewModel must not know about navigation implementation details, but it should define the logic for handling user actions, like:
user clicked item -> check if it is a folder -> send command to open folder

val folderPath = "$currentNodeUuid/${it.name}"
navigator.navigate(
NavigationCommand(
ConversationFilesWithSlideInTransitionScreenDestination(
folderPath,
it.name
),
BackStackMode.NONE,
launchSingleTop = false
)
)
},
showPublicLinkScreen = { assetId, fileName, linkId ->
navigator.navigate(
NavigationCommand(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Wire
* Copyright (C) 2025 Wire Swiss GmbH
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see http://www.gnu.org/licenses/.
*/
package com.wire.android.feature.cells.ui

import androidx.compose.runtime.Composable
import androidx.hilt.navigation.compose.hiltViewModel
import androidx.paging.compose.collectAsLazyPagingItems
import com.ramcosta.composedestinations.annotation.Destination
import com.wire.android.navigation.WireNavigator
import com.wire.android.navigation.style.SlideNavigationAnimation
import com.wire.android.ui.common.topappbar.NavigationIconType

@Destination(
style = SlideNavigationAnimation::class,
navArgsDelegate = CellFilesNavArgs::class,
)
@Composable
fun ConversationFilesWithSlideInTransitionScreen(
navigator: WireNavigator,
cellFilesNavArgs: CellFilesNavArgs,
viewModel: CellViewModel = hiltViewModel(),
) {

ConversationFilesScreenContent(
navigator = navigator,
currentNodeUuid = viewModel.currentNodeUuid(),
screenTitle = cellFilesNavArgs.screenTitle,
actions = viewModel.actions,
pagingListItems = viewModel.nodesFlow.collectAsLazyPagingItems(),
downloadFileSheet = viewModel.downloadFileSheet,
menu = viewModel.menu,
sendIntent = { viewModel.sendIntent(it) },
navigationIconType = NavigationIconType.Back()
)
}
Loading
Loading