feat(iOS): add support for medium detent #1649

kkafar · 2022-12-01T09:33:36Z

Description

Add support for sheetPresentationController by exposing new props. These changes work on both architectures (Fabric & Paper).

Known issues:

Transition animations between detent sizes are not working on Fabric.

Nevertheless after the review process I'm gonna merge this PR as it is getting to big and address these issues in separate PRs.

Changes

Added sheetAllowedDetens prop
- Describes heights where a sheet can rest.
- Works only when stackPresentation is set to formSheet.
- Defaults to large.
Added sheetCornerRadius prop
- The corner radius that the sheet will try to render with.
- Works only when stackPresentation is set to formSheet.
- If left unset system default is used.
Added sheetLargestUndimmedDetent prop
- The largest sheet detent for which a view underneath won't be dimmed.
- Works only when stackPresentation is set to formSheet.
- If this prop is set to:
- - large - the view underneath won't be dimmed at any detent level
- - medium - the view underneath will be dimmed only when detent level is large
- - all - the view underneath will be dimmed for any detent level
- Defaults to all.
Added sheetGrabberVisible prop
- Boolean indicating whether the sheet shows a grabber at the top.
- Works only when stackPresentation is set to formSheet.
- Defaults to false.
Added sheetExpandsWhenScrolledToEdge prop
- Whether the sheet should expand to larger detent when scrolling.
- Works only when stackPresentation is set to formSheet.
- Defaults to true.
Added noop implementations for Android
Updated view manager interfaces for Paper

Screenshots / GIFs

Test code and steps to reproduce

Test1649 in TestsExample & FabricTestExample

Checklist

Included code example that can be used to test this change
Updated TS types
Updated documentation:
Ensured that CI passes

kkafar

Some notes for reviewers below.

I would gladly hear some comments on prop naming.

kkafar · 2022-12-08T19:03:14Z

ios/RNSConvert.h

+ (RNSScreenDetentType)RNSScreenDetentTypeFromAllowedDetents:
+    (facebook::react::RNSScreenSheetAllowedDetents)allowedDetents;
+
+ (RNSScreenDetentType)RNSScreenDetentTypeFromLargestUndimmedDetent:
+    (facebook::react::RNSScreenSheetLargestUndimmedDetent)detent;
+


Codegen generates two different types of enums despite the fact, that only single type is used in JS component spec, thus 2 distinct conversion methods are required.

kkafar · 2022-12-08T19:07:27Z

ios/RNSScreen.mm

+  // This must be called after setter for stackPresentation
+  [self updatePresentationStyle];


This it the case, because retrieving sheetPresentationController from view controller requires modalPresentationStyle to be set to one of UIModalPresentationPageSheet or UIModalPresentationFormSheet. We do not support UIModalPresentationPageSheet, nevertheless this is done inside stackPresentation setter.

On Paper this effect is achieved by calling this method in didSetProps: method.

kkafar · 2022-12-08T19:10:20Z

src/native-stack/views/NativeStackView.tsx

+    sheetAllowedDetents = 'large',
+    sheetLargestUndimmedDetent = 'all',
+    isSheetGrabberVisible = false,
+    sheetCornerRadius = -1.0,


I wonder whether these default values will work with react-navigation, because I remember some shenanigans with default value for gestureResponseDistance prop.

react-navigation will have to add this code by itself since they have their own NativeStackView.

WoLewicki

LGTM! I left some comments, please answer them before merging.

guides/GUIDE_FOR_LIBRARY_AUTHORS.md

src/native-stack/types.tsx

WoLewicki · 2022-12-09T12:20:40Z

src/native-stack/views/NativeStackView.tsx

+    sheetAllowedDetents = 'large',
+    sheetLargestUndimmedDetent = 'all',
+    isSheetGrabberVisible = false,
+    sheetCornerRadius = -1.0,


react-navigation will have to add this code by itself since they have their own NativeStackView.

src/types.tsx

ios/RNSScreen.h

kacperkapusciak

Amazing job on this PR so far 🎉
Some general remarks:

Android has a similar concept to iOS detents with bottom sheets. If we ever add a support for this in screens (and I think we should) we definitely should have a unified API for both iOS detents and Android bottom sheets at the same time.

For API I kinda like the @gorhom 's API with snapPoints naming and due to the popularity of react-native-bottom-sheet it's something react-native developers are familiar with: https://gorhom.github.io/react-native-bottom-sheet/props#snappoints

