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

[expo-notifications] Mixed notification icons on Android after udpate to SDK v39 #10469

Closed
ivansenic opened this issue Sep 30, 2020 · 35 comments · Fixed by #10492, #10471, expo/expo-cli#2837 or #10828

Comments

@ivansenic
Copy link

🐛 Bug Report

Summary of Issue

After updating to SDK v39 and using the next notifications API the Android notifications icons are not mixed in the way that status bar icon is the Expo icon and notification media image (or what ever the name is) is our app icon. The example shown below is the app screenshot from OnePlus 6 running Anroid 10. I don't have the information if the behavior is some on other Android phones (@Lorezz mentioned in #9062 that this issue is only on some Android OS versions).

Screenshot_20200926-095653__01.jpg

Environment - output of expo diagnostics & the platform(s) you're targeting

Our app.json config related to notifications is empty, while the android part looks like:

    "android": {
      ...
      "icon": "./assets/icon.png",
      "adaptiveIcon": {
        "foregroundImage": "./assets/android/icon.png",
        "backgroundColor": "#50dbb6"
      },
      "useNextNotificationsApi": true
    },

Expo diagnostics:

  Expo CLI 3.27.10 environment info:
    System:
      OS: Linux 5.4 Ubuntu 18.04.5 LTS (Bionic Beaver)
      Shell: 4.4.20 - /bin/bash
    Binaries:
      Node: 10.22.1 - /usr/bin/node
      Yarn: 1.22.5 - /usr/bin/yarn
      npm: 6.14.6 - /usr/bin/npm
    npmPackages:
      expo: ^39.0.0 => 39.0.3 
      react: 16.13.1 => 16.13.1 
      react-native: https://github.com/expo/react-native/archive/sdk-39.0.0.tar.gz => 0.63.2 
      react-navigation: ^4.4.0 => 4.4.0 
    npmGlobalPackages:
      expo-cli: 3.27.10
    Expo Workflow: managed

Reproducible Demo

I don't have the reproducible demo, as you need to build the app to expect the correct notifications icon.

Steps to Reproduce

Expected Behavior vs Actual Behavior

Both icons should be set correctly.

@lukmccall
Copy link
Contributor

Please correct me if I don't understand correctly, but in your case
image

  • the small icon (1) is expo log (but the color is set correctly)
  • the large icon (4) is set correctly
    and the expected behavior is to have those two icons the same?

Is it a built application? Is it working in the expo-client?

@lukmccall lukmccall added needs more info To be used when awaiting reporter response and removed needs validation Issue needs to be validated labels Sep 30, 2020
@ivansenic
Copy link
Author

@lukmccall

We just moved from legacy notifications to this. In legacy notifications we never had (4) set (as I remeber), as we are not setting explicitly the large icon. The small icon was correctly set before to what is now the large icon. Just moving to new API, without any other configuration, is producing the output as show above in issue description.

I am not sure if this is due to the not setting the icon explicitly in the notifications path of the app.json config? You said color is OK, we are also not setting the colour either in the config.

This is from the built app. Same behavior is in the expo-client as well.

Setting the color or icon in the notifications part of the app.json config seems not to work (at least in the expo-client where we tested this). If I set any of those, then the big icon (4) is gone, and small icon (1) is the same expo icon.

@viljark
Copy link

viljark commented Sep 30, 2020

Hi, I'm having similar issue in standalone build in android. Notification coloring and the big icon on the right are inconsistent.

image

The difference in my case is that I have notification.icon and notification.color configured in the app.json.
The big icon on the right is the icon I specified in the notification.icon and I can't change/remove it.

I described it in more detail here:

https://forums.expo.io/t/sdk-39-inconsistent-notification-ui-in-android-standalone-build/43295

@ivansenic
Copy link
Author

@viljark Can you share your app.json config related to the notifications? Also can you share the icon that you actually use for the notifications? I see in the expo reference that 96x96 PNG grayscale should be used for the notification, but this is not working for me? Maybe you can give me more advice on this please..

@lukmccall
Copy link
Contributor

Ohh so it's more complicated than I thought :/

In #10471, I've added a way to remove the large icon. If you provide the notifications section without the icon key, it should remove it.

Moreover, in my case, the small icon was always set to the app's icon (it's expected behavior).

Note: this doesn't work in expo-client.

Unfortunately, I don't have OnePlus 6 :/ but on the Samsung s8 it works fine. I'll test on the phone with Android 10 later.

@viljark
Copy link

