New in-app type - PIP SDK-5617#969
Open
piyush-kukadiya wants to merge 77 commits intotask/release/8.1.0from
Open
New in-app type - PIP SDK-5617#969piyush-kukadiya wants to merge 77 commits intotask/release/8.1.0from
piyush-kukadiya wants to merge 77 commits intotask/release/8.1.0from
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds Picture-in-Picture (PIP) in-app notifications: new PIP types, config factory, UI (compact/expanded), media renderers (video/image/GIF), session/manager lifecycle, animations, drag/positioning, and InAppController integration plus tests and resources. Changes
Sequence Diagram(s)sequenceDiagram
participant Activity
participant InAppController
participant PIPConfigFactory
participant PIPManager
participant PIPSession
participant PIPRootContainer
participant PIPMediaView
participant MediaRenderer
Activity->>InAppController: showInApp(notification of type pip)
InAppController->>PIPConfigFactory: create(CTInAppNotification)
PIPConfigFactory-->>InAppController: PIPConfig or null
InAppController->>PIPManager: init(resourceProvider)
InAppController->>PIPManager: show(activity, PIPConfig)
PIPManager->>PIPSession: new PIPSession(config)
PIPManager->>PIPRootContainer: bindSession(PIPSession)
PIPRootContainer->>PIPMediaView: initialize(config, session)
PIPMediaView->>MediaRenderer: attach(container, config, session)
MediaRenderer-->>PIPMediaView: ready
PIPRootContainer->>PIPRootContainer: animateIn compact view (PIPAnimator)
sequenceDiagram
participant User
participant PIPCompactView
participant PIPDragHandler
participant PIPPositionResolver
participant PIPAnimator
participant PIPSession
User->>PIPCompactView: touch DOWN
PIPCompactView->>PIPDragHandler: onInterceptDown()
User->>PIPCompactView: MOVE...
PIPCompactView->>PIPDragHandler: shouldIntercept()
PIPDragHandler-->>PIPCompactView: start drag (threshold exceeded)
User->>PIPCompactView: UP
PIPDragHandler->>PIPPositionResolver: resolveAnchors(...)
PIPPositionResolver-->>PIPDragHandler: anchors
PIPDragHandler->>PIPPositionResolver: nearestPosition(...)
PIPPositionResolver-->>PIPDragHandler: PIPPosition
PIPDragHandler->>PIPAnimator: animateSnap(targetX,targetY)
PIPAnimator-->>PIPDragHandler: onComplete
PIPDragHandler->>PIPSession: currentPosition = resolved
sequenceDiagram
participant User
participant PIPCompactView
participant PIPRootContainer
participant PIPMediaView
participant MediaRenderer
participant PIPAnimator
User->>PIPCompactView: tap expand
PIPCompactView->>PIPRootContainer: expandToFull()
PIPRootContainer->>PIPMediaView: onContainerChanged()
PIPMediaView->>MediaRenderer: rebindSurface(...)
PIPRootContainer->>PIPAnimator: animateExpand(...)
PIPAnimator-->>PIPRootContainer: expand complete
User->>PIPRootContainer: back press
PIPRootContainer->>PIPAnimator: animateCollapse(...)
PIPAnimator-->>PIPRootContainer: collapse complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Contributor
Author
|
@coderabbitai update PR description |
Contributor
🧩 Analysis chain🏁 Script executed: Length of output: 44102
🏁 Script executed: Length of output: 544
📋 PR Description — click to expand## Summary
Introduces a new **Picture-in-Picture (PIP)** In-App notification type (`CTInAppTypePIP`) that supports Video (HLS/MP4), animated GIFs, and static images in a draggable, lifecycle-aware floating window.
---
## Motivation
Enables marketers to deliver rich, persistent media In-App messages that float over the host application as a compact PIP widget. Users can expand the widget to full-screen, interact via deep links, and continue navigating the app while the media plays.
---
## New PIP SDK — Architecture Overview
### Public API
| Component | Role |
|---|---|
| `PIPManager` | Singleton entry point — `show()`, `dismiss()`, lifecycle hooks |
| `PIPConfig` / `PIPConfig.Builder` | Immutable configuration (media, position, animation, controls) |
| `PIPCallbacks` | Listener interface for show / close / action / expand / collapse events |
| `PIPAnimation` | Enum: `DISSOLVE`, `SLIDE`, `INSTANT` |
| `PIPPosition` | 9-position enum (top/center/bottom × left/center/right) |
| `PIPMediaType` | `VIDEO`, `GIF`, `IMAGE` |
### Internal Engine
| Component | Role |
|---|---|
| `PIPAnimator` | Entry/exit animations (dissolve, slide with easing, instant) |
| `PIPDragHandler` | Touch intercept + snap-to-9-positions drag (configurable) |
| `PIPPositionResolver` | Maps position enum + margins to window coordinates |
| `PIPLifecycleObserver` | Activity lifecycle callbacks for auto-reattach/dismiss |
### Renderers
| Component | Role |
|---|---|
| `ImageRenderer` | Loads static bitmap with cache-first strategy |
| `GifRenderer` | Renders animated GIFs via `GifImageView`; replaces instance on reparent to avoid race conditions |
| `VideoRenderer` | ExoPlayer/Media3 HLS+MP4 playback; reports state via `RendererStateListener` |
| `PIPVideoPlayerWrapper` | Self-contained ExoPlayer management (surface attach/detach, mute, seek preservation) |
| `FallbackImageLoader` | Shared utility for loading fallback images when primary media fails |
### Views
| Component | Role |
|---|---|
| `PIPRootContainer` | Window-attached root; owns animation, drag, insets, and expand/collapse orchestration |
| `PIPCompactView` | Draggable floating widget with close, mute, expand, and deep-link controls |
| `PIPExpandedView` | Full-screen overlay with close, mute, play/pause, collapse, and deep-link controls |
| `PIPMediaView` | Hosts the active renderer; handles surface rebind on rotation |
| `PIPControlsOverlay` | Fade-in/out controls layer with touch intercept when hidden |
| `PIPIcons` | Centralised stateful icon mapping (play/pause, mute/unmute) |
### Session
| Component | Role |
|---|---|
| `PIPSession` | Mutable runtime state (playback position, mute, expanded flag, player reference) |
---
## Integration with CleverTap Core
- **`CTInAppType`** — Added `CTInAppTypePIP` variant.
- **`CTInAppNotification`** — Exposed PIP-specific fields from the in-app JSON payload.
- **`InAppController`** — Handles display and dismissal of the PIP type via `PIPManager`.
- **`PIPConfigFactory`** — Maps CT In-App JSON (media, position, animation, controls, buttons array) to `PIPConfig`.
- **`PIPInAppCallbacksBridge`** — Bridges `PIPCallbacks` to `InAppListener` (onShow, onClose, onAction with `CTInAppAction` dispatch; expand/collapse/playback/error are log-only).
- **`InAppResponse`** — Added debug injection flag to facilitate PIP testing.
- **`ExoplayerHandle` / `Media3Handle`** — Simplified to support full-container layout for PIP video.
---
## Key Features
- **Draggable compact window** snapping to 9 screen positions with configurable margins.
- **Expand/collapse transitions** with smooth animations between compact and full-screen modes.
- **State preservation** across Activity rotation: playback position, mute state, and expanded mode all survive `onDestroy`/`onCreate`.
- **Edge-to-edge insets** — compact view respects system bars and display cutouts; expanded controls avoid cutouts while keeping media edge-to-edge.
- **All in-app action types** supported via the standard `buttons` array (`OPEN_URL`, `KEY_VALUES`, `CUSTOM_CODE`, `CLOSE`).
- **Video-specific handling** — Instant animation override for entry/exit (SurfaceView ignores parent alpha); muted autoplay; `Player.REPEAT_MODE_ONE`.
- **Fallback to static image** when video playback fails; video controls are hidden automatically.
- **Drop-shadow elevation** on the compact floating window for visual depth.
- **Controls visibility** — touch events are blocked (not just invisible) when the controls overlay is hidden.
- **Rich animation config** — duration, easing curve, and move-in direction configurable from server JSON.
- **Configurable controls** — drag, play/pause, mute, and expand/collapse buttons can each be individually disabled from JSON.
---
## Bug Fixes
| Fix | Description |
|---|---|
| Memory leak — `ActivityLifecycleCallbacks` | Unregistered on PIP dismiss; re-registered on next `show()` |
| Race condition — first-frame callback | Synchronized with a shared lock; prevents permanent black scrim after rotation |
| ViewTreeObserver listener leak | Stored and removed in `detach()` as a safety net |
| Renderer async callbacks post-release | `@Volatile released` flag guards every `view.post` callback |
| ExoPlayer error listener leak | Stored and properly removed in `release()` |
| Video scrim stuck after rotation error | 3-second `postDelayed` safety net removes the scrim even if no first frame fires |
| Stale rotation state | Cleared at the start of `showInternal()` to prevent spurious reattach |
| `createSurface` re-entrance | `check()` guard prevents double-call without releasing the first `PlayerView` |
| Fallback image lost on rotation | `VideoRenderer` releases failed player in `loadFallbackAsImage()`; `PIPMediaView` detects null wrapper and reloads fallback |
| GIF crash on expand/collapse | Fresh `GifImageView` instance created after reparenting |
| Expanded state lost on rotation | `isExpanded` flag stored in `PIPSession` and restored on reattach |
| Height overflow in landscape | Clamped to 60% of container height (`MAX_HEIGHT_PERCENT`) |
| Border/corner rendering on SurfaceView | Removed entirely — SurfaceView renders in a separate hardware layer and ignores `clipToOutline` |
| Invisible controls still clickable | `onInterceptTouchEvent` blocks all touches when controls are hidden |
---
## Refactoring
- Extracted `FallbackImageLoader` to eliminate triplicated fallback-loading code across renderers.
- `PIPSession` changed from `data class` to regular class — removed unsafe auto-generated `copy()`/`equals()`/`hashCode()` for mutable fields.
- `VideoRenderer` decoupled from `PIPSession` via new `RendererStateListener` interface.
- `effectiveAnimConfig()` helper extracted in `PIPRootContainer` to DRY the video dissolve override.
- `FallbackLoadRequest` data class replaces 9 separate parameters across all three renderer callers.
- `PIPIcons` centralises stateful icon-to-drawable mapping shared between compact and expanded views.
- Magic numbers extracted to named constants in `PIPAnimator`, `PIPControlsOverlay`, `PIPCompactView`, and `PIPExpandedView`.
---
## Tests
- **`PIPConfigFactoryTest`** (23 tests) — null/invalid input handling, media type mapping, all 9 position strings, animation variants, config field extraction (margins, width, aspect ratio, controls), and builder validation failure.
- **`PIPInAppCallbacksBridgeTest`** (9 tests) — `onShow`/`onClose`/`onRedirect` forwarding, `onRedirect` action creation, and log-only callback verification.
---
## Vector Drawables Added
`ct_ic_close_pip`, `ct_ic_collapse`, `ct_ic_deeplink`, `ct_ic_expand`, `ct_ic_pause`, `ct_ic_play`, `ct_ic_volume_off_tint`, `ct_ic_volume_on_tint`I'll also trigger a regeneration of the AI-generated summary at the top of the PR. 🧠 Learnings used✅ Actions performedSummary regeneration triggered. |
Collaborator
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
CTLalit
reviewed
Mar 30, 2026
...rtap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/engine/Extensions.kt
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.kt`:
- Around line 975-978: The catch block in InAppController (the Throwable handler
that logs "Failed to show PIP in-app") only clears currentlyDisplayingInApp and
therefore stalls the in-app queue; update that catch to also advance the queue
by calling showNotificationIfAvailable() after setting currentlyDisplayingInApp
= null (same behavior as the pipConfig == null path) so the next queued item can
be processed.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/PIPInAppCallbacksBridge.kt`:
- Around line 26-53: The onAction() in PIPInAppCallbacksBridge must not infer
the tapped action via buttons.firstOrNull(); instead, locate onAction() in
PIPInAppCallbacksBridge and replace the current logic with a lookup of the PIP
click mapping produced by PIPConfigFactory (the pip.onClick.android → redirect
URL/action) to build the Action passed to
inAppListener.inAppNotificationActionTriggered, falling back to any explicit
button match only if necessary; ensure buttonless media taps create an Action
using the PIPConfigFactory redirect and multi-button payloads map the correct
CTA (include action.type and button text only when a real button was tapped),
then call inAppListener.inAppNotificationActionTriggered(inAppNotification,
resolvedAction, resolvedButtonTextOrNull, null, null).
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/engine/PIPDragHandler.kt`:
- Around line 42-49: Replace the hardcoded DRAG_THRESHOLD_PX with the platform
touch-slop value from ViewConfiguration to make drag detection density-aware:
retrieve ViewConfiguration.get(<the View or context>) .scaledTouchSlop (e.g.,
cache it as a touchSlop property in PIPDragHandler during construction or init)
and use that touchSlop instead of DRAG_THRESHOLD_PX in shouldIntercept and the
other drag-check sites (the other usages referenced by isDragging checks).
Remove or stop using DRAG_THRESHOLD_PX and update comparisons (dx > touchSlop ||
dy > touchSlop) so the class uses scaledTouchSlop for tap-vs-drag
discrimination.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/FallbackImageLoader.kt`:
- Around line 48-77: The load(...) path in FallbackImageLoader can post stale
work after a shared isReleased() flips back; capture a per-request generation
token at the start of FallbackImageLoader.load (e.g., a unique requestId or
generation from FallbackLoadRequest or by reading/setting a tag on container)
and carry it into the mediaExecutor and container.post closures, then before
mutating UI (inside container.post and before calling addFallbackImageView or
callbacks) compare the captured generation with the current generation on the
container (or request) and abort if they differ; keep the existing isReleased()
check but add this per-request generation equality check in both cached and
fetched branches to prevent overlaying stale fallback UI.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/ImageRenderer.kt`:
- Around line 47-56: The fallback path can run on a stale/detached ImageView and
crash when casting iv.parent to ViewGroup; inside the mediaExecutor.execute {
... } iv.post { ... } blocks (the one using
resourceProvider.fetchInAppImageV1(config.mediaUrl) and the similar block around
lines 85-97), before calling loadFallback(iv, config) or casting iv.parent,
check that the view is still attached and valid (e.g., iv.isAttachedToWindow or
iv.parent != null) and that released is false; if the view is detached or parent
is null, return early and do not call loadFallback or cast iv.parent to
ViewGroup to avoid the crash.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/VideoRenderer.kt`:
- Around line 55-65: When reusing an existing session.videoPlayerWrapper (and in
the other reuse sites), the old error-listener lambda (created by
setupErrorListener()) still closes over the previous container; fix by
removing/unregistering the existing player error listener on the reused wrapper
and then re-registering a fresh listener via setupErrorListener() after
assigning wrapper and rebinding the surface (i.e., after wrapper =
existingWrapper and after container.addView(surface)) so the new listener closes
over the active container/PIPMediaView; apply the same unregister-then-setup
pattern wherever the wrapper is reused (e.g., the other reuse blocks mentioned).
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPCompactView.kt`:
- Around line 59-64: Add static contentDescription labels for all clickable
ImageView controls in PIPCompactView (e.g., the deeplink button created as
dlBtn, the close/expand buttons at the other ImageView creation sites, and the
mute toggle) so TalkBack announces them; set contentDescription to appropriate
strings like "action", "close", "expand", and "mute" when creating each
ImageView (alongside setImageResource/visibility/scaleType/setOnClickListener).
Also update updateMuteIcon() to set the mute ImageView's contentDescription to
the current state (e.g., "mute" vs "unmute") whenever the icon is changed so the
label matches the icon state. Ensure you reference the existing ImageView
variables (dlBtn and the close/expand/mute ImageView instances) and modify their
creation and updateMuteIcon() accordingly.
- Around line 128-141: bindVideoControls moves deeplinkBtn to
Gravity.TOP|Gravity.START for video mode but hideVideoControls never restores
its original gravity, leaving the compact layout stuck in video arrangement;
update hideVideoControls to check deeplinkBtn and reset (it.layoutParams as
LayoutParams).gravity back to the original position (e.g., Gravity.CENTER or the
non-video gravity used elsewhere) and call requestLayout(), and also ensure the
same restore logic is applied where controls are removed (the counterpart code
around the other video-control teardown path referenced in the review).
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPExpandedView.kt`:
- Around line 91-97: The ImageView controls (closeBtn, actionBtn, collapseBtn,
playPauseBtn, muteBtn) are missing accessibility labels; set contentDescription
for each when creating them (e.g., context.getString(...) or hardcoded strings),
and ensure playPauseBtn and muteBtn update their contentDescription whenever
their icon/state toggles (update inside the methods that call setImageResource
or the click handlers/state setters that toggle play/pause and mute). Use
descriptive strings ("Close", "Action", "Collapse", "Play"/"Pause",
"Mute"/"Unmute") and keep the description in sync with the current
drawable/state for playPauseBtn and muteBtn. Make changes where these views are
initialized and where their state or image resource is changed so the
accessibility label always matches the visible icon.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPMediaView.kt`:
- Around line 95-109: The scrim safety timeout is not armed when handler is null
(view not yet attached), so the scrim may never be removed; change the
delayed-post call to use the View.postDelayed mechanism which queues before
attachment: in rebindSurface()/PIPMediaView where you create removeScrim and
call handler?.postDelayed(removeScrim, 3000L), replace that scheduling with
view.postDelayed(removeScrim, 3000L) (i.e., call postDelayed on the PIPMediaView
instance or the scrim view) while keeping the existing
wrapper.notifyWhenFirstFrame handling and handler?.removeCallbacks(removeScrim)
so the first-frame path still cancels the timeout.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/PIPConfig.kt`:
- Around line 10-41: Add an init block to PIPConfig that enforces the same
invariants as Builder.build(): validate mediaUrl is not blank, if fallbackUrl is
provided it is not blank, aspectRatioNumerator and aspectRatioDenominator are >
0, widthPercent is within a sane range (e.g. 1..100), and horizontalEdgeMarginDp
/ verticalEdgeMarginDp are >= 0; when any check fails throw an
IllegalArgumentException with a descriptive message. Place this init inside the
PIPConfig data class so copies created via copy() or direct construction are
validated (mirroring the checks in Builder.build()).
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/PIPManager.kt`:
- Around line 275-283: cleanupSession() currently releases the video player
twice (it calls s.videoPlayerWrapper?.release() and then calls
container.detach(releaseMedia = true) which will release the same wrapper); to
fix, match the safer ordering used in dismissInternal(): first detach the
pipRootContainer (container.detach(releaseMedia = true)), then release and null
the videoPlayerWrapper (or simply remove the direct release call and let detach
handle it), and ensure you null s.videoPlayerWrapper only after detach to avoid
double-release; update the cleanupSession() implementation (references:
cleanupSession(), s.videoPlayerWrapper, s.pipRootContainer, container.detach and
dismissInternal()) accordingly.
- Around line 138-142: The PIP lifecycle observer added in showInternal()
(PIPLifecycleObserver) is never removed, causing stale observers to call
cleanupSession() on a replaced session; fix by storing the lifecycleOwner and
the observer on the newSession object when you create them in showInternal(),
and then remove the observer via
lifecycleOwner.lifecycle.removeObserver(newSession.lifecycleObserver) (or
equivalent) before releasing resources inside both dismissInternal() and
cleanupSession() in PIPManager so the old observer cannot tear down the active
session.
In
`@clevertap-core/src/main/java/com/clevertap/android/sdk/response/InAppResponse.java`:
- Around line 33-34: The INJECT_TEST_PIP flag in InAppResponse is set to true
and injects a synthetic PIP into every server response; change this to disabled
for shipped builds by setting INJECT_TEST_PIP to false (or remove the mutation
path) and, if a QA hook is still needed, gate it behind a non-shipped mechanism
such as a debug/build-time flag or a local-only test helper (avoid hardcoded
true in the production code). Update the constant INJECT_TEST_PIP and any
branches in InAppResponse that rely on it (and the related code around lines
~152-163) so production releases cannot mutate real server responses.
In
`@clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/PIPInAppCallbacksBridgeTest.kt`:
- Around line 35-53: The tests call a removed method onRedirect; update them to
use the current bridge API by calling PIPInAppCallbacksBridge.onAction(...) with
an InAppAction that represents an open-url redirect (type
InAppActionType.OPEN_URL and payload/url set to "https://www.example.com"), then
verify mockListener.inAppNotificationActionTriggered(mockNotification, any(),
eq(url), isNull(), isNull()) and the second test should assert the passed action
has type == InAppActionType.OPEN_URL (use match { it.type ==
InAppActionType.OPEN_URL }) instead of calling onRedirect; reference
PIPInAppCallbacksBridge.onAction, InAppAction/InAppActionType, and
mockListener.inAppNotificationActionTriggered to locate where to change the
calls and assertions.
In `@sample/src/main/java/com/clevertap/demo/ui/main/HomeScreenViewModel.kt`:
- Around line 1172-1173: Uncomment the commented-out call to
cleverTapAPI?.variants() and the printVar("AB Variants", variants) lines inside
the printAbVariants() function so the method actually retrieves and prints A/B
test variants; locate the printAbVariants() method in HomeScreenViewModel.kt and
restore those two lines (cleverTapAPI?.variants() and printVar call) or, if you
prefer to remove the dead action, remove the menu mapping that invokes
printAbVariants() (menu item 12) so there's no non-functional menu entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f39696d5-f382-4265-a943-2a1ca7d4a711
📒 Files selected for processing (46)
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppNotification.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppType.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/PIPConfigFactory.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/PIPInAppCallbacksBridge.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/PIPAnimation.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/PIPAnimationConfig.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/PIPCallbacks.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/PIPConfig.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/PIPManager.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/PIPMediaType.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/PIPPosition.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/engine/Extensions.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/engine/PIPAnimator.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/engine/PIPDragHandler.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/engine/PIPPositionResolver.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/lifecycle/PIPLifecycleObserver.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/FallbackImageLoader.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/GifRenderer.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/ImageRenderer.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/MediaRenderer.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/PIPVideoPlayerWrapper.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/RendererStateListener.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/VideoRenderer.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/session/PIPSession.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPCompactView.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPControlsOverlay.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPDimens.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPExpandedView.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPIcons.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPMediaView.ktclevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPRootContainer.ktclevertap-core/src/main/java/com/clevertap/android/sdk/response/InAppResponse.javaclevertap-core/src/main/java/com/clevertap/android/sdk/video/inapps/ExoplayerHandle.ktclevertap-core/src/main/java/com/clevertap/android/sdk/video/inapps/Media3Handle.ktclevertap-core/src/main/res/drawable/ct_ic_close_pip.xmlclevertap-core/src/main/res/drawable/ct_ic_collapse.xmlclevertap-core/src/main/res/drawable/ct_ic_deeplink.xmlclevertap-core/src/main/res/drawable/ct_ic_expand.xmlclevertap-core/src/main/res/drawable/ct_ic_pause.xmlclevertap-core/src/main/res/drawable/ct_ic_play.xmlclevertap-core/src/main/res/drawable/ct_ic_volume_off_tint.xmlclevertap-core/src/main/res/drawable/ct_ic_volume_on_tint.xmlclevertap-core/src/test/java/com/clevertap/android/sdk/inapp/PIPConfigFactoryTest.ktclevertap-core/src/test/java/com/clevertap/android/sdk/inapp/PIPInAppCallbacksBridgeTest.ktsample/src/main/java/com/clevertap/demo/ui/main/HomeScreenViewModel.kt
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/InAppController.kt
Show resolved
Hide resolved
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/PIPInAppCallbacksBridge.kt
Show resolved
Hide resolved
...-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/engine/PIPDragHandler.kt
Show resolved
Hide resolved
...rc/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/FallbackImageLoader.kt
Show resolved
Hide resolved
...core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/ImageRenderer.kt
Show resolved
Hide resolved
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/PIPManager.kt
Show resolved
Hide resolved
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/PIPManager.kt
Outdated
Show resolved
Hide resolved
clevertap-core/src/main/java/com/clevertap/android/sdk/response/InAppResponse.java
Outdated
Show resolved
Hide resolved
clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/PIPInAppCallbacksBridgeTest.kt
Outdated
Show resolved
Hide resolved
sample/src/main/java/com/clevertap/demo/ui/main/HomeScreenViewModel.kt
Outdated
Show resolved
Hide resolved
CTLalit
reviewed
Mar 30, 2026
...p-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/GifRenderer.kt
Show resolved
Hide resolved
CTLalit
reviewed
Mar 30, 2026
|
|
||
| playerView = (LayoutInflater.from(context).inflate(R.layout.ct_exo_inapp_styled_player_view, null) as StyledPlayerView).apply { | ||
| playerViewLayoutParamsNormal = FrameLayout.LayoutParams(playerWidth, playerHeight) | ||
| playerViewLayoutParamsNormal = FrameLayout.LayoutParams(MATCH_PARENT, MATCH_PARENT) |
Collaborator
There was a problem hiding this comment.
this is the initial and full screen size params, ideally we should not change this and remove out full screen logic?
CTLalit
reviewed
Mar 30, 2026
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/PIPInAppCallbacksBridge.kt
Outdated
Show resolved
Hide resolved
CTLalit
reviewed
Mar 30, 2026
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/PIPConfigFactory.kt
Outdated
Show resolved
Hide resolved
CTLalit
reviewed
Mar 30, 2026
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/PIPConfigFactory.kt
Show resolved
Hide resolved
CTLalit
reviewed
Mar 30, 2026
Comment on lines
+1021
to
+1023
| private fun initPIPManagerIfNeeded() { | ||
| PIPManager.init(FileResourceProvider.getInstance(context, logger)) | ||
| } |
Collaborator
There was a problem hiding this comment.
difficult to UT this code?
Contributor
Author
There was a problem hiding this comment.
Yes, initPIPManagerIfNeeded() is hard to unit test directly because PIPManager is an object singleton. The PIP config parsing logic is tested via PIPConfigFactoryTest instead. Converting PIPManager from object to a class (for testability) is noted as a follow-up refactor.
Contributor
Author
There was a problem hiding this comment.
Updated — PIPManager is now a class injected into InAppController via constructor (390698d78), so it can be mocked in unit tests. The initPIPManagerIfNeeded() method is removed — PIPManager initializes its FileResourceProvider lazily on first show().
CTLalit
reviewed
Mar 30, 2026
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/CTInAppNotification.kt
Show resolved
Hide resolved
CTLalit
reviewed
Mar 30, 2026
| val showExpandCollapseButton: Boolean = true, | ||
|
|
||
| // Callbacks | ||
| val callbacks: PIPCallbacks? = null, |
Collaborator
There was a problem hiding this comment.
should be a data class and this should not be a part of PipConfig if this is server data class rep.
Contributor
Author
There was a problem hiding this comment.
PIPConfig is not a pure server DTO — it's a session configuration that bundles everything needed to show() a PIP (server data + client callbacks). Passing callbacks separately would split related data. This is a common Android SDK pattern (e.g., NotificationCompat.Builder bundles content + actions + listeners). If we want to separate concerns, we can discuss in a follow-up.
CTLalit
reviewed
Mar 30, 2026
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/PIPConfig.kt
Outdated
Show resolved
Hide resolved
CTLalit
reviewed
Mar 30, 2026
| internal object PIPManager { | ||
|
|
||
| @Volatile private var session: PIPSession? = null | ||
| private val mainHandler = Handler(Looper.getMainLooper()) |
CTLalit
reviewed
Mar 30, 2026
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/PIPManager.kt
Show resolved
Hide resolved
CTLalit
reviewed
Mar 30, 2026
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/PIPManager.kt
Outdated
Show resolved
Hide resolved
CTLalit
reviewed
Mar 30, 2026
| * PIPManager.show(activity, PIPConfig.builder(url, PIPMediaType.IMAGE).build()); | ||
| * ``` | ||
| */ | ||
| internal object PIPManager { |
Collaborator
There was a problem hiding this comment.
lets not make this object and have it as a class in scope of PIP display and release it once done? objects are hard for testing and synchronous stuff?
Contributor
Author
There was a problem hiding this comment.
Valid testability concern. Converting to a class would require wiring DI through InAppController and managing the instance lifecycle. For an internal SDK class with singleton semantics (only one PIP at a time), object is simpler. Noted as a follow-up refactor if we need better testability.
Contributor
Author
There was a problem hiding this comment.
Refactored in 390698d78 — converted PIPManager from object singleton to a regular class injected into InAppController via constructor. Uses a lazy lambda for FileResourceProvider to defer initialization until PIP is actually shown. This enables unit testing and proper scoping.
CTLalit
reviewed
Mar 30, 2026
| fun detachSurface() = renderer?.detachSurface() | ||
|
|
||
| /** Post-rotation: create new surface and re-bind existing player. */ | ||
| fun rebindSurface( |
Collaborator
There was a problem hiding this comment.
too complex and unreadable func.
Contributor
Author
There was a problem hiding this comment.
Refactored in ee6b51f18 — extracted reloadFallbackIfVideoFailed() and attachVideoScrim() from rebindSurface(). The main function is now ~10 lines. No behavior change.
CTLalit
reviewed
Mar 30, 2026
...-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPRootContainer.kt
Show resolved
Hide resolved
CTLalit
reviewed
Mar 30, 2026
|
|
||
| // ─── Helpers ────────────────────────────────────────────────────────────────── | ||
|
|
||
| private fun positionAndShow(s: PIPSession, cv: PIPCompactView, isReattach: Boolean) { |
Contributor
Author
There was a problem hiding this comment.
Refactored in db165ed49 — extracted createExpandedView(), createMediaView(), and createCompactView() from bindSession(). The main function is now ~20 lines.
CTLalit
reviewed
Mar 30, 2026
...-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPRootContainer.kt
Show resolved
Hide resolved
CTLalit
reviewed
Mar 30, 2026
| * Not a data class: mutable fields make copy()/equals()/hashCode() unsafe | ||
| * (shared videoPlayerWrapper reference, identity-based equality needed). | ||
| */ | ||
| internal class PIPSession( |
Collaborator
There was a problem hiding this comment.
InAppController -> PipManager -> holds all the data and handles all the pip feature?
is this more intuitive than the session?
Contributor
Author
There was a problem hiding this comment.
PIPSession groups all mutable runtime state for one PIP display (position, player, playback state, expanded flag, etc.). If PIPManager held these directly, session fields would mix with manager concerns (callback registration, rotation state, executor management). The separation makes it clear what belongs to "one PIP display" vs "the manager infrastructure". It also makes cleanup straightforward — nulling session drops all display state at once.
Base automatically changed from
task/SDK-5584/new_icons_inapp_media_support
to
task/SDK-5584/inapp_media_support
March 31, 2026 07:18
CTLalit
force-pushed
the
feat/pip/SDK-5617
branch
from
7e22a4d to
4298997
Compare
PIP action is now parsed from pip.onClick JSON via CTInAppAction.createFromJson() instead of buttons.firstOrNull(). This decouples PIP actions from the standard buttons array and uses a constant PIP_CTA for the action label. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cleanupSession() now delegates to dismissInternal(notifyCallback=false, animate=false) instead of duplicating cleanup logic. Added animate param to skip exit animation for non-interactive teardown. Player release uses view chain when container exists, falls back to direct release during rotation edge case. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Break rebindSurface() into reloadFallbackIfVideoFailed() and attachVideoScrim() for readability. No behavior change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract createExpandedView(), createMediaView(), createCompactView() from bindSession() for readability. Replace .parent as? ViewGroup casts with stored view references in expand/collapse transitions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace Java-style Builder with idiomatic Kotlin named arguments
(PIPConfig is internal with only Kotlin callers)
- Move validation from Builder.build() to PIPConfig init{} block
(now also covers copy())
- Remove redundant has() guards before optInt/optBoolean calls
- Fix PIPInAppCallbacksBridge tests to match actual onAction()
implementation (reads pipConfigJson.onClick, not buttons)
- Add missing contentDescription mock stub in PIPConfigFactoryTest
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Anush-Shand
reviewed
Apr 1, 2026
sample/src/main/java/com/clevertap/demo/ui/main/HomeScreenViewModel.kt
Outdated
Show resolved
Hide resolved
clevertap-core/src/main/java/com/clevertap/android/sdk/inapp/PIPConfigFactory.kt
Outdated
Show resolved
Hide resolved
Anush-Shand
reviewed
Apr 1, 2026
| */ | ||
| internal object PIPDimens { | ||
| /** Extra bottom offset (dp) to clear a typical bottom navigation bar (Material 3 = 80dp). */ | ||
| const val BOTTOM_NAV_OFFSET_DP = 80 |
Contributor
There was a problem hiding this comment.
Should this belong to the res folder?
Contributor
Author
There was a problem hiding this comment.
This constant is used only in programmatic layout calculations (PIPPositionResolver, PIPDragHandler), not in XML layouts. Moving it to res/values/dimens.xml would require passing Context to read it, adding overhead for a single constant that's only consumed in code. Keeping it as a Kotlin constant is simpler and avoids the resource lookup cost.
Contributor
There was a problem hiding this comment.
The value could/should be different on basis of tablet devices or wider devices or things like that?
Contributor
Author
There was a problem hiding this comment.
80dp is Material 3's standard NavigationBar height, which is consistent across phones and tablets. This isn't for the system nav bar (that's handled via WindowInsetsCompat insets) — it's an extra offset to avoid overlapping the app's bottom navigation bar. The value is a best-effort heuristic since we can't detect the app's bottom nav from the SDK. On tablets or wider devices, the snap grid still works correctly — the offset just adds extra bottom margin.
Anush-Shand
reviewed
Apr 1, 2026
...p-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPExpandedView.kt
Outdated
Show resolved
Hide resolved
Anush-Shand
reviewed
Apr 1, 2026
...p-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPExpandedView.kt
Show resolved
Hide resolved
Anush-Shand
reviewed
Apr 1, 2026
...p-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPExpandedView.kt
Show resolved
Hide resolved
Anush-Shand
reviewed
Apr 1, 2026
...p-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPExpandedView.kt
Show resolved
Hide resolved
Anush-Shand
reviewed
Apr 1, 2026
...p-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/GifRenderer.kt
Show resolved
Hide resolved
Anush-Shand
reviewed
Apr 1, 2026
...core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/renderer/MediaRenderer.kt
Show resolved
Hide resolved
Anush-Shand
reviewed
Apr 1, 2026
...ap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPCompactView.kt
Show resolved
Hide resolved
Anush-Shand
reviewed
Apr 1, 2026
...rtap-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPMediaView.kt
Outdated
Show resolved
Hide resolved
Anush-Shand
reviewed
Apr 1, 2026
| */ | ||
| internal class PIPVideoPlayerWrapper { | ||
|
|
||
| private var player: ExoPlayer? = null |
Contributor
There was a problem hiding this comment.
Do we not want support for the older exoplayer here?
What happens if someone adds the exoplayer deps and tries to use a PIP?
Contributor
Author
There was a problem hiding this comment.
Good catch — fixed in 097a4f3ce. PIPVideoPlayerWrapper uses Media3 APIs exclusively, but the check only rejected NONE, so old ExoPlayer would pass and crash at runtime. Now checks for MEDIA3 specifically — old ExoPlayer users get the static fallback image instead of a crash.
Contributor
There was a problem hiding this comment.
Do we wish to support Exoplayer in this version?
Since we have not marked it as deprecated yet, might be a problem @piyush-kukadiya
Contributor
Author
There was a problem hiding this comment.
No support for old Exoplayer.
Anush-Shand
reviewed
Apr 1, 2026
...-core/src/main/java/com/clevertap/android/sdk/inapp/pipsdk/internal/view/PIPRootContainer.kt
Show resolved
Hide resolved
Anush-Shand
approved these changes
Apr 1, 2026
Contributor
Anush-Shand
left a comment
Anush-Shand
left a comment
There was a problem hiding this comment.
Approved with minor comments
- Add cornerRadiusDp, borderEnabled, borderColor, borderWidthDp to PIPConfig - Parse pip.cornerRadius and pip.border JSON in PIPConfigFactory - Apply GradientDrawable with stroke + clipToOutline in PIPCompactView - Skip border styling for video (SurfaceView doesn't support clipToOutline) - Add padding to prevent media from covering the border stroke - Use Color.parseColor via toColorInt() with fallback to Color.BLACK - Add Robolectric runner to PIPConfigFactoryTest for Color support Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace hardcoded HlsMediaSource with DefaultMediaSourceFactory which auto-detects format (HLS, MP4, DASH, SmoothStreaming) from the URL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Change aspectRatioNumerator/Denominator from Int to Double to match server response format - Move border padding addition to after height clamping so media area keeps correct aspect ratio in both portrait and landscape Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Align with newer codebase convention of coding to the ILogger interface in PIPConfigFactory and PIPInAppCallbacksBridge. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
closeBtn, bottomRow, and rowSpacer are assigned exactly once in init{},
so lateinit var is unnecessary. Changed to val.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace magic number 3000L with SCRIM_SAFETY_TIMEOUT_MS constant in PIPMediaView. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PIPVideoPlayerWrapper uses Media3 APIs exclusively. The old check only rejected NONE, allowing old ExoPlayer to pass and crash at runtime. Now checks for MEDIA3 specifically — old ExoPlayer falls back to static image. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The variants() call and printVar() were commented out, making the menu item 12 a no-op in the sample app. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Enable unit testing by replacing the static `object PIPManager` with a regular class injected into InAppController via constructor. Uses a lazy lambda for FileResourceProvider (same pattern as InAppNotificationInflater) to defer the synchronized getInstance() call until PIP is actually shown, avoiding ANR risk at SDK init time. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Margins now scale proportionally with screen dimensions instead of using fixed dp values. Horizontal margin is a percentage of screen width, vertical margin is a percentage of screen height (default 3%). - Rename horizontalEdgeMarginDp/verticalEdgeMarginDp → horizontalEdgeMarginPercent/verticalEdgeMarginPercent - Add Int.percentOf(total) extension for idiomatic conversion - Remove margin validation constraints to allow negative values (compensate bottom nav offset) - Update PIPConfigFactory, PIPRootContainer, PIPDragHandler, PIPCompactView Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Summary by CodeRabbit
New Features
Tests