Notifications #554

Merged
6543 merged 12 commits from opyale/GitNex:notifications into master 2 years ago
opyale commented 2 years ago
Collaborator

[DONE] This PR has to wait until this PR got merged and backported to 1.12

Closes #5

Help is very much appreciated in the following topics:

  • Implementing a BottomSheetFragment for the notifications list (some progress was already made)
  • Completing contentDescription in ImageViews and strings.xml
  • Hiding Notifications in Drawer if current version of Gitea does not meet requirements
  • Implementing 'load more' in notifications list.
  • Dont show FAB when viewing seen notifications.
  • Implementing a filter icon in the notifications fragment to switch between seen and unseen notifications.
  • Implementing a solution (onItemClickListener) where the user is being redirected to the notifications source (issue, pr).
  • Mark notifications as read when clicked.

This work is already done and represents the basis:

  • Implementing the notifications API
  • Implementing a WorkManager to display and poll for new notifications
  • Added option in settings to customize the polling delay.
  • Added a NotificationFragment with corresponding NotificationsAdapter.
  • Version check to prevent polling if current version does not meet requirements
  • Created basic and a non-setup BottomSheetNotificationsFragment

I would be very grateful if you, @mmarif and @6543 could help me. Working together on notifications would make much more fun.

**[DONE] This PR has to wait until [this PR](https://github.com/go-gitea/gitea/pull/12164) got merged and backported to 1.12** Closes #5 Help is very much appreciated in the following topics: - [x] Implementing a BottomSheetFragment for the notifications list (some progress was already made) - [x] Completing contentDescription in ImageViews and strings.xml - [x] Hiding Notifications in Drawer if current version of Gitea does not meet requirements - [x] Implementing 'load more' in notifications list. - [x] Dont show FAB when viewing seen notifications. - [x] Implementing a filter icon in the notifications fragment to switch between seen and unseen notifications. - [x] Implementing a solution (onItemClickListener) where the user is being redirected to the notifications source (issue, pr). - [x] Mark notifications as read when clicked. This work is already done and represents the basis: - [x] Implementing the notifications API - [x] Implementing a WorkManager to display and poll for new notifications - [x] Added option in settings to customize the polling delay. - [x] Added a NotificationFragment with corresponding NotificationsAdapter. - [x] Version check to prevent polling if current version does not meet requirements - [x] Created basic and a non-setup BottomSheetNotificationsFragment I would be very grateful if you, @mmarif and @6543 could help me. Working together on notifications would make much more fun.
opyale added the
Enhancement
Long-term
labels 2 years ago
opyale self-assigned this 2 years ago
opyale added this to the 3.0.0 milestone 2 years ago
opyale added the
Needs-help
label 2 years ago
opyale added the due date 2020-07-10 2 years ago
Owner

Few obervations using 1.13-dev

  • Pin notification shuold be Pin Notification
  • Mark as unread shuold be Mark as Unread
  • Bottomsheet Open should be Unread with proper icon
  • Bottomsheet Closed should be Read with proper icon
  • On tap Pin Notification returns an instance error
  • Same goes for Mark as Unread when tap on it

Related to this PR. Some things are not implemented yet.

Pagination issues - this is for another PR and has to be solved within the whole app.

  • If I switch to Read notifications, and close the app. When start it should launch with Unread notifications screen(clear the saved value on start?)
  • If Gitea is below 1.12 then hide the nav menu.

These are the few things I noticed which will help improve the feature. :)

I will post back if found anything else.

And without diving to the code itself, the feature is great and working nicely. Good work.

