Skip to content

refactor: [SDK-5661] extract IAM horizontal inset math and unit test it - #1791

Open
fadi-george wants to merge 1 commit into
fadi/sdk-5654from
fadi/sdk-5661
Open

fadi-george wants to merge 1 commit into
fadi/sdk-5654from
fadi/sdk-5661

Conversation

@fadi-george

Copy link
Copy Markdown
Collaborator

Description

One Line Summary

Move the IAM horizontal inset math into a pure OSInAppMessageInsets helper and unit test it.

Details

Motivation

Follow-up to #1789 review: horizontalInsets mixed window lookup, safe area, margin and iOS 27.1 reserved regions, so it was only verified by screenshot runs.

Scope

  • OSInAppMessageInsets takes margin, safeArea, width, region frames and isFullscreen, and returns the left/right insets. Same rules as before: each side is max(margin, safe area), full screen ignores the safe area and regions, and a region adds clearance only when it sits inside a side safe area and isn't flush with the edge.
  • OSInAppMessageViewController keeps the scene view and reserved-region lookup (still behind the local protocol shim for Xcode 15.2 builds) and passes plain CGRects in.
  • No layout changes.

Testing

Unit testing

OSInAppMessageInsetsTests (12 cases): max vs sum, with and without the width margin, full screen ignoring regions, regions inside, flush with and straddling a side safe area, left, right and both sides, and the 1pt tolerance. OneSignalInAppMessagesTests passes on an iOS 26.5 simulator (52 tests).

Manual testing

Not rerun. This is a pure refactor and the unit tests cover the math. The Duo capture comparison from #1789 can be rerun if wanted.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fadi-george
fadi-george added this pull request to stack #1792 November 3, 2026 20:44
@fadi-george
fadi-george requested review from abdulraqeeb33 and nan-li and removed request for abdulraqeeb33 November 3, 2026 20:44
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.

1 participant