refactor(iOS, FormSheet v5): Move AppearanceCoordinator/Applicator to ContentController by t0maboro · Pull Request #4074 · software-mansion/react-native-screens

t0maboro · 2026-05-21T09:44:15Z

Description

Previously, there was a tight coupling where RNSFormSheetContentController and RNSFormSheetHostComponentView had a direct dependency. By introducing dedicated provider protocols, the UIKit is now completely agnostic of React.

Closes: https://github.com/software-mansion/react-native-screens-labs/issues/1454

Changes

defining contracts that Host now implements to supply data.
removed all RNSFormSheetHostComponentView dependencies from RNSFormSheetContentController and RNSFormSheetAppearanceApplicator
moved invalidation flags logic from the Host to the Controller

Before & after - visual documentation

N/A - refactor

Test plan

No regression in existing examples

Checklist

Included code example that can be used to test this change.
For visual changes, included screenshots / GIFs / recordings documenting the change.
For API changes, updated relevant public types.
Ensured that CI passes

Copilot

Pull request overview

This PR refactors the iOS FormSheet (v5) implementation to align with the Split host pattern: the host component now only records invalidation flags on prop/window changes, and the content controller owns the appearance coordinator/applicator and performs the batched updates after React mounting transactions complete.

Changes:

Moved RNSFormSheetAppearanceCoordinator/RNSFormSheetAppearanceApplicator ownership and update execution into RNSFormSheetContentController.
Added invalidation flags on RNSFormSheetHostComponentView and forwarded them to the controller in finalizeUpdates / didMoveToWindow.
Wired mounting-transaction callbacks from the host to the controller so the controller flushes pending appearance/presentation work in mountingTransactionDidMount.

Reviewed changes

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

File	Description
ios/gamma/modals/form-sheet/RNSFormSheetHostComponentView.mm	Replaces direct appearance/presentation updates with invalidation flags + controller signals, and forwards mounting-transaction callbacks.
ios/gamma/modals/form-sheet/RNSFormSheetHostComponentView.h	Exposes `isOpen` to allow the controller to derive presentation state from host props.
ios/gamma/modals/form-sheet/RNSFormSheetContentController.mm	Takes over appearance coordinator/applicator and executes pending appearance/presentation updates after mounting transactions.
ios/gamma/modals/form-sheet/RNSFormSheetContentController.h	Adds controller “signal” APIs, mounting-transaction hooks, and a weak back-reference to the host view.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

kkafar

I have few remarks. Please answer them before proceeding.

kkafar · 2026-05-21T11:23:01Z

+- (void)reactMountingTransactionWillMount;
+- (void)reactMountingTransactionDidMount;


It was my mistake to name these like this.

The content controller should ideally not be aware of the react inner workings.
We also likely don't need will callback.

Single flushPendingUpdates should be enough. Just as it is done in tabs.

kkafar · 2026-05-21T11:30:11Z

+  if (_needsInitialDetentReset) {
+    _needsInitialDetentReset = NO;
+    [_appearanceApplicator resetInitialDetent];
+  }


I do my first reading & don't understand the code fully yet, but detent update in appearance method doesn't fit for me. Aren't detents rather a behaviour?

Thinking now. Maybe call it configuration or something and then have both detents and appearance here.

you're right, there are a few things I don't like here:

appearance & behavior separation

that AppearanceApplicator is not stateless

my current plan is to:

separate appearance and behavior on Providers level for this PR

refactor AppearanceApplicator to extract Behavior configuration (next PR)

make AppearanceApplcator stateless (next PR)

let me know if you're okay with that

kkafar · 2026-05-21T13:17:09Z


+#pragma mark - Appearance
+
+- (void)updateAppearanceIfNeededForHost:(nonnull RNSFormSheetHostComponentView *)host


...ForHost suggests that you apply the appearance configuration on the host object, which is not the case. I'd name it ...withConfigProvider or somethig like this.

This method ideally, shouldn't take RNSFormSheetHostComponentView, but rather an interface of a delegate / provider. This is not a priority though.

kkafar · 2026-05-21T13:28:13Z

+  // Invalidation flags
+  BOOL _needsPresentationUpdate;
+  BOOL _needsAppearanceUpdate;
+  BOOL _needsInitialDetentReset;


Should these live on the host?

All the invalidation flags should leave on the content controller. Take a look at how Tabs are implemented. Host & Screens report the need (directly or not) of the update to the controller & it holds it, and clears the flag when it runs the update logic.

We can talk about it some more if that's not fully clear.

kkafar · 2026-05-21T13:31:31Z