Few obervations using 1.13-dev - [x] `Pin notification` shuold be `Pin Notification` - [x] `Mark as unread` shuold be `Mark as Unread` - [x] Bottomsheet `Open` should be `Unread` with proper icon - [x] Bottomsheet `Closed` should be `Read` with proper icon - [x] On tap `Pin Notification` returns an instance error - [x] Same goes for `Mark as Unread` when tap on it Related to [this PR](https://github.com/go-gitea/gitea/pull/12164). Some things are not implemented yet. - [x] Notifications icon in the nav should be the notifications(bell) icon - [x] Taping on the icon `mark all notification as read` should clear the list - [ ] Same goes(step 8) for when on tap on individual notification(three dotted menu) to remove from the list. For this you can just remove the position from the adapter and mark the notification as read via API. https://codeberg.org/gitnex/GitNex/src/branch/master/app/src/main/java/org/mian/gitnex/adapters/IssueCommentsAdapter.java#L209 Pagination issues - this is for another PR and has to be solved within the whole app. - [x] If I switch to `Read` notifications, and close the app. When start it should launch with `Unread` notifications screen(clear the saved value on start?) - [x] If Gitea is below 1.12 then hide the nav menu. These are the few things I noticed which will help improve the feature. :) I will post back if found anything else. And without diving to the code itself, the feature is great and working nicely. Good work.
Owner

Some more improvements

  • Select polling delay should be Select Polling Delay in the popup
  • Polling Delay in the settings should be Notifications Polling Delay
  • The hint text under Polling Delay should be 12sp
    • Really? Everything else is sized in 14sp.

Also not necessary but would be useful, if we can show the current selected value under Polling Delay like cache.

Some more improvements - [x] `Select polling delay` should be `Select Polling Delay` in the popup - [x] `Polling Delay` in the settings should be `Notifications Polling Delay` - [x] The hint text under `Polling Delay` should be 12sp - Really? Everything else is sized in 14sp. Also not necessary but would be useful, if we can show the current selected value under `Polling Delay` like cache.
Collaborator

I think we should move this into 3.1.0

I think we should move this into 3.1.0
opyale removed the due date 2020-07-10 2 years ago
Owner

Really? Everything else is sized in 14sp.

Yes, they are. But they are the saved values not hint text. Hint text is more like information and in grey color. Example is drafts.

Let's say I want to add some text for Theme, Hint text will be 12sp, while the saved which is in green will be 14sp.

Theme (16sp)
Dark (14sp)
hint text (12sp)

> Really? Everything else is sized in 14sp. Yes, they are. But they are the saved values not hint text. Hint text is more like information and in grey color. Example is drafts. Let's say I want to add some text for Theme, Hint text will be 12sp, while the saved which is in green will be 14sp. Theme (16sp) Dark (14sp) hint text (12sp)
opyale added the due date 2020-07-21 2 years ago
opyale removed the
Needs-help
label 2 years ago
6543 reviewed 2 years ago
<string-array name="tipsLoadingMoreView">
<item>You can drink something meanwhile</item>
<item>I don't know what to write here</item>
6543 commented 2 years ago
Collaborator
-        <item>I don't know what to write here</item>
+        <item>I don\t know what to write here</item>

need escabe \

```diff - <item>I don't know what to write here</item> + <item>I don\t know what to write here</item> ``` need escabe \
opyale marked this conversation as resolved
6543 reviewed 2 years ago
private static String[] cacheSizeImagesList = {"50 MB", "100 MB", "250 MB", "500 MB", "1 GB"};
private static int cacheSizeImagesSelectedChoice = 0;
private static int MINIMUM_POLLING_DELAY = 15;
6543 commented 2 years ago
Collaborator

if a user want to poll every 1min?

if a user want to poll every 1min?
opyale commented 2 years ago
Poster
Collaborator

This is not possible due to Google's constraints on WorkManager.
https://developer.android.com/reference/androidx/work/PeriodicWorkRequest

This is not possible due to Google's constraints on WorkManager. https://developer.android.com/reference/androidx/work/PeriodicWorkRequest
opyale marked this conversation as resolved
Collaborator

Few thins:

  • Read & Pinned Notifications are not shown
  • Filter list do not work
  • "mark all as read" dont clean the listview
  • If GitNex is closed / minimized you dont get any notification of new ones
Few thins: * Read & Pinned Notifications are not shown * Filter list do not work * "mark all as read" dont clean the listview * If GitNex is closed / minimized you dont get any notification of new ones
Collaborator

an suggestion would be to have two different times for the checking iterfall:

  • if the app is open (user interacts with gitnex): pull each 10-30sec

  • if the app is closed/minimized: pull each 1-270min

an suggestion would be to have two different times for the checking iterfall: - if the app is open (user interacts with gitnex): pull each 10-30sec - if the app is closed/minimized: pull each 1-270min
Poster
Collaborator

an suggestion would be to have two different times for the checking iterfall:

  • if the app is open (user interacts with gitnex): pull each 10-30sec

  • if the app is closed/minimized: pull each 1-270min

This would require an additional and dedicated background service, since WorkManager is only able to pull each 15 Minutes - as described before. It's just better to stay with WorkManager and accept its downsides, because im pretty sure it will not change much in the future (not as much deprecations which makes services like the BackgroundService unusable for long term background tasks).

> an suggestion would be to have two different times for the checking iterfall: > > - if the app is open (user interacts with gitnex): pull each 10-30sec > > - if the app is closed/minimized: pull each 1-270min This would require an additional and dedicated background service, since WorkManager is only able to pull each 15 Minutes - as described before. It's just better to stay with WorkManager and accept its downsides, because im pretty sure it will not change much in the future (not as much deprecations which makes services like the BackgroundService unusable for long term background tasks).
Collaborator

This would require an additional and dedicated background service, since WorkManager is only able to pull each 15 Minutes - as described before. It's just better to stay with WorkManager and accept its downsides, because im pretty sure it will not change much in the future (not as much deprecations which makes services like the BackgroundService unusable for long term background tasks).

this is bad - realy would like this feature - but this is no blocking point

#554 comment-69151 is what we need to fix

> This would require an additional and dedicated background service, since WorkManager is only able to pull each 15 Minutes - as described before. It's just better to stay with WorkManager and accept its downsides, because im pretty sure it will not change much in the future (not as much deprecations which makes services like the BackgroundService unusable for long term background tasks). this is bad - realy would like this feature - but this is no blocking point [#554 comment-69151](https://codeberg.org/gitnex/GitNex/pulls/554#issuecomment-69151) is what we need to fix
Owner

if the app is open (user interacts with gitnex): pull each 10-30sec

If the app is open/background notificatin will happen instantly.

if the app is closed/minimized: pull each 1-270min

As described, this is limitation from Google. When app is terminated(not even in background) delay is minimum 15 minutes.

> if the app is open (user interacts with gitnex): pull each 10-30sec If the app is open/background notificatin will happen instantly. > if the app is closed/minimized: pull each 1-270min As described, this is limitation from Google. When app is terminated(not even in background) delay is minimum 15 minutes.
Poster
Collaborator

#554 comment-69151 is what we need to fix

Working on it.

> [#554 comment-69151](https://codeberg.org/gitnex/GitNex/pulls/554#issuecomment-69151) is what we need to fix Working on it.
mmarif requested changes 2 years ago
<vector xmlns:android="http://schemas.android.com/apk/res/android"
mmarif commented 2 years ago
Owner

You can delete this icon and use ic_pull_request instead.

You can delete this icon and use `ic_pull_request` instead.
mmarif marked this conversation as resolved
<vector android:height="24dp" android:tint="#368F73"
mmarif commented 2 years ago
Owner

You can use ic_question and delete this icon.

You can use `ic_question` and delete this icon.
mmarif marked this conversation as resolved
<vector android:height="24dp" android:tint="#368F73"
mmarif commented 2 years ago
Owner

Use icon ic_unwatch and delete this icon.

Use icon `ic_unwatch` and delete this icon.
mmarif marked this conversation as resolved
opyale changed title from Notifications to [WIP] Notifications 2 years ago
opyale changed title from [WIP] Notifications to Notifications 2 years ago
6543 approved these changes 2 years ago
6543 left a comment
Collaborator

🎉

:tada:
6543 requested review from mmarif 2 years ago
mmarif approved these changes 2 years ago
6543 merged commit 39ac49b258 into master 2 years ago
opyale deleted branch notifications 2 years ago

Reviewers

6543 approved these changes 2 years ago
mmarif approved these changes 2 years ago
The pull request has been merged as 39ac49b258.
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date

2020-07-21

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.