Skip to content

feat: improve notification icon fallback to check standard Android conventions (#976)#1038

Closed
franco-zalamena-iterable wants to merge 1 commit intomasterfrom
fix/976-notification-icon-fallback-conventions
Closed

feat: improve notification icon fallback to check standard Android conventions (#976)#1038
franco-zalamena-iterable wants to merge 1 commit intomasterfrom
fix/976-notification-icon-fallback-conventions

Conversation

@franco-zalamena-iterable
Copy link
Copy Markdown
Contributor

@franco-zalamena-iterable franco-zalamena-iterable commented Apr 7, 2026

Summary

Improves the notification icon fallback chain to check standard icon conventions before using the app launcher icon (which renders as a white square on Android 5.0+).

Changes

Updated IterableNotificationHelper.getIconId() fallback order:

  1. iterable_notification_icon resource (existing behavior, unchanged)
  2. Firebase com.google.firebase.messaging.default_notification_icon meta-data
  3. @drawable/notification_icon — Expo plugin and React Native SDK convention
  4. @drawable/ic_notification — common Android convention
  5. App launcher icon (last resort, existing behavior)

Motivation

Expo users who configure notification icons via app.json have @drawable/notification_icon generated automatically. Without this change, they see a white square unless they also manually add iterable_notification_icon to AndroidManifest.xml.

Test plan

  • Verify notifications use iterable_notification_icon when configured
  • Verify notifications use Firebase meta-data icon when configured
  • Verify notifications use @drawable/notification_icon when present
  • Verify notifications use @drawable/ic_notification when present
  • Verify fallback to launcher icon still works when none of the above are present

Made with Cursor

…nventions (#976)

Before falling back to the app launcher icon (which appears as white square
on Android 5.0+), check for standard notification icon locations:
1. Firebase default_notification_icon meta-data
2. @drawable/notification_icon (Expo/React Native convention)
3. @drawable/ic_notification (common Android convention)

This improves out-of-the-box experience for Expo and React Native users.

Made-with: Cursor
@franco-zalamena-iterable
Copy link
Copy Markdown
Contributor Author

PR Analysis

Problem: When iterable_notification_icon is not configured, the SDK falls back directly to the app launcher icon, which renders as a white square on Android 5.0+ because launcher icons are full-color rather than the required silhouette style. Expo/React Native users who already have proper notification icons at standard drawable resource names get no benefit from them.

Ideal fix plan:

  • Extend the fallback chain in getIconId() to probe well-known drawable resource names (notification_icon, ic_notification) and the Firebase meta-data key before falling back to the launcher icon
  • Add debug logging for each fallback step so developers can trace which icon was selected
  • Avoid duplicating the PackageManager.getApplicationInfo() call that already exists a few lines above -- ideally read meta-data once and check both the Iterable key and the Firebase key from the same Bundle
  • Add unit tests covering each fallback level (at minimum: Firebase meta-data present, notification_icon drawable present, ic_notification drawable present, none present -> launcher icon)

What the PR did:

  • Added three new fallback steps in exactly the order proposed in the issue (Firebase meta-data, @drawable/notification_icon, @drawable/ic_notification)
  • Added debug log lines for each new fallback
  • Updated Javadoc on getIconId() to document the full fallback chain
  • Did not add any tests
  • Did not consolidate the duplicate ApplicationInfo lookup

Assessment:

  • Root cause identified: yes -- the direct fall-through to the launcher icon with no intermediate checks is the cause
  • Fix correctness: correct -- the new fallback order is sensible, resource lookups use the existing patterns already in the codebase, and the original behavior is preserved as the final fallback
  • Missed:
    • Unit tests. The repo already has IterableNotificationTest.java which exercises icon resolution (it sets context.getApplicationInfo().icon). New tests covering each fallback step should be straightforward and would prevent regressions if the method is refactored later.
    • Minor duplication. The PackageManager.getApplicationInfo(..., GET_META_DATA) call is now made twice in getIconId() (once for the Iterable meta-data key, once for the Firebase key). These could be consolidated into a single lookup, reading both keys from the same info.metaData bundle. This is a nit, not a blocker.
  • Wrong assessment: none -- the approach matches the issue request and is a reasonable enhancement
  • Tests: needed but missing -- the existing test file demonstrates that icon resolution is testable in this codebase; coverage for the new fallback steps should be added

@franco-zalamena-iterable franco-zalamena-iterable deleted the fix/976-notification-icon-fallback-conventions branch April 8, 2026 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant