(#600) Notification system for settings (for crash logs and apk updates) #656
Conversation
- Implement SettingsNotificationManager to accumulate update events and distribute them to observers. - Implement displaying of the notifications in the settings' views.
K1rakishou
changed the title
|
Alright, now it's ready. |
|
I had to change a couple of things in UpdateManager so you better review it more carefully. |
… when disabling crash log collect setting
...c/main/java/com/github/adamantcheese/chan/ui/controller/settings/MainSettingsController.java
Outdated
Show resolved
Hide resolved
Kuroba/app/src/main/java/com/github/adamantcheese/chan/ui/layout/crashlogs/CrashLog.kt
Outdated
Show resolved
Hide resolved
…ystem-for-settings # Conflicts: # Kuroba/app/src/main/java/com/github/adamantcheese/chan/core/manager/UpdateManager.java
…ystem-for-settings # Conflicts: # Kuroba/app/src/main/java/com/github/adamantcheese/chan/Chan.java # Kuroba/app/src/main/java/com/github/adamantcheese/chan/core/manager/UpdateManager.java # Kuroba/app/src/main/java/com/github/adamantcheese/chan/core/settings/ChanSettings.java # Kuroba/app/src/main/res/values/strings.xml
.../app/src/main/java/com/github/adamantcheese/chan/core/manager/SettingsNotificationManager.kt
Show resolved
Hide resolved
.../app/src/main/java/com/github/adamantcheese/chan/core/manager/SettingsNotificationManager.kt
Show resolved
Hide resolved
Kuroba/app/src/main/java/com/github/adamantcheese/chan/core/manager/UpdateManager.java
Outdated
Show resolved
Hide resolved
| return false; | ||
| } | ||
|
|
||
| private void notifyNewApkUpdate() { | ||
| ChanSettings.hasNewApkUpdate.set(true); |
There was a problem hiding this comment.
This is not a setting. This is a persistable app state. I think I should extract it into a separate class that uses it's own shared preferences file.
There was a problem hiding this comment.
You probably won't need a setting at all since the update API is run on every startup.
There was a problem hiding this comment.
But it only checks for the new apk only once in 24h, or is it not?
There was a problem hiding this comment.
It checks for a new APK only when the app is started cold, with a minimum delay of 24h for dev and 5 days for stable. There's no scheduled task to check for an update, just like how it was in Clover.
There was a problem hiding this comment.
Well that's why I needed "hasNewApkUpdate" flag. Because if we start the app during the 24h or 5d window when we do not check for the updates, and there were actually an update the last time we checked it, the notification won't show up.
There was a problem hiding this comment.
It's still fine to keep it in ChanSettings anyways, there's a bunch of things in there that aren't strictly settings.
There was a problem hiding this comment.
Well those will have to be moved too. But we can do that later. Or I could move them.
There was a problem hiding this comment.
Please just keep it to one thing per PR if possible, these super huge PRs with all sorts of barely related stuff in them makes it difficult to review.
There was a problem hiding this comment.
Alright then I will just move this one setting to a new class.
Kuroba/app/src/main/java/com/github/adamantcheese/chan/ui/controller/DrawerController.java
Outdated
Show resolved
Hide resolved
...p/src/main/java/com/github/adamantcheese/chan/ui/controller/settings/SettingsController.java
Outdated
Show resolved
Hide resolved
|
Already done in another PR and already merged. |
…ystem-for-settings # Conflicts: # Kuroba/app/src/main/java/com/github/adamantcheese/chan/Chan.java # Kuroba/app/src/main/java/com/github/adamantcheese/chan/ui/controller/settings/DeveloperSettingsController.java
Kuroba/app/src/main/java/com/github/adamantcheese/chan/core/manager/ReportManager.kt
Outdated
Show resolved
Hide resolved
.../app/src/main/java/com/github/adamantcheese/chan/core/manager/SettingsNotificationManager.kt
Outdated
Show resolved
Hide resolved
.../app/src/main/java/com/github/adamantcheese/chan/core/manager/SettingsNotificationManager.kt
Outdated
Show resolved
Hide resolved
Kuroba/app/src/main/java/com/github/adamantcheese/chan/core/settings/ChanState.kt
Outdated
Show resolved
Hide resolved
Kuroba/app/src/main/java/com/github/adamantcheese/chan/core/settings/ChanState.kt
Outdated
Show resolved
Hide resolved
…ystem-for-settings # Conflicts: # Kuroba/app/src/main/res/values/strings.xml
…ystem-for-settings # Conflicts: # Kuroba/app/src/main/java/com/github/adamantcheese/chan/ui/layout/ReportProblemLayout.kt
Closes #600
Not ready yet.So, basically, instead of spamming AlertDialogs on every important update (like new apk update or new crash log that awaits to be either uploaded or not) there is now a more user-friendly system which does not nag the user with any windows whatsoever. Instead, a small alert icon is being shown in specific setting option (and in the drawer too) so the user sees it every time he opens the drawer or the settings. Once the conditions for this icon are all satisfied it disappears (like all crash logs are either uploaded or deleted). If there are multiple notifications (say apk update and couple crash logs) then a notification with highest priority is shown in the drawer (once it's cleared the other one replaces it, etc).
It's pretty easy to add new notifications, too.
Oh yeah, you can now also review crash logs and decide which you upload and which not (you can even delete them all). No crash logs are now being uploaded automatically.