Unify Notifications #12907
Unify Notifications #12907
Conversation
alperozturk96
force-pushed
the
feature/unify-notifications
branch
2 times, most recently
from
46bf27c
to
32d0efb
Compare
alperozturk96
force-pushed
the
feature/unify-notifications
branch
3 times, most recently
from
f8cc620
to
fb239e5
Compare
Signed-off-by: alperozturk <alper_ozturk@proton.me>
Signed-off-by: alperozturk <alper_ozturk@proton.me>
Signed-off-by: alperozturk <alper_ozturk@proton.me>
Signed-off-by: alperozturk <alper_ozturk@proton.me>
Signed-off-by: alperozturk <alper_ozturk@proton.me>
Signed-off-by: alperozturk <alper_ozturk@proton.me>
Signed-off-by: alperozturk <alper_ozturk@proton.me>
Signed-off-by: alperozturk <alper_ozturk@proton.me>
Signed-off-by: alperozturk <alper_ozturk@proton.me>
Signed-off-by: alperozturk <alper_ozturk@proton.me>
Signed-off-by: alperozturk <alper_ozturk@proton.me>
alperozturk96
force-pushed
the
feature/unify-notifications
branch
from
fb239e5
to
248ee8a
Compare
|
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/12907.apk |
| @@ -182,8 +182,11 @@ | |||
| <string name="active_user">Active user</string> | |||
| <string name="upload_chooser_title">Upload from…</string> | |||
| <string name="uploader_info_dirname">Folder name</string> | |||
|
|
|||
| <string name="upload_notification_manager_start_text" translatable="false">%1$d / %2$d - %3$s</string> | |||
There was a problem hiding this comment.
I'm not sure how translatable="false" affects RTL languages and how this should be displayed in this case.
There was a problem hiding this comment.
It won't be added on transifex, text within the UI element will be right aligned, nothing else. Hence the text block will be right aligned while the information in its order ir ltr sorted.
Hence it makes sense to make it translatable even thought it won't differ in its from except for RTL / LTR
| run uploads@{ | ||
| uploads.forEachIndexed { currentUploadIndex, upload -> | ||
| if (isStopped) { | ||
| return@uploads |
There was a problem hiding this comment.
This run could be removed and the return@uploads replaced with a regular return, if I'm not mistaken.
| uploadsStorageManager.removeUpload(upload.uploadId) | ||
| } | ||
|
|
||
| notificationManager.dismissWorkerNotifications() |
There was a problem hiding this comment.
Wouldn't this cause the notification to be cancelled immediately after notificationManager.notify() is called?
| notification = notificationBuilder.build() | ||
| } | ||
| private val notificationManager = context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager | ||
| private var currentOperationTitle: String? = "null" |
There was a problem hiding this comment.
| private var currentOperationTitle: String? = "null" | |
| private var currentOperationTitle: String? = null |
This attribute should be moved to the same place as currentOperationTitle is placed in DownloadNotificationManager.
| startIntent: PendingIntent | ||
| startIntent: PendingIntent, | ||
| currentUploadIndex: Int, | ||
| totalUploadSize: Int | ||
| ) { |
There was a problem hiding this comment.
The function prepareForStart() is called by FileUploadWorker#uploadFiles(), which is in turn called from FileUploadWorker#retrievePagesBySortingUploadsByID(). This function queries the database for pending uploads using UploadsStorageManager#getCurrentAndPendingUploadsForAccountPageAscById().
Because of QUERY_PAGE_SIZE = 100, totalUploadSize will never exceed 100. Even if one uploads a large folder containing several hundred files.
All of this results in the notification always displaying n / 100 - filename.ext.
Pull request was converted to draft
Before
before.mp4
After
after.mp4
Improvements
All notifications are dismissible. Some users are experiencing problems with notifications that are stuck.
One notification can be used for downloads and uploads. Instead of showing progress with one by one now we can show all progress with one notification.
Example
1 / 10 abc.rtf …
2 / 10 abc2.docx…
...
...
10 / 10 abc3.pdf…
How to Test
Test one file download
Test one file upload
Test multiple file upload
Test multiple file download
Test all above scenarios for e2e