feat: push permission prompt #101

ami-aman · 2023-02-24T08:06:52Z

closes: https://github.com/customerio/issues/issues/8746
closes: https://github.com/customerio/issues/issues/9493

This PR adds the ability to display a push notification permission prompt in the app. This gives the user the freedom to display a prompt at a later time in the app, which was previously only possible at app launch or by requiring the customer to utilise additional libraries.

Use case:
Show push permission prompt by calling CustomerIO.showPromptForPushNotifications from the javascript with configurable options such as badge and sound.

var options = {"ios" : {"sound" : true}}
    CustomerIO.showPromptForPushNotifications(options).then(response => {
      alert("Permission Status -> " + response)
      console.log("Permission status is - ", response)
    }).catch(error => {
      alert("Failed to show push permission prompt.")
      console.log(error)
})

This PR also includes support for obtaining push permission authorisation status with a callback to perform action based on the response received. Call CustomerIO.getPushPermissionStatus to get the status.

Use case:

const getPushPermissionStatus = () => {
	CustomerIO.getPushPermissionStatus((response) => {
        performActionOnPushStatus(response)
}) }
        
const performActionOnPushStatus = (status) => {
 	alert("Push permission status is - " + status)
	console.log("Push permission status is - " + status)
}

…y been taken

github-actions · 2023-02-24T08:07:07Z

Pull request title looks good 👍!

If this pull request gets merged, it will cause a new release of the software. Example: If this project's latest release version is 1.0.0. If this pull request gets merged in, the next release of this project will be 1.1.0. This pull request is not a breaking change.

All merged pull requests will eventually get deployed. But some types of pull requests will trigger a deployment (such as features and bug fixes) while some pull requests will wait to get deployed until a later time.

This project uses a special format for pull requests titles. Expand this section to learn more (expand by clicking the ᐅ symbol on the left side of this sentence)...

This project uses a special format for pull requests titles. Don't worry, it's easy!

This pull request title should be in this format:

<type>: short description of change being made

If your pull request introduces breaking changes to the code, use this format:

<type>!: short description of breaking change

where <type> is one of the following:

feat: - A feature is being added or modified by this pull request. Use this if you made any changes to any of the features of the project.
fix: - A bug is being fixed by this pull request. Use this if you made any fixes to bugs in the project.
docs: - This pull request is making documentation changes, only.
refactor: - A change was made that doesn't fix a bug or add a feature.
test: - Adds missing tests or fixes broken tests.
style: - Changes that do not effect the code (whitespace, linting, formatting, semi-colons, etc)
perf: - Changes improve performance of the code.
build: - Changes to the build system (maven, npm, gulp, etc)
ci: - Changes to the CI build system (Travis, GitHub Actions, Circle, etc)
chore: - Other changes to project that don't modify source code or test files.
revert: - Reverts a previous commit that was made.

Examples:

feat: edit profile photo
refactor!: remove deprecated v1 endpoints
build: update npm dependencies
style: run formatter

Need more examples? Want to learn more about this format? Check out the official docs.

Note: If your pull request does multiple things such as adding a feature and makes changes to the CI server and fixes some bugs then you might want to consider splitting this pull request up into multiple smaller pull requests.

github-actions · 2023-02-24T09:17:16Z

Hey, there @ami-aman 👋🤖. I'm a bot here to help you.

⚠️ Pull requests into the branch main typically only allows changes with the types: fix. From the pull request title, the type of change this pull request is trying to complete is: feat. ⚠️

This pull request might still be allowed to be merged. However, you might want to consider make this pull request merge into a different branch other then main.

This project uses a special format for pull requests titles. Expand this section to learn more (expand by clicking the ᐅ symbol on the left side of this sentence)...

This project uses a special format for pull requests titles. Don't worry, it's easy!

This pull request title should be in this format:

<type>: short description of change being made

If your pull request introduces breaking changes to the code, use this format:

<type>!: short description of breaking change

where <type> is one of the following:

feat: - A feature is being added or modified by this pull request. Use this if you made any changes to any of the features of the project.
fix: - A bug is being fixed by this pull request. Use this if you made any fixes to bugs in the project.
docs: - This pull request is making documentation changes, only.
refactor: - A change was made that doesn't fix a bug or add a feature.
test: - Adds missing tests or fixes broken tests.
style: - Changes that do not effect the code (whitespace, linting, formatting, semi-colons, etc)
perf: - Changes improve performance of the code.
build: - Changes to the build system (maven, npm, gulp, etc)
ci: - Changes to the CI build system (Travis, GitHub Actions, Circle, etc)
chore: - Other changes to project that don't modify source code or test files.
revert: - Reverts a previous commit that was made.

Examples:

feat: edit profile photo
refactor!: remove deprecated v1 endpoints
build: update npm dependencies
style: run formatter

Need more examples? Want to learn more about this format? Check out the official docs.

Note: If your pull request does multiple things such as adding a feature and makes changes to the CI server and fixes some bugs then you might want to consider splitting this pull request up into multiple smaller pull requests.

ios/CustomerioReactnative.swift

levibostian · 2023-02-24T20:58:20Z

src/CustomerioTracking.tsx

+   * @param options
+   * @returns Success & Failure promises
+   */
+  static async showPromptForPushNotifications(options?: any) : Promise<any>{


Is there a reason why any is used for options parameter?

What about using a typescript interface instead?

That's a good point. I used any to prevent customer from importing another interface or config class. But this can be changed.

Ok, I can see how not needing to include another import could be nice. However, I believe that if a customer is using typescript in their project, they are OK with adding more imports for types in their code. Also, they probably prefer a strongly typed API when interacting with our SDK. With this in mind, I do believe it's preferred that we replace any with a strongly typed interface.

If we were to organize all of the interfaces in the project, I think the import statements for our SDK could look nice in a customer's app.

Here is an example of the imports could look like in a customer's app:

import type { PushPermissionOptions, Foo, Bar } from "customerio-reactnative/types" import { CustomerIOTracking } from "customerio-reactnative" ...

I do not see this as a blocker for this PR.

Small idea to share, I think changing the function name to something such as:

Suggested change

static async showPromptForPushNotifications(options?: any) : Promise<any>{

static async askForPushPermission(options?: any) : Promise<any>{

would match the function naming of getPushPermissionStatus.

This is a great suggestion. I will add this in a separate PR. I appreciate the suggestion.

I used "prompt" to make it specific that calling this method would show a prompt on the screen and not do something in the background.

I am OK with that while I am also OK with adding the changes to this PR.

I am OK with either name. I hope using the word "prompt" doesn't confuse anyone where there are some cases where a prompt will not show up when this function is called. That's one reason why I suggested the name change.

I tried implementing customerio-reactnative/types and found that it has to be a completely new library inside our customerio-reactnative with it's own package.json, index.ts and other important files. Since we are waiting to release this feature on priority, I do not want to take more time learning & implementing it at the moment. But I really loved the suggestion and will make sure to add it in the future updates. I would also like to add other configs like CioConfigOptions to /types so that the importing customerio-reactive could get reclustered as we release new features in future.

I still lean towards having promt in the function name as it makes things clear to me as a user. I discussed with Rehan and we both had same POV.

My code sample might have been bad as the solution I am thinking shouldn't be that complex. Here is a start to this work that I am testing now to see if we can sneak it into this release. I think our typescript customers will appreciate the strong types.

Good idea to ask others. If other people are behind it, I am for it 👍

src/CustomerioTracking.tsx

levibostian · 2023-02-28T19:48:04Z

src/CustomerioTracking.tsx

+   * @param options
+   * @returns Success & Failure promises
+   */
+  static async showPromptForPushNotifications(options?: any) : Promise<any>{


Small idea to share, I think changing the function name to something such as:

Suggested change

static async showPromptForPushNotifications(options?: any) : Promise<any>{

static async askForPushPermission(options?: any) : Promise<any>{

would match the function naming of getPushPermissionStatus.

ios/CustomerioReactnative.swift

This reverts commit 43b55a6.

levibostian · 2023-03-02T17:17:14Z

src/CustomerioTracking.tsx

+   * Get status of push permission for the app
+   * @returns Promise with status of push permission as a string
+   */
+  static getPushPermissionStatus() : Promise<any> {


Suggested change

static getPushPermissionStatus() : Promise<any> {

static getPushPermissionStatus() : Promise<PushPermissionStatus> {

Going along with the comment about a strongly typed parameter for showPromptForPushNotifications, we know the data type that is being returned from the Promise.

We should be providing customers with a data type return value that they can easily parse in their code.

ios/CustomerioReactnative.swift

mrehan27

LGTM 👍🏻

## [2.2.0](2.1.0...2.2.0) (2023-03-03) ### Features * push permission prompt ([#101](#101)) ([1abe9b3](1abe9b3))

ami-aman added 20 commits February 20, 2023 16:24

method to show push permissions prompt

05b5141

enum with possible authorization status

3ab0477

get status

f284358

check if prompt works from javascript

b54a680

function to show prompt later in app

7ed40a8

comments

98a9cc9

configurable options

3facddf

add sound & badge only if configured true

4c47773

handling promise

9261f8e

method with resolver and rejector promise

948aa94

shooting promises from native

8e6ccc5

completing callback

3fc5213

fixing minor issue

50dccfd

updating to not determined

8b563b2

send current status as string if action of push permission has alread…

a53d30d

…y been taken

modulated showPromptForPushNotifications

010ea97

added granted as push status

9446e0b

adding getPushPermissionStatus method

763ff12

callback method for status

42aa796

added method in js

2559412

ami-aman added 2 commits February 24, 2023 14:37

comments on js functions

bc3a5bb

comments

09538c6

ami-aman changed the title ~~Aman/push notifications~~ feat: push permission prompt Feb 24, 2023

Merge branch 'main' into aman/push-notifications

f2e187e

levibostian reviewed Feb 24, 2023

View reviewed changes

mrehan27 mentioned this pull request Feb 27, 2023

feat: push permission prompt [android] #104

Merged

levibostian reviewed Feb 28, 2023

View reviewed changes

levibostian approved these changes Feb 28, 2023

View reviewed changes

levibostian reviewed Feb 28, 2023

View reviewed changes

ios/CustomerioReactnative.swift Outdated Show resolved Hide resolved

feat: push permission prompt [android] (#104)

8abcb6e

ami-aman marked this pull request as ready for review March 1, 2023 16:50

ami-aman and others added 11 commits March 2, 2023 16:19

push config options

43b55a6

Revert "push config options"

7f63d0c

This reverts commit 43b55a6.

push config options

45d35d6

index file

805245a

removing push config

b11b6fc

simplifying push permission status

203efad

updated permission status handler to promise for android

4a2414f

removing callback, adding promise in js

2091998

callback to promise in ios

5a9bf2c

updating comment

91a245e

fixing objc error

f7e4845

levibostian reviewed Mar 2, 2023

View reviewed changes

levibostian suggested changes Mar 2, 2023

View reviewed changes

ios/CustomerioReactnative.swift Show resolved Hide resolved

ami-aman and others added 3 commits March 2, 2023 21:37

fix

0af13ec

not determined as default

13d2f5b

fix: replace any data types with strongly typed objects (#107)

bf640df

mrehan27 approved these changes Mar 3, 2023

View reviewed changes

mrehan27 merged commit 1abe9b3 into main Mar 3, 2023

mrehan27 deleted the aman/push-notifications branch March 3, 2023 12:28

github-actions bot pushed a commit that referenced this pull request Mar 3, 2023

chore: prepare for 2.2.0

662b480

## [2.2.0](2.1.0...2.2.0) (2023-03-03) ### Features * push permission prompt ([#101](#101)) ([1abe9b3](1abe9b3))

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: push permission prompt #101

feat: push permission prompt #101

ami-aman commented Feb 24, 2023 •

edited

Loading

github-actions bot commented Feb 24, 2023 •

edited

Loading

Examples:

github-actions bot commented Feb 24, 2023 •

edited

Loading

Examples:

levibostian Feb 24, 2023

ami-aman Feb 27, 2023

levibostian Feb 28, 2023

levibostian Feb 28, 2023

ami-aman Mar 1, 2023

levibostian Mar 1, 2023

ami-aman Mar 2, 2023

levibostian Mar 2, 2023

levibostian Feb 28, 2023

levibostian Mar 2, 2023

mrehan27 left a comment

	static async showPromptForPushNotifications(options?: any) : Promise<any>{
	static async askForPushPermission(options?: any) : Promise<any>{

	static getPushPermissionStatus() : Promise<any> {
	static getPushPermissionStatus() : Promise<PushPermissionStatus> {

feat: push permission prompt #101

feat: push permission prompt #101

Conversation

ami-aman commented Feb 24, 2023 • edited Loading

github-actions bot commented Feb 24, 2023 • edited Loading

Examples:

github-actions bot commented Feb 24, 2023 • edited Loading

Examples:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrehan27 left a comment

Choose a reason for hiding this comment

ami-aman commented Feb 24, 2023 •

edited

Loading

github-actions bot commented Feb 24, 2023 •

edited

Loading

github-actions bot commented Feb 24, 2023 •

edited

Loading