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.

Sign up for GitHub

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

feat: V3 Split Logic #1089

Closed
wants to merge 6 commits into from
Closed

feat: V3 Split Logic #1089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b0469ac
WIP
alexrisch Oct 21, 2024
0991c16
WIP
alexrisch Oct 21, 2024
267c1c3
feat: V3 Split WIP
alexrisch Oct 23, 2024
194052e
WIP
alexrisch Oct 23, 2024
9576073
Finalized Conversation List Component
alexrisch Oct 24, 2024
1b00846
Frames stuff
alexrisch Oct 24, 2024
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
248 changes: 248 additions & 0 deletions components/Chat/ChatDumb.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,248 @@
import { FlashList, ListRenderItem } from "@shopify/flash-list";
import {
backgroundColor,
itemSeparatorColor,
tertiaryBackgroundColor,
} from "@styles/colors";
import React, { useCallback, useEffect, useMemo, useRef } from "react";
import {
Dimensions,
FlatList,
Platform,
StyleSheet,
View,
useColorScheme,
} from "react-native";
import Animated, {
useAnimatedStyle,
useDerivedValue,
useSharedValue,
} from "react-native-reanimated";
import { useSafeAreaInsets } from "react-native-safe-area-context";

import ChatInput from "./Input/Input";
import TransactionInput from "./Transaction/TransactionInput";
import { ReanimatedFlashList, ReanimatedView } from "../../utils/animations";
import { useKeyboardAnimation } from "../../utils/animations/keyboardAnimation";
import { converseEventEmitter } from "../../utils/events";

type ChatDumbProps<T> = {
onReadyToFocus: () => void;
transactionMode: boolean;
frameTextInputFocused: boolean;
items: T[];
renderItem: ListRenderItem<T>;
keyExtractor: (item: T) => string;
showChatInput: boolean;
ListFooterComponent: React.JSX.Element | null;
showPlaceholder: boolean;
key?: string;
displayList: boolean;
refreshing: boolean;
getItemType: (
item: T,
index: number,
extraData?: any
) => string | number | undefined;
placeholderComponent: React.JSX.Element | null;
extraData?: any;
itemToId: (id: T) => string;
};
Comment on lines +29 to +50
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

Consider adding common list interaction props.

The ChatDumbProps interface could benefit from additional props commonly needed in chat interfaces:

  • onRefresh?: () => void - Callback for pull-to-refresh
  • onEndReached?: () => void - Callback for pagination
  • error?: Error - Error state handling
  • onRetry?: () => void - Error retry callback
 type ChatDumbProps<T> = {
   onReadyToFocus: () => void;
   transactionMode: boolean;
   frameTextInputFocused: boolean;
   items: T[];
   renderItem: ListRenderItem<T>;
   keyExtractor: (item: T) => string;
   showChatInput: boolean;
   ListFooterComponent: React.JSX.Element | null;
   showPlaceholder: boolean;
   key?: string;
   displayList: boolean;
   refreshing: boolean;
+  onRefresh?: () => void;
+  onEndReached?: () => void;
+  error?: Error;
+  onRetry?: () => void;
   getItemType: (
     item: T,
     index: number,
     extraData?: any
   ) => string | number | undefined;
   placeholderComponent: React.JSX.Element | null;
   extraData?: any;
   itemToId: (id: T) => string;
 };
📝 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
type ChatDumbProps<T> = {
onReadyToFocus: () => void;
transactionMode: boolean;
frameTextInputFocused: boolean;
items: T[];
renderItem: ListRenderItem<T>;
keyExtractor: (item: T) => string;
showChatInput: boolean;
ListFooterComponent: React.JSX.Element | null;
showPlaceholder: boolean;
key?: string;
displayList: boolean;
refreshing: boolean;
getItemType: (
item: T,
index: number,
extraData?: any
) => string | number | undefined;
placeholderComponent: React.JSX.Element | null;
extraData?: any;
itemToId: (id: T) => string;
};
type ChatDumbProps<T> = {
onReadyToFocus: () => void;
transactionMode: boolean;
frameTextInputFocused: boolean;
items: T[];
renderItem: ListRenderItem<T>;
keyExtractor: (item: T) => string;
showChatInput: boolean;
ListFooterComponent: React.JSX.Element | null;
showPlaceholder: boolean;
key?: string;
displayList: boolean;
refreshing: boolean;
onRefresh?: () => void;
onEndReached?: () => void;
error?: Error;
onRetry?: () => void;
getItemType: (
item: T,
index: number,
extraData?: any
) => string | number | undefined;
placeholderComponent: React.JSX.Element | null;
extraData?: any;
itemToId: (id: T) => string;
};


