refactor(iOS, FormSheet v5): Separate Behavior and Appearance#4084
Merged
Conversation
t0maboro
changed the base branch from
main
to
@t0maboro/move-config-to-controller
t0maboro
requested review from
Copilot,
kkafar,
kligarski,
kmichalikk and
sgaczol
Merged
4 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the iOS FormSheet v5 update pipeline by splitting “appearance” vs “behavior” changes, introducing a new RNSFormSheetUpdateCoordinator/RNSFormSheetUpdateFlags mechanism, and updating the content controller/applicator to apply configuration separately from presentation.
Changes:
- Introduces
RNSFormSheetUpdateFlagsandRNSFormSheetUpdateCoordinatorto track pending updates for Presentation/Appearance/Behavior. - Refactors
RNSFormSheetContentControllerto flush configuration updates first, then presentation updates. - Replaces/removes the old “Appearance” coordinator/update-flags types and wires host prop invalidations to appearance vs behavior appropriately.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ios/gamma/modals/form-sheet/RNSFormSheetUpdateFlags.h | Adds new bitmask flags for presentation/appearance/behavior updates. |
| ios/gamma/modals/form-sheet/RNSFormSheetUpdateCoordinator.h | Declares the new coordinator API to aggregate and flush update flags. |
| ios/gamma/modals/form-sheet/RNSFormSheetUpdateCoordinator.mm | Implements flag bookkeeping and conditional execution helpers. |
| ios/gamma/modals/form-sheet/RNSFormSheetHostComponentView.mm | Updates invalidation logic to distinguish appearance vs behavior changes. |
| ios/gamma/modals/form-sheet/RNSFormSheetContentController.mm | Splits configuration vs presentation flush logic; adopts new coordinator/applicator. |
| ios/gamma/modals/form-sheet/RNSFormSheetContentController.h | Exposes setNeedsBehaviorUpdate signal. |
| ios/gamma/modals/form-sheet/RNSFormSheetConfigurationApplicator.h | Introduces a configuration applicator interface accepting appearance + behavior providers. |
| ios/gamma/modals/form-sheet/RNSFormSheetConfigurationApplicator.mm | Applies sheet configuration (detents/scrolling + visual settings) using new flags/coordinator. |
| ios/gamma/modals/form-sheet/RNSFormSheetAppearanceUpdateFlags.h | Removes legacy appearance update flags. |
| ios/gamma/modals/form-sheet/RNSFormSheetAppearanceCoordinator.h | Removes legacy appearance coordinator API. |
| ios/gamma/modals/form-sheet/RNSFormSheetAppearanceCoordinator.mm | Removes legacy appearance coordinator implementation. |
| ios/gamma/modals/form-sheet/RNSFormSheetAppearanceApplicator.h | Removes legacy appearance applicator interface. |
Comments suppressed due to low confidence (1)
ios/gamma/modals/form-sheet/RNSFormSheetConfigurationApplicator.mm:39
applyConfigurationIfNeeded…triggers on any of (Appearance|Behavior) flags, but the apply block always recalculates and reapplies both behavior (detents, scrolling, selected detent) and appearance. This means an appearance-only change (e.g. grabber/corner radius) will still reassign detents and scrolling config, which can cause unnecessary animations/layout work and undermines the intended separation. Consider capturing which sub-flags are actually set and only applying the corresponding property updates insideanimateChanges:.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
t0maboro
force-pushed
the
@t0maboro/separate-behavior-from-appearance
branch
from
8a4f39d to
77fe85e
Compare
t0maboro
force-pushed
the
@t0maboro/separate-behavior-from-appearance
branch
from
77fe85e to
0dea3f9
Compare
kligarski
approved these changes
May 26, 2026
|
|
||
| - (void)mountChildComponentView:(UIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index | ||
| { | ||
| [_controller.contentView insertReactSubview:childComponentView atIndex:index]; |
Contributor
There was a problem hiding this comment.
It's out of scope of this PR but it would be nice to remove React infix here to make the separation complete.
Comment on lines
+44
to
+46
| - (void)applyConfigurationWithAppearanceProvider:(id<RNSFormSheetAppearanceProvider>)appearanceProvider | ||
| behaviorProvider:(id<RNSFormSheetBehaviorProvider>)behaviorProvider | ||
| controller:(RNSFormSheetContentController *)controller |
Contributor
There was a problem hiding this comment.
I guess that in the future we can split this to appearance and behavior (but still do it in one animateChanges block) but for now, there is just no need to do so, right?
Contributor
Author
There was a problem hiding this comment.
yup it makes sense, and we might consider that as a follow-up: https://github.com/software-mansion/react-native-screens-labs/issues/1501
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.
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
RNSFormSheetAppearanceCoordinatortoRNSFormSheetUpdateCoordinator.RNSFormSheetUpdateFlags, having distinct Appearance, Behavior and Presentation flags.RNSFormSheetAppearanceApplicatorwithRNSFormSheetConfigurationApplicatorthat processes both Appearance and Behavior providers and applies them inside a single block.updateConfigurationIfNeededfromupdatePresentationIfNeeded.flushPendingUpdatesnow configures the sheet first, then presents it.Before & after - visual documentation
N/A - refactor
Test plan
Tested with existing SFTs for FormSheet.
Checklist