Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android notification channels (categories) SAF-782 #1276

Open
tindn opened this issue Jul 16, 2020 · 3 comments
Open

Android notification channels (categories) SAF-782 #1276

tindn opened this issue Jul 16, 2020 · 3 comments
Assignees

Comments

@tindn
Copy link
Collaborator

tindn commented Jul 16, 2020

Why this issue
The notification problem is a bit confusing, so I wanted to create a document to explain my findings. Also, there will be PRs in 2 repositories, so I wanted a place to describe the whole solution.

Existing problem

Issue reported here on Jira.
There are 4 different notification categories (called Channels in code) on Android, and they're a bit confusing.
In addition to being confusing, having a Sync Service notification might cause concerns that the app is syncing collected data.

Cause

  • Every notification on android requires having a channel.
  • The existing 4 channels are created in 2 different places
    • Setting up the react-native-push-notification to be used for pushing local notifications. This is a valid category of notifications, however, the configuration in the AndroidManifest here used the code name "covidsafepaths_kit_notification_channel" for the category instead of a human readable name.
    • The Path-Check/background-geolocation-android submodule setting up the other 3 categories
      (a) Service channel - required to do background notification. This is valid, but should be renamed.
      (b) Sync channel - this should be removed as we're not doing any syncing work.
      (c) Android Permission Channel - as far as I can tell, this is not being used, as we're creating our own notifications for the permission here. However, I don't want to remove it without additional knowledge. We should still rename this to avoid the duplication and make it clear.

Proposed solution

  1. Remove the sync service category, and rename the other categories as follows

Note: the unreadable channel name was configured in AndroidManifest.xml instead of code, so the change will not be reflected after the app update, but only after a fresh install or reinstall. I have tried to find solutions to this but haven't found any. Perhaps someone with better Android experience can help.

  1. Once we've determined that the Android Permission category is not actually used, we should remove it, leaving only 2 valid categories, Background service and General notifications.

  2. Rename the background notification from

as it is not clear, and some text is cut off (no option to expand) to the newer version (copy can be changed):

I will submit the related PRs for this proposed solutions and update the issue.

@sifat120
Copy link

Hi! Is this issue still available? I am willing to contribute to fix the issue.

@juancruzgs juancruzgs self-assigned this Jul 23, 2020
@juancruzgs
Copy link
Collaborator

Hi @sifat120 , I'm already working on it
Sorry that I forgot to change the assignee

@sifat120
Copy link

Oh that's ok. Thanks for letting me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants