Fix Modal first frame being rendered on top-left corner (#50704) - #51048
Fix Modal first frame being rendered on top-left corner (#50704)#51048cortinico wants to merge 1 commit into
Conversation
|
This pull request was exported from Phabricator. Differential Revision: D73948178 |
ab44331 to
2247f5a
Compare
|
This pull request was exported from Phabricator. Differential Revision: D73948178 |
2247f5a to
c931481
Compare
|
This pull request was exported from Phabricator. Differential Revision: D73948178 |
c931481 to
f39e549
Compare
|
Hey @cortinico any next steps to know when this can land in main? |
We're still discussing internally on a couple of edge case to handle. I'll get back to you once we find an agreement |
|
This pull request was exported from Phabricator. Differential Revision: D73948178 |
f39e549 to
2c1fe34
Compare
|
This pull request was exported from Phabricator. Differential Revision: D73948178 |
2c1fe34 to
010ff1c
Compare
|
This pull request was exported from Phabricator. Differential Revision: D73948178 |
010ff1c to
fcce3b3
Compare
fcce3b3 to
ef6acf9
Compare
Summary: Pull Request resolved: react#51048 Fixes react#50442 Closes react#50704 Users reported that Modals on Android are first renderer anchored in 0,0. That results in them being on the top left corner of the screen for some seconds. This is happening because the native state of the Modal on Android as width/height set at 0,0 - which we then update in a subsequent callback. I'm fixing this by making sure we render the Modal the first time with the right screen size - the status bar size Changelog: [Android] [Fixed] - Fix Modal first frame being rendered on top-left corner Reviewed By: javache Differential Revision: D73948178
|
This pull request was exported from Phabricator. Differential Revision: D73948178 |
ef6acf9 to
cf5b1b8
Compare
|
This pull request was successfully merged by @cortinico in b950fa2 When will my fix make it into a release? | How to file a pick request? |
|
This pull request has been merged in b950fa2. |
Co-authored-by: Przemysław Sosna<przemyslaw.sosna@swmansion.com>
# message auto-generated for no-merge-commit merge:
merge @sosen/modal-host-view-state into 0.77.1-rc.1-ohos
Fix: KeyboardAvoidingView jitters in modals without set height
Created-by: PrzemekSosna
Commit-by: Przemysław Sosna
Merged-by: huangyouhua
Description: ## Description
ModalHostViewState was previously initialized with height and width {0,0}. This would cause the first onLayout events to return the wrong height. KeyboardAvoidingView uses the first onLayout to set initialFrameHeight, which is then used to calculate the height of the components. This would lead to negative values being calculated for the height causing jittering etc.
This is the PR in the core RN repo for reference: react/react-native#51048
## Changes
- fixed KeyboardAvoidingView jittering by initializing ModalHostViewState with sensible values.
## Test Plan
1. Run the below snippet and focus the TextInput. You shouldn't see any jittering.
```ts
import React, {useState} from 'react';
import {
View,
Text,
TextInput,
Pressable,
StyleSheet,
Modal,
KeyboardAvoidingView,
} from 'react-native';
const App = () => {
const [modalOpen, setModalOpen] = useState(false);
return (
<View
style={styles.outerContainer}
onLayout={layoutEvent => {
console.log('Outer container layout:', layoutEvent.nativeEvent.layout);
}}>
<Modal
animationType="fade"
visible={modalOpen}
onLayout={layoutEvent => {
console.log('modal layout:', layoutEvent.nativeEvent.layout);
}}>
<KeyboardAvoidingView behavior="height" style={styles.container}>
<View style={[styles.closeView, {marginHorizontal: 25}]}>
<Pressable
onPress={() => setModalOpen(false)}
style={styles.closeButton}>
<Text>Close</Text>
</Pressable>
</View>
<TextInput placeholder="TextInput" style={styles.textInput} />
</KeyboardAvoidingView>
</Modal>
<Pressable onPress={() => setModalOpen(true)} style={styles.launchButton}>
<Text>Open Example</Text>
</Pressable>
</View>
);
};
const styles = StyleSheet.create({
outerContainer: {
flex: 1,
justifyContent: 'center',
alignItems: 'center',
backgroundColor: 'cyan',
},
container: {
flex: 1,
justifyContent: 'center',
paddingHorizontal: 20,
paddingTop: 20,
backgroundColor: 'red',
alignItems: 'center',
},
textInput: {
borderRadius: 5,
borderWidth: 1,
height: 44,
width: 300,
paddingHorizontal: 10,
},
closeView: {
alignSelf: 'stretch',
},
closeButton: {
alignSelf: 'flex-end',
},
launchButton: {
padding: 10,
backgroundColor: '#eee',
borderRadius: 5,
},
});
export default App;
```
## Checklist
- [x] Does not involve incompatible changes; if involved, has been reviewed accordingly.
- [x] Does not impact performance, or performance testing has been conducted without degradation.
- [x] Complies with the relevant coding standards.
- [x] Does not involve documentation updates, or the documentation has been updated.
- [x] Meets testability requirements with necessary self-test cases, appropriate logging, or trace information added.
- [x] No illegal file inclusions exist, such as images or code.
See merge request: openharmony-sig/ohos_react_native!1184
Summary: Pull Request resolved: react#51048 Fixes react#50442 Closes react#50704 Users reported that Modals on Android are first renderer anchored in 0,0. That results in them being on the top left corner of the screen for some seconds. This is happening because the native state of the Modal on Android as width/height set at 0,0 - which we then update in a subsequent callback. I'm fixing this by making sure we render the Modal the first time with the right screen size - the status bar size Changelog: [Android] [Fixed] - Fix Modal first frame being rendered on top-left corner Reviewed By: javache Differential Revision: D73948178 fbshipit-source-id: 055c12aa62d70acc1e4c5a2a5c4ea0c5608e22c7
|
This pull request was successfully merged by @cortinico in 710d499 When will my fix make it into a release? | How to file a pick request? |
Summary: Pull Request resolved: #51048 Fixes #50442 Closes #50704 Users reported that Modals on Android are first renderer anchored in 0,0. That results in them being on the top left corner of the screen for some seconds. This is happening because the native state of the Modal on Android as width/height set at 0,0 - which we then update in a subsequent callback. I'm fixing this by making sure we render the Modal the first time with the right screen size - the status bar size Changelog: [Android] [Fixed] - Fix Modal first frame being rendered on top-left corner Reviewed By: javache Differential Revision: D73948178 fbshipit-source-id: 055c12aa62d70acc1e4c5a2a5c4ea0c5608e22c7
Two Android-only failure modes share this remedy, both traced to RN 0.79's
ModalHostViewScreenSize() returning Size{0,0} on Android while iOS returns a
real RCTScreenSize (react/react-native#51048, fixed only in RN 0.81):
the dialog's open() call is gated on onLayout measuring a non-zero size, so
inside a 0x0 Modal it can either never fire (visibility stuck at 0) or fire
but have its spring orphaned mid-flight by the same underlying glitch
(visibility stuck partway).
The watchdog polls visibility every 400ms and re-opens only when the value
is unchanged since the last sample - never while it is decreasing, which is
what a close() in progress looks like before modalVisibility flips - and
gives up permanently after 8 attempts.
This treats the symptom, not the underlying RN bug, which stays open until
RN 0.81.
Verified in production CI: previously-deterministic Android failures now
pass on attempt 1 with no retries - sites-list-android-move-to-trash-confirm,
sites-list-android-action-sheet-opens, invites-admin-android-action-sheet-opens,
and the sites-list suite on both platforms.
…n Android (#4038) * fix: Dialog - prevent open animation interruption by residual touch on Android (minDistance) A bottom Dialog/ActionSheet opened from a gesture-driven trigger (e.g. List.Item's TapGestureHandler firing onPress on END) can rest part-way open on Android: the residual touch leaks into the Dialog's own panGesture and drives `visibility` mid-open, interrupting the open spring. Adding a minDistance activation threshold to the pan prevents a near-static residual touch from engaging it, while drag-to-dismiss keeps working. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci: trigger snapshot build * fix: clarify minDistance rationale (MOBAPP-2994) * test: add minDistance to Pan gesture jest mock (MOBAPP-2994) * fix: Dialog - add open-animation watchdog for stranded Android opens Two Android-only failure modes share this remedy, both traced to RN 0.79's ModalHostViewScreenSize() returning Size{0,0} on Android while iOS returns a real RCTScreenSize (react/react-native#51048, fixed only in RN 0.81): the dialog's open() call is gated on onLayout measuring a non-zero size, so inside a 0x0 Modal it can either never fire (visibility stuck at 0) or fire but have its spring orphaned mid-flight by the same underlying glitch (visibility stuck partway). The watchdog polls visibility every 400ms and re-opens only when the value is unchanged since the last sample - never while it is decreasing, which is what a close() in progress looks like before modalVisibility flips - and gives up permanently after 8 attempts. This treats the symptom, not the underlying RN bug, which stays open until RN 0.81. Verified in production CI: previously-deterministic Android failures now pass on attempt 1 with no retries - sites-list-android-move-to-trash-confirm, sites-list-android-action-sheet-opens, invites-admin-android-action-sheet-opens, and the sites-list suite on both platforms. * test: cover Dialog open-animation watchdog Mounts the dialog already visible so open()/close() and the watchdog effect close over the same render's shared value (react-native-reanimated's jest mock, unlike the real implementation, allocates a fresh value per call rather than persisting it across renders). Covers: recovering a dialog stuck since mount and self-clearing once it reaches full visibility; retrying while frozen and permanently stopping at the attempt cap; never re-opening while visibility is decreasing (a close in progress). * refactor: trim Dialog watchdog comments to the non-obvious rationale * chore: retrigger CI --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Yedidya Kennard <yedidyak@wix.com>
…n Android (#4038) * fix: Dialog - prevent open animation interruption by residual touch on Android (minDistance) A bottom Dialog/ActionSheet opened from a gesture-driven trigger (e.g. List.Item's TapGestureHandler firing onPress on END) can rest part-way open on Android: the residual touch leaks into the Dialog's own panGesture and drives `visibility` mid-open, interrupting the open spring. Adding a minDistance activation threshold to the pan prevents a near-static residual touch from engaging it, while drag-to-dismiss keeps working. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci: trigger snapshot build * fix: clarify minDistance rationale (MOBAPP-2994) * test: add minDistance to Pan gesture jest mock (MOBAPP-2994) * fix: Dialog - add open-animation watchdog for stranded Android opens Two Android-only failure modes share this remedy, both traced to RN 0.79's ModalHostViewScreenSize() returning Size{0,0} on Android while iOS returns a real RCTScreenSize (react/react-native#51048, fixed only in RN 0.81): the dialog's open() call is gated on onLayout measuring a non-zero size, so inside a 0x0 Modal it can either never fire (visibility stuck at 0) or fire but have its spring orphaned mid-flight by the same underlying glitch (visibility stuck partway). The watchdog polls visibility every 400ms and re-opens only when the value is unchanged since the last sample - never while it is decreasing, which is what a close() in progress looks like before modalVisibility flips - and gives up permanently after 8 attempts. This treats the symptom, not the underlying RN bug, which stays open until RN 0.81. Verified in production CI: previously-deterministic Android failures now pass on attempt 1 with no retries - sites-list-android-move-to-trash-confirm, sites-list-android-action-sheet-opens, invites-admin-android-action-sheet-opens, and the sites-list suite on both platforms. * test: cover Dialog open-animation watchdog Mounts the dialog already visible so open()/close() and the watchdog effect close over the same render's shared value (react-native-reanimated's jest mock, unlike the real implementation, allocates a fresh value per call rather than persisting it across renders). Covers: recovering a dialog stuck since mount and self-clearing once it reaches full visibility; retrying while frozen and permanently stopping at the attempt cap; never re-opening while visibility is decreasing (a close in progress). * refactor: trim Dialog watchdog comments to the non-obvious rationale * chore: retrigger CI --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Yedidya Kennard <yedidyak@wix.com>
Summary:
Fixes #50442
Closes #50704
Users reported that Modals on Android are first renderer anchored in 0,0.
That results in them being on the top left corner of the screen for some seconds.
This is happening because the native state of the Modal on Android as width/height set at 0,0 - which we then update in a subsequent callback.
I'm fixing this by making sure we render the Modal the first time with the right screen size - the status bar size
Changelog:
[Android] [Fixed] - Fix Modal first frame being rendered on top-left corner
Differential Revision: D73948178