[FEATURE REQUEST] Allow to go to the destination folder when the copy/move operation is finished#4802
[FEATURE REQUEST] Allow to go to the destination folder when the copy/move operation is finished#4802mykh-hailo wants to merge 3 commits intoowncloud:masterfrom
Conversation
mykh-hailo
force-pushed
the
feat/copy_snack_bar
branch
from
4c713b5 to
b088f36
Compare
mykh-hailo
force-pushed
the
feat/copy_snack_bar
branch
from
b088f36 to
9a416e0
Compare
|
Thanks for your contribution @mykh-hailo! 🍻 Just a small note before the CR: please be careful with the branch name. The branch prefixes we use in the project are: I will take care of this as soon as possible and I'll ping you when it's ready 😄 |
|
@joragua I'd appreciate it if you share any feedback on the result. |
|
@joragua Can you check my PR please? |
|
@mykh-hailo I'll ping you when the CR is finished. Stay tuned! 😄 |
|
@joragua Can you provide me any updates on this? |
|
Hi @mykh-hailo! I'm working on the PR but it will not be available until next week. I'll ping you once it's done. No worries! 😄 Thanks for your patience! |
joragua
left a comment
joragua
left a comment
There was a problem hiding this comment.
Nice job @mykh-hailo! 🙌🏻 Somme comments here.
NOTE: I'd add a release note since this is an important and useful improvement for end users. They should be aware of it this and see the new feature in the release notes screen. You can add a new entry in ReleaseNotesViewModel.kt with an appropriate title and subtitle. Let us know if you have any question about this process. 😄
| fun Activity.showSnackbarWithAction( | ||
| message: CharSequence, | ||
| action: () -> Unit, | ||
| actionText: CharSequence, | ||
| duration: Int = Snackbar.LENGTH_LONG, | ||
| layoutId: Int = android.R.id.content | ||
| ) { | ||
| Snackbar.make(findViewById(layoutId), message, duration) | ||
| .setAction(actionText) { action() } | ||
| .show() | ||
| } | ||
|
|
There was a problem hiding this comment.
I think that there are more snackbars with actions in the app (the re-login snackbar), so you could use this method in those cases: FileDetailsFragment.kt and FileActivity.java Up to you! If not, we can address this in a future PR 😄
| } else { | ||
| showMessageInSnackbar(R.id.list_layout, message) | ||
| } | ||
| copyMoveTargetFolder = null |
There was a problem hiding this comment.
Just a question: Is it necessary to set null at the end? Since the copyMoveTargetFolder value is set at the begin of requestMoveOperation and copyMoveOperation methods
| <string name="copy_file_error">An error occurred while trying to copy this file or folder.</string> | ||
| <string name="copy_success_msg">The file has been copied</string> | ||
| <string name="move_success_msg">The file has been moved</string> | ||
| <string name="go_to_destination_folder">Go to target folder</string> |
There was a problem hiding this comment.
IMO, this string is too long for a snackbar action. I'd say: Open folder. What do you think @jesmrec?
My suggestion:
<string name="open_folder_action">Open folder</string>
There was a problem hiding this comment.
Open folder sounds good. The first approach i suggested is more complete, but space is also reduced.
| <string name="copy_success_msg">The file has been copied</string> | ||
| <string name="move_success_msg">The file has been moved</string> |
There was a problem hiding this comment.
I'd separate these strings into different blocks. There is one block for the move operation strings and a another for copy operation strings. I'd also re-write the strings like this:
<string name="move_file_correctly">File moved correctly</string>
...
<string name="copy_file_correctly">File copied correctly</string>
3 participants
Related Issues
App: #4379
ReleaseNotesViewModel.ktcreating a newReleaseNote()with String resources (if required)