Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sheet / detent buggy jumping behavior on iOS #1722

Closed
ferrannp opened this issue Feb 22, 2023 · 4 comments
Closed

Sheet / detent buggy jumping behavior on iOS #1722

ferrannp opened this issue Feb 22, 2023 · 4 comments
Assignees
@kkafar
Labels
Bug Something isn't working Missing repro This issue need minimum repro scenario Platform: iOS This issue is specific to iOS

Comments

@ferrannp
Copy link
Contributor

Description

Not sure if you seen my answer here #1649 (comment), but trying the new formSheet with detent has a buggy behavior. You can see the content from the bottom when you slide. I think this bug is in modal too but it is not so visible. See the video:

bottomJump.mov

By the way, are you planning to implement this also on Android? Would love testing it or even helping. I think a native sheet really makes a difference.

Steps to reproduce

In your Example app, go to StackPresentation.tsx, and change this example:

    <Stack.Screen
      name="FormSheet"
      component={FormScreen}
      options={{ stackPresentation: 'formSheet', sheetAllowedDetents: 'all' }}
    />

Can be any combo of sheetAllowedDetents.

Snack or a link to a repository

https://github.com/software-mansion/react-native-screens/blob/main/Example/src/screens/StackPresentation.tsx#L187

Screens version

Latest master of your Example

React Native version

Latest master of your Example

Platforms

iOS

JavaScript runtime

None

Workflow

React Native (without Expo)

Architecture

Paper (Old Architecture)

Build type

Debug mode

Device

iOS simulator

Device model

No response

Acknowledgements

Yes
@github-actions github-actions bot added Platform: iOS This issue is specific to iOS Missing repro This issue need minimum repro scenario labels Feb 22, 2023
@github-actions
Copy link

Hey! 👋

The issue doesn't seem to contain a minimal reproduction.

Could you provide a snack or a link to a GitHub repository under your username that reproduces the problem?

@kkafar
Copy link
Member

kkafar commented Mar 13, 2023

Thank you for reporting, I'll try to handle it.

I've spent some time on Android implementation, but it is not so straightforward with current react-native-screens architecture - the very short story: we wrap every view inside custom Fragment subclass & to display modal view (I'm talking material components here) the view needs to be presented inside special ModalFragment (or sth close) & we can't inherit from multiple classes. If Android only had used composition over inheritance there would be no issue here, but it is like it is :D However I'm not abandoning the idea, will surely give it a second try.

@kkafar kkafar self-assigned this Mar 13, 2023
@ferrannp
Copy link
Contributor Author

Thanks for answering @kkafar ! Hopefully we can make it work in Android.

By the way, any ideas about the iOS bug reported in here?

@okdltd okdltd mentioned this issue May 3, 2023
Closed
8 tasks
@kkafar kkafar mentioned this issue Aug 9, 2023
Closed
13 tasks
@kkafar kkafar mentioned this issue Aug 21, 2023
Merged
2 tasks
@grahammendick grahammendick mentioned this issue Oct 17, 2023
Closed
kkafar added a commit that referenced this issue Nov 15, 2023
@kkafar
fix(iOS): modal view flickering (#1870)
c74b2d1 
## Description

Should be merged to #1852 or first rebased and then merged to main after
#1852.

Closes #1722 

So the exact roots of the issue are unclear & obfuscated. It seems that
it might be related to following (not 100% sure):

1. It looks like during animation `viewDidLayoutSubviews` is being
called which in turn calls `setSize:forView:` on UIManager. This
triggers Yoga layout underneath which sets view dimensions to the target
values (end animation values) resulting in view clipping during the
animation. There are two more important facts:

a. its hard to determine whether its Yoga who sets invalid value or it
gets invalid value from us (passed in `updateBounds` method after
`viewDidLayoutSubviews` is triggered by system.

b. when uimanager is not notified of bounds size change everything works
fine

I've considered various approaches:

1. Do not pass the value to UIManager when animation is ongoing.
Presence of animation was detected by checking `animationKeys` property
of view's layer. This still resulted in visual glitches. Moreover if I
sent the final value after animation finish (via completion block) it
resulted in content jumping (see
[here](facebook/react-native#34834 (comment))).
2. Use `CADisplayLink` & report to UIManager bounds size from
`presentationLayer` (that should be (almost) accurate value), but it
still resulted in visual glitches (even when sliding down), both
flickering and content jumping or just content had wrong top offset /
padding.
3. Do not notify UIManager at all on bounds change. 


## Changes

I went with this approach for now. That is: I do not notify uimanager on
bounds change when `stackPresentation == formSheet`. This is wild I
know. I experimented a bit trying to find out what did it broke, but I
did not find anything on my toy example (Test1649), however it is highly
unlikely that such approach does not have negative impact, but I believe
it is better that way, than having formsheets unusable due to this
flickering.


## Test code and steps to reproduce

Test1649

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes
@kkafar kkafar added the Bug Something isn't working label Dec 14, 2023
kkafar added a commit that referenced this issue Feb 21, 2024
@kkafar
fix(iOS): modal view flickering (#1870)
b4a81d9 
## Description

Should be merged to #1852 or first rebased and then merged to main after
#1852.

Closes #1722 

So the exact roots of the issue are unclear & obfuscated. It seems that
it might be related to following (not 100% sure):

1. It looks like during animation `viewDidLayoutSubviews` is being
called which in turn calls `setSize:forView:` on UIManager. This
triggers Yoga layout underneath which sets view dimensions to the target
values (end animation values) resulting in view clipping during the
animation. There are two more important facts:

a. its hard to determine whether its Yoga who sets invalid value or it
gets invalid value from us (passed in `updateBounds` method after
`viewDidLayoutSubviews` is triggered by system.

b. when uimanager is not notified of bounds size change everything works
fine

I've considered various approaches:

1. Do not pass the value to UIManager when animation is ongoing.
Presence of animation was detected by checking `animationKeys` property
of view's layer. This still resulted in visual glitches. Moreover if I
sent the final value after animation finish (via completion block) it
resulted in content jumping (see
[here](facebook/react-native#34834 (comment))).
2. Use `CADisplayLink` & report to UIManager bounds size from
`presentationLayer` (that should be (almost) accurate value), but it
still resulted in visual glitches (even when sliding down), both
flickering and content jumping or just content had wrong top offset /
padding.
3. Do not notify UIManager at all on bounds change. 


## Changes

I went with this approach for now. That is: I do not notify uimanager on
bounds change when `stackPresentation == formSheet`. This is wild I
know. I experimented a bit trying to find out what did it broke, but I
did not find anything on my toy example (Test1649), however it is highly
unlikely that such approach does not have negative impact, but I believe
it is better that way, than having formsheets unusable due to this
flickering.


## Test code and steps to reproduce

Test1649

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes
@kkafar kkafar mentioned this issue Apr 26, 2024
Closed
@kkafar
Copy link
Member

kkafar commented Oct 21, 2024

I believe #2045 fixed this issue at a cost of impaired flexbox model support in sheet.

@kkafar kkafar closed this as completed Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kkafar kkafar

Labels
Bug Something isn't working Missing repro This issue need minimum repro scenario Platform: iOS This issue is specific to iOS
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ferrannp @kkafar