feat: Navigate between folders (WPB-17392)#4018
Conversation
…-attachement-type # Conflicts: # kalium
…-attachement-type # Conflicts: # features/cells/src/main/java/com/wire/android/feature/cells/ui/CellFilesScreen.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/ConversationFilesScreen.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/createfolder/CreateFolderScreen.kt
# Conflicts: # app/src/main/kotlin/com/wire/android/di/accountScoped/CellsModule.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/AllFilesScreen.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/CellFilesScreen.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/CellListItem.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/CellScreenContent.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/CellViewModel.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/ConversationFilesScreen.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/download/DownloadFileBottomSheet.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/model/CellFileUi.kt # features/cells/src/main/res/values/strings.xml # kalium
Ups 🫰🟨This PR is too big. Please try to break it up into smaller PRs. |
| actionsFlow = viewModel.actions, | ||
| pagingListItems = pagingListItems, | ||
| sendIntent = { viewModel.sendIntent(it) }, | ||
| onFolderClick = { |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
We will not need the viewModel here as we will navigate to a different screen
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
features/cells/src/main/java/com/wire/android/feature/cells/ui/CellViewModel.kt
Show resolved
Hide resolved
| downloadFileState = downloadFileSheet, | ||
| menuState = menu, | ||
| isAllFiles = false, | ||
| onFolderClick = { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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..
There was a problem hiding this comment.
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
...res/cells/src/main/java/com/wire/android/feature/cells/ui/createfolder/CreateFolderScreen.kt
Show resolved
Hide resolved
...res/cells/src/main/java/com/wire/android/feature/cells/ui/dialog/FolderActionsBottomSheet.kt
Show resolved
Hide resolved
# Conflicts: # features/cells/src/main/java/com/wire/android/feature/cells/ui/CellListItem.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/CellScreenContent.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/CellViewModel.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/ConversationFilesScreen.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/createfolder/CreateFolderScreenNavArgs.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/dialog/FileActionsBottomSheet.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/dialog/FolderActionsBottomSheet.kt # features/cells/src/main/java/com/wire/android/feature/cells/ui/model/CellNodeUi.kt # kalium
|
|
Built wire-android-staging-compat-pr-4018.apk is available for download |
|
Built wire-android-dev-debug-pr-4018.apk is available for download |
3 participants
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764The PR Description
What's new in this PR?
Issues
Follow up PR of #4007
It aims to allow user to navigate between folders and sub folders
Needs releases with:
Testing
Test Coverage (Optional)
How to Test
Briefly describe how this change was tested and if applicable the exact steps taken to verify that it works as expected.
Notes (Optional)
Specify here any other facts that you think are important for this issue.
Attachments (Optional)
Attachments like images, videos, etc. (drag and drop in the text box)
PR Post Submission Checklist for internal contributors (Optional)
PR Post Merge Checklist for internal contributors
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.