-                      }];
+  if (_needsPresentationUpdate) {
+    _needsPresentationUpdate = NO;
+    [_controller setNeedsPresentationUpdate];


Instead of holding these additional flags on the host and passing this information to the controller here we can directly signalise to the controller when we e.g. detect prop change.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

t0maboro · 2026-05-22T09:39:41Z

-                             controller:(RNSFormSheetContentController *)controller
-                            coordinator:(RNSFormSheetAppearanceCoordinator *)coordinator;
+- (void)updateAppearanceIfNeededForAppearanceProvider:(id<RNSFormSheetAppearanceProvider>)appearanceProvider
+                                     behaviorProvider:(id<RNSFormSheetBehaviorProvider>)behaviorProvider


note: behavior will have a dedicated path, but I'm extracting it to a separate PR in which I'm going to introduce a dedicated class for that

associated followup PR: #4084

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

kkafar

Looks good. I still have few remarks / questions. Let's answer them.

kkafar · 2026-05-22T10:50:28Z

- (void)updateAppearanceIfNeededForHost:(RNSFormSheetHostComponentView *)host
-                             controller:(RNSFormSheetContentController *)controller
-                            coordinator:(RNSFormSheetAppearanceCoordinator *)coordinator;
+- (void)updateAppearanceIfNeededForAppearanceProvider:(id<RNSFormSheetAppearanceProvider>)appearanceProvider


I have one remark regarding naming.

Suggested change

- (void)updateAppearanceIfNeededForAppearanceProvider:(id<RNSFormSheetAppearanceProvider>)appearanceProvider

- (void)updateAppearanceIfNeededWithAppearanceProvider:(id<RNSFormSheetAppearanceProvider>)appearanceProvider

The For part does sound inappropriate and misleading in this context.

kkafar · 2026-05-22T10:57:35Z

+- (nullable UIWindow *)hostWindow
+{
+  return self.window;
+}


I don't understand what hostWindow is for. The window is shared for both host & content hierarchies. Why do we need additional method here? Do you need window earlier than onWillMoveToWindow on RNSFormSheetContentController.view?

When I navigate to the screen that has FormSheet with initially isOpen=true set, what I'm observing is that on the 1st mounting transaction, flushPendingUpdates updates the presentation and calls updatePresentationState before willMoveToWindow is called on ContentView

Okay, but why do you even need a window for before you receive willMoveToWindow?

updatePresentationState triggers presentFromWindow and in that method I'm searching for a VC that will present this ContentController

Okay. I don't like the approach that RNSFormSheetContentController is responsible for its own presentation, however it's not crucial atm, it's more important to progress.

Let's create a ticket to clean this up.

Host should be responsible for presentation of the content controller and the logic should leave there.

https://github.com/software-mansion/react-native-screens-labs/issues/1488

kkafar · 2026-05-22T10:58:41Z

+- (BOOL)prefersGrabberVisible
+{
+  return _prefersGrabberVisible;
+}
+
+- (CGFloat)preferredCornerRadius
+{
+  return _preferredCornerRadius;
+}
+
+- (NSInteger)largestUndimmedDetentIndex
+{
+  return _largestUndimmedDetentIndex;
+}


Are these necessary? You have declared properites on RNSFormSheetHostComponentView.h, aren't they synthesized by the compiler?

dc3f9ac
note: having these providers, I think that we shouldn't expose properties from HostComponent at all, but rather operate on proper providers

Sorry, I think I didn't communicate clearly.

These properties SHOULD exist on RNSFormSheetHostComponentView interface. We always expose the props.

My remark is that you don't have to write their getters in .mm file. IIRC the getters should be synthesized by the compiler.

correct, these getters will be synthesized, okay will expose props

a8a43b2 + 6fd75c0

Let's fix the code then. Restore the properties on the interface and remove unnecessary implementations.

This reverts commit dc3f9ac.

## Description A follow-up for #4074 . It addresses the overloaded "Appearance" concept by logically separating visual updates from behavioral updates. Previously, both visual props (e.g. grabber visibility) and structural props (e.g. detents) were grouped together under a single "Appearance" applicator. This PR introduces a more granular UpdateCoordinator and separates the logic. ## Changes - Renamed `RNSFormSheetAppearanceCoordinator` to `RNSFormSheetUpdateCoordinator`. - Replaced old flags with `RNSFormSheetUpdateFlags`, having distinct Appearance, Behavior and Presentation flags. - Replaced `RNSFormSheetAppearanceApplicator` with `RNSFormSheetConfigurationApplicator` that processes both Appearance and Behavior providers and applies them inside a single block. - Refactored RNSFormSheetContentController to separate `updateConfigurationIfNeeded` from `updatePresentationIfNeeded`. `flushPendingUpdates` now configures the sheet first, then presents it. ## Before & after - visual documentation N/A - refactor ## Test plan Tested with existing SFTs for FormSheet. ## Checklist - [ ] Included code example that can be used to test this change. - [ ] For visual changes, included screenshots / GIFs / recordings documenting the change. - [ ] For API changes, updated relevant public types. - [ ] Ensured that CI passes

Move AppearanceCoordinator/Applicator to ContentController

8742fff

t0maboro requested review from KaHarlacz, Copilot, kkafar, kmichalikk and sgaczol May 21, 2026 09:44

Copilot started reviewing on behalf of t0maboro May 21, 2026 09:45 View session

t0maboro requested review from kligarski and removed request for KaHarlacz May 21, 2026 09:48

Copilot AI reviewed May 21, 2026

View reviewed changes

kkafar reviewed May 21, 2026

View reviewed changes

t0maboro added 5 commits May 22, 2026 10:03

Define FormSheetProviders

529e7b1

Decouple host from controller

06be498

Implement protocols on Host

33ede2c

Drop invalidation flags from Host

c6b9efe

Fix broken didMoveToWindow after refactor

414255e

t0maboro requested review from Copilot and kkafar May 22, 2026 08:30

Copilot started reviewing on behalf of t0maboro May 22, 2026 08:31 View session

Copilot AI reviewed May 22, 2026

View reviewed changes

Comment thread ios/gamma/modals/form-sheet/RNSFormSheetHostComponentView.mm Outdated

Comment thread ios/gamma/modals/form-sheet/RNSFormSheetContentController.h

Comment thread ios/gamma/modals/form-sheet/RNSFormSheetAppearanceApplicator.mm

t0maboro added 3 commits May 22, 2026 11:20

Fix signature

9022290

Remove forward declararion from controller

1fb8840

Add guards for providers

e6c5928

t0maboro requested a review from Copilot May 22, 2026 09:32

Copilot started reviewing on behalf of t0maboro May 22, 2026 09:33 View session

Add TODO

abbef2d

Copilot AI reviewed May 22, 2026

View reviewed changes

Comment thread ios/gamma/modals/form-sheet/RNSFormSheetHostComponentView.mm Outdated

Comment thread ios/gamma/modals/form-sheet/RNSFormSheetContentController.mm Outdated

Minor cleanups

c56b89c

t0maboro commented May 22, 2026

View reviewed changes

Remove forward declaration

fa5861a

t0maboro and others added 2 commits May 22, 2026 11:42

Wording

579fffb

Potential fix for pull request finding

c18345f

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

t0maboro mentioned this pull request May 22, 2026

refactor(iOS, FormSheet v5): Separate Behavior and Appearance #4084

Merged

4 tasks

kkafar reviewed May 22, 2026

View reviewed changes

t0maboro added 2 commits May 22, 2026 13:02

Rename

c5473d9

Expose via providers, drop properties

dc3f9ac

t0maboro requested a review from kkafar May 22, 2026 11:15

t0maboro added 2 commits May 22, 2026 13:56

Revert "Expose via providers, drop properties"

a8a43b2

This reverts commit dc3f9ac.

Drop redundant getters

6fd75c0

kkafar approved these changes May 22, 2026

View reviewed changes

t0maboro merged commit d0b645e into main May 22, 2026
5 of 6 checks passed

t0maboro deleted the @t0maboro/move-config-to-controller branch May 22, 2026 12:36

		- (void)reactMountingTransactionWillMount;
		- (void)reactMountingTransactionDidMount;


		#pragma mark - Appearance

		- (void)updateAppearanceIfNeededForHost:(nonnull RNSFormSheetHostComponentView *)host

	- (void)updateAppearanceIfNeededForAppearanceProvider:(id<RNSFormSheetAppearanceProvider>)appearanceProvider
	- (void)updateAppearanceIfNeededWithAppearanceProvider:(id<RNSFormSheetAppearanceProvider>)appearanceProvider

Uh oh!

Conversation

t0maboro commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Before & after - visual documentation

Test plan

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

kkafar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t0maboro May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kkafar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t0maboro May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t0maboro May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t0maboro May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

t0maboro commented May 21, 2026 •

edited

Loading

t0maboro May 22, 2026 •

edited

Loading

t0maboro May 22, 2026 •

edited

Loading

t0maboro May 22, 2026 •

edited

Loading

t0maboro May 22, 2026 •

edited

Loading