fix(native-view): resolve nested view content loss after rotation#110
Merged
kherembourg merged 2 commits intomainfrom May 4, 2026
Merged
fix(native-view): resolve nested view content loss after rotation#110kherembourg merged 2 commits intomainfrom
kherembourg merged 2 commits intomainfrom
Conversation
…ce rotation The nested Purchasely view (PLYPresentationView) lost all content when switching tabs (Annual/Monthly) after a device orientation change. Root cause: the native UIViewController was embedded as a bare UIView without proper view controller containment. This meant it never received `viewWillTransition(to:with:)` callbacks, so the SDK's internal layout (webview/collection view) kept stale dimensions and rendered empty content on tab switch. Fix (iOS): - Wrap the controller's view in a NativeContainerView that forces child relayout on bounds changes - Set autoresizingMask for flexible width/height - Add proper VC containment (addChild/didMove) so the controller receives lifecycle events - Observe UIDevice.orientationDidChangeNotification and explicitly call viewWillTransition(to:with:) with a NoAnimationTransitionCoordinator - Add proper cleanup in deinit (removeFromParent, removeObserver) Fix (Android): - Implement dispose() to call removeAllViews() instead of no-op - Fix misleading log message on fallback path Fix (Dart): - Replace hardcoded TextDirection.ltr with Directionality.of(context) for RTL support - Remove debug print statements Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughMultiple platform layers (iOS, Android, Flutter) updated to improve native view container lifecycle management and orientation handling. iOS now properly manages UIViewController containment with automatic frame synchronization and orientation observers. Android disposal improved. Flutter layout direction now respects app directionality context instead of hardcoding left-to-right. Changes
Sequence Diagram(s)sequenceDiagram
actor System as System Orientation Change
participant Flutter as Flutter Platform View
participant Native as NativeView Controller
participant Container as NativeContainerView
participant Child as UIViewController (Child)
System->>Native: UIDevice.orientationDidChangeNotification
Native->>Native: Delay & execute layout update
Native->>Child: Update controller.view.frame to bounds
Native->>Child: Call viewWillTransition(to:with:) with coordinator
Native->>Container: setNeedsLayout()
Native->>Container: layoutIfNeeded()
Container->>Container: layoutSubviews() - sync child frames
Container->>Child: Recursively force layout on subviews
Child-->>Native: Layout updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 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)
📝 Coding Plan
Comment |
|
| Filename | Overview |
|---|---|
| purchasely/ios/Classes/NativeView.swift | Major rewrite: adds NativeContainerView, proper VC containment via findRootViewController(), orientation observer with async relayout, and NoAnimationTransitionCoordinator — addresses the core rotation/content-loss bug; two P2 issues around stacked async closures and stale controller references after cleanup. |
| purchasely/android/src/main/kotlin/io/purchasely/purchasely_flutter/NativeView.kt | Small, clean fixes: dispose() now calls removeAllViews(), redundant ViewGroup cast removed in closeCallback(), misleading Log.e corrected to Log.d, and trailing newline added. |
| purchasely/lib/native_view_widget.dart | Debug prints removed and Directionality.maybeOf() applied for Android RTL support; iOS UiKitView branch is missing the same layoutDirection fix, leaving RTL incomplete on iOS. |
| purchasely/example/ios/RunnerTests/SwiftPurchaselyFlutterPluginTests.swift | Adds 6 NativeView tests, but all test plain UIKit/UIViewController primitives rather than NativeView itself (SDK dependency prevents direct instantiation). |
| purchasely/test/native_view_widget_test.dart | Adds Android RTL/LTR layout direction tests; correctly verifies Directionality inheritance and fallback, but no corresponding iOS UiKitView tests. |
Sequence Diagram
sequenceDiagram
participant D as UIDevice
participant NV as NativeView
participant CQ as DispatchQueue.main
participant CV as NativeContainerView
participant VC as PLY UIViewController
Note over NV: init()
NV->>CV: NativeContainerView(frame:)
NV->>VC: controller.view addSubview(childView)
NV->>VC: rootVC.addChild(controller)
NV->>D: beginGeneratingDeviceOrientationNotifications()
NV->>NV: addObserver(orientationDidChange)
Note over D: Device rotates
D-->>NV: orientationDidChangeNotification
NV->>CQ: asyncAfter(+0.1s)
CQ->>CV: read bounds.size (newSize)
CQ->>VC: controller.view.frame = containerView.bounds
CQ->>VC: viewWillTransition(to: newSize, with: coordinator)
CQ->>VC: setNeedsLayout() / layoutIfNeeded()
CQ->>NV: forceLayoutRecursive(controller.view)
Note over NV: PLYEvent.presentationClosed
NV->>VC: willMove(toParent: nil)
NV->>VC: removeFromParent()
NV->>VC: view.removeFromSuperview()
Note over NV: deinit
NV->>NV: removeObserver(self)
NV->>D: endGeneratingDeviceOrientationNotifications()
NV->>VC: cleanupController()
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
purchasely/lib/native_view_widget.dart:55-60
**`UiKitView` missing `layoutDirection` — RTL fix is Android-only**
The PR's stated goal includes RTL support via `Directionality.maybeOf(context)`, but the iOS branch's `UiKitView` is left without the same fix. `UiKitView` accepts an identical `layoutDirection` parameter, so omitting it means iOS will silently default to `TextDirection.ltr` regardless of the app's inherited directionality. The new tests only exercise the Android path.
```suggestion
return SafeArea(
// Wrap UiKitView with SafeArea
child: UiKitView(
viewType: viewType,
layoutDirection: Directionality.maybeOf(context) ?? TextDirection.ltr,
creationParams: creationParams,
```
### Issue 2 of 3
purchasely/ios/Classes/NativeView.swift:47-50
**Stale `controller` reference survives `cleanupController()` in async block**
In `orientationDidChange()`, `controller` is captured as a strong local variable by the `asyncAfter` closure. If `cleanupController()` fires (via `presentationClosed` event or `deinit`) within the 100 ms window, the closure will still execute — calling `controller.view.frame = ...` and `viewWillTransition(to:with:)` on an already-detached, parentless view controller. While unlikely to crash, this can trigger unexpected relayout callbacks inside the Purchasely SDK against a controller that has been removed from the hierarchy. Guard against the cleaned-up state inside the async block by re-reading `self._controller` inside the closure instead of capturing the local.
### Issue 3 of 3
purchasely/ios/Classes/NativeView.swift:44-57
**Rapid rotation stacks multiple async relayout closures**
Every `orientationDidChange` notification enqueues a new `asyncAfter(deadline: .now() + 0.1)` block without cancelling any prior pending block. During a quick double-rotation the first closure fires against stale `_containerView.bounds` (from the intermediate orientation), resizes the content view to the wrong size, and 100 ms later the second closure corrects it — producing visible flicker. Consider using a `DispatchWorkItem` stored as a property so each new notification cancels the previous pending relayout before scheduling a fresh one.
Reviews (2): Last reviewed commit: "fix(native-view): address review comment..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (6)
purchasely/test/native_view_widget_test.dart (1)
203-235: Consider adding explicit expectations.The test verifies no crash occurs but doesn't include explicit assertions. Adding at least a
findexpectation would make test intent clearer and catch potential regressions in widget structure.♻️ Suggestion to add explicit expectations
// RTL context await tester.pumpWidget( MaterialApp( home: Directionality( textDirection: TextDirection.rtl, child: Scaffold(body: view), ), ), ); + + // Verify AndroidView is rendered + expect(find.byType(AndroidView), findsOneWidget); // No crash = layout direction is resolved from context debugDefaultTargetPlatformOverride = null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@purchasely/test/native_view_widget_test.dart` around lines 203 - 235, The test currently only ensures no crash when switching layout directions; add explicit assertions after each pump to verify the widget is present and layout direction applied: after pumping the LTR MaterialApp and RTL MaterialApp, call expectations such as expect(find.byType(PLYPresentationView), findsOneWidget) and a directional-dependent check (e.g., expect(find.byWidgetPredicate(w => /* check Directionality or a child layout property tied to TextDirection */), findsOneWidget)) so the test asserts the view is rendered and responds to context; reference PLYPresentationView, testWidgets, tester.pumpWidget and debugDefaultTargetPlatformOverride to locate where to insert the expectations.purchasely/android/src/main/kotlin/io/purchasely/purchasely_flutter/NativeView.kt (1)
27-29: Redundant cast toViewGroup.
FrameLayoutalready extendsViewGroup, so the cast is unnecessary. The same applies to the existingcloseCallback()method.♻️ Optional simplification
override fun dispose() { - (layout as ViewGroup).removeAllViews() + layout.removeAllViews() }And optionally in
closeCallback():private fun closeCallback() { - (layout as ViewGroup).removeAllViews() + layout.removeAllViews() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@purchasely/android/src/main/kotlin/io/purchasely/purchasely_flutter/NativeView.kt` around lines 27 - 29, The cast to ViewGroup is redundant because layout is a FrameLayout (which already extends ViewGroup); update the dispose() method to call removeAllViews() directly on layout (e.g., layout.removeAllViews()) and likewise remove the unnecessary cast in closeCallback() where layout is cast to ViewGroup; reference the dispose(), closeCallback(), and layout symbols when making these edits.purchasely/ios/Classes/NativeView.swift (4)
44-57: Hard-coded delay is fragile for layout coordination.The 0.1-second delay at line 47 assumes Flutter will have completed its resize within that time. On slower devices or under load, this may not hold true, potentially causing layout glitches.
Consider a more robust approach such as observing bounds changes on the container view via KVO or overriding
layoutSubviewsinNativeContainerViewto trigger the controller re-layout when the frame actually changes.Alternative using bounds observation in NativeContainerView
private class NativeContainerView: UIView { weak var controller: UIViewController? + private var lastBounds: CGRect = .zero override func layoutSubviews() { super.layoutSubviews() + if bounds != lastBounds, let controller = controller { + lastBounds = bounds + controller.viewWillTransition(to: bounds.size, with: NoAnimationTransitionCoordinator()) + } for child in subviews { if child.frame != bounds {Then remove the
asyncAfterdelay and the notification observer, relying onlayoutSubviewsinstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@purchasely/ios/Classes/NativeView.swift` around lines 44 - 57, The hard-coded 0.1s delay in orientationDidChange is fragile; remove the DispatchQueue.main.asyncAfter and the orientation notification usage and instead trigger re-layout when the container's actual bounds change by overriding layoutSubviews in NativeContainerView (or adding a bounds observer/KVO on _containerView). In that override/observer, compute the new size from _containerView.bounds, set controller.view.frame = _containerView.bounds, call controller.viewWillTransition(to:with:) using NoAnimationTransitionCoordinator(), then call view.setNeedsLayout(), view.layoutIfNeeded(), and forceLayoutRecursive(controller.view) so the controller re-layouts exactly when the container actually changes.
22-33: Consider safer patterns for controller setup.Two observations:
Line 23: Force unwrapping
controller.view!could crash if view loading fails. Consider usingguard let childView = controller.view else { return }.Lines 30-31: Accessing
UIApplication.shared.delegate?.window??is deprecated on iOS 13+ with multi-scene support. While Flutter apps typically use a single scene, consider using the view's window scene for future-proofing:Suggested improvement
- if let rootVC = UIApplication.shared.delegate?.window??.rootViewController { + if let rootVC = _containerView.window?.rootViewController ?? UIApplication.shared.delegate?.window??.rootViewController { rootVC.addChild(controller) controller.didMove(toParent: rootVC) }Note: Since
_containerViewwon't have a window at init time, you may need to defer containment setup to when the view is added to the hierarchy (e.g., indidMoveToWindow).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@purchasely/ios/Classes/NativeView.swift` around lines 22 - 33, Replace the unsafe force-unwrap and deprecated window access: instead of using controller.view! and UIApplication.shared.delegate?.window??, guard-let unwrap controller.view (e.g., guard let childView = _controller?.view else { return }) and add it to _containerView; defer attaching the view controller to a parent until the view has a window (implement containment in _containerView.didMoveToWindow or when view.window != nil) and use the view's window/scene to find a parent view controller (e.g., view.window?.windowScene or view.window?.rootViewController) rather than UIApplication.shared.delegate?.window?? to avoid deprecation and crashes.
108-108:containerViewcreates a new instance on each access.The computed property returns
UIView()each time it's accessed. While likely harmless given the coordinator's brief lifetime, returning a stable instance would be more correct:+ private let _containerView = UIView() - var containerView: UIView { UIView() } + var containerView: UIView { _containerView }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@purchasely/ios/Classes/NativeView.swift` at line 108, The computed property containerView currently returns a new UIView each time (var containerView: UIView { UIView() }); change it to a stored instance so the same UIView is returned on every access (e.g., make containerView a private let or lazy var initialized once in the NativeView class/struct) so the coordinator and any other consumers reference a stable view instance instead of creating a new one on each access.
71-76: Duplicate cleanup logic betweendeinitandeventTriggered.Both
deinit(lines 71-76) and the.presentationClosedhandler (lines 144-148) perform the same controller detachment sequence. If the event fires before deallocation,deinitwill redundantly attempt to clean up an already-detached controller.Consider consolidating cleanup into a single method and guarding against double execution:
Proposed consolidation
+ private var isCleanedUp = false + + private func cleanupController() { + guard !isCleanedUp, let controller = _controller else { return } + isCleanedUp = true + controller.willMove(toParent: nil) + controller.view.removeFromSuperview() + controller.removeFromParent() + } + deinit { NotificationCenter.default.removeObserver(self) - _controller?.willMove(toParent: nil) - _controller?.view.removeFromSuperview() - _controller?.removeFromParent() + cleanupController() }And in
eventTriggered:if event == .presentationClosed { DispatchQueue.main.async { [weak self] in - self?._controller?.willMove(toParent: nil) - self?._controller?.view.removeFromSuperview() - self?._controller?.removeFromParent() + self?.cleanupController() } }Also applies to: 144-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@purchasely/ios/Classes/NativeView.swift` around lines 71 - 76, Duplicate cleanup is happening in deinit and the .presentationClosed handler; extract the detach/remove sequence into a single private helper (e.g., cleanupController()) that checks/guards for already-cleared state (inspect _controller, _controller?.parent, or _controller?.view.superview) before calling willMove(toParent:), view.removeFromSuperview(), removeFromParent(), and finally nil-ing _controller; then call cleanupController() from both deinit and the eventTriggered/.presentationClosed handler to avoid double execution.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@purchasely/android/src/main/kotlin/io/purchasely/purchasely_flutter/NativeView.kt`:
- Around line 27-29: The cast to ViewGroup is redundant because layout is a
FrameLayout (which already extends ViewGroup); update the dispose() method to
call removeAllViews() directly on layout (e.g., layout.removeAllViews()) and
likewise remove the unnecessary cast in closeCallback() where layout is cast to
ViewGroup; reference the dispose(), closeCallback(), and layout symbols when
making these edits.
In `@purchasely/ios/Classes/NativeView.swift`:
- Around line 44-57: The hard-coded 0.1s delay in orientationDidChange is
fragile; remove the DispatchQueue.main.asyncAfter and the orientation
notification usage and instead trigger re-layout when the container's actual
bounds change by overriding layoutSubviews in NativeContainerView (or adding a
bounds observer/KVO on _containerView). In that override/observer, compute the
new size from _containerView.bounds, set controller.view.frame =
_containerView.bounds, call controller.viewWillTransition(to:with:) using
NoAnimationTransitionCoordinator(), then call view.setNeedsLayout(),
view.layoutIfNeeded(), and forceLayoutRecursive(controller.view) so the
controller re-layouts exactly when the container actually changes.
- Around line 22-33: Replace the unsafe force-unwrap and deprecated window
access: instead of using controller.view! and
UIApplication.shared.delegate?.window??, guard-let unwrap controller.view (e.g.,
guard let childView = _controller?.view else { return }) and add it to
_containerView; defer attaching the view controller to a parent until the view
has a window (implement containment in _containerView.didMoveToWindow or when
view.window != nil) and use the view's window/scene to find a parent view
controller (e.g., view.window?.windowScene or view.window?.rootViewController)
rather than UIApplication.shared.delegate?.window?? to avoid deprecation and
crashes.
- Line 108: The computed property containerView currently returns a new UIView
each time (var containerView: UIView { UIView() }); change it to a stored
instance so the same UIView is returned on every access (e.g., make
containerView a private let or lazy var initialized once in the NativeView
class/struct) so the coordinator and any other consumers reference a stable view
instance instead of creating a new one on each access.
- Around line 71-76: Duplicate cleanup is happening in deinit and the
.presentationClosed handler; extract the detach/remove sequence into a single
private helper (e.g., cleanupController()) that checks/guards for
already-cleared state (inspect _controller, _controller?.parent, or
_controller?.view.superview) before calling willMove(toParent:),
view.removeFromSuperview(), removeFromParent(), and finally nil-ing _controller;
then call cleanupController() from both deinit and the
eventTriggered/.presentationClosed handler to avoid double execution.
In `@purchasely/test/native_view_widget_test.dart`:
- Around line 203-235: The test currently only ensures no crash when switching
layout directions; add explicit assertions after each pump to verify the widget
is present and layout direction applied: after pumping the LTR MaterialApp and
RTL MaterialApp, call expectations such as
expect(find.byType(PLYPresentationView), findsOneWidget) and a
directional-dependent check (e.g., expect(find.byWidgetPredicate(w => /* check
Directionality or a child layout property tied to TextDirection */),
findsOneWidget)) so the test asserts the view is rendered and responds to
context; reference PLYPresentationView, testWidgets, tester.pumpWidget and
debugDefaultTargetPlatformOverride to locate where to insert the expectations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32490b05-cb16-4426-88b4-ee6eeffe2cf5
📒 Files selected for processing (5)
purchasely/android/src/main/kotlin/io/purchasely/purchasely_flutter/NativeView.ktpurchasely/example/ios/RunnerTests/SwiftPurchaselyFlutterPluginTests.swiftpurchasely/ios/Classes/NativeView.swiftpurchasely/lib/native_view_widget.dartpurchasely/test/native_view_widget_test.dart
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an iOS nested PLYPresentationView rendering issue where paywall content disappears after an orientation change followed by a tab switch, by improving native view controller embedding and relayout behavior; also includes small Android cleanup and Flutter RTL support.
Changes:
- iOS: Wrap embedded controller view in a container view, add VC containment, and trigger relayout on orientation changes.
- Flutter: Use inherited
Directionalityfor Android platform view layout direction; remove debug prints. - Android: Implement
dispose()cleanup and correct a fallback-path log message; add/update tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| purchasely/ios/Classes/NativeView.swift | Adds container view + VC containment and forces relayout on orientation changes to prevent blank nested content. |
| purchasely/android/src/main/kotlin/io/purchasely/purchasely_flutter/NativeView.kt | Ensures native view cleanup on disposal and fixes fallback-path logging. |
| purchasely/lib/native_view_widget.dart | Uses inherited Directionality for Android platform view layout direction; removes debug prints. |
| purchasely/test/native_view_widget_test.dart | Adds a widget test intended to cover layout direction behavior. |
| purchasely/example/ios/RunnerTests/SwiftPurchaselyFlutterPluginTests.swift | Adds iOS unit tests related to the new native view approach. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
iOS: - Stable container view in NoAnimationTransitionCoordinator (lazy stored property) to honor UIViewControllerTransitionCoordinatorContext - Scene-aware host VC lookup with iOS 12 fallback - Extract cleanupController() helper (deinit + presentationClosed) - Begin/end device orientation notifications around the observer - Drop unused NativeContainerView.controller property Dart: - Use Directionality.maybeOf(context) ?? TextDirection.ltr to avoid FlutterError when no Directionality ancestor exists Tests: - Assert AndroidView.layoutDirection in LTR/RTL contexts - Add fallback test for missing Directionality - Reset debugDefaultTargetPlatformOverride via try/finally so the invariant check sees an unset value Android: - Drop redundant (layout as ViewGroup) casts (FrameLayout is a ViewGroup) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@greptileai review |
EPIKorial
approved these changes
May 4, 2026
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
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
Fixes a bug where the nested Purchasely view (
PLYPresentationView) loses all content when switching tabs (e.g. Annual ↔ Monthly) after a device orientation change. Reported by Headspace via Slack.Bug
Steps to reproduce:
PLYPresentationView)Screenshots (before fix):
Root Cause
The
NativeViewon iOS embedded the nativeUIViewController's view as a bareUIViewwithout proper view controller containment. This meant:viewWillTransition(to:with:)callbacks during rotationFix
iOS (
NativeView.swift)NativeContainerViewthat overrideslayoutSubviews()to force child relayout on bounds changes[.flexibleWidth, .flexibleHeight]so the view follows frame changesaddChild/didMove(toParent:)so the controller participates in the view controller lifecycleUIDevice.orientationDidChangeNotificationand explicitly callviewWillTransition(to:with:)with aNoAnimationTransitionCoordinatorto trigger the SDK's internal relayoutdeinitwithremoveFromParent,removeObserver, andremoveFromSuperviewAndroid (
NativeView.kt)dispose()to callremoveAllViews()(was a no-op)Log.e"built successfully" →Log.dwith correct message)Dart (
native_view_widget.dart)TextDirection.ltrwithDirectionality.of(context)for RTL supportprint()statementsTest plan
NativeViewTests— 6 tests: container view layout, autoresizing, VC containment, rotation propagation)PLYPresentationView layout direction— RTL/LTR context test)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests