-
Notifications
You must be signed in to change notification settings - Fork 42
[SDK-303] Make the functionality for ios and android the same #800
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
base: loren/embedded/SDK-231-ios-add-click-handling-and-track
Are you sure you want to change the base?
Conversation
|
Coverage Impact ⬇️ Merging this pull request will decrease total coverage on 🛟 Help
|
1 new issue
|
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.
Pull request overview
This PR removes the getPlacementIds (Android-only) functionality from the embedded messaging API to align iOS and Android behavior, and updates the example app to use a text input for placement IDs instead.
Changes:
- Removed
getPlacementIdsmethod from TypeScript SDK and all Android native implementations - Updated example app to allow users to manually enter comma-separated placement IDs via text input
- Added warning UI in example app when embedded messaging is disabled
- Modified MainActivity.kt to set Iterable context and handle savedInstanceState differently
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/embedded/classes/IterableEmbeddedManager.ts | Removed getPlacementIds method from the embedded manager class |
| src/embedded/classes/IterableEmbeddedManager.test.ts | Removed test coverage for the removed getPlacementIds method |
| src/core/classes/IterableApi.ts | Removed static getEmbeddedPlacementIds method |
| src/api/NativeRNIterableAPI.ts | Removed getEmbeddedPlacementIds from the TurboModule interface |
| src/mocks/MockRNIterableAPI.ts | Removed mock implementation of getEmbeddedPlacementIds |
| example/src/hooks/useIterableApp.tsx | Removed duplicate enableEmbeddedMessaging configuration line |
| example/src/components/Embedded/Embedded.tsx | Replaced automatic placement ID fetching with manual text input and added warning banner |
| example/src/components/Embedded/Embedded.styles.ts | Added styles for text input, disabled state, and warning banner |
| example/android/app/src/main/java/iterable/reactnativesdk/example/MainActivity.kt | Added IterableApi.setContext call and modified onCreate to pass null to super |
| android/src/oldarch/java/com/RNIterableAPIModule.java | Removed getEmbeddedPlacementIds React method from old architecture module |
| android/src/newarch/java/com/RNIterableAPIModule.java | Removed getEmbeddedPlacementIds React method from new architecture module |
| android/src/main/java/com/iterable/reactnative/RNIterableAPIModuleImpl.java | Removed implementation of getEmbeddedPlacementIds |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const parsedPlacementIds = placementIdsInput | ||
| .split(',') | ||
| .map((id) => id.trim()) | ||
| .filter((id) => id !== '') | ||
| .map((id) => parseInt(id, 10)) | ||
| .filter((id) => !isNaN(id)); |
Copilot
AI
Jan 12, 2026
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.
The parsedPlacementIds array is recalculated on every render because it's not memoized. This could cause unnecessary re-renders and performance issues. Consider wrapping the parsing logic in useMemo to ensure it only recalculates when placementIdsInput changes.
| // Call super.onCreate with null to prevent savedInstanceState restoration issues | ||
| super.onCreate(null) |
Copilot
AI
Jan 12, 2026
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.
Passing null to super.onCreate instead of savedInstanceState prevents the restoration of the activity's saved state. This could affect the app's ability to restore its state after configuration changes (like screen rotation) or when the app is killed and restored by the system. While this is example code, it sets a poor pattern for developers who might copy this implementation. If this is necessary for the Iterable SDK to work correctly, it should be documented in the SDK's integration guide with an explanation of why it's needed and what trade-offs it entails.

🔹 JIRA Ticket(s) if any
✏️ Description
getPlacementsByIdcallTesting
Setup
yarn installyarn installbundle exec pod installwatchman watch-del-allyarn start --reset-cacheopen ReactNativeSdkExample.xcworkspace