To your implementation:
besides passing a few predefined detents iOS 16 also allows to pass a custom detent, I would love to see this implemented as well - customizing and resizing sheets in UIKit. Of course it can be in a separate PR but it's something we should keep in mind.

Cheers ✌️

kkafar · 2022-12-14T09:39:40Z

For API I kinda like the @gorhom 's API with snapPoints naming and due to the popularity of react-native-bottom-sheet it's something react-native developers are familiar with: https://gorhom.github.io/react-native-bottom-sheet/props#snappoints

I'll address naming issues when API for Android will be ready (yes, it is planned), because I do not know yet what terminology / conventions Android uses exactly. Thank you for suggestions though, I will most likely go into that direction.

besides passing a few predefined detents iOS 16 also allows to pass a custom detent, I would love to see this implemented as well - customizing and resizing sheets in UIKit. Of course it can be in a separate PR but it's something we should keep in mind.

I'm aware of possibility of specifying custom detent sizes, although it comes with few extra steps. I'll do it separate PR for sure, as this one gets too big.

I'm going to merge this PR and iterate over the new API in followups.

gorbypark · 2023-01-19T08:43:29Z

It appears that when using 3.19.0 with react-navigation, presentation: 'formSheet' is defaulting to "half screen" height, when reading above it seems like it was intended to be "large" (which I assume is the old nearly full screen (~90%) height)? It doesn't seem like the props to configure the formSheet behaviour has been added to react-navigation yet, afaict. I've attached a screenshot of what detent I'm getting by default.

kkafar · 2023-01-19T08:57:43Z

Hi @gorbypark,

This PR was not included in version 3.19.0 of react-native-screens, because we are still having some internal conversations on prop naming and shape of the final API (especially, that we need to have it unified with Android implementation which is not ready yet).

It will most likely be released with next minor version.

gorbypark · 2023-01-19T09:31:02Z

Hi @gorbypark,

This PR was not included in version 3.19.0 of react-native-screens, because we are still having some internal conversations on prop naming and shape of the final API (especially, that we need to have it unified with Android implementation which is not ready yet).

It will most likely be released with next minor version.

Aha! Sorry for jumping the gun, I've been tracking this PR for a while since I'm interested in the feature! When I saw the weird half-height formSheet and a new release just a few days ago, I just assumed it had been merged! I guess I have to do some more investigation as to what's going on with my formSheet.

Thanks!

ferrannp · 2023-01-25T18:30:38Z

@kkafar this is awesome that we will finally have native sheets. Excited to see them on Android too. Is there any branch I can watch? Let me know if you need any help with implementation or testing 🙏 .

By the way I noticed something on iOS that seems like a bug, when you collapse the sheet fast, there is a jumping in the bottom (seeing the content behind from the bottom):

bottomJump.mov

## Description Earlier when calling `updatePresentationStyle` in `RNSScreen` we have relied on order of method calls and kept our fingers crossed that the order would not be violated with future changes. Now we leverage `finalizeUpdates:` method from `RCTComponentViewProtocol` which is called after all updates are applied to the component. For broader context see #1649 (comments). ## Changes Moved call to `updatePresentationStyle` from `updateProps` to `finalizeUpdates`. ## Test code and steps to reproduce See `Test1649` (everything should work as before) ## Checklist - [x] Ensured that CI passes (iOS e2e is a known issue)

kkafar added 11 commits November 30, 2022 13:44

bookmark: begin

795678b

Add example

aedb1c0

Pod install in TestExample/ios

2f3ab0e

Add native implementation

947188a

Add draft JS implementation

c035f92

Rename prop to allowedDetent

e807b00

Rename on native site

fab266d

Add FormSheetDetentTypes

a7ea09c

Add largestUndimmedDetent

2a2768a

add formSheetGrabberVisible prop

3aa55b2

Update presentation style in didSetProps

587cfc1

kkafar self-assigned this Dec 1, 2022

kkafar added 15 commits December 8, 2022 15:18

Add radius configuration

47d7f69

Unify prop names

5c08472

Rename sheetAllowedDetents#both to all

f985fb5

Fix refactoring artifacts

4eb7522

Fix typos

7d4085a

Set props for Fabric

43f79ae

Add example to FabricTestExample

434310f

Log in case of error on native side

2841cc9

Rename TestSheet -> Test1649

092bee9

Merge branch 'main' into @kkafar/ios-medium-detent

df7002c

Use Test42 in TestsExample

1f65502

Remove commented out code

d3d6780

Update paper interfaces

c997ee0

Add docs for types

1eb1d10

Types for native stack

bbbcecc

kkafar changed the title ~~feat: add support for iOS medium detent~~ feat(iOS): add support for medium detent Dec 8, 2022

Update readmes

8062914

kkafar marked this pull request as ready for review December 8, 2022 19:00

kkafar commented Dec 8, 2022

View reviewed changes

kkafar requested review from WoLewicki and kacperkapusciak December 8, 2022 19:11

kkafar added 2 commits December 9, 2022 11:59

Add compilation time check for sdk version

eb82c1b

Add some comments

aa85164

WoLewicki approved these changes Dec 9, 2022

View reviewed changes

kacperkapusciak requested a review from satya164 December 9, 2022 13:03

kkafar added 7 commits December 9, 2022 14:57

Add sheetExpandsWhenScrolledToEdge prop

a37596a

Add ScrollView to example in TestsExample

1a2648f

Animate transitions between detent states

7f7062a

Fabric & tvOS

7d3d36e

Update FabricExample

7d45647

Update paper interfaces

12daddf

Do not export props on tvOS

6295d99

kacperkapusciak reviewed Dec 13, 2022

View reviewed changes

kkafar added 2 commits December 14, 2022 09:55

Add value description to types & docs

bcfbc09

Rename isSheetGrabberVisible -> sheetGrabberVisible

fe7b019

kkafar merged commit 914730d into main Dec 14, 2022

kkafar deleted the @kkafar/ios-medium-detent branch December 14, 2022 10:25

gorbypark mentioned this pull request Jan 19, 2023

formSheet presentation only opening half-way in 3.19.0 #1686

Closed

Gregoirevda mentioned this pull request Jan 25, 2023

[Question] How do I make a half-opened Modal/ActionSheet? #530

Closed

kkafar mentioned this pull request Jan 27, 2023

chore: refactor medium detent iOS implementation #1699

Merged

1 task

ferrannp mentioned this pull request Feb 22, 2023

Sheet / detent buggy jumping behavior on iOS #1722

Closed

delphinebugner mentioned this pull request Jan 9, 2024

iOS 15 - function deinit not called on custom native components on goback #2007

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(iOS): add support for medium detent #1649

feat(iOS): add support for medium detent #1649

kkafar commented Dec 1, 2022 •

edited

Loading

kkafar left a comment •

edited

Loading

kkafar Dec 8, 2022

kkafar Dec 8, 2022

kkafar Dec 8, 2022

WoLewicki Dec 9, 2022

WoLewicki left a comment

WoLewicki Dec 9, 2022

kacperkapusciak left a comment

kkafar commented Dec 14, 2022

gorbypark commented Jan 19, 2023

kkafar commented Jan 19, 2023

gorbypark commented Jan 19, 2023

ferrannp commented Jan 25, 2023 •

edited

Loading

		// This must be called after setter for stackPresentation
		[self updatePresentationStyle];

feat(iOS): add support for medium detent #1649

feat(iOS): add support for medium detent #1649

Conversation

kkafar commented Dec 1, 2022 • edited Loading

Description

Changes

Screenshots / GIFs

Test code and steps to reproduce

Checklist

kkafar left a comment • edited Loading

Choose a reason for hiding this comment

kkafar Dec 8, 2022

Choose a reason for hiding this comment

kkafar Dec 8, 2022

Choose a reason for hiding this comment

kkafar Dec 8, 2022

Choose a reason for hiding this comment

WoLewicki Dec 9, 2022

Choose a reason for hiding this comment

WoLewicki left a comment

Choose a reason for hiding this comment

WoLewicki Dec 9, 2022

Choose a reason for hiding this comment

kacperkapusciak left a comment

Choose a reason for hiding this comment

kkafar commented Dec 14, 2022

gorbypark commented Jan 19, 2023

kkafar commented Jan 19, 2023

gorbypark commented Jan 19, 2023

ferrannp commented Jan 25, 2023 • edited Loading

kkafar commented Dec 1, 2022 •

edited

Loading

kkafar left a comment •

edited

Loading

ferrannp commented Jan 25, 2023 •

edited

Loading