const useStyles = () => {
const colorScheme = useColorScheme();
return useMemo(
() =>
StyleSheet.create({
chatContainer: {
flex: 1,
justifyContent: "flex-end",
backgroundColor: backgroundColor(colorScheme),
},
chatContent: {
backgroundColor: backgroundColor(colorScheme),
flex: 1,
},
chatPreviewContent: {
backgroundColor: backgroundColor(colorScheme),
flex: 1,
paddingBottom: 0,
},
chat: {
backgroundColor: backgroundColor(colorScheme),
},
inputBottomFiller: {
position: "absolute",
width: "100%",
bottom: 0,
backgroundColor: backgroundColor(colorScheme),
zIndex: 0,
},
inChatRecommendations: {
borderBottomWidth: 0.5,
borderBottomColor: itemSeparatorColor(colorScheme),
marginHorizontal: 20,
marginBottom: 10,
},
}),
[colorScheme]
);
};

export function ChatDumb<T>({
onReadyToFocus,
transactionMode,
frameTextInputFocused,
items,
renderItem,
keyExtractor,
showChatInput,
showPlaceholder,
key,
displayList,
refreshing,
ListFooterComponent,
getItemType,
placeholderComponent,
extraData,
itemToId,
}: ChatDumbProps<T>) {
const colorScheme = useColorScheme();
const styles = useStyles();

const hideInputIfFrameFocused = Platform.OS !== "web";
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

Improve platform check robustness.

The platform check could be more explicit to handle web platforms consistently.

-const hideInputIfFrameFocused = Platform.OS !== "web";
+const hideInputIfFrameFocused = Platform.OS === "ios" || Platform.OS === "android";
📝 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
const hideInputIfFrameFocused = Platform.OS !== "web";
const hideInputIfFrameFocused = Platform.OS === "ios" || Platform.OS === "android";


const DEFAULT_INPUT_HEIGHT = 58;
const chatInputHeight = useSharedValue(0);
const chatInputDisplayedHeight = useDerivedValue(() => {
return frameTextInputFocused && hideInputIfFrameFocused
? 0
: chatInputHeight.value + DEFAULT_INPUT_HEIGHT;
});
Comment on lines +116 to +121
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

Consider memoizing shared values.

The shared values and derived values should be memoized to prevent unnecessary recalculations during re-renders.

-const chatInputHeight = useSharedValue(0);
-const chatInputDisplayedHeight = useDerivedValue(() => {
+const chatInputHeight = useMemo(() => useSharedValue(0), []);
+const chatInputDisplayedHeight = useMemo(() => useDerivedValue(() => {
   return frameTextInputFocused && hideInputIfFrameFocused
     ? 0
     : chatInputHeight.value + DEFAULT_INPUT_HEIGHT;
-});
+}), [frameTextInputFocused, hideInputIfFrameFocused, chatInputHeight]);
📝 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
const chatInputHeight = useSharedValue(0);
const chatInputDisplayedHeight = useDerivedValue(() => {
return frameTextInputFocused && hideInputIfFrameFocused
? 0
: chatInputHeight.value + DEFAULT_INPUT_HEIGHT;
});
const chatInputHeight = useMemo(() => useSharedValue(0), []);
const chatInputDisplayedHeight = useMemo(() => useDerivedValue(() => {
return frameTextInputFocused && hideInputIfFrameFocused
? 0
: chatInputHeight.value + DEFAULT_INPUT_HEIGHT;
}), [frameTextInputFocused, hideInputIfFrameFocused, chatInputHeight]);


const insets = useSafeAreaInsets();

const { height: keyboardHeight } = useKeyboardAnimation();
const tertiary = tertiaryBackgroundColor(colorScheme);
const textInputStyle = useAnimatedStyle(
() => ({
position: "absolute",
width: "100%",
backgroundColor: tertiary,
height: "auto",
zIndex: 1,
transform: [
{ translateY: -Math.max(insets.bottom, keyboardHeight.value) },
] as any,
}),
[keyboardHeight, colorScheme, insets.bottom]
);

const chatContentStyle = useAnimatedStyle(
() => ({
...styles.chatContent,
paddingBottom: showChatInput
? chatInputDisplayedHeight.value +
Math.max(insets.bottom, keyboardHeight.value)
: insets.bottom,
}),
[showChatInput, keyboardHeight, chatInputDisplayedHeight, insets.bottom]
);

const messageListRef = useRef<FlatList<T> | FlashList<T> | undefined>();

const scrollToMessage = useCallback(
(data: { messageId?: string; index?: number; animated?: boolean }) => {
let index = data.index;
if (index === undefined && data.messageId) {
index = items.findIndex((m) => itemToId(m) === data.messageId);
}
if (index !== undefined) {
messageListRef.current?.scrollToIndex({
index,
viewPosition: 0.5,
animated: !!data.animated,
});
}
},
[itemToId, items]
);

useEffect(() => {
converseEventEmitter.on("scrollChatToMessage", scrollToMessage);
return () => {
converseEventEmitter.off("scrollChatToMessage", scrollToMessage);
};
}, [scrollToMessage]);

const handleOnLayout = useCallback(() => {
setTimeout(() => {
onReadyToFocus();
}, 50);
}, [onReadyToFocus]);
Comment on lines +178 to +182
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

Clean up setTimeout on unmount.

The setTimeout in handleOnLayout should be cleaned up to prevent potential memory leaks.

 const handleOnLayout = useCallback(() => {
-  setTimeout(() => {
-    onReadyToFocus();
-  }, 50);
+  const timer = setTimeout(() => {
+    onReadyToFocus();
+  }, 50);
+  return () => clearTimeout(timer);
 }, [onReadyToFocus]);

Committable suggestion was skipped due to low confidence.


return (
<View style={styles.chatContainer} key={key}>
<Animated.View style={chatContentStyle}>
{displayList && (
<ReanimatedFlashList
contentContainerStyle={styles.chat}
data={items}
refreshing={refreshing}
extraData={extraData}
renderItem={renderItem}
onLayout={handleOnLayout}
ref={(r) => {
// if (r) {
// messageListRef.current = r;
// }
}}
Comment on lines +196 to +199
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

Uncomment and assign messageListRef to enable scrolling functionality

The messageListRef is currently not assigned because the code assigning it is commented out. This prevents the scrollToMessage function from working as intended, as messageListRef.current will be undefined.

Apply this diff to fix the issue:

196             ref={(r) => {
197-              // if (r) {
198-              //   messageListRef.current = r;
199-              // }
197+              if (r) {
198+                messageListRef.current = r;
199+              }
200             }}
📝 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
// if (r) {
// messageListRef.current = r;
// }
}}
ref={(r) => {
if (r) {
messageListRef.current = r;
}
}}

keyboardDismissMode="interactive"
automaticallyAdjustContentInsets={false}
contentInsetAdjustmentBehavior="never"
// Causes a glitch on Android, no sure we need it for now
// maintainVisibleContentPosition={{
// minIndexForVisible: 0,
// autoscrollToTopThreshold: 100,
// }}
estimatedListSize={Dimensions.get("screen")}
inverted
keyExtractor={keyExtractor}
getItemType={getItemType}
keyboardShouldPersistTaps="handled"
estimatedItemSize={80}
// Size glitch on Android
showsVerticalScrollIndicator={Platform.OS === "ios"}
pointerEvents="auto"
ListFooterComponent={ListFooterComponent}
/>
)}
{showPlaceholder && placeholderComponent}
</Animated.View>
{showChatInput && (
<>
<ReanimatedView
style={[
textInputStyle,
{
display:
frameTextInputFocused && hideInputIfFrameFocused
? "none"
: "flex",
},
]}
>
{!transactionMode && <ChatInput inputHeight={chatInputHeight} />}
{transactionMode && <TransactionInput />}
</ReanimatedView>
<View
style={[
styles.inputBottomFiller,
{ height: insets.bottom + DEFAULT_INPUT_HEIGHT },
]}
/>
</>
)}
</View>
);
}
11 changes: 5 additions & 6 deletions components/Chat/Frame/FrameBottom.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@
import FrameButton from "./FrameButton";
import FrameTextInput from "./FrameTextInput";
import { FrameButtonType, FrameToDisplay } from "../../../utils/frames";
import { MessageToDisplay } from "../Message/Message";

