Skip to content
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
174 changes: 57 additions & 117 deletions app/containers/ActionSheet/ActionSheet.tsx
Original file line number Diff line number Diff line change
@@ -1,98 +1,57 @@
import { useBackHandler } from '@react-native-community/hooks';
import * as Haptics from 'expo-haptics';
import React, { forwardRef, isValidElement, useEffect, useImperativeHandle, useRef, useState, useCallback } from 'react';
import { Keyboard, type LayoutChangeEvent, useWindowDimensions } from 'react-native';
import { Easing, useDerivedValue, useSharedValue } from 'react-native-reanimated';
import BottomSheet, { BottomSheetBackdrop, type BottomSheetBackdropProps } from '@discord/bottom-sheet';
import React, { forwardRef, isValidElement, useCallback, useEffect, useImperativeHandle, useRef, useState } from 'react';
import { Keyboard, type LayoutChangeEvent } from 'react-native';
import { TrueSheet } from '@lodev09/react-native-true-sheet';
import { useSafeAreaInsets } from 'react-native-safe-area-context';

import { useResponsiveLayout } from '../../lib/hooks/useResponsiveLayout/useResponsiveLayout';
import { useTheme } from '../../theme';
import { isIOS, isTablet } from '../../lib/methods/helpers';
import { isTablet } from '../../lib/methods/helpers';
import { Handle } from './Handle';
import { type TActionSheetOptions } from './Provider';
import BottomSheetContent from './BottomSheetContent';
import styles from './styles';
import { ACTION_SHEET_MAX_HEIGHT_FRACTION } from './utils';
import { useActionSheetDetents } from './useActionSheetDetents';

export const ACTION_SHEET_ANIMATION_DURATION = 250;
const HANDLE_HEIGHT = 28;
const CANCEL_HEIGHT = 64;

const ANIMATION_CONFIG = {
duration: ACTION_SHEET_ANIMATION_DURATION,
// https://easings.net/#easeInOutCubic
easing: Easing.bezier(0.645, 0.045, 0.355, 1.0)
};

const ActionSheet = React.memo(
forwardRef(({ children }: { children: React.ReactElement }, ref) => {
const { colors } = useTheme();
const { height: windowHeight } = useWindowDimensions();
const { height: windowHeight, fontScale } = useResponsiveLayout();
const { bottom, right, left } = useSafeAreaInsets();
const { fontScale } = useWindowDimensions();
const itemHeight = 48 * fontScale;
const bottomSheetRef = useRef<BottomSheet>(null);
const sheetRef = useRef<TrueSheet>(null);
const [data, setData] = useState<TActionSheetOptions>({} as TActionSheetOptions);
const [isVisible, setVisible] = useState(false);
const animatedContentHeight = useSharedValue(0);
const animatedHandleHeight = useSharedValue(0);
const animatedDataSnaps = useSharedValue<TActionSheetOptions['snaps']>([]);
const animatedSnapPoints = useDerivedValue(() => {
if (animatedDataSnaps.value?.length) {
return animatedDataSnaps.value;
}
const contentWithHandleHeight = animatedContentHeight.value + animatedHandleHeight.value;
// Bottom sheet requires a default value to work
if (contentWithHandleHeight === 0) {
return ['25%'];
}
return [contentWithHandleHeight];
}, [data]);
const [contentHeight, setContentHeight] = useState(0);

const itemHeight = 48 * fontScale;

const detents = useActionSheetDetents({
data,
windowHeight,
contentHeight,
itemHeight,
bottom
});

const handleContentLayout = useCallback(
({
nativeEvent: {
layout: { height }
}
}: LayoutChangeEvent) => {
/**
* This logic is only necessary to prevent the action sheet from
* occupying the entire screen when the dynamic content is too big.
*/
animatedContentHeight.value = Math.min(height, windowHeight * 0.8);
({ nativeEvent: { layout } }: LayoutChangeEvent) => {
const height = Math.min(layout.height, windowHeight * ACTION_SHEET_MAX_HEIGHT_FRACTION);
setContentHeight(height);
},
[animatedContentHeight, windowHeight]
[windowHeight]
);

const maxSnap = Math.min(
(itemHeight + 0.5) * (data?.options?.length || 0) +
HANDLE_HEIGHT +
// Custom header height
(data?.headerHeight || 0) +
// Insets bottom height (Notch devices)
bottom +
// Cancel button height
(data?.hasCancel ? CANCEL_HEIGHT : 0),
windowHeight * 0.8
);

/*
* if the action sheet cover more than 60% of the screen height,
* we'll provide more one snap of 50%
*/
const snaps = maxSnap > windowHeight * 0.6 && !data.snaps ? ['50%', maxSnap] : [maxSnap];

const toggleVisible = () => setVisible(!isVisible);

const hide = () => {
bottomSheetRef.current?.close();
sheetRef.current?.dismiss();
};

const show = (options: TActionSheetOptions) => {
setData(options);
if (options.snaps?.length) {
animatedDataSnaps.value = options.snaps;
}
toggleVisible();
setVisible(true);
};

useBackHandler(() => {
Expand All @@ -102,6 +61,12 @@ const ActionSheet = React.memo(
return isVisible;
});

useEffect(() => {
if (isVisible) {
sheetRef.current?.present(0);
}
}, [isVisible]);

useEffect(() => {
if (isVisible) {
Keyboard.dismiss();
Expand All @@ -121,62 +86,37 @@ const ActionSheet = React.memo(
</>
);

const onClose = () => {
toggleVisible();
data?.onClose && data?.onClose();
animatedDataSnaps.value = [];
const onDidDismiss = () => {
setVisible(false);
setContentHeight(0);
data?.onClose?.();
};

const renderBackdrop = useCallback(
(props: BottomSheetBackdropProps) => (
<BottomSheetBackdrop
{...props}
appearsOnIndex={0}
// Backdrop should be visible all the time bottom sheet is open
disappearsOnIndex={-1}
opacity={colors.backdropOpacity}
/>
),
[]
);

const bottomSheet = isTablet ? styles.bottomSheet : { marginRight: right, marginLeft: left };

// Must need this prop to avoid keyboard dismiss
// when is android tablet and the input text is focused
const androidTablet: any = isTablet && !isIOS ? { android_keyboardInputMode: 'adjustResize' } : {};
const bottomSheetStyle = isTablet ? styles.bottomSheet : { marginRight: right, marginLeft: left };

return (
<>
{children}
{isVisible && (
<BottomSheet
ref={bottomSheetRef}
// If data.options exist, we calculate snaps to be precise, otherwise we cal
snapPoints={data.options?.length ? snaps : animatedSnapPoints}
handleHeight={animatedHandleHeight}
// We need undefined to enable vertical swipe gesture inside the bottom sheet like in reaction picker
contentHeight={data.snaps?.length || data.options?.length ? undefined : animatedContentHeight}
animationConfigs={ANIMATION_CONFIG}
animateOnMount={true}
backdropComponent={renderBackdrop}
handleComponent={renderHandle}
enablePanDownToClose
style={{ ...styles.container, ...bottomSheet }}
backgroundStyle={{ backgroundColor: colors.surfaceLight }}
onChange={index => index === -1 && onClose()}
// We need this to allow horizontal swipe gesture inside the bottom sheet like in reaction picker
enableContentPanningGesture={data?.enableContentPanningGesture ?? true}
{...androidTablet}>
<BottomSheetContent
options={data?.options}
hide={hide}
children={data?.children}
hasCancel={data?.hasCancel}
onLayout={handleContentLayout}
/>
</BottomSheet>
)}
<TrueSheet
ref={sheetRef}
detents={detents}
maxHeight={windowHeight * ACTION_SHEET_MAX_HEIGHT_FRACTION}
backgroundColor={colors.surfaceLight}
cornerRadius={16}
dimmed
grabber={false}
header={renderHandle()}
scrollable={!!data?.options}
style={[styles.container, bottomSheetStyle]}
onDidDismiss={onDidDismiss}>
<BottomSheetContent
options={data?.options}
hide={hide}
children={data?.children}
hasCancel={data?.hasCancel}
onLayout={handleContentLayout}
/>
Comment on lines +112 to +118
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Pass children via JSX elements instead of the children prop.

As flagged by static analysis (Biome noChildrenProp), passing children via a prop is not the canonical React pattern. Use JSX nesting instead.

♻️ Proposed fix
 				<BottomSheetContent
 					options={data?.options}
 					hide={hide}
-					children={data?.children}
 					hasCancel={data?.hasCancel}
-					onLayout={handleContentLayout}
-				/>
+					onLayout={handleContentLayout}>
+					{data?.children}
+				</BottomSheetContent>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<BottomSheetContent
options={data?.options}
hide={hide}
children={data?.children}
hasCancel={data?.hasCancel}
onLayout={handleContentLayout}
/>
<BottomSheetContent
options={data?.options}
hide={hide}
hasCancel={data?.hasCancel}
onLayout={handleContentLayout}>
{data?.children}
</BottomSheetContent>
🧰 Tools
🪛 Biome (2.3.13)

[error] 115-115: Avoid passing children using a prop

The canonical way to pass children in React is to use JSX elements

(lint/correctness/noChildrenProp)

🤖 Prompt for AI Agents
In `@app/containers/ActionSheet/ActionSheet.tsx` around lines 112 - 118, The
BottomSheetContent usage is passing children via the children prop
(data?.children), which violates React conventions and triggers the
noChildrenProp rule; change the invocation of BottomSheetContent to nest the
children JSX instead (e.g., replace the children={data?.children} prop with
<BottomSheetContent options={data?.options} hide={hide}
hasCancel={data?.hasCancel}
onLayout={handleContentLayout}>{data?.children}</BottomSheetContent>) and update
any related prop typings in the BottomSheetContent component (prop name/type for
children) if necessary.

</TrueSheet>
</>
);
})
Expand Down
11 changes: 6 additions & 5 deletions app/containers/ActionSheet/BottomSheetContent.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { Text, useWindowDimensions, type ViewProps } from 'react-native';
import { FlatList, Text, useWindowDimensions, View, type ViewProps } from 'react-native';
import React from 'react';
import { BottomSheetView, BottomSheetFlatList } from '@discord/bottom-sheet';
import { useSafeAreaInsets } from 'react-native-safe-area-context';

import I18n from '../../i18n';
import { useTheme } from '../../theme';
import { isAndroid } from '../../lib/methods/helpers';
import { type IActionSheetItem, Item } from './Item';
import { type TActionSheetOptionsItem } from './Provider';
import styles from './styles';
Expand Down Expand Up @@ -41,7 +41,7 @@ const BottomSheetContent = React.memo(({ options, hasCancel, hide, children, onL

if (options) {
return (
<BottomSheetFlatList
<FlatList
testID='action-sheet'
data={options}
refreshing={false}
Expand All @@ -56,13 +56,14 @@ const BottomSheetContent = React.memo(({ options, hasCancel, hide, children, onL
ListHeaderComponent={List.Separator}
ListFooterComponent={renderFooter}
onLayout={onLayout}
nestedScrollEnabled={isAndroid}
/>
);
}
return (
<BottomSheetView testID='action-sheet' style={styles.contentContainer} onLayout={onLayout}>
<View testID='action-sheet' style={styles.contentContainer} onLayout={onLayout}>
{children}
</BottomSheetView>
</View>
);
});

Expand Down
26 changes: 26 additions & 0 deletions app/containers/ActionSheet/useActionSheetDetents.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { useMemo } from 'react';

import type { TActionSheetOptions } from './Provider';
import { getDetents } from './utils';

interface UseActionSheetDetentsParams {
data: TActionSheetOptions;
windowHeight: number;
contentHeight: number;
itemHeight: number;
bottom: number;
}

export function useActionSheetDetents({ data, windowHeight, contentHeight, itemHeight, bottom }: UseActionSheetDetentsParams) {
return useMemo(
() =>
getDetents({
data,
windowHeight,
contentHeight,
itemHeight,
bottom
}),
[data, windowHeight, contentHeight, itemHeight, bottom]
);
}
61 changes: 61 additions & 0 deletions app/containers/ActionSheet/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import type { SheetDetent } from '@lodev09/react-native-true-sheet';

import type { TActionSheetOptions } from './Provider';

export const ACTION_SHEET_MIN_HEIGHT_FRACTION = 0.35;
export const ACTION_SHEET_MAX_HEIGHT_FRACTION = 0.75;
export const HANDLE_HEIGHT = 28;
export const CANCEL_HEIGHT = 64;

interface IGetDetentsParams {
data: TActionSheetOptions;
windowHeight: number;
contentHeight: number;
itemHeight: number;
bottom: number;
}

export const normalizeSnapsToDetents = (snaps: (string | number)[]): number[] =>
snaps
.slice(0, 3)
.map(snap => {
if (typeof snap === 'number') {
if (snap <= 0 || snap > 1) return Math.min(1, Math.max(0.1, snap));
return snap;
}
const match = String(snap).match(/^(\d+(?:\.\d+)?)\s*%$/);
if (match) return Math.min(1, Math.max(0.1, Number(match[1]) / 100));
return 0.5;
})
.sort((a, b) => a - b);
Comment on lines +18 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

rg -n --type=ts --type=tsx 'snaps\s*[:=]' -C 3 | head -100

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 105


🏁 Script executed:

# Look at the actual file to understand the function better
cat -n app/containers/ActionSheet/utils.ts

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 2509


🏁 Script executed:

# Check Provider.tsx for context about snaps type and usage
rg -n 'snaps' app/containers/ActionSheet/Provider.tsx -C 2

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 401


🏁 Script executed:

rg -n 'snaps\s*[=:]\s*\[' --type ts -C 2

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 1045


🏁 Script executed:

# Also search for ActionSheet component usage
rg -n 'ActionSheet' --type ts -C 1 | grep -A 2 -B 2 'snaps' | head -60

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 353


🏁 Script executed:

# Look for any comments or migration notes about snaps/pixel values
rg -n 'pixel|detent|snap' app/containers/ActionSheet/ --type ts

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 1110


🏁 Script executed:

cat -n app/lib/hooks/useVideoConf/index.tsx | sed -n '55,75p'

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 651


🏁 Script executed:

# Check git history or comments around this line
git log --oneline -n 20 -- app/lib/hooks/useVideoConf/index.tsx 2>/dev/null || echo "Git history not available"

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 125


🏁 Script executed:

# Check if there are any other references to understanding what 480 means in this context
rg -n '480|windowHeight' app/lib/hooks/useVideoConf/index.tsx -C 5

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 375


🏁 Script executed:

# Check if there's a test or if 480 is actually being used
rg -n 'StartACallActionSheet' --type ts -C 3 | head -40

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 1712


🏁 Script executed:

# Check the old bottom-sheet or previous implementation for context
rg -n 'bottom.?sheet|BottomSheet' --type ts -i | head -20

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 2068


🏁 Script executed:

# Check if SheetDetent type is defined locally or imported
fd 'SheetDetent' --type ts -x cat -n {} 2>/dev/null | head -20

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 60


🏁 Script executed:

# Check if there's documentation about what the expected snap values should be
rg -n 'TrueSheet|react-native-true-sheet' --type ts -C 2 | head -40

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 2025


🏁 Script executed:

cat -n app/containers/ActionSheet/utils.ts | sed -n '18,65p'

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 1943


🏁 Script executed:

# Check ActionSheet.tsx to see how detents is used
cat -n app/containers/ActionSheet/ActionSheet.tsx | sed -n '30,50p'

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 693


🏁 Script executed:

# Get the exact function signatures and return types
rg -n 'useActionSheetDetents' app/containers/ActionSheet/ -A 10 -B 2

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 3252


🏁 Script executed:

# Verify the type of detents variable
grep -n 'detents.*=' app/containers/ActionSheet/ActionSheet.tsx | head -5

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 149


normalizeSnapsToDetents silently converts pixel-based snap values to incorrect detents.

The code supports two snap formats—legacy pixel values (e.g., snaps: [480]) and the new fraction/percentage format. However, line 23's clamping logic is flawed: any numeric value > 1 is treated as a pixel value to clamp, but without windowHeight as a parameter, normalizeSnapsToDetents cannot convert pixels to fractions correctly.

Example: snaps: [480] (used in useVideoConf/index.tsx) becomes Math.min(1, Math.max(0.1, 480)) = 1.0, resulting in a full-height sheet instead of the intended ~480px detent.

To fix this, either:

  1. Pass windowHeight to normalizeSnapsToDetents and convert pixel values to fractions (e.g., 480 / windowHeight)
  2. Migrate all callers to use fractions/percentages only and reject numeric values > 1

Also note: getDetents returns SheetDetent[] but normalizeSnapsToDetents only returns number[], missing the possibility of 'auto' detents.

🤖 Prompt for AI Agents
In `@app/containers/ActionSheet/utils.ts` around lines 18 - 30,
normalizeSnapsToDetents currently treats numeric snaps >1 as already-fractional
and clamps them, which incorrectly turns pixel values (e.g., 480) into 1.0;
update normalizeSnapsToDetents to accept a windowHeight parameter and convert
numeric snaps >1 into fractions by dividing by windowHeight before applying the
existing clamp (Math.min(1, Math.max(0.1, ...))), ensure string percentage
handling remains unchanged, and update callers (e.g., useVideoConf/index.tsx) to
pass window.innerHeight (or equivalent) or migrate them to percent/fraction
inputs; also make normalizeSnapsToDetents able to return SheetDetent[]
(including 'auto') consistent with getDetents or adjust getDetents to map the
returned number[] into proper SheetDetent entries so 'auto' detents are
preserved.


export const getDetents = ({ data, windowHeight, contentHeight, itemHeight, bottom }: IGetDetentsParams): SheetDetent[] => {
const hasOptions = (data?.options?.length || 0) > 0;
const maxSnap = hasOptions
? Math.min(
(itemHeight + 0.5) * (data?.options?.length || 0) +
HANDLE_HEIGHT +
(data?.headerHeight || 0) +
bottom +
(data?.hasCancel ? CANCEL_HEIGHT : 0),
windowHeight * ACTION_SHEET_MAX_HEIGHT_FRACTION
)
: 0;

if (data?.snaps?.length) {
return normalizeSnapsToDetents(data.snaps);
}
if (hasOptions) {
if (maxSnap > windowHeight * 0.6) {
return [0.5, ACTION_SHEET_MAX_HEIGHT_FRACTION];
}
const fraction = Math.max(0.25, Math.min(maxSnap / windowHeight, ACTION_SHEET_MAX_HEIGHT_FRACTION));
return [fraction];
}
if (contentHeight > 0) {
const fraction = Math.min(contentHeight / windowHeight, ACTION_SHEET_MAX_HEIGHT_FRACTION);
const contentDetent = Math.max(0.25, fraction);
return contentDetent > ACTION_SHEET_MIN_HEIGHT_FRACTION ? [ACTION_SHEET_MIN_HEIGHT_FRACTION, contentDetent] : [contentDetent];
}
return [ACTION_SHEET_MIN_HEIGHT_FRACTION, 'auto'];
};
4 changes: 1 addition & 3 deletions app/containers/TextInput/FormTextInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
View,
type ViewStyle
} from 'react-native';
import { BottomSheetTextInput } from '@discord/bottom-sheet';
import Touchable from 'react-native-platform-touchable';
import { A11y } from 'react-native-a11y-order';

Expand Down Expand Up @@ -124,7 +123,6 @@ export const FormTextInput = ({
const { colors } = useTheme();
const [showPassword, setShowPassword] = useState(false);
const showClearInput = onClearInput && value && value.length > 0;
const Input = bottomSheet ? BottomSheetTextInput : TextInput;

const inputError = getInputError(error);
const accessibilityLabelText = useMemo(() => {
Expand All @@ -151,7 +149,7 @@ export const FormTextInput = ({
) : null}

<View accessible={false} style={styles.wrap}>
<Input
<TextInput
accessible
accessibilityLabel={accessibilityLabelText}
style={[
Expand Down
Loading