feat(iOS, FormSheet v5): Add basic setup for standalone FormSheet native component#3947
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an iOS-only FormSheet (v5) component to the gamma/experimental API surface, backed by a new Fabric component view and supporting native controller/state plumbing, plus manual test scenarios in the test app.
Changes:
- Introduces a new
FormSheetJS component (with Android/Web stubs) and exports it viareact-native-screens/experimental. - Adds iOS native implementation (
RNSFormSheetComponentView+RNSFormSheetController) and registers the component for codegen. - Adds C++ shadow node/state/descriptor glue and adds new test-app scenarios for manual validation.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/fabric/gamma/modals/form-sheet/FormSheetNativeComponent.ts | New codegen native component interface for RNSFormSheet (iOS-only). |
| src/components/gamma/modals/form-sheet/* | JS public API wrapper (FormSheet), types, and Android/Web fallbacks. |
| src/components/gamma/modals/form-sheet/index.ts | Barrel export for the new component. |
| src/experimental/index.ts | Exposes FormSheet from the experimental entrypoint. |
| package.json | Registers RNSFormSheet in codegenConfig.ios.components. |
| ios/gamma/modals/form-sheet/* | Native controller, component view, and event emitter for iOS FormSheet presentation + events. |
| common/cpp/react/renderer/components/rnscreens/RNSFormSheet* | New shadow node/state/component descriptor for layout + origin offset handling. |
| apps/src/tests//form-sheet/ | Adds new manual scenarios for single-feature and integration testing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kkafar
left a comment
There was a problem hiding this comment.
Nice job!
I've left remarks below. Please answer them before we proceed.
kligarski
left a comment
There was a problem hiding this comment.
What about this situation:
<View style={{ position: 'absolute', top: 200, left: 100 }}>
<FormSheet
isOpen={isOpen}
onDismiss={() => setIsOpen(false)}
detents={[0.6, 1.0]}>
<View style={styles.sheetContent}>
<Text style={styles.sheetTitle}>FormSheet content</Text>
<View style={styles.spacing} />
<Button
title="Dismiss from JS"
color={Colors.primary}
onPress={() => setIsOpen(false)}
/>
</View>
</FormSheet>
</View>absolute.mov
LKuchno
left a comment
There was a problem hiding this comment.
Screen and scenarios looks good :)
I added few comments about naming conventions and directory structure and small suggestion about scenario to be consider.
|
|
||
| const scenarioDescription: ScenarioDescription = { | ||
| name: 'Basic functionality', | ||
| key: 'test-formsheet-base', |
There was a problem hiding this comment.
Following naming convention for directories I would set key to 'test-form-sheet-base-ios'
|
|
||
| const scenarioDescription: ScenarioDescription = { | ||
| name: 'FormSheet with Nested Stack v5', | ||
| key: 'test-formsheet-nested-stack-v5', |
There was a problem hiding this comment.
Since this is a CIT scenario, the directory name should look slightly different. I would propose: test-form-sheet-in-stack-v5-ios. The platform must be included as this test is iOS-only.
@kkafar , what do you think about the naming? Since no additional props are being tested, I wouldn't include the <component-a>-<component-b>-<test-case-name> part here, as the test case is basically a <component-a>-in-<component-b> check.
There was a problem hiding this comment.
I think we should keep to what's decided in the RFC on test naming. It might result in long names, but we did that for a reason. Let's stick to it. I see that the name now is test-form-sheet-stack-v5-nesting-stack-in-form-sheet-ios and that makes sense to me.
kligarski
left a comment
There was a problem hiding this comment.
Something seems off regarding the shadow tree offset synchronization.
First, I wondered why we're using [self convertPoint:CGPointZero toView:formSheetContentView] and then sending offset as negative instead of [formSheetContentView convertPoint:CGPointZero toView:self] which makes more sense to me (no "-" necessary). But after I compared both, I get different (absolute) values:
[dbg123] original: {-2.9599811375705726e-14, -388.80829015544043} alt: {2.8421709430404007e-14, 373.33333333333337}
I decided to check with devtools what's going on and I discovered that the offset is incorrect on first update and after trying to change the size of the sheet, it's still incorrect but it's different. You can see that pressable loses focus earlier when I swipe at the top.
original.mov
Then I used the second version. The offset is incorrect on first render as well but after trying to resize the sheet, it looks correct.
modified.mov
I think that we should debug this further. Maybe due to sheet padding on iOS 26 there are some transforms applied and they mess with the calculations (maybe also timing-related?)?
Native code
// For Yoga, we need to apply the offset in RNSFormSheetHostContentView coordinates
auto formSheetContentView = (RNSFormSheetContentView *)_controller.view;
CGPoint hostOriginInContentViewSpace = [self convertPoint:CGPointZero toView:formSheetContentView];
CGPoint contentViewOriginInHostSpace = [formSheetContentView convertPoint:CGPointZero toView:self];
NSLog(@"[dbg123] original: %@ alt: %@", NSStringFromCGPoint(hostOriginInContentViewSpace), NSStringFromCGPoint(contentViewOriginInHostSpace));
CGPoint contentOriginOffset = CGPointMake(-hostOriginInContentViewSpace.x, -hostOriginInContentViewSpace.y);
// CGPoint contentOriginOffset = CGPointMake(contentViewOriginInHostSpace.x, contentViewOriginInHostSpace.y);JS test (note the disabled hitSlop for debugging!)
```tsx import React, { useState } from 'react'; import { Button, StyleSheet, Text, View } from 'react-native'; import { FormSheet } from 'react-native-screens/experimental'; import type { ScenarioDescription } from '@apps/tests/shared/helpers'; import { createScenario } from '@apps/tests/shared/helpers'; import { Colors } from '@apps/shared/styling'; import PressableWithFeedback from '@apps/shared/PressableWithFeedback';const scenarioDescription: ScenarioDescription = {
name: 'Basic functionality',
key: 'test-form-sheet-base-ios',
details: 'Allows to test the basic functionality of FormSheet component.',
platforms: ['ios'],
};
export function App() {
const [isOpen, setIsOpen] = useState(false);
return (
FormSheet Test
<Button
title="Open FormSheet"
color={Colors.primary}
onPress={() => setIsOpen(true)}
/>
<FormSheet
isOpen={isOpen}
onNativeDismiss={() => setIsOpen(false)}
detents={[0.6]}>
{/FormSheet content
/}
{/<Button
title="Dismiss from JS"
color={Colors.primary}
onPress={() => setIsOpen(false)}
/>/}
<View style={{ width: 100, height: 50 }} />
);
}
const styles = StyleSheet.create({
container: {
flex: 1,
justifyContent: 'center',
alignItems: 'center',
backgroundColor: Colors.offBackground,
},
title: {
fontSize: 20,
fontWeight: 'bold',
marginBottom: 20,
color: Colors.text,
},
sheetContent: {
flex: 1,
backgroundColor: Colors.background,
// padding: 24,
// justifyContent: 'center',
// alignItems: 'center',
},
sheetTitle: {
fontSize: 22,
fontWeight: '600',
marginBottom: 12,
color: Colors.text,
},
spacing: {
height: 32,
},
});
export default createScenario(App, scenarioDescription);
</details>
|
|
||
| #pragma mark - Presentation Logic | ||
|
|
||
| - (void)updatePresentationState |
There was a problem hiding this comment.
I wasn't sure about how we handle isOpen from JS and native, I asked LLM for some edge cases and I tried quick change false -> open from JS:
Code and logs
<FormSheet
isOpen={isOpen}
onNativeDismiss={() => {
console.log('[dbg123] onNativeDismiss');
setIsOpen(false);
}}
detents={[0.6, 1.0]}>
<View style={styles.sheetContent}>
<Text style={styles.sheetTitle}>FormSheet content</Text>
<View style={styles.spacing} />
<Button
title="Dismiss from JS"
color={Colors.primary}
onPress={() => {
console.log('[dbg123] Dismiss From JS (setIsOpen(false))');
setIsOpen(false);
setTimeout(() => {
console.log(
'[dbg123] Dismiss From JS - Timeout (setIsOpen(true))',
);
setIsOpen(true);
}, 500);
}}
/>
</View>
</FormSheet>The sheet doesn't reappear:
JS-native-desync.mov
This is what LLM suspected but I didn't check if that's true:
- The "Animation In-Flight" Trap (Rapid Toggle Desync)
The Scenario: JS rapidly toggles isOpen from true -> false -> true.
The Problem: UIKit takes time to animate dismissals, but React updates props almost instantly.
JS sets isOpen={false}.
Native updateProps triggers updatePresentationState.
Native calls [_controller dismissViewControllerAnimated:YES completion:...]. The dismissal animation begins.
While the animation is running, JS sets isOpen={true}.
Native updateProps triggers updatePresentationState again.
The code checks isPresented = _controller.presentingViewController != nil. Because UIKit is still in the middle of animating the dismissal, presentingViewController is still not nil, so isPresented evaluates to YES.
Your logic hits this block:
// 2. _isOpen == YES and isPresented == YES: This occurs when the sheet is already visible ... require no action
The native code does nothing.
A fraction of a second later, the UIKit dismissal animation completes, and the sheet disappears from the screen.
The Result: JS thinks the sheet is open (isOpen={true}). Native _isOpen is YES. But the physical UI is completely gone. Total desynchronization.
There was a problem hiding this comment.
thanks for catching that, let me add it to my issue tracker, this might be more complicated issue to fix, so let me fix it in the followup PR: https://github.com/software-mansion/react-native-screens-labs/issues/1420
There was a problem hiding this comment.
Yeah. I do not know how we'll control the sheet yet. It's down to the discussion in the RFC. For now this is fine as PoC.
ed20408 to
bb8786c
Compare
|
Regarding the layout, it may currently become slightly desynchronized, as noted in the discussion: #3947 (review) Ticket: https://github.com/software-mansion/react-native-screens-labs/issues/1421 |
kligarski
left a comment
There was a problem hiding this comment.
I suggest that you add a separate ticket to https://github.com/software-mansion/react-native-screens-labs/issues/1419 for the drop shadow scaling issue (not matching 1:1 to what's on screen with margin on the left).
… sending zeroed frames
This reverts commit faf0d58.
9eebb45 to
7efa595
Compare
|
android e2e failed with reasons that are unrelated to this PR, I'm pushing it forward |
Description
Introducing the iOS implementation of the standalone FormSheet component.
Key Architectural Decisions:
UIModalPresentationFormSheetpresentation.RNSFormSheetHostController. This controller creates a transparent top-level UIView and is presented modally over the current UIWindow hierarchy via the closest validpresentingViewController.isOpenprop. This declarative boolean is translated into imperativepresentViewControlleranddismissViewControllercalls. When a user natively dismisses the sheet (e.g., via a swipe-down gesture), theRNSFormSheetHostControllerDelegatetriggers theRNSFormSheetHostComponentEventEmitterto fire theonNativeDismissevent, allowing the JS to synchronize its local state.viewDidLayoutSubviewsinside the presented controller. When the native bounds change, the new size and origin offset are captured and dispatched via a C++ state update. This explicitly forces Yoga to synchronously recalculate the content layout using the sheet's actual native dimensions.RNSFormSheetContentView.Implementation details:
RCTSurfaceTouchHandlerattached to the controller's view, utilizingviewOriginOffsetto correctly align the React touch coordinate space with the natively presented window.Caution
Dynamic content origin updates aren't supported in the context of synchronization with the ShadowTree state. If an ancestor offset changes, the frame of our Host view might not be updated at all, because from the Host perspective, the frame doesn't change, so there's no need to re-layout the content.
Caution
Regarding the layout, it may currently become slightly desynchronized, as noted in the discussion: #3947 (review)
The root cause is that
UIDropShadowView(an ancestor ofContentView) applies a transform: scale, which is not straightforward to detect. As a result, the bounds ofContentViewremain unchanged, and layout updates triggered byUIDropShadowVieware not observable from our components.This can lead to a minor offset during the initial layout pass on iOS 26, since the
FormSheetis not anchored to the leading and trailing edges of the screen.I wouldn’t classify this as a regression - we observed similar behaviors in the v4 implementation, and it did not raise any reported issues. For now, I prefer to leave it as-is rather than introduce fragile workarounds. I'll open a follow-up ticket to track this in case the desynchronization becomes problematic.
(ik that too many tasks were covered in this single PR, I'm refining this approach for other modal components)
Closes https://github.com/software-mansion/react-native-screens-labs/issues/1254
Closes https://github.com/software-mansion/react-native-screens-labs/issues/1255
Closes https://github.com/software-mansion/react-native-screens-labs/issues/1256
Closes https://github.com/software-mansion/react-native-screens-labs/issues/1257
Closes https://github.com/software-mansion/react-native-screens-labs/issues/1258
Closes https://github.com/software-mansion/react-native-screens-labs/issues/1274
Changes
RNSFormSheetHostComponentViewto handle the Fabric lifecycle.RNSFormSheetHostControllerto manage modal presentation.RNSFormSheetHostContentViewto isolate react subviews reparenting.RNSFormSheetHostComponentEventEmitterand updated the JS spec to emit theonNativeDismissevent.RNSFormSheetHostShadowNodedefinitions for synchronous layout updates via C++ state proxy.Before & after - visual documentation
basic-layout.mov
formsheet-with-stack-v5.mov
Test plan
isOpenis set totrue.StackContainerfills the FormSheet's boundsChecklist