fix(iOS): modal content flicker during enter transition on iPad #3345
Conversation
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
This is done to prevent exactly the same issue we have with native header. Basically e.g. on iPads, the horizontal space available is constrained (the modal view is narrower than the view underneath) and the native layout differs from first Yoga layout leading to content jumps & flickers in the very beginning, before we send first state updates. I filter these out now. Relying fully on native layout. Really hard to state whether it won't break anything. Until now manual testing seems ok.
kkafar
requested review from
kligarski,
kmichalikk and
t0maboro
and removed request for
t0maboro
kligarski
approved these changes
Oct 27, 2025
| export { default as TestScreenStack } from './TestScreenStack'; | ||
| export { default as TestSplitView } from './TestSplitView'; | ||
| export { default as TestSafeAreaViewIOS } from './TestSafeAreaViewIOS'; | ||
| export { default as Test3345 } from './Test3345'; |
There was a problem hiding this comment.
nitpick: I'd move this a few lines above, closer to other numbered tests.
| if (parentVC == nil && [self isKindOfClass:RNSModalScreen.class]) { | ||
| // If we're in modal presentation, we don't want to set the frame from RN, | ||
| // as the available space is most likely restriced & differs from what Yoga | ||
| // resolves during firt layout. We want to rely on native layout here. |
There was a problem hiding this comment.
Suggested change
| // resolves during firt layout. We want to rely on native layout here. | |
| // resolves during first layout. We want to rely on native layout here. |
t0maboro
approved these changes
Oct 27, 2025
|
|
||
| if (parentVC == nil && [self isKindOfClass:RNSModalScreen.class]) { | ||
| // If we're in modal presentation, we don't want to set the frame from RN, | ||
| // as the available space is most likely restriced & differs from what Yoga |
There was a problem hiding this comment.
Suggested change
| // as the available space is most likely restriced & differs from what Yoga | |
| // as the available space is most likely restricted & differs from what Yoga |
|
The problem is still reproducible when using very specific styles on scrollview. Added reproduction in ad8eda8 Screen.Recording.2025-11-04.at.11.42.25.mov |
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
Closes https://github.com/software-mansion/react-native-screens-labs/issues/531
This PR filters out calls to
updateLayoutMetrics:oldLayoutMetrics:for modal screen.This is done to prevent exactly the same issue we have with native
header. Basically e.g. on iPads, the horizontal space available is
constrained (the modal view is narrower than the view underneath) and
the native layout differs from first Yoga layout leading to content
jumps & flickers in the very beginning, before we send first state
updates.
I filter these out now. Relying fully on native layout. Really hard to
state whether it won't break anything. Until now manual testing indicates that the change
is ok.
Code remarks
I've put the code into
- [RNSScreenView updateLayoutMetrics:oldLayoutMetrics:]and not into- [RNSModalScreen updateLayoutMetrics:oldLayoutMetrics:]deliberately. We're not extending theRNSScreenViewbehaviour. We alter it. We can not omit calling super in- [RNSModalScreen updateLayoutMetrics:oldLayoutMetrics:]as we'd fail to update state gathered there on every call, potentially reintroducing #1463. Therefore I decided to handle this special case of subclass related code in superclass. What we could do here is to treat these as separate components & make them separate (do not inherit). This is something that we plan to do in next major, therefore decided against such big change here.Changes
Before
repro.mov
Notice this frame and the lack of green bar on the right side:
After
fix.mov
Test code and steps to reproduce
Checklist