export default function FrameBottom({
message,
messageFromMe,
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

The prop renaming is incomplete across the codebase

There are inconsistencies in how message properties are accessed:

  • FrameBottom.tsx still has a reference to message.fromMe for background color styling
  • Other Frame components (FramePreview, FrameButton, FrameTextInput) correctly use the new messageFromMe prop
  • Multiple other components in the codebase still use message.fromMe pattern

The prop renaming should be consistent throughout the codebase to avoid confusion and potential bugs.

🔗 Analysis chain

Props interface refactoring looks good, but verify message.fromMe usage.

The prop renaming from message to messageFromMe improves clarity by making the boolean nature explicit. However, there's still a reference to message.fromMe in the render logic that needs to be updated.

Let's verify all message.fromMe references are updated:

Also applies to: 24-24

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining message.fromMe references
rg "message\.fromMe" --type tsx

Length of output: 61

Script:

#!/bin/bash
# Search for any remaining message.fromMe references with correct file extension
rg "message\.fromMe"

# Also search for messageFromMe to verify the new prop usage
rg "messageFromMe"

# Use ast-grep to find component props and their usage
ast-grep --pattern 'interface $_ {
  $$$
  messageFromMe: $_
  $$$
}'

Length of output: 5555

frame,
textInput,
buttons,
Expand All @@ -22,7 +21,7 @@
postingActionForButton,
onButtonPress,
}: {
message: MessageToDisplay;
messageFromMe: boolean;
frame: FrameToDisplay;
textInput: string | undefined;
buttons: FrameButtonType[];
Expand All @@ -39,7 +38,7 @@
style={[
styles.frameBottom,
{
backgroundColor: message.fromMe

Check failure on line 41 in components/Chat/Frame/FrameBottom.tsx

View workflow job for this annotation

GitHub Actions / tsc 

Cannot find name 'message'. Did you mean 'onmessage'?
? myMessageInnerBubbleColor(colorScheme)
: messageInnerBubbleColor(colorScheme),
paddingVertical:
Expand All @@ -55,7 +54,7 @@
setFrameTextInputFocused={setFrameTextInputFocused}
setFrameTextInputValue={setFrameTextInputValue}
frameTextInputValue={frameTextInputValue}
messageFromMe={message.fromMe}
messageFromMe={messageFromMe}
/>
)}
{buttons.length > 0 &&
Expand All @@ -72,7 +71,7 @@
Haptics.impactAsync();
onButtonPress(button);
}}
messageFromMe={message.fromMe}
messageFromMe={messageFromMe}
/>
))}
</>
Expand All @@ -82,7 +81,7 @@
style={[
styles.frameBottomText,
{
color: message.fromMe ? "white" : textPrimaryColor(colorScheme),
color: messageFromMe ? "white" : textPrimaryColor(colorScheme),
fontWeight: frame.type === "PREVIEW" ? "600" : "400",
},
]}
Expand Down
Loading
Loading