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.

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

fix: fix issues with BottomSheetModal implementation #1083

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/frontend/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ import { LogBox } from "react-native";
import SplashScreen from "react-native-splash-screen";
import AsyncStorage from "@react-native-async-storage/async-storage";

// We need to wrap the app with this provider to fix an issue with the bottom sheet modal backdrop
// not overlaying the navigation header. Without this, the header is accessible even when
// the modal is open, which we don't want (e.g. header back button shouldn't be reachable).
// See https://github.com/gorhom/react-native-bottom-sheet/issues/1157
import { BottomSheetModalProvider } from "@gorhom/bottom-sheet";

import ErrorScreen from "./screens/UncaughtError";
import { AppLoading } from "./AppLoading";
import AppContainerWrapper from "./AppContainerWrapper";
Expand Down Expand Up @@ -51,7 +57,9 @@ const App = () => (
<PermissionsProvider>
<AppLoading>
<AppProvider>
<AppContainerWrapper />
<BottomSheetModalProvider>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment explaining why this is here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed in db02e2d

<AppContainerWrapper />
</BottomSheetModalProvider>
<UpdateNotifier />
</AppProvider>
</AppLoading>
Expand Down
6 changes: 4 additions & 2 deletions src/frontend/screens/AppPasscode/ConfirmPasscodeSheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ export const ConfirmPasscodeSheet = ({
}: NativeRootNavigationProps<"ConfirmPasscodeSheet">) => {
const { formatMessage: t } = useIntl();
const { setAuthValues } = React.useContext(SecurityContext);
const { sheetRef } = useBottomSheetModal({ openOnMount: true });
const { sheetRef, isOpen } = useBottomSheetModal({
openOnMount: true,
});
const { passcode } = route.params;

function setPasscodeAndNavigateBack() {
Expand All @@ -47,7 +49,7 @@ export const ConfirmPasscodeSheet = ({
}

return (
<BottomSheetModal ref={sheetRef} onDismiss={() => {}}>
<BottomSheetModal ref={sheetRef} isOpen={isOpen}>
<BottomSheetContent
titleStyle={styles.text}
descriptionStyle={styles.text}
Expand Down
10 changes: 8 additions & 2 deletions src/frontend/screens/JoinRequestModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ export const JoinRequestModal = ({

const [config] = React.useContext(ConfigContext);

const { sheetRef, closeSheet } = useBottomSheetModal({ openOnMount: true });
const { sheetRef, isOpen, closeSheet } = useBottomSheetModal({
openOnMount: true,
});

const projectName = config.metadata.name;
const deviceName = route.params?.deviceName || "";
Expand All @@ -77,7 +79,11 @@ export const JoinRequestModal = ({
};

return (
<BottomSheetModal ref={sheetRef} onDismiss={navigation.goBack}>
<BottomSheetModal
ref={sheetRef}
isOpen={isOpen}
onDismiss={navigation.goBack}
>
{step === "prompt" ? (
<BottomSheetContent
buttonConfigs={[
Expand Down
5 changes: 4 additions & 1 deletion src/frontend/screens/PhotosModal/ConfirmDeleteModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { ErrorIcon } from "../../sharedComponents/icons";

interface ModalProps {
sheetRef: React.RefObject<BottomSheetModalMethods>;
isOpen: boolean;
photoIndex: number;
closeSheet: () => void;
disableBackdrop: boolean;
Expand All @@ -35,6 +36,7 @@ const m = defineMessages({

export const ConfirmDeleteModal = ({
sheetRef,
isOpen,
photoIndex,
closeSheet,
disableBackdrop,
Expand All @@ -56,9 +58,10 @@ export const ConfirmDeleteModal = ({
return (
<BottomSheetModal
ref={sheetRef}
isOpen={isOpen}
disableBackrop={disableBackdrop}
onDismiss={closeSheet}
onHardwareBackPress={closeSheet}
onBack={closeSheet}
>
<BottomSheetContent
buttonConfigs={[
Expand Down
1 change: 1 addition & 0 deletions src/frontend/screens/PhotosModal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ const PhotosModal = ({
renderTabBar={() => null}
/>
<ConfirmDeleteModal
isOpen={isOpen}
disableBackdrop={true}
closeSheet={closeModal}
sheetRef={sheetRef}
Expand Down
10 changes: 8 additions & 2 deletions src/frontend/screens/ProjectInviteModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ export const ProjectInviteModal = ({
}: NativeRootNavigationProps<"ProjectInviteModal">) => {
const { formatMessage: t } = useIntl();
const isMounted = useIsMounted();
const { sheetRef, closeSheet } = useBottomSheetModal({ openOnMount: true });
const { sheetRef, isOpen, closeSheet } = useBottomSheetModal({
openOnMount: true,
});
const [{ observations }] = React.useContext(ObservationsContext);
const [config] = React.useContext(ConfigContext);

Expand Down Expand Up @@ -162,7 +164,11 @@ export const ProjectInviteModal = ({
}, [fetchInviteDetails, inviteKey]);

return (
<BottomSheetModal ref={sheetRef} onDismiss={navigation.goBack}>
<BottomSheetModal
ref={sheetRef}
isOpen={isOpen}
onDismiss={navigation.goBack}
>
{status.type === "loading" ? (
<Loading />
) : status.type === "error" ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,7 @@ export const BackgroundMapInfo = ({
mapName={name}
mapId={id}
closeSheet={closeSheet}
// If open, hardware backpress returns true, which disables the back button and vice versa
onHardwareBackPress={() => isOpen}
isOpen={isOpen}
/>
</View>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ interface DeleteMapBottomSheetProps {
mapName: string;
mapId: string;
closeSheet: () => void;
onHardwareBackPress: () => boolean;
isOpen: boolean;
}

export const DeleteMapBottomSheet = React.forwardRef<
BottomSheetModalMethods,
DeleteMapBottomSheetProps
>(({ mapName, closeSheet, mapId, onHardwareBackPress }, sheetRef) => {
>(({ isOpen, mapName, closeSheet, mapId }, sheetRef) => {
const { navigate } = useNavigationFromRoot();
const { formatMessage: t } = useIntl();
const { selectedStyleId, setSelectedStyleId } = useMapStyles();
Expand All @@ -68,11 +68,7 @@ export const DeleteMapBottomSheet = React.forwardRef<
}

return (
<BottomSheetModal
ref={sheetRef}
onDismiss={closeSheet}
onHardwareBackPress={onHardwareBackPress}
>
<BottomSheetModal ref={sheetRef} onDismiss={closeSheet} isOpen={isOpen}>
<BottomSheetContent
descriptionStyle={{ fontSize: 16 }}
title={
Expand Down
92 changes: 49 additions & 43 deletions src/frontend/sharedComponents/BottomSheetModal/index.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import * as React from "react";
import { BackHandler, useWindowDimensions } from "react-native";
import {
BackHandler,
NativeEventSubscription,
useWindowDimensions,
} from "react-native";
import {
BottomSheetModal as RNBottomSheetModal,
BottomSheetModalProvider,
BottomSheetView,
} from "@gorhom/bottom-sheet";
import { NativeStackNavigationOptions } from "@react-navigation/native-stack";
Expand All @@ -23,7 +26,7 @@ export const useBottomSheetModal = ({
}) => {
const initiallyOpenedRef = React.useRef(false);
const sheetRef = React.useRef<RNBottomSheetModal>(null);
const [isOpen, setIsOpen] = React.useState(false);
const [isOpen, setIsOpen] = React.useState(openOnMount);

const closeSheet = React.useCallback(() => {
if (sheetRef.current) {
Expand All @@ -49,24 +52,27 @@ export const useBottomSheetModal = ({
return { sheetRef, closeSheet, openSheet, isOpen };
};

const useBackPressHandler = (onHardwareBackPress?: () => void | boolean) => {
// Handles cases where Android hardware backpress button is used to trigger navigation events
// not covered by usePreventNavigationBack
const useBackHandler = (enable: boolean, onBack?: () => void) => {
React.useEffect(() => {
const onBack = () => {
if (onHardwareBackPress) {
const backPress = onHardwareBackPress();
if (typeof backPress === "boolean") {
return backPress;
}
}
let subscription: NativeEventSubscription;

// We don't allow the back press to navigate/dismiss this modal by default
return true;
};
if (enable) {
// This event is triggered by both Android hardware back press and gesture back swipe
subscription = BackHandler.addEventListener("hardwareBackPress", () => {
if (onBack) {
onBack();
}

BackHandler.addEventListener("hardwareBackPress", onBack);
return true;
});
}

return () => BackHandler.removeEventListener("hardwareBackPress", onBack);
}, [onHardwareBackPress]);
return () => {
if (subscription) subscription.remove();
};
}, [enable, onBack]);
};

const useSnapPointsCalculator = () => {
Expand Down Expand Up @@ -97,42 +103,42 @@ const useSnapPointsCalculator = () => {
};

interface Props extends React.PropsWithChildren<{}> {
onDismiss: () => void;
onHardwareBackPress?: () => void | boolean;
isOpen: boolean;
onDismiss?: () => void;
// Triggered by: Android hardware back press and gesture back swipe
onBack?: () => void;
snapPoints?: (string | number)[];
disableBackrop?: boolean;
}

export const BottomSheetModal = React.forwardRef<RNBottomSheetModal, Props>(
({ children, onDismiss, onHardwareBackPress, disableBackrop }, ref) => {
useBackPressHandler(onHardwareBackPress);
({ children, isOpen, onDismiss, onBack, disableBackrop }, ref) => {
useBackHandler(isOpen, onBack);

const { snapPoints, updateSheetHeight } = useSnapPointsCalculator();

return (
<BottomSheetModalProvider>
<RNBottomSheetModal
ref={ref}
backdropComponent={disableBackrop ? null : Backdrop}
enableContentPanningGesture={false}
enableHandlePanningGesture={false}
handleComponent={() => null}
index={0}
onDismiss={onDismiss}
snapPoints={snapPoints}
<RNBottomSheetModal
ref={ref}
backdropComponent={disableBackrop ? null : Backdrop}
enableContentPanningGesture={false}
enableHandlePanningGesture={false}
handleComponent={() => null}
index={0}
onDismiss={onDismiss}
snapPoints={snapPoints}
>
<BottomSheetView
onLayout={updateSheetHeight}
style={{
flex: 1,
paddingHorizontal: 20,
paddingTop: 30,
}}
>
<BottomSheetView
onLayout={updateSheetHeight}
style={{
flex: 1,
paddingHorizontal: 20,
paddingTop: 30,
}}
>
{children}
</BottomSheetView>
</RNBottomSheetModal>
</BottomSheetModalProvider>
{children}
</BottomSheetView>
</RNBottomSheetModal>
);
}
);
Expand Down