viljark commented Sep 30, 2020

@ivansenic
I created a simple repo here: https://github.com/viljark/expo-notifications-10469, note that it requires google-services.json file for remote notifications in standalone build (it is not included in the repo, you must create your own in the Firebase Console)

I'm waiting for standalone build to finish so I can show the screenshots.

@lukmccall
Copy link
Contributor

@ivansenic, I assumed that you have the primaryColor key in your manifest. That's why I said the icon color was set correctly.

When you don't provide the custom one, the primaryColor will be applied to the notification icon.

lukmccall added a commit that referenced this issue Sep 30, 2020
…on icon (#10471)

# Why

Connected with #10469.

# How

We should fallback to the app icon only if the `iconUrl` is null or empty.

# Test Plan

- `NCL` with diffrent app.json configs ✅
@ivansenic
Copy link
Author

@ivansenic, I assumed that you have the primaryColor key in your manifest. That's why I said the icon color was set correctly.

When you don't provide the custom one, the primaryColor will be applied to the notification icon.

@lukmccall We do have primary color set to "primaryColor": "#50dbb6", but I don't think that the color is correct in the notification? Am i completely blind or?

@viljark
Copy link

viljark commented Sep 30, 2020

@lukmccall so here are the results when custom notification color and notification icon are provided.
I can provide the built .apk if needed, all other code (without the google-services.json) is available here: https://github.com/viljark/expo-notifications-10469

Remote and local notifications in Expo client:
Screenshot_20200930-145754 (1)

Remote and local notifications in standalone Android bundle:
Screenshot_20200930-161238 (1)

So my 2 issues are:

  1. In standalone bundle, the image on the right can not be removed (or changed to something other than notification.icon) when remote notification is displayed. I think using the notification.icon there is incorrect, as it is really low resolution and grayscale.
  2. In standalone bundle, the icon and text coloring is different for local and remote notifications.

@ivansenic
Copy link
Author

ivansenic commented Oct 1, 2020

@viljark Thanks so much for the effort here and the example project. I think I managed to figure out what's going on based on the settings in the app.json:

  1. notification part not set:
    • small icon will be expo one
    • big icon will be set as the app icon
  2. notification part set with either icon or color:
    • small icon will be expo one
    • big icon will not be set
  3. notification part set with both icon and color:
    • small icon will be the notification icon
    • big icon will be the notification icon as well

Do you see this as correct?

@lukmccall
Copy link
Contributor

lukmccall commented Oct 1, 2020

@ivansenic, the original icon color is blue 🤷🏼‍♂️ So I assumed it was changed correctly.

In standalone bundle, the image on the right can not be removed (or changed to something other than notification.icon) when remote notification is displayed.

It was fixed by 710776d

@viljark
Copy link

viljark commented Oct 1, 2020

@viljark Thanks so much for the effort here and the example project. I think I managed to figure out what's going on based on the settings in the app.json:

  1. notification part not set:
  • small icon will be expo one
  • big icon will be set as the app icon
  1. notification part set with either icon or color:

    • small icon will be expo one
    • big icon will not be set
  2. notification part set with both icon and color:

  • small icon will be the notification icon
  • big icon will be the notification icon as well

Do you see this as correct?

  1. yes, I get the same result. In SDK38, the small icon was also APP icon in this case, so is this regression?
    image (8)

  2. I have not tested

  3. yes, I get the same result

@Aryk
Copy link

Aryk commented Oct 1, 2020

@viljark Thanks so much for the effort here and the example project. I think I managed to figure out what's going on based on the settings in the app.json:

  1. notification part not set:

    • small icon will be expo one
    • big icon will be set as the app icon
  2. notification part set with either icon or color:

    • small icon will be expo one
    • big icon will not be set
  3. notification part set with both icon and color:

    • small icon will be the notification icon
    • big icon will be the notification icon as well

Do you see this as correct?

I'm also trying to fix the issue with the small Android icon showing the Expo icon. I don't have notification.icon and notification.color set. I'm worried about screwing up iOS notification icon which is working fine.

I also noticed that we cannot put "notification" keys under "ios" or "android" in app.json.

What is the recommended way of solving the small Expo icon on Android without having to set "notification.color" which might be different between iOS and Android?

@lukmccall
Copy link
Contributor

lukmccall commented Oct 2, 2020

@Aryk, I think that icon and color which are in the notification object, don't work on iOS. The notification icon is always set to the app icon.

To change the icon on iOS, you need to change the app icon and it's the only way to achieve this.

lukmccall added a commit that referenced this issue Oct 2, 2020
)

