-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Added Channel Group #2607
base: master
Are you sure you want to change the base?
Added Channel Group #2607
Conversation
Added group functionality
with createChannelGroup,deleteChannelGroup,ListChannelGroup createChannel can add channel group created and listChannel can show the channel group that channel belong to
Thanks for contributing! Can you update the type definitions to reflect on those new APIs and attributes? |
What is it,is it the API.md |
It's the https://github.com/phonegap/phonegap-plugin-push/blob/master/types/index.d.ts file. That typescript "interface descriptor" needs to reflect the new APIs you're adding, so that the project compiles for the users that consume our API through a typescript project. |
Also added comments and clean up code to match the coding style
There you have it |
types/index.d.ts
Outdated
/** | ||
* Set notitfication visibility | ||
*/ | ||
visibility: integer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same integer
vs number
comment from above.
types/index.d.ts
Outdated
/** | ||
* Set Channel Group Name | ||
*/ | ||
name: string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same "can these be optional" suggestion applies to these attributes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not optional
*/ | ||
id: string | ||
/** | ||
* camillebeaumont name and description fix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these notes to fix later? We don't have the name
attribute yet, so we don't have an official doc to copy from, so it would make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup based on a previous pull request
Co-Authored-By: OliverOng1995 <[email protected]>
Co-Authored-By: OliverOng1995 <[email protected]>
Co-Authored-By: OliverOng1995 <[email protected]>
Co-Authored-By: OliverOng1995 <[email protected]>
Co-Authored-By: OliverOng1995 <[email protected]>
Co-Authored-By: OliverOng1995 <[email protected]>
Sorry I am not familiar with typescript variable,sorry to inconvenience you with my new features that I want to add on.I thought it seemed like javascript |
Co-Authored-By: OliverOng1995 <[email protected]>
Co-Authored-By: OliverOng1995 <[email protected]>
Co-Authored-By: OliverOng1995 <[email protected]>
Co-Authored-By: OliverOng1995 <[email protected]>
Co-Authored-By: OliverOng1995 <[email protected]>
Co-Authored-By: OliverOng1995 <[email protected]>
Co-Authored-By: OliverOng1995 <[email protected]>
Not at all an inconvenience! Please pat yourself on the back, you're doing all of us a big favor. You even walked the extra mile to document/type missing parts that weren't even related to your additions to the API, which is awesome. I won't bother you with some of the other missing parts I noticed, as they're definitely not related to your PR, and I'll work on them later myself. |
The type definitions, as they are right now, are fine. Missing bits will be considered my responsibility soon. @macdonst or someone else should review the native part soon. |
Description
In Android O,Push Channel also have group to categorize notification channel such as Work and Personal Notifications
Motivation and Context
It can help developer to assign notification channel to a group,easily categorizing it.
Which help user experience better.
Example use cases are chat app between Personal and Business account messages notifications
How Has This Been Tested?
Using Outsystem,I use the Firebase Push Plugin and modified the source code and the wrapper on the plugin on Outsystem itself.Also reference the source code instead of the original author git with the channels related function in the latest version of this plugin.
Also tested on Mi Max on lineageOS 15.1 and Mi A1 on Android 8.0 for creation,listing and deletion of channel.Setting and listing channels with the group
Types of changes
Checklist: