Skip to content

feat: Create Rokt.close method #373

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

Merged
merged 4 commits into from
Jul 20, 2025
Merged

Conversation

jamesnrokt
Copy link
Collaborator

@jamesnrokt jamesnrokt commented Jul 14, 2025

Instructions

  1. PR target branch should be against development
  2. PR title name should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-title-check.yml
  3. PR branch prefix should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-branch-check-name.yml

Summary

  • Create Rokt.close method and passthrough to kit
  • Changed app delegate key placeholders for easy selection
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-07-14.at.23.33.50.mp4

Testing Plan

  • Tested using the Example app and creating a new entry for auto close

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

@Copilot Copilot AI review requested due to automatic review settings July 14, 2025 13:32
@jamesnrokt jamesnrokt requested a review from a team as a code owner July 14, 2025 13:32
Copy link

@Copilot Copilot AI left a 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 adds a new close method to the Rokt integration for programmatic teardown of overlay placements, and updates the example app and placeholder keys accordingly.

  • Introduce - (void)close in MPRokt to forward a close call to attached kits
  • Expose the new close API in the public header with a doc comment
  • Update the Example app to demonstrate auto-close behavior and tweak key placeholders

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
mParticle-Apple-SDK/MPRokt.m Implemented -close to dispatch on main and forward to kits
mParticle-Apple-SDK/Include/MPRokt.h Added declaration and doc comment for -close
Example/mParticleExample/ViewController.m Added a cell and selector for auto-close scenario
Example/mParticleExample/AppDelegate.m Changed placeholder strings to use underscores
Comments suppressed due to low confidence (4)

mParticle-Apple-SDK/MPRokt.m:123

  • Add unit tests for the new -close method to verify it correctly forwards the SDK call and runs on the main thread.
- (void)close {

Example/mParticleExample/ViewController.m:82

  • [nitpick] The UI string uses inconsistent casing; consider changing (auto close) to (Auto Close) to match the capitalization style of other entries.
                    @"Set Session Attribute", @"Increment Session Attribute", @"Register Remote", @"Get Audience", @"Log Media Events", @"Toggle CCPA Consent", @"Toggle GDPR Consent", @"Request & Set IDFA", @"Logout", @"Login", @"Set IDFA", @"Decrease Upload Timer", @"Increase Upload Timer", @"Display Rokt Overlay Placement", @"Display Rokt Dark Mode Overlay Placement", @"Display Rokt Embedded Placement", @"Display Rokt Overlay (auto close)"];

mParticle-Apple-SDK/Include/MPRokt.h:113

  • Expand the doc comment for -close to clarify when it should be called (e.g., only after an overlay is displayed) and any threading guarantees or side effects.
 * Used to close Rokt overlay placements

Example/mParticleExample/ViewController.m:75

  • [nitpick] Maintaining parallel cellTitles and selectorNames arrays can be error-prone; consider defining a single model pairing titles with selectors to ensure consistency.
- (NSArray *)cellTitles {

thomson-t
thomson-t previously approved these changes Jul 14, 2025
Copy link
Collaborator

@BrandonStalnaker BrandonStalnaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jamesnrokt jamesnrokt merged commit cdc45bb into development Jul 20, 2025
21 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants