Skip to content

Commit e6b1525

Browse files
authored
fix: bottom sheet android content issues (#3340)
## 🎯 Goal I unfortunately noticed today that there was a serious concurrency issue introduced on Android with [this PR](#3339), where the bottom sheet content would simply not load 90% of the time. The issue was that the animation got cancelled in-flight and so the animation callback of `withTiming` fired with `finished: false` every time, causing the content to never really be shown. These changes should address that now. I also introduced optionality for the lazy loading so that the sheet can properly be reused in the future. ## 🛠 Implementation details <!-- Provide a description of the implementation --> ## 🎨 UI Changes <!-- Add relevant screenshots --> <details> <summary>iOS</summary> <table> <thead> <tr> <td>Before</td> <td>After</td> </tr> </thead> <tbody> <tr> <td> <!--<img src="" /> --> </td> <td> <!--<img src="" /> --> </td> </tr> </tbody> </table> </details> <details> <summary>Android</summary> <table> <thead> <tr> <td>Before</td> <td>After</td> </tr> </thead> <tbody> <tr> <td> <!--<img src="" /> --> </td> <td> <!--<img src="" /> --> </td> </tr> </tbody> </table> </details> ## 🧪 Testing <!-- Explain how this change can be tested (or why it can't be tested) --> ## ☑️ Checklist - [ ] I have signed the [Stream CLA](https://docs.google.com/forms/d/e/1FAIpQLScFKsKkAJI7mhCr7K9rEIOpqIDThrWxuvxnwUq2XkHyG154vQ/viewform) (required) - [ ] PR targets the `develop` branch - [ ] Documentation is updated - [ ] New code is tested in main example apps, including all possible scenarios - [ ] SampleApp iOS and Android - [ ] Expo iOS and Android
1 parent 60d115e commit e6b1525

3 files changed

Lines changed: 123 additions & 57 deletions

File tree

examples/SampleApp/src/screens/ThreadScreen.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React, { useCallback } from 'react';
2-
import { Platform, StyleSheet, View } from 'react-native';
2+
import { Platform, StyleSheet } from 'react-native';
33
import { SafeAreaView } from 'react-native-safe-area-context';
44
import {
55
Channel,

package/src/components/MessageMenu/MessageReactionPicker.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ export const MessageReactionPicker = (props: MessageReactionPickerProps) => {
155155
renderItem={renderItem}
156156
/>
157157
{emojiViewerOpened ? (
158-
<BottomSheetModal height={300} onClose={closeModal} visible={true}>
158+
<BottomSheetModal height={300} lazy={true} onClose={closeModal} visible={true}>
159159
<FlatList
160160
columnWrapperStyle={styles.bottomSheetColumnWrapper}
161161
contentContainerStyle={styles.bottomSheetContentContainer}

package/src/components/UIComponents/BottomSheetModal.tsx

Lines changed: 121 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,31 @@ import { useStableCallback } from '../../hooks';
2626
import { KeyboardControllerPackage } from '../KeyboardCompatibleView/KeyboardControllerAvoidingView';
2727

2828
export type BottomSheetModalProps = {
29+
/**
30+
* Function to call when the modal is closed.
31+
* @returns void
32+
*/
2933
onClose: () => void;
34+
/**
35+
* Whether the modal is visible.
36+
*/
3037
visible: boolean;
38+
/**
39+
* The height of the modal.
40+
*/
3141
height?: number;
42+
/**
43+
* Whether the sheet content should be lazy loaded or not. Particularly
44+
* useful when the content is something heavy and we don't want to disrupt
45+
* the animations while this is happening.
46+
*/
47+
lazy?: boolean;
3248
};
3349

50+
// TODO: V9: Animate the backdrop as well.
3451
export const BottomSheetModal = (props: PropsWithChildren<BottomSheetModalProps>) => {
3552
const { height: windowHeight, width: windowWidth } = useWindowDimensions();
36-
const { children, height = windowHeight / 2, onClose, visible } = props;
53+
const { children, height = windowHeight / 2, onClose, visible, lazy = false } = props;
3754

3855
const {
3956
theme: {
@@ -44,81 +61,119 @@ export const BottomSheetModal = (props: PropsWithChildren<BottomSheetModalProps>
4461

4562
const translateY = useSharedValue(height);
4663
const keyboardOffset = useSharedValue(0);
64+
4765
const isOpen = useSharedValue(false);
66+
const isOpening = useSharedValue(false);
4867

4968
const panStartY = useSharedValue(0);
5069

51-
const [renderContent, setRenderContent] = useState(false);
70+
const [renderContent, setRenderContent] = useState(!lazy);
71+
72+
const showContent = useStableCallback(() => {
73+
if (lazy) {
74+
setRenderContent(true);
75+
}
76+
});
77+
78+
const hideContent = useStableCallback(() => {
79+
if (lazy) {
80+
setRenderContent(false);
81+
}
82+
});
5283

5384
const close = useStableCallback(() => {
54-
// close always goes fully off-screen and only then notifies JS
55-
setRenderContent(false);
85+
// hide content immediately
86+
hideContent();
5687

5788
isOpen.value = false;
89+
isOpening.value = false;
90+
5891
cancelAnimation(translateY);
59-
translateY.value = withTiming(height, { duration: 200 }, (finished) => {
60-
if (finished) runOnJS(onClose)();
61-
});
92+
93+
translateY.value = withTiming(
94+
height,
95+
{ duration: 180, easing: Easing.out(Easing.cubic) },
96+
(finished) => {
97+
if (finished) runOnJS(onClose)();
98+
},
99+
);
62100
});
63101

64-
// Open animation: keep it simple (setting shared values from JS still runs on UI)
102+
// modal opening layout effect - we make sure to only show the content
103+
// after the animation has finished if `lazy` has been set to true
65104
useLayoutEffect(() => {
66105
if (!visible) return;
67106

68107
isOpen.value = true;
69-
keyboardOffset.value = 0;
108+
isOpening.value = true;
70109

71-
// clean up any leftover animations
72110
cancelAnimation(translateY);
73-
// kick animation on UI thread so JS congestion can't delay the start; only render content
74-
// once the animation finishes
111+
112+
// start from closed
75113
translateY.value = height;
76114

115+
// Snapshot current keyboard offset as the open target.
116+
// If keyboard changes during opening, we’ll adjust after.
117+
const initialTarget = keyboardOffset.value;
118+
77119
translateY.value = withTiming(
78-
keyboardOffset.value,
79-
{ duration: 200, easing: Easing.inOut(Easing.ease) },
120+
initialTarget,
121+
{ duration: 220, easing: Easing.out(Easing.cubic) },
80122
(finished) => {
81-
if (finished) runOnJS(setRenderContent)(true);
123+
if (!finished) return;
124+
125+
// opening the modal has now truly finished
126+
isOpening.value = false;
127+
128+
// reveal the content if we want to load it lazily
129+
runOnJS(showContent)();
130+
131+
// if keyboard offset changed while we were opening, we do a
132+
// follow-up adjustment (we do not gate the content however)
133+
const latestTarget = keyboardOffset.value;
134+
if (latestTarget !== initialTarget && isOpen.value) {
135+
cancelAnimation(translateY);
136+
translateY.value = withTiming(latestTarget, {
137+
duration: 200,
138+
easing: Easing.inOut(Easing.ease),
139+
});
140+
}
82141
},
83142
);
84-
}, [visible, height, isOpen, keyboardOffset, translateY]);
143+
}, [visible, height, hideContent, isOpen, isOpening, keyboardOffset, showContent, translateY]);
85144

86145
// if `visible` gets hard changed, we force a cleanup
87146
useEffect(() => {
88147
if (visible) return;
89148

90-
setRenderContent(false);
91-
92149
isOpen.value = false;
150+
isOpening.value = false;
93151
keyboardOffset.value = 0;
94152

95153
cancelAnimation(translateY);
96154
translateY.value = height;
97-
}, [visible, height, isOpen, keyboardOffset, translateY]);
155+
}, [visible, height, isOpen, isOpening, keyboardOffset, translateY]);
98156

99-
const keyboardDidShow = useStableCallback((event: KeyboardEvent) => {
157+
const keyboardDidShowRN = useStableCallback((event: KeyboardEvent) => {
100158
const offset = -event.endCoordinates.height;
101159
keyboardOffset.value = offset;
102160

103-
if (isOpen.value) {
104-
cancelAnimation(translateY);
105-
translateY.value = withTiming(offset, {
106-
duration: 250,
107-
easing: Easing.inOut(Easing.ease),
108-
});
109-
}
161+
// We just record the offset, but we avoid cancelling the animation
162+
// if it's in the process of opening. The same logic applies to all
163+
// other keyboard related callbacks in this specific conditional.
164+
if (!isOpen.value || isOpening.value) return;
165+
166+
cancelAnimation(translateY);
167+
translateY.value = withTiming(offset, { duration: 250, easing: Easing.inOut(Easing.ease) });
110168
});
111169

112170
const keyboardDidHide = useStableCallback(() => {
113171
keyboardOffset.value = 0;
114172

115-
if (isOpen.value) {
116-
cancelAnimation(translateY);
117-
translateY.value = withTiming(0, {
118-
duration: 250,
119-
easing: Easing.inOut(Easing.ease),
120-
});
121-
}
173+
if (!isOpen.value || isOpening.value) return;
174+
175+
cancelAnimation(translateY);
176+
translateY.value = withTiming(0, { duration: 250, easing: Easing.inOut(Easing.ease) });
122177
});
123178

124179
useEffect(() => {
@@ -127,31 +182,27 @@ export const BottomSheetModal = (props: PropsWithChildren<BottomSheetModalProps>
127182
const listeners: EventSubscription[] = [];
128183

129184
if (KeyboardControllerPackage?.KeyboardEvents) {
130-
const keyboardDidShow = (event: KeyboardEventData) => {
185+
const keyboardDidShowKC = (event: KeyboardEventData) => {
131186
const offset = -event.height;
132187
keyboardOffset.value = offset;
133188

134-
if (isOpen.value) {
135-
cancelAnimation(translateY);
136-
translateY.value = withTiming(offset, {
137-
duration: 250,
138-
easing: Easing.inOut(Easing.ease),
139-
});
140-
}
189+
if (!isOpen.value || isOpening.value) return;
190+
191+
cancelAnimation(translateY);
192+
translateY.value = withTiming(offset, { duration: 250, easing: Easing.inOut(Easing.ease) });
141193
};
142194

143195
listeners.push(
144-
KeyboardControllerPackage.KeyboardEvents.addListener('keyboardDidShow', keyboardDidShow),
196+
KeyboardControllerPackage.KeyboardEvents.addListener('keyboardDidShow', keyboardDidShowKC),
145197
KeyboardControllerPackage.KeyboardEvents.addListener('keyboardDidHide', keyboardDidHide),
146198
);
147199
} else {
148-
listeners.push(Keyboard.addListener('keyboardDidShow', keyboardDidShow));
200+
listeners.push(Keyboard.addListener('keyboardDidShow', keyboardDidShowRN));
149201
listeners.push(Keyboard.addListener('keyboardDidHide', keyboardDidHide));
150202
}
151-
return () => {
152-
listeners.forEach((listener) => listener.remove());
153-
};
154-
}, [visible, keyboardDidHide, keyboardDidShow, keyboardOffset, isOpen, translateY]);
203+
204+
return () => listeners.forEach((l) => l.remove());
205+
}, [visible, keyboardDidHide, keyboardDidShowRN, keyboardOffset, isOpen, isOpening, translateY]);
155206

156207
const sheetAnimatedStyle = useAnimatedStyle(() => ({
157208
transform: [{ translateY: translateY.value }],
@@ -160,13 +211,16 @@ export const BottomSheetModal = (props: PropsWithChildren<BottomSheetModalProps>
160211
const gesture = useMemo(
161212
() =>
162213
Gesture.Pan()
214+
// disable pan until content is rendered (prevents canceling the opening timing).
215+
.enabled(renderContent)
163216
.onBegin(() => {
164217
cancelAnimation(translateY);
165218
panStartY.value = translateY.value;
166219
})
167220
.onUpdate((event) => {
168221
const minY = keyboardOffset.value;
169-
translateY.value = Math.max(panStartY.value + event.translationY, minY);
222+
const next = panStartY.value + event.translationY;
223+
translateY.value = Math.max(next, minY);
170224
})
171225
.onEnd((event) => {
172226
const openY = keyboardOffset.value;
@@ -177,9 +231,15 @@ export const BottomSheetModal = (props: PropsWithChildren<BottomSheetModalProps>
177231

178232
if (shouldClose) {
179233
isOpen.value = false;
180-
translateY.value = withTiming(height, { duration: 100 }, (finished) => {
181-
if (finished) runOnJS(onClose)();
182-
});
234+
isOpening.value = false;
235+
236+
translateY.value = withTiming(
237+
height,
238+
{ duration: 140, easing: Easing.out(Easing.cubic) },
239+
(finished) => {
240+
if (finished) runOnJS(onClose)();
241+
},
242+
);
183243
} else {
184244
isOpen.value = true;
185245
translateY.value = withTiming(openY, {
@@ -188,13 +248,13 @@ export const BottomSheetModal = (props: PropsWithChildren<BottomSheetModalProps>
188248
});
189249
}
190250
}),
191-
[height, isOpen, keyboardOffset, onClose, panStartY, translateY],
251+
[height, isOpen, isOpening, keyboardOffset, onClose, panStartY, renderContent, translateY],
192252
);
193253

194254
return (
195255
<View style={[styles.wrapper, wrapper]}>
196256
<Modal onRequestClose={onClose} transparent visible={visible}>
197-
<GestureHandlerRootView style={{ flex: 1 }}>
257+
<GestureHandlerRootView style={styles.sheetContentContainer}>
198258
<GestureDetector gesture={gesture}>
199259
<View style={[styles.overlay, { backgroundColor: overlay }, overlayTheme]}>
200260
<TouchableWithoutFeedback onPress={close}>
@@ -214,7 +274,10 @@ export const BottomSheetModal = (props: PropsWithChildren<BottomSheetModalProps>
214274
/>
215275
<View style={[styles.contentContainer, contentContainer]}>
216276
{renderContent ? (
217-
<Animated.View entering={FadeIn.duration(300)} style={{ flex: 1 }}>
277+
<Animated.View
278+
entering={FadeIn.duration(250)}
279+
style={styles.sheetContentContainer}
280+
>
218281
{children}
219282
</Animated.View>
220283
) : null}
@@ -247,6 +310,9 @@ const styles = StyleSheet.create({
247310
flex: 1,
248311
justifyContent: 'flex-end',
249312
},
313+
sheetContentContainer: {
314+
flex: 1,
315+
},
250316
wrapper: {
251317
alignItems: 'center',
252318
flex: 1,

0 commit comments

Comments
 (0)