Skip to content

Support InboxStyle of Android notifications. #1323

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

Closed
wants to merge 1 commit into from

Conversation

aMarCruz
Copy link
Contributor

@aMarCruz aMarCruz commented Jul 19, 2018

This commit adds suport for the InboxStyle of Android notifications.

An overview is in the guides Create a Group of Notifications and Create an Expandable Notification of the Google Developers docs.

The commit includes Flow and Typescript definitions, but no test nor documentation.

The InboxStyle is supported from API level 16 and NotificationCompat 22.1.0.

@aMarCruz
Copy link
Contributor Author

@Salakar this is an example:

  // use constant ID for notification used as group summary
  const SUMMARY_ID = '12345';
  const CHANNEL_ID = 'com.android.example.channel';
  const GROUP_KEY = 'com.android.example.group';
  const STATUS_ICON = 'ic_notification';

  const newNotification1 = new firebase.notifications.Notification()
    .setTitle('Justin Rhyss')
    .setBody('You will not believe...')
    .android.setChannelId(CHANNEL_ID)
    .android.setSmallIcon(STATUS_ICON)
    .android.setGroup(GROUP_KEY);

  const newNotification2 = new firebase.notifications.Notification()
    .setTitle('Ali Connors')
    .setBody('Please join us to celebrate the...')
    .android.setChannelId(CHANNEL_ID)
    .android.setSmallIcon(STATUS_ICON)
    .android.setGroup(GROUP_KEY);

  const summaryNotification = new firebase.notifications.Notification()
    .setNotificationId(SUMMARY_ID)
    .setTitle('Email Summary')
    // set content text to support devices running API level < 24
    .setBody('Two new messages')
    .setSound('default')
    .android.setChannelId(CHANNEL_ID)
    .android.setSmallIcon(STATUS_ICON)
    // build summary info into InboxStyle template
    .android.setInboxStyle(
      [],
      '2 new messages',
      '[email protected]'
    )
    // specify which group this notification belongs to
    .android.setGroup(GROUP_KEY)
    // set this notification as the summary for the group
    .android.setGroupSummary(true);

  firebase.notifications().displayNotification(newNotification1);
  firebase.notifications().displayNotification(newNotification2);
  firebase.notifications().displayNotification(summaryNotification);

Collapsed output:
inboxstyle-collapsed

Expanded:
inboxstyle-expanded

The summary notification ID should stay the same so that it is only posted once, and so you can update it later if the summary information changes (subsequent additions to the group should result in updating the existing summary).

@Salakar
Copy link
Contributor

Salakar commented Jul 20, 2018

@aMarCruz this is a great start to adding support for this 🎉 thank you!

Given that there are multiple Notification styles that could potentially be supported in the future would it not make more sense to have a generic setStyle function that takes a class of a style, e.g. something like:

const SUMMARY_ID = '12345';
const STATUS_ICON = 'ic_notification';
const GROUP_KEY = 'com.android.example.group';
const CHANNEL_ID = 'com.android.example.channel';

const notification = new firebase.notifications.Notification();
const inboxStyle = new firebase.notifications.AndroidNotificationStyleInbox();

notification.setSound('default');
notification.setTitle('Email Summary');
notification.setBody('Two new messages');
notification.setNotificationId(SUMMARY_ID);

// inboxStyle.addLine('foo'); // lines.push()
// inboxStyle.addLine('barb'); // lines.push();
inboxStyle.setTitle('2 new messages'); // contentTitle
inboxStyle.setBody('[email protected]'); // summaryText

// Android specific
notification.android.setGroup(GROUP_KEY);
notification.android.setStyle(inboxStyle);
notification.android.setGroupSummary(true);
notification.android.setChannelId(CHANNEL_ID);
notification.android.setSmallIcon(STATUS_ICON);

When it serializes to send to/from native it could just add an extra property of style to the style object that can be switch cased on, e.g. style === 'inbox', so something like:

const style = {
  style: 'inbox',
  lines: [],
  title: '2 new messages', // contentTitle
  body: '[email protected]',  // summaryText
};

Thoughts?


Loving react-native-firebase and the support we provide? Please consider supporting us with any of the below:

@Salakar Salakar added the Workflow: Waiting for User Response Blocked waiting for user response. label Jul 20, 2018
@aMarCruz
Copy link
Contributor Author

aMarCruz commented Jul 20, 2018

@Salakar I agree.
Different interfaces for the types accepted by a single setStyle method could simplify the API.

bigPicture, bigText, and inbox can be implemented in this way now and deprecate the current setBigPicture and setBigText.

The rest (Messaging, Media, DecoratedCustom, DecoratedMediaCustomView) seem a bit more complex and could be implemented gradually in future versions, as needed.

@aMarCruz
Copy link
Contributor Author

@Salakar,
Thinking a bit more about this, I think that introducing a class for each style will make verbose the code and can complicate the API, rather than simplify it.

On the other hand, the use of classes would be very similar to the Android API, which can help RNFB make a better check and can be an advantage for those who are related to Android.

I don't know, but if only these 3 are implemented, including that of this PR, it could be handled by TS or Flow typings, as follows:

// Typescript
interface BaseStyle {
  style: 'bigPicture' |  'bigText' | 'inbox';
  title?: string;
  body?: string;
}
interface BigPicture extends BaseStyle {
  style: 'bigPicture';
  picture: string;
  largeIcon?: string;
}
interface BigText extends BaseStyle {
  style: 'bigText';
  text: string;
}
interface Inbox extends BaseStyle {
  style: 'inbox';
  lines: string[];
}
type NotificationStyle = BigPicture | BigText | Inbox;

