-
Notifications
You must be signed in to change notification settings - Fork 38
AJO Optimize RN Package #177
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
Conversation
Updated Readme with following: 1. Installation instructions 2. APIs in AEPOptimize module
Added documentation on following public classes: - DecisionScope - Offer - Proposition
…optimize Merge the upstream
cacheung
left a comment
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.
Some feedback for Optimize.
| The Adobe Experience Platform Optimize extension has the following peer dependency, which must be installed prior to installing the optimize extension: | ||
| - [Core](../core/README.md) | ||
| - [Edge](../edge/README.md) | ||
| - [Edge Identity](../edgeidentity/README.md) |
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.
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.
Can we also register for iOS
[AEPMobileCore registerExtensions: @[AEPMobileEdge.class, AEPMobileEdgeIdentity.class, AEPMobileOptimize.class] completion:^
Register for Android
Identity.registerExtension();
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.
@dsoffiantini Can we add register AEPMobileEdgeIdentity.class in the sample code?
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.
Also add to Android, Identity.registerExtension();
Since EdgeIdentity is a dependency.
cacheung
left a comment
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.
"@adobe/react-native-aepcore": "^1.0.0",
"@adobe/react-native-aepedge": "^1.0.0",
"@adobe/react-native-aepedgeidentity": "^1.0.0",
| npm install {path to node package file} | ||
| ``` | ||
|
|
||
| **Podfile setup for `@adobe/react-native-aepmessaging` v1.0.0-beta.2 with in-app messaging support** |
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.
Podfile setup for @adobe/react-native-aepmessaging iOS SDK v1.0.0-beta.2 with in-app messaging support
This sounds clearer for iOS version?
packages/messaging/README.md
Outdated
|
|
||
| **Gradle setup for `@adobe/react-native-aepmessaging` v1.0.0-beta.2 with in-app messaging support** | ||
| AEPMessaging Android package v1.1.0 with in-app messaging support is not published to maven yet. In project level build.gradle file of Android project in your RN application add following changes in `allprojects -> repositories`. | ||
| **Gradle setup for `@adobe/react-native-aepmessaging` v1.2.0-beta.1 with in-app messaging support** |
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.
Gradle setup for @adobe/react-native-aepmessaging Android SDK v1.2.0-beta.1 with in-app messaging support
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 clear to me what user need to do for messaging extension by following the Readme.
Let revisit.
| ... | ||
| import android.app.Application; | ||
| ... |
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 here
| The Adobe Experience Platform Optimize extension has the following peer dependency, which must be installed prior to installing the optimize extension: | ||
| - [Core](../core/README.md) | ||
| - [Edge](../edge/README.md) | ||
| - [Edge Identity](../edgeidentity/README.md) |
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.
Can we also register for iOS
[AEPMobileCore registerExtensions: @[AEPMobileEdge.class, AEPMobileEdgeIdentity.class, AEPMobileOptimize.class] completion:^
Register for Android
Identity.registerExtension();
| @@ -1,7 +1,7 @@ | |||
|
|
|||
| # React Native Adobe Experience Platform Messaging Extension | |||
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.
nit: change all
javascript code block
to
typescript code block
| repositories { | ||
| google() | ||
| mavenCentral() | ||
| maven { url "https://oss.sonatype.org/content/repositories/snapshots/" } |
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.
We don't need this for Optimize right?
| repositories { | ||
| google() | ||
| mavenCentral() | ||
| maven { url "https://oss.sonatype.org/content/repositories/snapshots/" } |
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 we don't this for Optimize.
README.md
Outdated
| | [@adobe/react-native-aepmessaging](./packages/messaging) | [](https://www.npmjs.com/package/@adobe/react-native-aepmessaging) [](https://www.npmjs.com/package/@adobe/react-native-aepmessaging) | [Messaging](https://aep-sdks.gitbook.io/docs/beta/iam) | ||
| | [@adobe/react-native-aepassurance](./packages/assurance) | [](https://www.npmjs.com/package/@adobe/react-native-aepassurance) [](https://www.npmjs.com/package/@adobe/react-native-aepassurance) | [Assurance](https://aep-sdks.gitbook.io/docs/foundation-extensions/adobe-experience-platform-assurance) | ||
| | [@adobe/react-native-aepassurance](./packages/assurance) | [](https://www.npmjs.com/package/@adobe/react-native-aepassurance) [](https://www.npmjs.com/package/@adobe/react-native-aepassurance) | [Assurance](https://aep-sdks.gitbook.io/docs/foundation-extensions/adobe-experience-platform-assurance) | ||
| | [@adobe/react-native-aepoptimize](./packages/optimize) | [](https://www.npmjs.com/package/@adobe/react-native-aepoptimize)  | [Optimize](https://aep-sdks.gitbook.io/docs/using-mobile-extensions/adobe-journey-optimizer-decisioning) |
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.
(https://img.shields.io/npm/dm/@adobe/react-native-aepassurance)
The above URL should show the downloads for Assurance, can we correct it with theaepoptimize URL?
| "babel-preset-react-native": "5.0.2", | ||
| "jest": "^27.5.1", | ||
| "lerna": "^4.0.0", | ||
| "metro-react-native-babel-preset": "^0.70.1", |
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.
Can we move this metro dependency to the sample app level as it should only be used by the sample app?
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.
After changing this one, unit tests wont be able to run so it has to stay at this level
|
https://github.com/adobe/aepsdk-react-native/pull/177/files#diff-e7e2f2b83ea901f4ba22c4027cbee641a70899f27e9d03e5cf95b3cb3bf12acdR45 |
Good point for the description. We were providing a local build to the user which had to include Optimize and Messaging. So we have both Optimize and Messaging in this branch. We continued making all the changes in this PR after. @dsoffiantini Once this PR merges in to the Staging, we should also pull the staging down to Messaging branch to make it to the latest. |
apps/AEPSampleApp/package.json
Outdated
| "@adobe/react-native-aepedgeconsent": "^1.0.0", | ||
| "@adobe/react-native-aepedgeidentity": "^1.1.0", | ||
| "@adobe/react-native-aepmessaging": "^1.0.0-beta.2", | ||
| "@adobe/react-native-aepmessaging": "^1.0.0-beta.3", |
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.
Should we change it back to beta.2 since not releasing Messaging?
packages/messaging/README.md
Outdated
|
|
||
| **Installation instructions for `@adobe/react-native-aepmessaging` v1.0.0-beta.2 with in-app messaging support** | ||
| Download the node package `adobe-react-native-aepmessaging-1.0.0-beta.2.tgz` from [github release](https://github.com/adobe/aepsdk-react-native/releases/tag/%40adobe%2Freact-native-aepmessaging%401.0.0-beta.2) and save it in a folder. | ||
| **Installation instructions for `@adobe/react-native-aepmessaging` v1.0.0-beta.3 with in-app messaging support** |
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.
Change back to beta.2
packages/messaging/README.md
Outdated
| ```bash | ||
| cd MyReactApp | ||
| npm install {path to node package file} | ||
| npm install @adobe/react-native-aepmessaging |
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.
User should install from the instruction in prerelease md?
Do we want to user to use npm install @adobe/react-native-aepmessaging?
packages/messaging/README.md
Outdated
|
|
||
| **Podfile setup for `@adobe/react-native-aepmessaging` v1.0.0-beta.2 with in-app messaging support** | ||
| The In app Message APIs depends on the AEP Messaging, v1.1.0. This version is not yet published to the Cocoapods but is available in the public [github repository](https://github.com/adobe/aepsdk-messaging-ios/tree/staging). Add the following pod dependency in your applications Podfile under the application target. | ||
| **Podfile setup for `@adobe/react-native-aepmessaging` v1.0.0-beta.3 with in-app messaging support** |
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.
beta.2
packages/messaging/package.json
Outdated
| { | ||
| "name": "@adobe/react-native-aepmessaging", | ||
| "version": "1.0.0-beta.2", | ||
| "version": "1.0.0-beta.3", |
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.
Change back to 1.0.0-beta.2
cacheung
left a comment
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.
Looks good!!
Description
Contains the AJO Optimize RN package and sample app, as well as Messaging updates that enable In-App-Messaging functionality, and changes to the underlying packages that enable them now that optimize has been released officially.
Motivation and Context
Enables users to install the optimize sdk and use it on their react native apps, and sets them up to test IAM features if they want to
How Has This Been Tested?
Unit tests have been added to all new functionality
Types of changes
Checklist: