Skip to content

Commit

Permalink
Merge pull request #53440 from margelo/fix/53423-handle-insets-in-modal
Browse files Browse the repository at this point in the history
[CP Staging] fix: consume insets in Modals anyway because they are mounted in different native view hierarchy
  • Loading branch information
puneetlath authored Dec 3, 2024
2 parents a1585a9 + 6650009 commit 072b2a6
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 10 deletions.
1 change: 1 addition & 0 deletions src/components/Modal/BaseModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ function BaseModal(
const modalContextValue = useMemo(
() => ({
activeModalType: isVisible ? type : undefined,
default: false,
}),
[isVisible, type],
);
Expand Down
3 changes: 2 additions & 1 deletion src/components/Modal/ModalContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ type ModalContextType = {
// The type of the currently displayed modal, or undefined if there is no currently displayed modal.
// Note that React Native can only display one modal at a time.
activeModalType?: ModalType;
default: boolean;
};

// This context is meant to inform modal children that they are rendering in a modal (and what type of modal they are rendering in)
// Note that this is different than ONYXKEYS.MODAL.isVisible data point in that that is a global variable for whether a modal is visible or not,
// whereas this context is provided by the BaseModal component, and thus is only available to components rendered inside a modal.
const ModalContext = createContext<ModalContextType>({});
const ModalContext = createContext<ModalContextType>({default: true});

export default ModalContext;
13 changes: 11 additions & 2 deletions src/components/ScreenWrapper.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {UNSTABLE_usePreventRemove, useIsFocused, useNavigation, useRoute} from '@react-navigation/native';
import type {ForwardedRef, ReactNode} from 'react';
import React, {createContext, forwardRef, useEffect, useMemo, useRef, useState} from 'react';
import React, {createContext, forwardRef, useContext, useEffect, useMemo, useRef, useState} from 'react';
import type {StyleProp, ViewStyle} from 'react-native';
import {Keyboard, NativeModules, PanResponder, View} from 'react-native';
import {PickerAvoidingView} from 'react-native-picker-select';
Expand All @@ -25,6 +25,7 @@ import type FocusTrapForScreenProps from './FocusTrap/FocusTrapForScreen/FocusTr
import HeaderGap from './HeaderGap';
import ImportedStateIndicator from './ImportedStateIndicator';
import KeyboardAvoidingView from './KeyboardAvoidingView';
import ModalContext from './Modal/ModalContext';
import OfflineIndicator from './OfflineIndicator';
import withNavigationFallback from './withNavigationFallback';

Expand Down Expand Up @@ -149,6 +150,8 @@ function ScreenWrapper(
const navigation = navigationProp ?? navigationFallback;
const isFocused = useIsFocused();
const {windowHeight} = useWindowDimensions(shouldUseCachedViewportHeight);
// since Modals are drawn in separate native view hierarchy we should always add paddings
const ignoreInsetsConsumption = !useContext(ModalContext).default;

// We need to use isSmallScreenWidth instead of shouldUseNarrowLayout for a case where we want to show the offline indicator only on small screens
// eslint-disable-next-line rulesdir/prefer-shouldUseNarrowLayout-instead-of-isSmallScreenWidth
Expand Down Expand Up @@ -237,19 +240,25 @@ function ScreenWrapper(
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, []);

const {insets, paddingTop, paddingBottom, safeAreaPaddingBottomStyle} = useStyledSafeAreaInsets();
const {insets, paddingTop, paddingBottom, safeAreaPaddingBottomStyle, unmodifiedPaddings} = useStyledSafeAreaInsets();
const paddingStyle: StyleProp<ViewStyle> = {};

const isSafeAreaTopPaddingApplied = includePaddingTop;
if (includePaddingTop) {
paddingStyle.paddingTop = paddingTop;
}
if (includePaddingTop && ignoreInsetsConsumption) {
paddingStyle.paddingTop = unmodifiedPaddings.top;
}

// We always need the safe area padding bottom if we're showing the offline indicator since it is bottom-docked.
const isSafeAreaBottomPaddingApplied = includeSafeAreaPaddingBottom || (isOffline && shouldShowOfflineIndicator);
if (isSafeAreaBottomPaddingApplied) {
paddingStyle.paddingBottom = paddingBottom;
}
if (isSafeAreaBottomPaddingApplied && ignoreInsetsConsumption) {
paddingStyle.paddingBottom = unmodifiedPaddings.bottom;
}

const isAvoidingViewportScroll = useTackInputFocus(isFocused && shouldEnableMaxHeight && shouldAvoidScrollOnVirtualViewport && Browser.isMobileWebKit());
const contextValue = useMemo(
Expand Down
5 changes: 1 addition & 4 deletions src/components/TextPicker/TextSelectorModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import Text from '@components/Text';
import TextInput from '@components/TextInput';
import type {BaseTextInputRef} from '@components/TextInput/BaseTextInput/types';
import useLocalize from '@hooks/useLocalize';
import useSafeAreaInsets from '@hooks/useSafeAreaInsets';
import useThemeStyles from '@hooks/useThemeStyles';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
Expand All @@ -22,7 +21,6 @@ function TextSelectorModal({value, description = '', subtitle, onValueSelected,
const styles = useThemeStyles();

const [currentValue, setValue] = useState(value);
const {top} = useSafeAreaInsets();

const inputRef = useRef<BaseTextInputRef | null>(null);
const inputValueRef = useRef(value);
Expand Down Expand Up @@ -81,11 +79,10 @@ function TextSelectorModal({value, description = '', subtitle, onValueSelected,
shouldUseModalPaddingStyle={false}
>
<ScreenWrapper
includePaddingTop={false}
includePaddingTop
includeSafeAreaPaddingBottom
testID={TextSelectorModal.displayName}
shouldEnableMaxHeight
style={{paddingTop: top}}
>
<HeaderWithBackButton
title={description}
Expand Down
4 changes: 1 addition & 3 deletions src/components/ValidateCodeActionModal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import HeaderWithBackButton from '@components/HeaderWithBackButton';
import Modal from '@components/Modal';
import ScreenWrapper from '@components/ScreenWrapper';
import Text from '@components/Text';
import useSafeAreaInsets from '@hooks/useSafeAreaInsets';
import useThemeStyles from '@hooks/useThemeStyles';
import Navigation from '@libs/Navigation/Navigation';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -33,7 +32,6 @@ function ValidateCodeActionModal({
const themeStyles = useThemeStyles();
const firstRenderRef = useRef(true);
const validateCodeFormRef = useRef<ValidateCodeFormHandle>(null);
const {top} = useSafeAreaInsets();

const [validateCodeAction] = useOnyx(ONYXKEYS.VALIDATE_ACTION_CODE);

Expand Down Expand Up @@ -64,10 +62,10 @@ function ValidateCodeActionModal({
>
<ScreenWrapper
includeSafeAreaPaddingBottom
includePaddingTop
shouldEnableMaxHeight
testID={ValidateCodeActionModal.displayName}
offlineIndicatorStyle={themeStyles.mtAuto}
style={{paddingTop: top}}
>
<HeaderWithBackButton
title={title}
Expand Down
4 changes: 4 additions & 0 deletions src/hooks/useStyledSafeAreaInsets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ function useStyledSafeAreaInsets() {
return {
paddingTop: isSafeAreaTopPaddingApplied ? 0 : paddingTop,
paddingBottom: adaptedPaddingBottom,
unmodifiedPaddings: {
top: paddingTop,
bottom: paddingBottom,
},
insets: adaptedInsets,
safeAreaPaddingBottomStyle,
};
Expand Down

0 comments on commit 072b2a6

Please sign in to comment.