//. . .
setStyle(style: NotificationStyle): Notification;

usage:

  const notification = new firebase.notifications.Notification()
    .setStyle({
      style: 'bigText',
      title: 'BigText Sample',
      picture: 'mypicture.png',
    })

@Salakar Salakar added the Workflow: Needs Review Pending feedback or review from a maintainer. label Jul 21, 2018
@Salakar
Copy link
Contributor

Salakar commented Jul 23, 2018

@aMarCruz thanks for your feedback and suggestions. I agree also that BigText and BigPicture styles could be moved into this and their existing setters and getters from the AndroidNotification class be removed.

I think that as it is an Android-only thing it's good that it's similar to how the Android API is, it's also similar to the rest of the notifications library and other modules in RNFB that use a builder approach anyway, e.g. Dynamic Links uses a builder-like API.

Additionally, having it class based also allows future proofing for when we handle the more complex ones that may require asynchronous calls over the react native bridge, e.g. MediaStyle.

You could have a BaseStyle class that they all extend from - just like you defined in the types?

There should also probably be a default style if none provided as well, I guess BigText?

@aMarCruz
Copy link
Contributor Author

aMarCruz commented Jul 25, 2018

@Salakar ,
I think for a developer focused on JS and minimal relation with Java, C#, etc, the use of a plain objects as parameters feels more natural, and it is likely that the high number of issues about notifications is due to the fact that most RN developers come from the JS world and have little or no experience with the Android API. But you are right,
Anyway...

You could have a BaseStyle class that they all extend from - just like you defined in the types?

Yes, of course.

There should also probably be a default style if none provided as well, I guess BigText?

I'm not sure I understand this (English is not my language).
The notifications have an implicit style defined by the theme and version of the SDK, the styles set by setStyle are additionals, so BaseStyle should be an abstract class without defaults.
We can differentiate the derived instances with instanceof or by means of a private property.

The plubic API, based on classes and close to the Android API, could look like this:

// Typescript - Android namespace
declare abstract class BaseStyle {
  readonly summary?: string | void;
  readonly title: string | void;
}
declare class BigPictureStyle extends BaseStyle {
  readonly picture: string;
  readonly largeIcon: string | void;
  setTitle(title: string): this;
  setSummary(body: string): this;
  setPicture(picture: String): this;  // required?
  setLargeIcon(icon: string): this;
}
declare class BigTextStyle extends BaseStyle {
  readonly text: string;
  setTitle(title: string): this;
  setSummary(body: string): this;
  setText(text: String): this;        // required
}
declare class InboxStyle extends BaseStyle {
  readonly lines: string[]; // there's no getLines() in Android InboxStyle
  setTitle(title: string): this;
  setSummary(body: string): this;
  addLine(line: String): this;
}
// As example, the MediaStyle is simple and RNFB can expose it, but
// an external library must provide the MediaSession.Token object.
declare class MediaStyle extends BaseStyle {
  setMediaSession(token: Object): this;
  setShowActionsInCompactView(...actions: number[]): this;
}
// MessagingStyle is complex and require additional classes
declare class Person {
  // ...
}
declare class Message {
  constructor(text: string, timestamp: Date, sender?: Person)
  // ...
}
declare class MessagingStyle extends BaseStyle {
  readonly historicMessages: Message[];
  readonly messages: Message[];
  readonly user: Person;
  readonly userDisplayName: string;
  readonly groupConversation: boolean;
  constructor(user: Person);
  setTitle(title: string): this;
  setGroupConversation(group: boolean): this;
  addMessage(text: String, timestamp?: Date, sender?: Person): this;
  addMessage(message: Message): this;
  addHistoricMessage(message): this;
  static Message: typeof Message;
}
// DecoratedMediaCustomViewStyle requires some methods not yet implemented in AndroidNotification.

// ...
setStyle(style: BaseStyle): Notification;

with your feedback, in the next few days I can submit a new PR with the implementation of the first 3 or 4 styles and MessagingStyle, if required, in a later PR.

@Salakar Salakar added Workflow: Waiting for User Response Blocked waiting for user response. and removed Workflow: Needs Review Pending feedback or review from a maintainer. Workflow: Waiting for User Response Blocked waiting for user response. labels Aug 2, 2018
@Salakar Salakar added this to the v5.0.0 Release milestone Aug 2, 2018
@stale
Copy link

stale bot commented Sep 2, 2018

This issue has been automatically marked as stale because it has not had recent user activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Type: Stale Issue has become stale - automatically added by Stale bot label Sep 2, 2018
@Salakar Salakar added the Workflow: Needs Review Pending feedback or review from a maintainer. label Sep 2, 2018
@stale stale bot removed the Type: Stale Issue has become stale - automatically added by Stale bot label Sep 2, 2018
@aMarCruz aMarCruz mentioned this pull request Sep 13, 2018
@Salakar Salakar modified the milestones: v5.0.0 Release, v6.0.0 Sep 20, 2018
@Salakar Salakar added the help: android Needs help implementing a PR relating to Android code. label Oct 23, 2018
@Salakar Salakar closed this Dec 14, 2018
@24dev
Copy link

24dev commented May 23, 2019

@Salakar @aMarCruz Was this implemented? I can't find it in the docs!

@Ehesp
Copy link
Member

Ehesp commented May 23, 2019

@AlcuinGuest this will be introduced in version 6 once we get around to notifications.

@eramudeep
Copy link

Hi, any update on MessagingStyle .?
whats the current status?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help: android Needs help implementing a PR relating to Android code. platform: android Workflow: Needs Review Pending feedback or review from a maintainer. Workflow: Waiting for User Response Blocked waiting for user response.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants