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

fix: Frames Renders on initial Send #1113

Merged
merged 3 commits into from
Oct 28, 2024
Merged

fix: Frames Renders on initial Send #1113

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b6a1e94
fix: Frames Renders on initial Send
alexrisch Oct 27, 2024
0cecad2
cleanup
alexrisch Oct 27, 2024
5e5d353
Merge branch 'main' into ar/frames-refactor
alexrisch Oct 28, 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
22 changes: 13 additions & 9 deletions components/Chat/Chat.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,13 @@ const usePeerSocials = () => {
const useRenderItem = ({
xmtpAddress,
conversation,
framesStore,
messageFramesMap,
colorScheme,
}: {
xmtpAddress: string;
conversation: XmtpConversationWithUpdate | undefined;
framesStore: {
[frameUrl: string]: FrameWithType;
messageFramesMap: {
[messageId: string]: FrameWithType[];
};
colorScheme: ColorSchemeName;
}) => {
Expand All @@ -90,11 +90,11 @@ const useRenderItem = ({
message={{ ...item }}
colorScheme={colorScheme}
isGroup={!!conversation?.isGroup}
isFrame={!!framesStore[item.content.toLowerCase().trim()]}
hasFrames={messageFramesMap[item.id]?.length > 0}
alexrisch marked this conversation as resolved.
Show resolved Hide resolved
/>
);
},
[colorScheme, xmtpAddress, conversation?.isGroup, framesStore]
[colorScheme, xmtpAddress, conversation?.isGroup, messageFramesMap]
);
};

Expand Down Expand Up @@ -383,7 +383,9 @@ export function Chat() {
styles.inChatRecommendations,
]);

const { frames: framesStore } = useFramesStore(useSelect(["frames"]));
const { messageFramesMap, frames: framesStore } = useFramesStore(
useSelect(["messageFramesMap", "frames"])
);

const showPlaceholder = useIsShowingPlaceholder({
messages: listArray,
Expand All @@ -394,7 +396,7 @@ export function Chat() {
const renderItem = useRenderItem({
xmtpAddress,
conversation,
framesStore,
messageFramesMap,
colorScheme,
});

Expand Down Expand Up @@ -539,7 +541,9 @@ export function ChatPreview() {
]
);

const { frames: framesStore } = useFramesStore(useSelect(["frames"]));
const { frames: framesStore, messageFramesMap } = useFramesStore(
useSelect(["frames", "messageFramesMap"])
);

const showPlaceholder = useIsShowingPlaceholder({
messages: listArray,
Expand All @@ -550,7 +554,7 @@ export function ChatPreview() {
const renderItem = useRenderItem({
xmtpAddress,
conversation,
framesStore,
messageFramesMap,
colorScheme,
});

Expand Down
42 changes: 16 additions & 26 deletions components/Chat/Frame/FramesPreviews.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,58 +1,48 @@
import { useConversationContext } from "@utils/conversation";
import { useCallback, useRef, useState } from "react";
import { useCallback, useEffect, useRef } from "react";
import { View } from "react-native";
import { useShallow } from "zustand/react/shallow";

import FramePreview from "./FramePreview";
import { useCurrentAccount } from "../../../data/store/accountsStore";
import { useFramesStore } from "../../../data/store/framesStore";
import {
FrameWithType,
FramesForMessage,
fetchFramesForMessage,
} from "../../../utils/frames";
import { FramesForMessage, fetchFramesForMessage } from "../../../utils/frames";
import { MessageToDisplay } from "../Message/Message";

type Props = {
message: MessageToDisplay;
};

export default function FramesPreviews({ message }: Props) {
export function FramesPreviews({ message }: Props) {
const messageId = useRef<string | undefined>(undefined);
const tagsFetchedOnceForMessage = useConversationContext(
"tagsFetchedOnceForMessage"
);
const account = useCurrentAccount() as string;
const [framesForMessage, setFramesForMessage] = useState<{
[messageId: string]: FrameWithType[];
}>({
[message.id]: useFramesStore
.getState()
.getFramesForURLs(message.converseMetadata?.frames || []),
});
const framesToDisplay = useFramesStore(
useShallow((s) => s.messageFramesMap[message.id] ?? [])
);

const fetchTagsIfNeeded = useCallback(() => {
if (!tagsFetchedOnceForMessage.current[message.id]) {
tagsFetchedOnceForMessage.current[message.id] = true;
fetchFramesForMessage(account, message).then(
(frames: FramesForMessage) => {
setFramesForMessage({ [frames.messageId]: frames.frames });
useFramesStore
.getState()
.setMessageFramesMap(frames.messageId, frames.frames);
alexrisch marked this conversation as resolved.
Show resolved Hide resolved
}
);
}
}, [account, message, tagsFetchedOnceForMessage]);

// Components are recycled, let's fix when stuff changes
if (message.id !== messageId.current) {
messageId.current = message.id;
fetchTagsIfNeeded();
setFramesForMessage({
[message.id]: useFramesStore
.getState()
.getFramesForURLs(message.converseMetadata?.frames || []),
});
}

const framesToDisplay = framesForMessage[message.id] || [];
useEffect(() => {
if (message.id !== messageId.current) {
messageId.current = message.id;
fetchTagsIfNeeded();
}
}, [message.id, fetchTagsIfNeeded, message.converseMetadata?.frames]);
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 optimizing effect dependencies.

The effect's dependency array includes message.converseMetadata?.frames which might trigger unnecessary reruns. Since you're only checking for message ID changes, you could simplify this:

  useEffect(() => {
    if (message.id !== messageId.current) {
      messageId.current = message.id;
      fetchTagsIfNeeded();
    }
- }, [message.id, fetchTagsIfNeeded, message.converseMetadata?.frames]);
+ }, [message.id, fetchTagsIfNeeded]);
📝 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
useEffect(() => {
if (message.id !== messageId.current) {
messageId.current = message.id;
fetchTagsIfNeeded();
}
}, [message.id, fetchTagsIfNeeded, message.converseMetadata?.frames]);
useEffect(() => {
if (message.id !== messageId.current) {
messageId.current = message.id;
fetchTagsIfNeeded();
}
}, [message.id, fetchTagsIfNeeded]);


return (
<View>
Expand Down
81 changes: 45 additions & 36 deletions components/Chat/Message/Message.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useFramesStore } from "@data/store/framesStore";
import {
inversePrimaryColor,
messageInnerBubbleColor,
Expand Down Expand Up @@ -26,6 +27,7 @@ import Animated, {
useAnimatedStyle,
withTiming,
} from "react-native-reanimated";
import { useShallow } from "zustand/react/shallow";

import ChatMessageActions from "./MessageActions";
import ChatMessageReactions from "./MessageReactions";
Expand Down Expand Up @@ -64,7 +66,7 @@ import ClickableText from "../../ClickableText";
import ActionButton from "../ActionButton";
import AttachmentMessagePreview from "../Attachment/AttachmentMessagePreview";
import ChatGroupUpdatedMessage from "../ChatGroupUpdatedMessage";
import FramesPreviews from "../Frame/FramesPreviews";
import { FramesPreviews } from "../Frame/FramesPreviews";
import ChatInputReplyBubble from "../Input/InputReplyBubble";
import TransactionPreview from "../Transaction/TransactionPreview";

Expand All @@ -84,7 +86,7 @@ type Props = {
message: MessageToDisplay;
colorScheme: ColorSchemeName;
isGroup: boolean;
isFrame: boolean;
hasFrames: boolean;
};

// On iOS, the native context menu view handles the long press, but could potentially trigger the onPress event
Expand Down Expand Up @@ -151,7 +153,7 @@ const ChatMessage = ({
message,
colorScheme,
isGroup,
isFrame,
hasFrames,
}: Props) => {
const styles = useStyles();

Expand All @@ -163,6 +165,10 @@ const ChatMessage = ({
() => getLocalizedTime(message.sent),
[message.sent]
);
// The content is completely a frame so a larger full width frame will be shown
const isFrame = useFramesStore(
useShallow((s) => !!s.frames[message.content.toLowerCase().trim()])
);

// Reanimated shared values for time and date-time animations
const timeHeight = useSharedValue(0);
Expand Down Expand Up @@ -325,6 +331,37 @@ const ChatMessage = ({

const swipeableRef = useRef<Swipeable | null>(null);

const renderLeftActions = useCallback(
(
progressAnimatedValue: RNAnimated.AnimatedInterpolation<string | number>
) => {
return (
<RNAnimated.View
style={{
opacity: progressAnimatedValue.interpolate({
inputRange: [0, 0.7, 1],
outputRange: [0, 0, 1],
}),
height: "100%",
justifyContent: "center",
transform: [
{
translateX: progressAnimatedValue.interpolate({
inputRange: [0, 0.8, 1],
outputRange: [0, 0, 8],
extrapolate: "clamp",
}),
},
],
}}
>
<ActionButton picto="arrowshape.turn.up.left" />
</RNAnimated.View>
);
},
[]
);
alexrisch marked this conversation as resolved.
Show resolved Hide resolved

return (
<View
style={[
Expand Down Expand Up @@ -358,35 +395,7 @@ const ChatMessage = ({
overshootFriction={1.5}
containerStyle={styles.messageSwipeable}
childrenContainerStyle={styles.messageSwipeableChildren}
renderLeftActions={(
progressAnimatedValue: RNAnimated.AnimatedInterpolation<
string | number
>
) => {
return (
<RNAnimated.View
style={{
opacity: progressAnimatedValue.interpolate({
inputRange: [0, 0.7, 1],
outputRange: [0, 0, 1],
}),
height: "100%",
justifyContent: "center",
transform: [
{
translateX: progressAnimatedValue.interpolate({
inputRange: [0, 0.8, 1],
outputRange: [0, 0, 8],
extrapolate: "clamp",
}),
},
],
}}
>
<ActionButton picto="arrowshape.turn.up.left" />
</RNAnimated.View>
);
}}
renderLeftActions={renderLeftActions}
leftThreshold={10000} // Never trigger opening
onSwipeableWillClose={() => {
const translation = swipeableRef.current?.state.rowTranslation;
Expand Down Expand Up @@ -543,7 +552,7 @@ type RenderedChatMessage = {
message: MessageToDisplay;
colorScheme: ColorSchemeName;
isGroup: boolean;
isFrame: boolean;
hasFrames: boolean;
};

const renderedMessages = new LimitedMap<string, RenderedChatMessage>(50);
Expand All @@ -567,7 +576,7 @@ export default function CachedChatMessage({
message,
colorScheme,
isGroup,
isFrame = false,
hasFrames = false,
}: Props) {
const alreadyRenderedMessage = renderedMessages.get(
`${account}-${message.id}`
Expand All @@ -584,14 +593,14 @@ export default function CachedChatMessage({
message,
colorScheme,
isGroup,
isFrame,
hasFrames,
});
renderedMessages.set(`${account}-${message.id}`, {
message,
renderedMessage,
colorScheme,
isGroup,
isFrame,
hasFrames,
});
return renderedMessage;
} else {
Expand Down
17 changes: 16 additions & 1 deletion data/store/framesStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,24 @@ type FramesStoreType = {
frames: {
[frameUrl: string]: FrameWithType;
};
messageFramesMap: {
[messageId: string]: FrameWithType[];
};
setFrames: (framesToSet: { [frameUrl: string]: FrameWithType }) => void;
getFramesForURLs: (urls: string[]) => FrameWithType[];
setMessageFramesMap: (messageId: string, framesUrls: FrameWithType[]) => void;
};

export const useFramesStore = create<FramesStoreType>()(
persist(
(set, get) => ({
frames: {},
messageFramesMap: {},
setFrames: (framesToSet: { [frameUrl: string]: FrameWithType }) =>
set((state) => {
const existingFrames = pick(state.frames, Object.keys(framesToSet));
if (isDeepEqual(existingFrames, framesToSet)) return {};
return { frames: { ...state.frames, ...framesToSet } };
return { ...state, frames: { ...state.frames, ...framesToSet } };
}),
getFramesForURLs: (urls: string[]) => {
const framesToReturn: FrameWithType[] = [];
Expand All @@ -35,6 +40,16 @@ export const useFramesStore = create<FramesStoreType>()(
});
return framesToReturn;
},
setMessageFramesMap: (messageId: string, frames: FrameWithType[]) =>
set((state) => {
return {
...state,
messageFramesMap: {
...state.messageFramesMap,
[messageId]: frames,
},
};
}),
}),
{
name: `store-frames`,
Expand Down
1 change: 1 addition & 0 deletions utils/frames.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ export const fetchFramesForMessage = async (
};
// Save frame to store
useFramesStore.getState().setFrames(framesToSave);
useFramesStore.getState().setMessageFramesMap(message.id, fetchedFrames);
// Then update message to reflect change
saveMessageMetadata(account, message, messageMetadataToSave);
alexrisch marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
Loading