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

Chat Composer Glow Up / Changed Functionality #1513

Merged
merged 18 commits into from
Jan 15, 2025
Merged
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
18 changes: 17 additions & 1 deletion App.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import "expo-dev-client";
import "./polyfills";

import {
configureReanimatedLogger,
ReanimatedLogLevel,
} from "react-native-reanimated";
import { configure as configureCoinbase } from "@coinbase/wallet-mobile-sdk";
import DebugButton from "@components/DebugButton";
import { BottomSheetModalProvider } from "@design-system/BottomSheet/BottomSheetModalProvider";
Expand All @@ -10,7 +13,7 @@
import { queryClient } from "@queries/queryClient";
import { MaterialDarkTheme, MaterialLightTheme } from "@styles/colors";
import { QueryClientProvider } from "@tanstack/react-query";
import { useAppTheme, useThemeProvider } from "@theme/useAppTheme";

Check warning on line 16 in App.tsx

View workflow job for this annotation

GitHub Actions / lint

'useAppTheme' is defined but never used
import { useCoinbaseWalletListener } from "@utils/coinbaseWallet";
import { converseEventEmitter } from "@utils/events";
import logger from "@utils/logger";
Expand Down Expand Up @@ -48,8 +51,21 @@
"Privy: Expected status code 200, received 400", // Privy
"Error destroying session", // Privy
'event="noNetwork', // ethers
"[Reanimated] Reading from `value` during component render. Please ensure that you do not access the `value` property or use `get` method of a shared value while React is rendering a component.",
"Attempted to import the module",
Comment on lines +54 to +55
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

Fix shared value access in JoinGroup screen

The warning is caused by direct access to state.value during render in features/GroupInvites/joinGroup/JoinGroup.screen.tsx. To fix this:

  • Move the value access into a derived value using useDerivedValue
  • Or store the value in a regular React state if animation isn't needed
// Before
const pollingTimedOut = state.value === "Attempting to Join Group Timed Out";

// After
const pollingTimedOut = useDerivedValue(() => 
  state.value === "Attempting to Join Group Timed Out"
);
🔗 Analysis chain

Investigate the root cause of the Reanimated warning.

While ignoring the warning allows the app to function, it masks a potential performance or correctness issue. The warning indicates that shared values are being accessed during component rendering, which could lead to unnecessary re-renders or race conditions.

Let's search for potential causes:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for direct value access patterns in animation code
rg -A 2 "useAnimatedStyle|useSharedValue|SharedValue" 

# Search for .value access during render
ast-grep --pattern 'const $_ = $_?.value'

Length of output: 20729


Script:

#!/bin/bash
# Search for shared value access in hooks and handlers
rg -A 2 "useEffect.*\.value|useMemo.*\.value|const.*=.*\.value"

# Look for value access in event handlers
ast-grep --pattern 'onPress={$_}'

# Search for any remaining direct value access
ast-grep --pattern '$_.value'

Length of output: 10328

]);

// This is the default configuration
configureReanimatedLogger({
level: ReanimatedLogLevel.warn,
strict: /*
Ignores the following warning:
"[Reanimated] Reading from `value` during component render. Please ensure that you do not access the `value` property or use `get` method of a shared value while React is rendering a component.",
todo investigate

*/ false,
});
Comment on lines +58 to +67
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 the TODO comment and reconsider strict mode configuration.

  1. The TODO comment should follow a standard format with ownership and timeline:
-todo investigate
+TODO(owner): Investigate shared value access during render and determine if strict mode can be enabled
+Target: DD/MM/YYYY
  1. Disabling strict mode to ignore warnings is not recommended as it may mask other issues. Consider keeping strict mode enabled and properly fixing the underlying shared value access issue.

Committable suggestion skipped: line range outside the PR's diff.


configureCoinbase({
callbackURL: new URL(`https://${config.websiteDomain}/coinbase`),
hostURL: new URL("https://wallet.coinbase.com/wsegue"),
Expand Down Expand Up @@ -79,10 +95,10 @@
}, []);

const showDebugMenu = useCallback(() => {
if (!debugRef.current || !(debugRef.current as any).showDebugMenu) {

Check warning on line 98 in App.tsx

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
return;
}
(debugRef.current as any).showDebugMenu();

Check warning on line 101 in App.tsx

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
}, []);

useEffect(() => {
Expand Down Expand Up @@ -126,7 +142,7 @@
const AppKeyboardProvider =
Platform.OS === "ios" ? KeyboardProvider : React.Fragment;

export default function AppWithProviders() {

Check warning on line 145 in App.tsx

View workflow job for this annotation

GitHub Actions / lint

Prefer named exports
const colorScheme = useColorScheme();

const paperTheme = useMemo(() => {
Expand All @@ -144,7 +160,7 @@
<ActionSheetProvider>
<ThemeProvider value={{ themeScheme, setThemeContextOverride }}>
<PaperProvider theme={paperTheme}>
<GestureHandlerRootView style={{ flex: 1 }}>

Check warning on line 163 in App.tsx

View workflow job for this annotation

GitHub Actions / lint

Inline style: { flex: 1 }
<BottomSheetModalProvider>
<App />
<Snackbars />
Expand Down
4 changes: 2 additions & 2 deletions components/ClickableText.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { ParsedText } from "@components/ParsedText/ParsedText";
import Clipboard from "@react-native-clipboard/clipboard";
import { actionSheetColors } from "@styles/colors";
import * as Linking from "expo-linking";
import { useCallback } from "react";
import { useCallback, useEffect } from "react";
import { StyleProp, StyleSheet, TextStyle, useColorScheme } from "react-native";
import { navigate } from "../utils/navigation";
import {
Expand Down Expand Up @@ -38,7 +38,7 @@ export function ClickableText({ children, style }: Props) {
}, []);

const handleNewConversationPress = useCallback((peer: string) => {
navigate("NewConversation", { peer });
navigate("Conversation", { peer });
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

Navigation route inconsistency detected

The change to use "Conversation" in ClickableText.tsx aligns with the majority of navigation patterns in the codebase. However, there is still one instance using the old "NewConversation" route:

  • features/conversation-list/conversation-list.screen.tsx:350: navigation.navigate("NewConversation", {});
🔗 Analysis chain

Verify navigation flow changes.

The navigation destination has changed from "NewConversation" to "Conversation". Ensure this change is intentional and all related navigation flows have been updated accordingly.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining references to the old navigation route
rg '"NewConversation"' --type ts --type tsx

# Search for navigation.navigate calls to verify consistency
ast-grep --pattern 'navigate("Conversation"'

Length of output: 120


Script:

#!/bin/bash
# Search for both old and new navigation routes
rg -t typescript -t javascriptreact -t typescriptreact '"NewConversation"|"Conversation"'

# Search for navigation patterns using ast-grep
ast-grep --pattern 'navigate($ROUTE, $$$)'

# Also search for navigation.navigate pattern
ast-grep --pattern 'navigation.navigate($ROUTE, $$$)'

Length of output: 4641

}, []);

const showCopyActionSheet = useCallback(
Expand Down
66 changes: 0 additions & 66 deletions components/Onboarding/WarpcastConnect.tsx

This file was deleted.

212 changes: 0 additions & 212 deletions components/Recommendations/Recommendation.tsx

This file was deleted.

Loading
Loading