SDK-5611: tvOS App Inbox Support#528
Open
reshab-code wants to merge 11 commits intofeature/tvos-supportfrom
Open
SDK-5611: tvOS App Inbox Support#528reshab-code wants to merge 11 commits intofeature/tvos-supportfrom
reshab-code wants to merge 11 commits intofeature/tvos-supportfrom
Conversation
Contributor
WalkthroughThis PR adds comprehensive Apple TV (tvOS) support to the CleverTap iOS SDK by enabling Inbox functionality, adding tvOS-specific UI components and layouts, implementing focus navigation and remote control handling, and creating a tvOS-compatible sample app with card-based navigation. Changes
Sequence DiagramsequenceDiagram
participant User as tvOS User
participant Cell as CTInboxBaseMessageCell
participant VC as CleverTapInboxViewController
participant Player as AVPlayerViewController
User->>Cell: Focus on video message
Cell->>Cell: didUpdateFocusInContext (animate scale)
User->>Cell: Press Select button
Cell->>Cell: bodyFocusButtonTapped
Cell->>VC: Post CLTAP_INBOX_VIDEO_PLAYER_REQUESTED_NOTIFICATION
VC->>VC: handleVideoPlayerRequested (notification received)
VC->>Player: Create AVPlayerViewController with mediaUrl
Player->>Player: Seek to currentTime (if provided)
Player->>Player: Begin playback
VC->>User: Present Player modally
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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 docstrings
🧪 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. |
Collaborator
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
akashvercetti
changed the title
![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: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CleverTapSDK.xcodeproj/project.pbxproj`:
- Around line 1024-1026: Remove the duplicate PBXFileReference for
CTCarouselMessageCell~tv.xib and its duplicate inclusion in the resources group:
locate the second entry named "CTCarouselMessageCell~tv.xib" (the
PBXFileReference block with id 58D237B32F6ACEC100063FB9) and delete that file
reference, then remove the duplicate reference from the resources group where
that same id is added (the duplicate resource entry around the resources list).
Keep the single original PBXFileReference (58D237B02F6AC54100063FB9) and ensure
only that id is present in the resources group to avoid duplicate project
entries.
In `@CleverTapSDK/Inbox/cells/CTCarouselImageMessageCell.m`:
- Around line 102-110: The shouldUpdateFocusInContext: implementation currently
blocks all left/right focus exits; update it so lateral presses only get
consumed when the carousel can move to a previous/next item and otherwise fall
back to the system focus engine. In shouldUpdateFocusInContext: use
context.nextFocusedView first (keep returning YES if it's a descendant), then
for UIFocusHeadingLeft/Right check whether there is an actual previous/next card
to focus/scroll to (the same condition used elsewhere to decide scrolling); if
such an item exists consume the event (return NO or handle scroll), but if there
is no previous/next item return YES so tvOS can move focus out of the carousel.
Ensure you reference and reuse the carousel’s existing checks for "has previous"
/ "has next" rather than unconditionally returning NO for left/right.
In `@CleverTapSDK/Inbox/cells/CTInboxBaseMessageCell.m`:
- Around line 77-82: The cell's primary-action target (bodyFocusButton -> action
bodyFocusButtonTapped) only special-cases video, so audio messages never start
when the remote's select/primary action is used; modify the primary-action
handling in CTInboxBaseMessageCell (bodyFocusButtonTapped / the primary action
path that currently defers to handleOnMessageTapGesture:) to detect audio
message types and start audio playback the same way video is handled (mirror the
video-start logic used elsewhere), or call the same audio-start routine that
handleOnMessageTapGesture: uses for taps; ensure bodyFocusButton's action covers
both video and audio message types so remote select will play audio from the
cell body.
- Around line 77-82: The bodyFocusButton created in CTInboxBaseMessageCell is
the tvOS focus target but never given accessibility metadata; update the cell
configuration (e.g., in the method that applies a message to the cell) to set
bodyFocusButton.isAccessibilityElement = YES and populate
bodyFocusButton.accessibilityLabel (and accessibilityValue if appropriate) from
the message's title/body summary, or alternatively expose the cell as a grouped
accessible container and move the accessibilityLabel to that container; ensure
changes reference CTInboxBaseMessageCell and the created bodyFocusButton (and
keep existing bodyFocusButtonTapped behavior intact).
In `@CleverTapSDK/Inbox/controllers/CleverTapInboxViewController.m`:
- Around line 548-568: The method handleVideoPlayerRequested: may pass a nil
NSURL to AVPlayer when mediaUrl is malformed; before calling [AVPlayer
playerWithURL:], create an NSURL from mediaUrl and guard it (e.g., NSURL *url =
[NSURL URLWithString:mediaUrl]; if (!url) return;), then use that url with
[AVPlayer playerWithURL:] and proceed with player setup and presentation; update
all references to use the validated NSURL to avoid undefined behavior.
In `@CleverTapSDK/Inbox/resources/CleverTapInboxViewController`~tv.xib:
- Around line 14-21: The tableView element currently uses autoresizingMask
attributes flexibleMaxX="YES" flexibleMaxY="YES" which only makes margins
flexible; update the <autoresizingMask> on the tableView in
CleverTapInboxViewController~tv.xib to use widthSizable="YES"
heightSizable="YES" (replace flexibleMaxX/flexibleMaxY) so the tableView
stretches full-screen; keep the existing tableView element and its outlets
(dataSource/delegate) unchanged.
In `@SwiftStarter/SwiftTvOS/ViewController.swift`:
- Around line 140-142: The recordEvent method is sending a misspelled event
name; in the recordEvent() function update the call to
CleverTap.sharedInstance()?.recordEvent("tesr") to use the correct event name
"test" (locate the recordEvent method and the
CleverTap.sharedInstance()?.recordEvent call and replace the string).
🪄 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: df602f24-56b8-4c9f-8c8a-13953e86ebd2
📒 Files selected for processing (26)
CleverTap-iOS-SDK.podspecCleverTapSDK.xcodeproj/project.pbxprojCleverTapSDK/CTConstants.hCleverTapSDK/CleverTap.hCleverTapSDK/Inbox/cells/CTCarouselImageMessageCell.mCleverTapSDK/Inbox/cells/CTCarouselMessageCell.mCleverTapSDK/Inbox/cells/CTInboxBaseMessageCell.mCleverTapSDK/Inbox/cells/CTInboxIconMessageCell.mCleverTapSDK/Inbox/cells/CTInboxSimpleMessageCell.mCleverTapSDK/Inbox/controllers/CleverTapInboxViewController.mCleverTapSDK/Inbox/models/CTInboxUtils.mCleverTapSDK/Inbox/resources/CTCarouselImageMessageCell~tv.xibCleverTapSDK/Inbox/resources/CTCarouselImageView~tv.xibCleverTapSDK/Inbox/resources/CTCarouselMessageCell~tv.xibCleverTapSDK/Inbox/resources/CTInboxIconMessageCell~tv.xibCleverTapSDK/Inbox/resources/CTInboxSimpleMessageCell~tv.xibCleverTapSDK/Inbox/resources/CleverTapInboxViewController~tv.xibCleverTapSDK/Inbox/views/CTInboxMessageActionView.mCleverTapSDK/Inbox/views/CTSwipeView.mCleverTapSDK/Inbox/views/UIView+CTToast.mSwiftStarter/SwiftStarter.xcodeproj/project.pbxprojSwiftStarter/SwiftTvOS/AppDelegate.swiftSwiftStarter/SwiftTvOS/CardModel.swiftSwiftStarter/SwiftTvOS/CardViewCell.swiftSwiftStarter/SwiftTvOS/Info.plistSwiftStarter/SwiftTvOS/ViewController.swift
💤 Files with no reviewable changes (1)
- CleverTapSDK/CleverTap.h
Comment on lines
+1024
to
+1026
| 58D237B02F6AC54100063FB9 /* CTCarouselMessageCell~tv.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = "CTCarouselMessageCell~tv.xib"; sourceTree = "<group>"; }; | ||
| 58D237B32F6ACEC100063FB9 /* CTCarouselMessageCell~tv.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = "CTCarouselMessageCell~tv.xib"; sourceTree = "<group>"; }; | ||
| 58D237B62F6AD23B00063FB9 /* CTCarouselImageView~tv.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = "CTCarouselImageView~tv.xib"; sourceTree = "<group>"; }; |
Contributor
There was a problem hiding this comment.
Remove duplicate tvOS CTCarouselMessageCell~tv.xib file reference.
Line 1025 introduces a second PBXFileReference for the same CTCarouselMessageCell~tv.xib path, and Line 1535 adds that duplicate into the resources group. This creates duplicate project entries and avoidable merge churn.
🛠️ Proposed cleanup
- 58D237B32F6ACEC100063FB9 /* CTCarouselMessageCell~tv.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = "CTCarouselMessageCell~tv.xib"; sourceTree = "<group>"; };
...
- 58D237B32F6ACEC100063FB9 /* CTCarouselMessageCell~tv.xib */,Also applies to: 1535-1536
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CleverTapSDK.xcodeproj/project.pbxproj` around lines 1024 - 1026, Remove the
duplicate PBXFileReference for CTCarouselMessageCell~tv.xib and its duplicate
inclusion in the resources group: locate the second entry named
"CTCarouselMessageCell~tv.xib" (the PBXFileReference block with id
58D237B32F6ACEC100063FB9) and delete that file reference, then remove the
duplicate reference from the resources group where that same id is added (the
duplicate resource entry around the resources list). Keep the single original
PBXFileReference (58D237B02F6AC54100063FB9) and ensure only that id is present
in the resources group to avoid duplicate project entries.
Comment on lines
+102
to
+110
| - (BOOL)shouldUpdateFocusInContext:(UIFocusUpdateContext *)context { | ||
| if ([context.nextFocusedView isDescendantOfView:self]) { | ||
| return YES; | ||
| } | ||
| UIFocusHeading heading = context.focusHeading; | ||
| if (heading == UIFocusHeadingLeft || heading == UIFocusHeadingRight) { | ||
| return NO; | ||
| } | ||
| return YES; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the CTCarouselImageMessageCell.m file
find . -name "CTCarouselImageMessageCell.m" -type fRepository: CleverTap/clevertap-ios-sdk
Length of output: 126
🏁 Script executed:
# Read the file to see the actual code at the specified lines
wc -l ./CleverTapSDK/Inbox/cells/CTCarouselImageMessageCell.mRepository: CleverTap/clevertap-ios-sdk
Length of output: 130
🏁 Script executed:
# Read the specific lines mentioned in the review
sed -n '95,136p' ./CleverTapSDK/Inbox/cells/CTCarouselImageMessageCell.m | cat -nRepository: CleverTap/clevertap-ios-sdk
Length of output: 1962
Let boundary arrows fall back to the focus engine.
Lines 107-108 block every left/right focus exit unconditionally, and lines 121 and 129 return even when there's no previous/next item to scroll to. On the first or last card, lateral presses are consumed locally instead of being handed to tvOS, preventing navigation out of the carousel. After applying the suggested fix, verify first/last-item navigation behavior in the tvOS simulator.
🎯 Suggested adjustment
- (BOOL)shouldUpdateFocusInContext:(UIFocusUpdateContext *)context {
if ([context.nextFocusedView isDescendantOfView:self]) {
return YES;
}
UIFocusHeading heading = context.focusHeading;
- if (heading == UIFocusHeadingLeft || heading == UIFocusHeadingRight) {
- return NO;
+ if (heading == UIFocusHeadingLeft) {
+ return self.swipeView.currentItemIndex <= 0;
+ }
+ if (heading == UIFocusHeadingRight) {
+ return self.swipeView.currentItemIndex >= (NSInteger)self.itemViews.count - 1;
}
return YES;
}
- (void)pressesBegan:(NSSet<UIPress *> *)presses withEvent:(UIPressesEvent *)event {
for (UIPress *press in presses) {
if (press.type == UIPressTypeLeftArrow) {
NSInteger prev = self.swipeView.currentItemIndex - 1;
if (prev >= 0) {
[self.swipeView scrollToItemAtIndex:prev duration:0.3];
self.pageControl.currentPage = (int)prev;
+ return;
}
- return;
}
if (press.type == UIPressTypeRightArrow) {
NSInteger next = self.swipeView.currentItemIndex + 1;
if (next < (NSInteger)self.itemViews.count) {
[self.swipeView scrollToItemAtIndex:next duration:0.3];
self.pageControl.currentPage = (int)next;
+ return;
}
- return;
}
}
[super pressesBegan:presses withEvent:event];
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CleverTapSDK/Inbox/cells/CTCarouselImageMessageCell.m` around lines 102 -
110, The shouldUpdateFocusInContext: implementation currently blocks all
left/right focus exits; update it so lateral presses only get consumed when the
carousel can move to a previous/next item and otherwise fall back to the system
focus engine. In shouldUpdateFocusInContext: use context.nextFocusedView first
(keep returning YES if it's a descendant), then for UIFocusHeadingLeft/Right
check whether there is an actual previous/next card to focus/scroll to (the same
condition used elsewhere to decide scrolling); if such an item exists consume
the event (return NO or handle scroll), but if there is no previous/next item
return YES so tvOS can move focus out of the carousel. Ensure you reference and
reuse the carousel’s existing checks for "has previous" / "has next" rather than
unconditionally returning NO for left/right.
Comment on lines
+77
to
+82
| self.bodyFocusButton = [UIButton buttonWithType:UIButtonTypeCustom]; | ||
| self.bodyFocusButton.backgroundColor = [UIColor clearColor]; | ||
| [self.contentView addSubview:self.bodyFocusButton]; | ||
| [self.bodyFocusButton addTarget:self | ||
| action:@selector(bodyFocusButtonTapped) | ||
| forControlEvents:UIControlEventPrimaryActionTriggered]; |
Contributor
There was a problem hiding this comment.
Handle audio in the tvOS primary-action path.
Lines 77-82 make bodyFocusButton the main focus target for the cell, but Lines 113-126 only special-case video. Audio messages fall through to handleOnMessageTapGesture: instead of starting playback, so the remote’s select action cannot play audio from the cell body.
🎵 One possible fix
- (void)bodyFocusButtonTapped {
CleverTapInboxMessageContent *content = self.message.content.firstObject;
+ if (content.mediaIsAudio && content.mediaUrl.length > 0) {
+ [self togglePlay];
+ return;
+ }
if (content.mediaIsVideo && content.mediaUrl.length > 0) {
CMTime currentTime = self.avPlayer ? self.avPlayer.currentTime : kCMTimeZero;
[self pause];
[[NSNotificationCenter defaultCenter]
postNotificationName:CLTAP_INBOX_VIDEO_PLAYER_REQUESTED_NOTIFICATIONAlso applies to: 111-126
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CleverTapSDK/Inbox/cells/CTInboxBaseMessageCell.m` around lines 77 - 82, The
cell's primary-action target (bodyFocusButton -> action bodyFocusButtonTapped)
only special-cases video, so audio messages never start when the remote's
select/primary action is used; modify the primary-action handling in
CTInboxBaseMessageCell (bodyFocusButtonTapped / the primary action path that
currently defers to handleOnMessageTapGesture:) to detect audio message types
and start audio playback the same way video is handled (mirror the video-start
logic used elsewhere), or call the same audio-start routine that
handleOnMessageTapGesture: uses for taps; ensure bodyFocusButton's action covers
both video and audio message types so remote select will play audio from the
cell body.
Label the tvOS overlay button for VoiceOver.
Because Lines 93-95 make the cell itself non-focusable, this code-created bodyFocusButton becomes the primary tvOS focus target. It never gets an accessibility label/value from the message, so VoiceOver users will land on an unlabeled button for every inbox card. Populate its accessibility metadata when the message is configured, or expose a grouped accessible container instead.
That XIB-based pattern does not cover this code-created overlay. Based on learnings: In CleverTap iOS SDK, accessibility element configuration for UI components like cellIcon in CTInboxIconMessageCell is handled in XIB files through Interface Builder rather than programmatically in code, so setting isAccessibilityElement = YES in code is not needed.
Also applies to: 93-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CleverTapSDK/Inbox/cells/CTInboxBaseMessageCell.m` around lines 77 - 82, The
bodyFocusButton created in CTInboxBaseMessageCell is the tvOS focus target but
never given accessibility metadata; update the cell configuration (e.g., in the
method that applies a message to the cell) to set
bodyFocusButton.isAccessibilityElement = YES and populate
bodyFocusButton.accessibilityLabel (and accessibilityValue if appropriate) from
the message's title/body summary, or alternatively expose the cell as a grouped
accessible container and move the accessibilityLabel to that container; ensure
changes reference CTInboxBaseMessageCell and the created bodyFocusButton (and
keep existing bodyFocusButtonTapped behavior intact).
Comment on lines
+548
to
+568
| #if TARGET_OS_TV | ||
| - (void)handleVideoPlayerRequested:(NSNotification *)notification { | ||
| NSString *mediaUrl = notification.userInfo[@"mediaUrl"]; | ||
| NSValue *currentTimeValue = notification.userInfo[@"currentTime"]; | ||
| NSString *title = notification.userInfo[@"title"]; | ||
| if (!mediaUrl) return; | ||
| AVPlayer *player = [AVPlayer playerWithURL:[NSURL URLWithString:mediaUrl]]; | ||
| AVPlayerViewController *playerVC = [[AVPlayerViewController alloc] init]; | ||
| playerVC.player = player; | ||
| playerVC.showsPlaybackControls = YES; | ||
| if (title.length > 0) { | ||
| playerVC.title = title; | ||
| } | ||
| [self presentViewController:playerVC animated:YES completion:^{ | ||
| if (currentTimeValue) { | ||
| CMTime seekTime = [currentTimeValue CMTimeValue]; | ||
| [player seekToTime:seekTime toleranceBefore:kCMTimeZero toleranceAfter:kCMTimeZero]; | ||
| } | ||
| [player play]; | ||
| }]; | ||
| } |
Contributor
There was a problem hiding this comment.
Add nil check for NSURL creation.
If mediaUrl is non-nil but malformed, [NSURL URLWithString:mediaUrl] returns nil, and passing nil to AVPlayer playerWithURL: may cause undefined behavior.
🛡️ Proposed fix
- (void)handleVideoPlayerRequested:(NSNotification *)notification {
NSString *mediaUrl = notification.userInfo[@"mediaUrl"];
NSValue *currentTimeValue = notification.userInfo[@"currentTime"];
NSString *title = notification.userInfo[@"title"];
if (!mediaUrl) return;
- AVPlayer *player = [AVPlayer playerWithURL:[NSURL URLWithString:mediaUrl]];
+ NSURL *url = [NSURL URLWithString:mediaUrl];
+ if (!url) return;
+ AVPlayer *player = [AVPlayer playerWithURL:url];
AVPlayerViewController *playerVC = [[AVPlayerViewController alloc] init];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CleverTapSDK/Inbox/controllers/CleverTapInboxViewController.m` around lines
548 - 568, The method handleVideoPlayerRequested: may pass a nil NSURL to
AVPlayer when mediaUrl is malformed; before calling [AVPlayer playerWithURL:],
create an NSURL from mediaUrl and guard it (e.g., NSURL *url = [NSURL
URLWithString:mediaUrl]; if (!url) return;), then use that url with [AVPlayer
playerWithURL:] and proceed with player setup and presentation; update all
references to use the validated NSURL to avoid undefined behavior.
Comment on lines
+14
to
+21
| <tableView clipsSubviews="YES" contentMode="scaleToFill" style="grouped" rowHeight="-1" estimatedRowHeight="-1" sectionHeaderHeight="1" sectionFooterHeight="6" id="Pal-ed-Io6"> | ||
| <rect key="frame" x="0.0" y="0.0" width="1920" height="1080"/> | ||
| <autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMaxY="YES"/> | ||
| <connections> | ||
| <outlet property="dataSource" destination="-1" id="mwF-we-s5N"/> | ||
| <outlet property="delegate" destination="-1" id="LpY-uj-7qh"/> | ||
| </connections> | ||
| </tableView> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other tvOS XIBs in this project configure autoresizing masks for root views
echo "=== Checking autoresizing masks in tvOS XIBs ==="
fd -e xib -p '~tv\.xib$' --exec grep -l "autoresizingMask" {} \; | while read -r file; do
echo "--- $file ---"
grep -A1 "autoresizingMask" "$file"
done
echo ""
echo "=== Checking if iOS version has different autoresizing ==="
fd -e xib -p 'CleverTapInboxViewController' --exec cat {} \;Repository: CleverTap/clevertap-ios-sdk
Length of output: 5628
Use widthSizable and heightSizable for full-screen table view.
The current autoresizing mask (flexibleMaxX, flexibleMaxY) keeps the view's dimensions constant and only makes margins flexible. For a full-screen table view that should stretch with its container, use widthSizable="YES" heightSizable="YES" instead. This pattern is already used in CTCarouselImageView~tv.xib for tvOS views in this codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CleverTapSDK/Inbox/resources/CleverTapInboxViewController`~tv.xib around
lines 14 - 21, The tableView element currently uses autoresizingMask attributes
flexibleMaxX="YES" flexibleMaxY="YES" which only makes margins flexible; update
the <autoresizingMask> on the tableView in CleverTapInboxViewController~tv.xib
to use widthSizable="YES" heightSizable="YES" (replace
flexibleMaxX/flexibleMaxY) so the tableView stretches full-screen; keep the
existing tableView element and its outlets (dataSource/delegate) unchanged.
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
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.
Summary
Changes
Testing
Summary by CodeRabbit