Conversation
|
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/16672.apk |
app/src/main/java/com/nextcloud/ui/tags/TagManagementViewModel.kt
Outdated
Show resolved
Hide resolved
| private val currentAccountProvider: CurrentAccountProvider | ||
| ) : ViewModel() { | ||
|
|
||
| sealed interface TagUiState { |
There was a problem hiding this comment.
Please add sealed interface in com.nextcloud.ui.tags.model
There was a problem hiding this comment.
What do you mean? Shall I extract it and move to com.nextcloud.ui.tags.model as a new file/interface?
There was a problem hiding this comment.
Yes please.
My folder structure idea:
ui
ui/tags
ui/tags/adapter
ui/tags/model
ui/tags/model/tagUiState
ui/tags/util -> if exists
ui/tags/tagsFragment
ui/tags/tagsViewModel
Thus we don't have huge model package or huge viewModels package every feature have same inner structure and all inner classes, interfaces etc only responsible for that feature.
Since this is a new feature lets follow this structure make sense?
e.g. not well structured (OCFileUtils only related with file listing feature but BuildHelper project scope util so its right place for it)
my suggestion (see notifications package in future everything related with notification will be under ui/notification package)
app/src/main/java/com/nextcloud/ui/tags/TagManagementViewModel.kt
Outdated
Show resolved
Hide resolved
| viewModelScope.launch(Dispatchers.IO) { | ||
| try { | ||
| val client = clientFactory.createNextcloudClient(currentAccountProvider.user) | ||
| val result = PutTagRemoteOperation(tag.id, fileId).execute(client) |
There was a problem hiding this comment.
Better to create repository for TagManagementViewModel e.g. TagManagementRepository and handle network call there and inject inside ViewModel instead of doing network call directly inside the ViewModel.
You can check AssistantViewModel as an example.
| } | ||
| } | ||
| } catch (e: ClientFactory.CreationException) { | ||
| // ignore |
| val nextcloudClient = clientFactory.createNextcloudClient(currentAccountProvider.user) | ||
| val createResult = CreateTagRemoteOperation(name).execute(nextcloudClient) | ||
|
|
||
| if (createResult.isSuccess) { |
There was a problem hiding this comment.
Apply fail fast princible instead of nested ifs.
| .show(fragmentManager, "actions"); | ||
| } | ||
|
|
||
| private void refreshTagChips(Context context) { |
There was a problem hiding this comment.
FileDetailFragment already 900+ lines of code. Let's not make it more harder to maintain. Please create a new Kotlin class and pass reference and handle the refresh tag chips there.
tobiasKaminsky
force-pushed
the
editTags
branch
from
1414f8e to
bf31347
Compare
|
APK file: https://github.com/nextcloud/android/actions/runs/24389201000/artifacts/6423672148 |
|
blue-Light-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed. |
2 participants
Disclaimer: Claude AI