CTInAppHtmlBannerOverlay #841
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) ✅ code/snyk check is complete. No issues have been found. (View Details) |
|
Caution Review failedFailed to post review comments. WalkthroughThis change completely rewrites the in-app notification system, removing all legacy Java-based in-app notification classes and fragments and replacing them with a new Kotlin-based architecture. The new implementation introduces redesigned models, controllers, fragments, overlays, and supporting infrastructure for queuing, inflating, displaying, and handling user interactions with in-app notifications. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application/Activity
participant InAppController
participant InAppQueue
participant InAppNotificationInflater
participant Fragment/Overlay
participant InAppHost
participant Listener
App->>InAppController: onAppLaunch / onEvent / onProfileChange
InAppController->>InAppQueue: Enqueue eligible in-app notifications
App->>InAppController: showNotificationIfAvailable()
InAppController->>InAppQueue: Dequeue next notification
InAppController->>InAppNotificationInflater: Inflate notification (async)
InAppNotificationInflater-->>InAppController: Notification ready
InAppController->>Fragment/Overlay: Show notification UI
Fragment/Overlay->>InAppHost: User interacts (click, dismiss, etc.)
InAppHost->>Listener: Notify action/click/dismiss
InAppHost->>InAppController: Trigger next notification if needed
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppHtmlBannerOverlay.java (1)
161-169: Consider configuration changes and verify window type compatibilityThe overlay doesn't handle configuration changes (e.g., device rotation) which could cause display issues. Also,
TYPE_APPLICATION_PANELbehavior should be verified across Android versions.Consider implementing configuration change handling or documenting that the overlay should be dismissed on configuration changes. Also, please verify that
TYPE_APPLICATION_PANELworks correctly on your target Android versions:What are the restrictions and best practices for using WindowManager.LayoutParams.TYPE_APPLICATION_PANEL in Android 12 and above?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppHtmlBannerOverlay.java(1 hunks)clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.java(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CTLalit
PR: CleverTap/clevertap-android-sdk#802
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppNotification.java:180-182
Timestamp: 2025-05-08T08:31:50.340Z
Learning: The CTInAppNotification class in the CleverTap Android SDK adds new fields `aspectRatio` and `isRequestForPushPermission` to its Parcel implementation in version 7.4.0, but doesn't require parcel versioning as these parcels only exist in memory during a single app session.
Learnt from: CTLalit
PR: CleverTap/clevertap-android-sdk#802
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppWebView.kt:60-78
Timestamp: 2025-05-08T08:32:26.358Z
Learning: The CleverTap Android SDK has backend validation checks that ensure zero/negative dimensions don't occur when instantiating CTInAppWebView, making additional guard clauses unnecessary at this level.
Learnt from: CTLalit
PR: CleverTap/clevertap-android-sdk#797
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppNotification.java:184-185
Timestamp: 2025-04-28T08:41:36.134Z
Learning: In the CleverTap Android SDK, parcels in the CTInAppNotification class exist only in memory and are not persisted, so backward compatibility concerns between different SDK versions are not applicable for these parcels.
Learnt from: Anush-Shand
PR: CleverTap/clevertap-android-sdk#734
File: clevertap-pushtemplates/src/main/res/layout-v31/timer.xml:58-60
Timestamp: 2025-05-28T11:10:51.166Z
Learning: For API 31+ (Android 12+) in CleverTap push templates, action buttons are handled through official Android notification APIs rather than custom layout includes. The API 31+ layout versions intentionally omit @layout/action_buttons inclusions because the action buttons are managed by the Android system's notification builder APIs directly.
Learnt from: CTLalit
PR: CleverTap/clevertap-android-sdk#741
File: clevertap-core/src/main/java/com/clevertap/android/sdk/AnalyticsManager.java:1143-1143
Timestamp: 2025-01-24T11:56:39.526Z
Learning: Privacy and security related changes in the CleverTap Android SDK, such as PII exposure in logs, should be discussed with the team before implementation to ensure proper handling of sensitive data.
Learnt from: vasct
PR: CleverTap/clevertap-android-sdk#831
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/fragment/CTInAppNativeCoverImageFragment.kt:55-59
Timestamp: 2025-06-18T09:47:50.616Z
Learning: In CleverTap Android SDK in-app notification fragments, the `isHideCloseButton` flag has counterintuitive logic where `!isHideCloseButton` sets visibility to GONE and `isHideCloseButton` sets visibility to VISIBLE. This is intentional behavior preserved from the original Java implementation and should not be "fixed" during Kotlin conversion.
Learnt from: vasct
PR: CleverTap/clevertap-android-sdk#831
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/fragment/CTInAppBasePartialHtmlFragment.kt:118-120
Timestamp: 2025-06-18T09:52:22.861Z
Learning: The onTouch method in CTInAppBasePartialHtmlFragment intentionally returns `gd.onTouchEvent(event) || (event.action == MotionEvent.ACTION_MOVE)` to maintain existing Java behavior during Kotlin conversion, even though this might consume MOVE events that could affect WebView scrolling.
Learnt from: vasct
PR: CleverTap/clevertap-android-sdk#831
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/fragment/CTInAppBaseFullHtmlFragment.kt:71-79
Timestamp: 2025-06-18T12:06:46.032Z
Learning: The CTInAppWebView constructor sets the view ID internally, so there's no need to explicitly call View.generateViewId() when creating CTInAppWebView instances. The ID is available for use in layout rules like RIGHT_OF and ABOVE.
Learnt from: Anush-Shand
PR: CleverTap/clevertap-android-sdk#802
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.java:751-759
Timestamp: 2025-05-08T08:30:59.935Z
Learning: The `showInApp()` method in InAppController is always called from the main thread, even when triggered via task callbacks.
Learnt from: CTLalit
PR: CleverTap/clevertap-android-sdk#802
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppBaseFullHtmlFragment.java:44-48
Timestamp: 2025-05-08T08:25:14.692Z
Learning: In the CleverTap Android SDK, fragments are managed using add and remove methods rather than replace, so resources are properly cleaned up in onDestroyView() without needing additional cleanup in onDestroy().
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.java (9)
Learnt from: Anush-Shand
PR: CleverTap/clevertap-android-sdk#802
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.java:751-759
Timestamp: 2025-05-08T08:30:59.935Z
Learning: The `showInApp()` method in InAppController is always called from the main thread, even when triggered via task callbacks.
Learnt from: CTLalit
PR: CleverTap/clevertap-android-sdk#797
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppNotification.java:184-185
Timestamp: 2025-04-28T08:41:36.134Z
Learning: In the CleverTap Android SDK, parcels in the CTInAppNotification class exist only in memory and are not persisted, so backward compatibility concerns between different SDK versions are not applicable for these parcels.
Learnt from: vasct
PR: CleverTap/clevertap-android-sdk#831
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/fragment/CTInAppNativeCoverImageFragment.kt:55-59
Timestamp: 2025-06-18T09:47:50.616Z
Learning: In CleverTap Android SDK in-app notification fragments, the `isHideCloseButton` flag has counterintuitive logic where `!isHideCloseButton` sets visibility to GONE and `isHideCloseButton` sets visibility to VISIBLE. This is intentional behavior preserved from the original Java implementation and should not be "fixed" during Kotlin conversion.
Learnt from: CTLalit
PR: CleverTap/clevertap-android-sdk#802
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppNotification.java:180-182
Timestamp: 2025-05-08T08:31:50.340Z
Learning: The CTInAppNotification class in the CleverTap Android SDK adds new fields `aspectRatio` and `isRequestForPushPermission` to its Parcel implementation in version 7.4.0, but doesn't require parcel versioning as these parcels only exist in memory during a single app session.
Learnt from: CTLalit
PR: CleverTap/clevertap-android-sdk#802
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppWebView.kt:60-78
Timestamp: 2025-05-08T08:32:26.358Z
Learning: The CleverTap Android SDK has backend validation checks that ensure zero/negative dimensions don't occur when instantiating CTInAppWebView, making additional guard clauses unnecessary at this level.
Learnt from: vasct
PR: CleverTap/clevertap-android-sdk#831
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.kt:435-448
Timestamp: 2025-06-18T09:50:46.757Z
Learning: During Java to Kotlin conversions in the CleverTap Android SDK, vasct maintains the exact same logic for activity blacklist checks using `activity.getLocalClassName()`, even if there are potential improvements, to keep functional changes separate from conversion changes.
Learnt from: Anush-Shand
PR: CleverTap/clevertap-android-sdk#734
File: clevertap-pushtemplates/src/main/res/layout-v31/timer.xml:58-60
Timestamp: 2025-05-28T11:10:51.166Z
Learning: For API 31+ (Android 12+) in CleverTap push templates, action buttons are handled through official Android notification APIs rather than custom layout includes. The API 31+ layout versions intentionally omit @layout/action_buttons inclusions because the action buttons are managed by the Android system's notification builder APIs directly.
Learnt from: CTLalit
PR: CleverTap/clevertap-android-sdk#802
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppBaseFullHtmlFragment.java:44-48
Timestamp: 2025-05-08T08:25:14.692Z
Learning: In the CleverTap Android SDK, fragments are managed using add and remove methods rather than replace, so resources are properly cleaned up in onDestroyView() without needing additional cleanup in onDestroy().
Learnt from: vasct
PR: CleverTap/clevertap-android-sdk#831
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/fragment/CTInAppBasePartialHtmlFragment.kt:118-120
Timestamp: 2025-06-18T09:52:22.861Z
Learning: The onTouch method in CTInAppBasePartialHtmlFragment intentionally returns `gd.onTouchEvent(event) || (event.action == MotionEvent.ACTION_MOVE)` to maintain existing Java behavior during Kotlin conversion, even though this might consume MOVE events that could affect WebView scrolling.
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppHtmlBannerOverlay.java (11)
Learnt from: CTLalit
PR: CleverTap/clevertap-android-sdk#797
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppNotification.java:184-185
Timestamp: 2025-04-28T08:41:36.134Z
Learning: In the CleverTap Android SDK, parcels in the CTInAppNotification class exist only in memory and are not persisted, so backward compatibility concerns between different SDK versions are not applicable for these parcels.
Learnt from: CTLalit
PR: CleverTap/clevertap-android-sdk#802
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppNotification.java:180-182
Timestamp: 2025-05-08T08:31:50.340Z
Learning: The CTInAppNotification class in the CleverTap Android SDK adds new fields `aspectRatio` and `isRequestForPushPermission` to its Parcel implementation in version 7.4.0, but doesn't require parcel versioning as these parcels only exist in memory during a single app session.
Learnt from: Anush-Shand
PR: CleverTap/clevertap-android-sdk#734
File: clevertap-pushtemplates/src/main/res/layout-v31/timer.xml:58-60
Timestamp: 2025-05-28T11:10:51.166Z
Learning: For API 31+ (Android 12+) in CleverTap push templates, action buttons are handled through official Android notification APIs rather than custom layout includes. The API 31+ layout versions intentionally omit @layout/action_buttons inclusions because the action buttons are managed by the Android system's notification builder APIs directly.
Learnt from: vasct
PR: CleverTap/clevertap-android-sdk#831
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/fragment/CTInAppNativeCoverImageFragment.kt:55-59
Timestamp: 2025-06-18T09:47:50.616Z
Learning: In CleverTap Android SDK in-app notification fragments, the `isHideCloseButton` flag has counterintuitive logic where `!isHideCloseButton` sets visibility to GONE and `isHideCloseButton` sets visibility to VISIBLE. This is intentional behavior preserved from the original Java implementation and should not be "fixed" during Kotlin conversion.
Learnt from: CTLalit
PR: CleverTap/clevertap-android-sdk#802
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppWebView.kt:60-78
Timestamp: 2025-05-08T08:32:26.358Z
Learning: The CleverTap Android SDK has backend validation checks that ensure zero/negative dimensions don't occur when instantiating CTInAppWebView, making additional guard clauses unnecessary at this level.
Learnt from: CTLalit
PR: CleverTap/clevertap-android-sdk#802
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppBaseFullHtmlFragment.java:44-48
Timestamp: 2025-05-08T08:25:14.692Z
Learning: In the CleverTap Android SDK, fragments are managed using add and remove methods rather than replace, so resources are properly cleaned up in onDestroyView() without needing additional cleanup in onDestroy().
Learnt from: vasct
PR: CleverTap/clevertap-android-sdk#831
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.kt:435-448
Timestamp: 2025-06-18T09:50:46.757Z
Learning: During Java to Kotlin conversions in the CleverTap Android SDK, vasct maintains the exact same logic for activity blacklist checks using `activity.getLocalClassName()`, even if there are potential improvements, to keep functional changes separate from conversion changes.
Learnt from: vasct
PR: CleverTap/clevertap-android-sdk#831
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/fragment/CTInAppBasePartialHtmlFragment.kt:118-120
Timestamp: 2025-06-18T09:52:22.861Z
Learning: The onTouch method in CTInAppBasePartialHtmlFragment intentionally returns `gd.onTouchEvent(event) || (event.action == MotionEvent.ACTION_MOVE)` to maintain existing Java behavior during Kotlin conversion, even though this might consume MOVE events that could affect WebView scrolling.
Learnt from: CTLalit
PR: CleverTap/clevertap-android-sdk#741
File: clevertap-core/src/main/java/com/clevertap/android/sdk/AnalyticsManager.java:1143-1143
Timestamp: 2025-01-24T11:56:39.526Z
Learning: Privacy and security related changes in the CleverTap Android SDK, such as PII exposure in logs, should be discussed with the team before implementation to ensure proper handling of sensitive data.
Learnt from: vasct
PR: CleverTap/clevertap-android-sdk#831
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/fragment/CTInAppBaseFullHtmlFragment.kt:71-79
Timestamp: 2025-06-18T12:06:46.032Z
Learning: The CTInAppWebView constructor sets the view ID internally, so there's no need to explicitly call View.generateViewId() when creating CTInAppWebView instances. The ID is available for use in layout rules like RIGHT_OF and ABOVE.
Learnt from: vasct
PR: CleverTap/clevertap-android-sdk#785
File: clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/customtemplates/system/PushPermissionTemplate.kt:17-27
Timestamp: 2025-03-31T13:24:07.074Z
Learning: In template presenters for the CleverTap Android SDK, it's sometimes intentional to call both `setPresented()` and `setDismissed()` on the same template context. Specifically in PushPermissionTemplate, the presenter should always call `setDismissed()`, and additionally call `setPresented()` when the permission prompt is successfully launched.
🔇 Additional comments (4)
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.java (1)
797-811: Well-designed fallback mechanism for non-FragmentActivity contextsThe integration of the overlay-based display as a fallback is clean and maintains backward compatibility. This successfully enables HTML banner notifications in environments like Unreal that use NativeActivity instead of FragmentActivity.
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppHtmlBannerOverlay.java (3)
49-84: Clean API design for overlay-based banner displayThe class provides a minimal, well-documented API that clearly explains its purpose as a fragment-free alternative for HTML banners. The defensive handling in
display()whencanDisplay()returns false ensures proper cleanup.
221-231: Proper WebView cleanup implementationThe WebView cleanup is thorough and includes proper error handling. The null checks and exception handling ensure safe cleanup even in edge cases.
1-454: Ensure comprehensive testing of the overlay approachSince this introduces a new display mechanism for in-app notifications, please ensure thorough testing across:
- Different Android versions (especially Android 12+)
- App lifecycle events (backgrounding, foregrounding)
- Configuration changes (rotation, theme changes)
- Multiple overlay scenarios
- Memory leak detection using LeakCanary
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.java
Outdated
Show resolved
Hide resolved
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppHtmlBannerOverlay.java
Outdated
Show resolved
Hide resolved
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppHtmlBannerOverlay.java
Outdated
Show resolved
Hide resolved
…rs without needing a FragmentActivity
Convert CTInAppHtmlBannerOverlay to Kotlin Extract common logic from CTInAppBaseFragment in CTInAppHost and remove the fragment dependencies from CTInAppWebView and CTWebInterface Move InAppWebViewClient inside CTInAppWebView
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds
CTInAppHtmlBannerOverlayto handle custom-html header and footerCTInAppNotificationbanners when the current activity is not aFragmentActivity.This allows
CTInAppTypeFooterHTMLandCTInAppTypeHeaderHTMLnotifications to work on Unreal, which can't easily provide aFragmentActivity.Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
Documentation