# Why

Fixes #10469.

# How

The `largeIcon` isn't set correctly. Moreover, for now, we don't have enough tools to config it properly. So we temporarily remove it. This functionality will be reimplemented in the future. 

# Test Plan

- NCL ✅
@lukmccall
Copy link
Contributor

Desired behavior:

  • iOS:
    • The small icon should always be the same as the app icon.
    • The color property shouldn't have any effect.
  • Android:
    • if notification.icon wasn't provided, the small icon should be set to the app icon.
    • if notification.color wasn't provided, the color should be set to primaryColor.
    • if only notification.icon was provided, the small icon should be set to the provided value, and the color should be set to blue.
    • if only notification.color was provided, the small icon should be set to the app icon.
    • if notification.color and notification.icon were provided, both values should be applied correctly.

The large icon was removed in the managed workflow. However, you can use it in the bare workflow. For more information, check https://github.com/expo/expo/blob/master/packages/expo-notifications/README.md#configure-for-android.

Note:

  • the small icon will be always set to the expo logo in the expo-client. However, the color property should work fine there.
  • if you change notification.icon you need to rebuild your app. Republishing your app won't have any effect in that case
  • the notification properties should be applied to remote and local notifications using the same rules.

@lukmccall
Copy link
Contributor

Unfortunately, I'm not sure if those fixes will help in all cases. So I reopen this issue. After the SDK 39.0.1 release, I'll come back to this and check if everything works fine.

@lukmccall lukmccall reopened this Oct 2, 2020
lukmccall added a commit that referenced this issue Oct 2, 2020
)

# Why

Fixes #10469.

# How

The `largeIcon` isn't set correctly. Moreover, for now, we don't have enough tools to config it properly. So we temporarily remove it. This functionality will be reimplemented in the future.

# Test Plan

- NCL ✅
# Conflicts:
#	packages/expo-notifications/CHANGELOG.md
lukmccall added a commit that referenced this issue Oct 2, 2020
)

# Why

Fixes #10469.

# How

The `largeIcon` isn't set correctly. Moreover, for now, we don't have enough tools to config it properly. So we temporarily remove it. This functionality will be reimplemented in the future.

# Test Plan

- NCL ✅
# Conflicts:
#	packages/expo-notifications/CHANGELOG.md
@Aryk
Copy link

Aryk commented Oct 2, 2020

@Aryk, I think that icon and color which are in the notification object, don't work on iOS. The notification icon is always set to the app icon.

To change the icon on iOS, you need to change the app icon and it's the only way to achieve this.

Can someone from the Expo team clarify this...could we also maybe add to the docs that the notification.icon and notification.color are android only even though they are not under the "android" part of the app.json?

https://docs.expo.io/versions/latest/config/app/#notification

@brentvatne can you confirm this?

@brentvatne
Copy link
Member

as far as i know this is correct, cc @cruzach

@brentvatne
Copy link
Member

brentvatne commented Oct 3, 2020

fixes for this have been deployed. more info at #10464 (comment)

@ivansenic
Copy link
Author

@brentvatne @lukmccall Followed the instructions, updated the app and build the new version (Android). The problem still persists on OnePlus 6 with Android 10. The small icon is still Expo's one, while the big icon is gone now. We have no notifications entry in the app.json..

@viljark I guess your problem is resolved or?

@viljark
Copy link

viljark commented Oct 6, 2020

@ivansenic yes, my problem is partially solved since I use notifications.icon, but the notifications.color is still not applied correctly to locally triggered notifications.

@summerkiflain
Copy link

Is it not possible to just show the default app icon for local or push notifications mentioned in expo.icon, it was working fine for me sdk38 with useNextNotificationsApi: true, I don't want to show mono-color app icon or expo icon to the user, just default app icon is good enough for me, I have not specified notifications key in app.json.

@esfxra
Copy link

esfxra commented Oct 16, 2020

Also experimenting issues with the notification icons on SDK39 and Android 11 (Pixel 3).

Initially, only the expo icon would appear with notifications.

I have set these 2 settings in app.json as recommended here:

notification

  • icon
  • color

The result is:
Screenshot_20201015-190028.png

@cruzach
Copy link
Contributor

cruzach commented Oct 16, 2020

According to Android, as of Android 5 you should:

Update or remove assets that involve color. The system ignores all non-alpha channels in action icons and in the main notification icon. You should assume that these icons will be alpha-only. The system draws notification icons in white and action icons in dark gray.

so no, it is not recommended that you use your app icon as the notification icon. If you do, then there's a good chance the OS will simply render a blank square or circle (see @diegoserranor 's image above).

It does look like there may have been a regression here, but for anyone needing a fix right now- I've confirmed that setting the notification.icon in app.json to an appropriate greyscale image works as expected on SDK 39. Maybe we can consider defaulting to taking your app icon and making it into a greyscale image, but I think for the vast majority of use cases you're better off explicitly creating an android notification icon that matches Google's recommended design.

Some things that do need to be addressed by us:

  • document these facts around android notification icons and make it clear that the notification.icon and .color apply only to android, not iOS (thanks @Aryk )
  • fix an issue where the notification.color is only being applied to remote push notifications, not local notifications

Edit- on second thought, I think it might be a good idea for us to default to a greyscale of your app icon by default

@summerkiflain
Copy link

@cruzach on sdk38 my colored app icon was showing fine in notifications, local and push notifications, its not about going against the recommendations, I'll definitely add those too, but whatever the case is standalone should never set the default notification icon to expo app icon, thats whats happening for me on sdk39, a sensible fallback would still be app's icon.

@cruzach
Copy link
Contributor

cruzach commented Oct 16, 2020

yeah i agree, I haven't figured out what's caused that behavior of falling back to the expo icon yet, that's definitely not what we want to be doing.

@esfxra
Copy link

esfxra commented Oct 16, 2020

Thank you for clarifying @cruzach. A greyscale with alpha did the trick.
Also, can confirm that notification.color is not applied to local notifications.

@ugurdnlr
Copy link

Screen Shot 2020-10-05 at 14 56 08

i have like this issue. i didn't any configiration to iOS but it is working correctly. display app icon but in android still display expo icon. when I added notification icon and color in app.json, icon becomes a gray square.

Screen Shot 2020-10-12 at 11 30 03 AM

@dariocostanzo
Copy link

Same here, since upgrading to expo 39 the notification icon is no longer the same as the app icon, but it is now using the expo icon instead, this is causing issues with my clients.

@cruzach
Copy link
Contributor

cruzach commented Oct 21, 2020

@dariocostanzo I shared a solution for now:

I've confirmed that setting the notification.icon in app.json to an appropriate greyscale image works as expected on SDK 39

@dariocostanzo
Copy link

@dariocostanzo I shared a solution for now:

I've confirmed that setting the notification.icon in app.json to an appropriate greyscale image works as expected on SDK 39

Yes, but I used to have the colorful app icon before, which came automatically without changing anything in the app.json (didn't have notification.icon) . Do you know how can I set it back to use the app icon? The client doesn't want a grayscale image unfortunately. Thanks

@cruzach
Copy link
Contributor

cruzach commented Oct 21, 2020

I can't really recommend a way to do that, as I said above

it is not recommended that you use your app icon as the notification icon. If you do, then there's a good chance the OS will simply render a blank square or circle

@dariocostanzo
Copy link

I can't really recommend a way to do that, as I said above

it is not recommended that you use your app icon as the notification icon. If you do, then there's a good chance the OS will simply render a blank square or circle

I understand that it is not recommended, but at least the option to set the app icon or whatever works should be left up to the developer, also it was working perfectly fine before.

@cruzach
Copy link
Contributor

cruzach commented Oct 22, 2020

Going to lock this thread so it's obvious for people coming here what the issues are & how to work around them:

The bug with expo-notifications (that is not present in the legacy Notifications module, which is still available if you'd like to fall back to that) is that notification.color is not applied to notifications. You can work around this by setting the color in your NotificationContentInput

There is also an issue that your notification icon no longer defaults to your app icon, but this can be worked around by setting the notification.icon attribute in app.json. We may be able to add in some handling to default to a greyscale of your app icon, but for now relying on notification.icon is your best best. Also, when creating that icon, we recommend following Googles guidelines (otherwise your app icon may just show up as a blank square).

@expo expo locked and limited conversation to collaborators Oct 22, 2020
@cruzach cruzach removed the needs more info To be used when awaiting reporter response label Oct 22, 2020
@cruzach cruzach closed this as completed Oct 30, 2020
@cruzach
Copy link
Contributor

cruzach commented Nov 2, 2020

This will be fixed in the next SDK release (SDK 40), but for now you can use the workarounds I stated above to set the proper notification icon

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.