Repository navigation
feedback changes - #78
Conversation
WalkthroughThis pull request introduces a series of UI and functionality improvements across multiple components. The changes focus on enhancing user interface consistency, implementing platform-specific navigation, and improving text rendering. Key modifications include adding a Changes
Sequence DiagramsequenceDiagram
participant User
participant PreSigninScreen
participant Swiper
participant Firestore
User->>PreSigninScreen: Navigates to screen
PreSigninScreen->>Swiper: Render with platform-specific buttons
Swiper-->>PreSigninScreen: Display navigation
User->>InfluencerCard: Views card
InfluencerCard->>Firestore: Fetch handle
Firestore-->>InfluencerCard: Return handle
InfluencerCard->>User: Display influencer details
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (11)
components/proposals/Invitations.tsx (2)
218-219: LGTM! Responsive layout improvements look good.The changes improve the layout responsiveness by:
- Implementing a 2-column grid layout for larger screens
- Ensuring proper spacing between columns
- Adding adequate bottom padding for better scrolling experience
However, consider adding a comment explaining the increased paddingBottom value for better maintainability.
contentContainerStyle={{ gap: 16, - paddingBottom: 64, + // Additional padding to account for the bottom navigation bar + paddingBottom: 64, width: xl ? "50%" : "100%", }}Also applies to: 229-229, 232-234
Line range hint
42-93: Consider improving error handling and loading states.The
fetchProposalsfunction could benefit from:
- More granular loading states for better UX
- User-friendly error handling
- Query optimization to reduce Firestore reads
const fetchProposals = async () => { + const loadingStates = { + proposals: false, + applications: false, + invitations: false, + }; try { + setError(null); + loadingStates.proposals = true; const collaborationCol = collection(FirestoreDB, "collaborations"); const q = query( collaborationCol, where("brandId", "==", selectedBrand?.id) ); const querySnapshot = await getDocs(q); + loadingStates.proposals = false; + loadingStates.applications = true; + loadingStates.invitations = true; const proposals = await Promise.all( querySnapshot.docs.map(async (doc) => { const data = { ...doc.data(), id: doc.id, }; // Batch get applications and invitations + const batch = []; + batch.push(getDocs(collection(FirestoreDB, "collaborations", data.id, "applications"))); + batch.push(getDocs(collection(FirestoreDB, "collaborations", data.id, "invitations"))); + const [applicationSnapshot, invitationSnapshot] = await Promise.all(batch); const applications = applicationSnapshot.docs.map((appDoc) => appDoc.data() ); const acceptedApplications = applications.filter( (application) => application.status === "accepted" ).length; const invitations = invitationSnapshot.docs.map((invDoc) => invDoc.data() ); return { ...data, applications: applications.length, invitations: invitations.length, acceptedApplications, }; }) ); setProposals(proposals); } catch (error) { console.error("Error fetching proposals: ", error); + setError("Failed to fetch proposals. Please try again later."); } finally { setIsLoading(false); + loadingStates.applications = false; + loadingStates.invitations = false; } };components/proposals/Applications.tsx (2)
242-242: LGTM! Consistent responsive layout implementation.The changes maintain consistency with the Invitations component, implementing the same responsive layout improvements.
Consider extracting these common FlatList styles into a shared configuration for better maintainability.
+// shared/styles/list-layouts.ts +export const getResponsiveListStyles = (xl: boolean) => ({ + contentContainerStyle: { + gap: 16, + paddingBottom: 64, + width: xl ? "50%" : "100%", + }, + numColumns: xl ? 2 : 1, + ...(xl && { + columnWrapperStyle: { + justifyContent: "space-between", + gap: 16, + }, + }), +}); +// In both components: +const responsiveListStyles = getResponsiveListStyles(xl); +<FlatList + {...responsiveListStyles} + // other props +/>Also applies to: 251-251, 254-256
Line range hint
276-309: Consider improving the create button's positioning.The absolute positioning of the create button might overlap with the list content. Consider using a sticky footer or padding to ensure visibility.
-<View - style={{ - position: "absolute", - bottom: 0, - right: 0, - left: 0, - paddingTop: 16, - paddingHorizontal: 16, - }} -> +<View + style={{ + position: "sticky", + bottom: 16, + paddingHorizontal: 16, + backgroundColor: Colors(theme).background, + shadowColor: "#000", + shadowOffset: { + width: 0, + height: -2, + }, + shadowOpacity: 0.25, + shadowRadius: 3.84, + elevation: 5, + }} +>components/collaboration-card/card-components/CollaborationDetails.tsx (2)
Line range hint
124-139: Consider using a mapping object for platform icons.The current implementation uses nested ternaries and repeats
faInstagramas a fallback. Consider using a mapping object for better maintainability.+const PLATFORM_ICONS = { + Instagram: faInstagram, + Facebook: faFacebook, + Youtube: faYoutube, +} as const; + {platform && platform.map((content, index) => ( <ChipCard key={index} chipText={content} - chipIcon={ - content === "Instagram" - ? faInstagram - : content === "Facebook" - ? faFacebook - : content === "Youtube" - ? faYoutube - : faInstagram - } + chipIcon={PLATFORM_ICONS[content as keyof typeof PLATFORM_ICONS] ?? faInstagram} /> ))}
Line range hint
140-159: Simplify content type icon mapping.Similar to the platform icons, consider using a mapping object for content type icons.
+const CONTENT_TYPE_ICONS = { + Posts: faPanorama, + Reels: faFilm, + Stories: faHeart, + Live: faRecordVinyl, + 'Product Reviews': faStarHalfStroke, +} as const; + {contentType && contentType.map((content, index) => ( <ChipCard key={index} chipText={content} - chipIcon={ - content === "Posts" - ? faPanorama - : content === "Reels" - ? faFilm - : content === "Stories" - ? faHeart - : content === "Live" - ? faRecordVinyl - : content === "Product Reviews" - ? faStarHalfStroke - : faPanorama - } + chipIcon={CONTENT_TYPE_ICONS[content as keyof typeof CONTENT_TYPE_ICONS] ?? faPanorama} /> ))}components/collaboration/collaboration-details/OverviewTabContent.tsx (2)
418-419: Document or remove empty callback functions.If these callbacks are placeholders for future implementation, consider adding TODO comments. If they're not needed, consider removing them from the component props.
-onMapRegionChange={(region) => {}} -onFormattedAddressChange={(address) => {}} +// TODO: Implement map region change handler +onMapRegionChange={(region) => {/* Implementation pending */}} +// TODO: Implement address change handler +onFormattedAddressChange={(address) => {/* Implementation pending */}}
319-331: Simplify budget text rendering logic.The nested ternary with template literals can be simplified for better readability.
-<Text - style={{ - fontSize: 16, - color: Colors(theme).text, - }} -> - Budget: - {props.collaboration?.budget?.min === - props.collaboration?.budget?.max - ? `$${props.collaboration?.budget?.min}` - : `$${props.collaboration?.budget?.min} - $${props.collaboration?.budget?.max}`} -</Text> +<Text style={{ fontSize: 16, color: Colors(theme).text }}> + {`Budget: $${props.collaboration?.budget?.min}${ + props.collaboration?.budget?.min !== props.collaboration?.budget?.max + ? ` - $${props.collaboration?.budget?.max}` + : '' + }`} +</Text>components/contract-card/ContractHeader.tsx (1)
46-46: Consider removing fixed width constraint on username Text.While adding
flex: 1improves layout flexibility, the username Text component still has a fixedwidth: 200which could cause truncation issues on different screen sizes. Consider using flexbox properties instead.<Text style={{ fontSize: 16, fontWeight: "bold", - width: 200, + flexShrink: 1, }} >app/(auth)/pre-signin.tsx (1)
Line range hint
100-127: Move inline styles to styles file and consolidate platform checks.The current implementation has two issues:
- Inline styles reduce maintainability and should be moved to the styles file
- Platform.OS check is duplicated (lines 53 and 100)
Consider applying these improvements:
- {slide.key !== "connect" && Platform.OS !== "web" && ( - <Pressable - style={{ - flexDirection: "row", - alignItems: "center", - justifyContent: "center", - marginTop: 15, - backgroundColor: Colors(theme).primary, - paddingHorizontal: 20, - paddingVertical: 10, - borderRadius: 5, - gap: 10, - }} - onPress={() => { - swiperRef.current?.scrollBy(1); - }} - > - <Text - style={{ - color: Colors(theme).white, - fontSize: 16, - }} - > - Next - </Text> - <FontAwesomeIcon - icon={faArrowRight} - size={16} - color={Colors(theme).white} - /> - </Pressable> - )} + {slide.key !== "connect" && Platform.OS !== "web" && ( + <Pressable + style={styles.nextButton} + onPress={() => { + swiperRef.current?.scrollBy(1); + }} + > + <Text style={styles.nextButtonText}>Next</Text> + <FontAwesomeIcon + icon={faArrowRight} + size={16} + color={Colors(theme).white} + /> + </Pressable> + )}Add these styles to your styles file:
nextButton: { flexDirection: "row", alignItems: "center", justifyContent: "center", marginTop: 15, backgroundColor: Colors(theme).primary, paddingHorizontal: 20, paddingVertical: 10, borderRadius: 5, gap: 10, }, nextButtonText: { color: Colors(theme).white, fontSize: 16, },components/InfluencerCard.tsx (1)
126-126: Add loading state to handle display.Consider showing a loading indicator while the handle is being fetched.
- <Text style={styles.handle}>@{handle}</Text> + {isLoading ? ( + <ActivityIndicator size="small" color={Colors(theme).primary} /> + ) : ( + <Text style={styles.handle}>@{handle}</Text> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/(auth)/pre-signin.tsx(5 hunks)components/InfluencerCard.tsx(4 hunks)components/collaboration-card/card-components/CollaborationDetails.tsx(6 hunks)components/collaboration-card/card-components/CollaborationHeader.tsx(2 hunks)components/collaboration/collaboration-details/OverviewTabContent.tsx(9 hunks)components/contract-card/ContractHeader.tsx(3 hunks)components/contracts/ContractDetailContent.tsx(2 hunks)components/proposals/Applications.tsx(2 hunks)components/proposals/Invitations.tsx(2 hunks)styles/CollaborationDetails.styles.tsx(0 hunks)styles/collaboration-details/CollaborationHeader.styles.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- styles/CollaborationDetails.styles.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
components/collaboration-card/card-components/CollaborationDetails.tsx
[error] 124-139: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (15)
components/collaboration-card/card-components/CollaborationHeader.tsx (2)
66-67: LGTM! Layout improvements enhance consistency.The increased gap and flex property changes improve spacing and layout consistency across components.
97-97: LGTM! Clean timePosted rendering.The simplified conditional rendering of timePosted improves code readability while maintaining functionality.
styles/collaboration-details/CollaborationHeader.styles.tsx (2)
5-6: LGTM! Well-structured style definition.The file follows React Native best practices with proper typing and theme support.
23-26: Great improvement to the layout flexibility!Adding
flex: 1to the title section is a good change as it allows the content to grow and fill available space while maintaining proper layout structure. This change aligns well with similar modifications in related components.components/collaboration-card/card-components/CollaborationDetails.tsx (1)
25-25: LGTM! Text truncation improves UI consistency.The addition of
truncateTextutility ensures consistent text display across components.Also applies to: 102-102
components/contracts/ContractDetailContent.tsx (1)
178-178: LGTM! Null-safe text truncation.The implementation correctly handles potential null/undefined values using the null coalescing operator.
components/collaboration/collaboration-details/OverviewTabContent.tsx (2)
366-369: Use consistent platform icon mapping.Similar to the suggestion in CollaborationDetails.tsx, consider using a mapping object for platform icons.
382-389: Use consistent content format icon mapping.Similar to the suggestion in CollaborationDetails.tsx, consider using a mapping object for content format icons.
components/contract-card/ContractHeader.tsx (2)
67-67: LGTM! Good layout improvement.Adding
flex: 1here creates a proper flex layout hierarchy, allowing the text container to grow appropriately within its parent.
97-97: Consider adding timestamp validation.While the code simplification is good, consider adding validation for the timestamp to handle potential invalid values.
- {timePosted ? <Text>{formatTimeToNow(timePosted)}</Text> : null} + {timePosted && !isNaN(timePosted) ? <Text>{formatTimeToNow(timePosted)}</Text> : null}Let's verify the
formatTimeToNowimplementation:app/(auth)/pre-signin.tsx (3)
2-2: LGTM! Import changes align with new features.The added imports support the platform-specific navigation implementation.
Also applies to: 16-17
86-87: LGTM! Theme-consistent button styling.The button color changes improve visual consistency by properly utilizing theme colors.
53-75: Verify the buttonWrapper styles are defined.The navigation buttons reference
styles.buttonWrapper, but this style isn't visible in the provided code. Ensure it's properly defined in the styles file.✅ Verification successful
The buttonWrapper style is properly defined
The style is correctly defined in
styles/tab1.styles.tsxwith appropriate styling for the navigation buttons, including background color, border radius, and padding properties.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for buttonWrapper style definition rg -A 5 "buttonWrapper.*[:=]" --type tsxLength of output: 70
Script:
#!/bin/bash # Search for buttonWrapper styles with multiple file extensions rg -A 5 "buttonWrapper.*[:=]" --type ts --type js --type jsx --type tsx # Check the specific file content cat "app/(auth)/pre-signin.tsx"Length of output: 5256
Script:
#!/bin/bash # Find and check the contents of the styles file fd "tab1.styles" --type f cat "styles/tab1.styles.ts" || cat "styles/tab1.styles.js"Length of output: 212
Script:
#!/bin/bash # Check the contents of the correct styles file cat "styles/tab1.styles.tsx"Length of output: 1437
components/InfluencerCard.tsx (2)
35-37: LGTM! Clean import additions.The new Firestore imports are properly organized and necessary for the added functionality.
Line range hint
1-248: Verify component usage across the application.Let's ensure that these changes don't affect other parts of the application that might be using this component.
✅ Verification successful
Changes are safe and improve data access patterns ✅
The component is only used in
components/explore-influencers/index.tsxand the changes make the social handle fetching more robust by properly accessing Firestore data instead of direct array access. No other components are affected by these changes.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for InfluencerCard usage echo "Searching for InfluencerCard imports and usage..." rg "import.*InfluencerCard|<InfluencerCard" --type tsx --type ts # Search for any direct access to influencer.socials that might be affected echo "Checking for direct access to influencer.socials..." rg "influencer\.socials\[" --type tsx --type tsLength of output: 394
Script:
#!/bin/bash # Find all TypeScript and TypeScript React files first echo "Finding TypeScript files..." fd "\.(tsx|ts)$" echo -e "\nSearching for InfluencerCard usage..." fd "\.(tsx|ts)$" | xargs rg "import.*InfluencerCard|<InfluencerCard" echo -e "\nChecking for direct access to influencer.socials..." fd "\.(tsx|ts)$" | xargs rg "influencer\.socials\["Length of output: 10266
| useEffect(() => { | ||
| if (props?.alreadyInvited) { | ||
| props.alreadyInvited(props.influencer.id).then((invited) => { | ||
| setIsInvited(invited); | ||
| }); | ||
| } | ||
| fetchHandle(); | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update useEffect dependencies and handle concurrent operations.
The current useEffect implementation has potential issues:
- Missing dependency on
influencer.primarySocial - Concurrent API calls could lead to race conditions
Consider separating the effects:
- useEffect(() => {
- if (props?.alreadyInvited) {
- props.alreadyInvited(props.influencer.id).then((invited) => {
- setIsInvited(invited);
- });
- }
- fetchHandle();
- }, []);
+ useEffect(() => {
+ if (props?.alreadyInvited) {
+ props.alreadyInvited(props.influencer.id).then((invited) => {
+ setIsInvited(invited);
+ });
+ }
+ }, [props?.alreadyInvited, props.influencer.id]);
+
+ useEffect(() => {
+ const cleanup = fetchHandle();
+ return cleanup;
+ }, [influencer.primarySocial]);📝 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.
| useEffect(() => { | |
| if (props?.alreadyInvited) { | |
| props.alreadyInvited(props.influencer.id).then((invited) => { | |
| setIsInvited(invited); | |
| }); | |
| } | |
| fetchHandle(); | |
| }, []); | |
| useEffect(() => { | |
| if (props?.alreadyInvited) { | |
| props.alreadyInvited(props.influencer.id).then((invited) => { | |
| setIsInvited(invited); | |
| }); | |
| } | |
| }, [props?.alreadyInvited, props.influencer.id]); | |
| useEffect(() => { | |
| fetchHandle(); | |
| }, [props.influencer?.primarySocial]); |
| const fetchHandle = async () => { | ||
| if (!influencer.primarySocial) { | ||
| setHandle("influencer-handle"); | ||
| return; | ||
| } | ||
| const userSocialRef = doc( | ||
| FirestoreDB, | ||
| "users", | ||
| influencer.id, | ||
| "socials", | ||
| influencer.primarySocial | ||
| ); | ||
| const socialDoc = await getDoc(userSocialRef); | ||
| const socialData = socialDoc.data() as ISocials; | ||
| if (socialDoc.exists()) { | ||
| if (socialData.isInstagram) { | ||
| setHandle(socialData.instaProfile?.username || "influencer-handle"); | ||
| } else { | ||
| setHandle(socialData.fbProfile?.name || "influencer-handle"); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and loading state to the Firestore fetch.
The fetchHandle function needs improvements in error handling and user experience:
- Add error handling for Firestore operations
- Prevent state updates if component unmounts
- Add loading state while fetching
- Consider moving the fallback value to constants
Here's a suggested implementation:
+ const [isLoading, setIsLoading] = useState(false);
+ const DEFAULT_HANDLE = "influencer-handle";
const fetchHandle = async () => {
+ setIsLoading(true);
if (!influencer.primarySocial) {
- setHandle("influencer-handle");
+ setHandle(DEFAULT_HANDLE);
+ setIsLoading(false);
return;
}
+
+ let isMounted = true;
try {
const userSocialRef = doc(
FirestoreDB,
"users",
influencer.id,
"socials",
influencer.primarySocial
);
const socialDoc = await getDoc(userSocialRef);
const socialData = socialDoc.data() as ISocials;
- if (socialDoc.exists()) {
+ if (socialDoc.exists() && isMounted) {
if (socialData.isInstagram) {
- setHandle(socialData.instaProfile?.username || "influencer-handle");
+ setHandle(socialData.instaProfile?.username || DEFAULT_HANDLE);
} else {
- setHandle(socialData.fbProfile?.name || "influencer-handle");
+ setHandle(socialData.fbProfile?.name || DEFAULT_HANDLE);
}
}
+ } catch (error) {
+ console.error('Error fetching handle:', error);
+ if (isMounted) {
+ setHandle(DEFAULT_HANDLE);
+ }
+ } finally {
+ if (isMounted) {
+ setIsLoading(false);
+ }
}
+
+ return () => {
+ isMounted = false;
+ };
};📝 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.
| const fetchHandle = async () => { | |
| if (!influencer.primarySocial) { | |
| setHandle("influencer-handle"); | |
| return; | |
| } | |
| const userSocialRef = doc( | |
| FirestoreDB, | |
| "users", | |
| influencer.id, | |
| "socials", | |
| influencer.primarySocial | |
| ); | |
| const socialDoc = await getDoc(userSocialRef); | |
| const socialData = socialDoc.data() as ISocials; | |
| if (socialDoc.exists()) { | |
| if (socialData.isInstagram) { | |
| setHandle(socialData.instaProfile?.username || "influencer-handle"); | |
| } else { | |
| setHandle(socialData.fbProfile?.name || "influencer-handle"); | |
| } | |
| } | |
| }; | |
| const [isLoading, setIsLoading] = useState(false); | |
| const DEFAULT_HANDLE = "influencer-handle"; | |
| const isMounted = useRef(true); | |
| useEffect(() => { | |
| return () => { | |
| isMounted.current = false; | |
| }; | |
| }, []); | |
| const fetchHandle = async () => { | |
| setIsLoading(true); | |
| if (!influencer.primarySocial) { | |
| setHandle(DEFAULT_HANDLE); | |
| setIsLoading(false); | |
| return; | |
| } | |
| try { | |
| const userSocialRef = doc( | |
| FirestoreDB, | |
| "users", | |
| influencer.id, | |
| "socials", | |
| influencer.primarySocial | |
| ); | |
| const socialDoc = await getDoc(userSocialRef); | |
| const socialData = socialDoc.data() as ISocials; | |
| if (socialDoc.exists() && isMounted.current) { | |
| if (socialData.isInstagram) { | |
| setHandle(socialData.instaProfile?.username || DEFAULT_HANDLE); | |
| } else { | |
| setHandle(socialData.fbProfile?.name || DEFAULT_HANDLE); | |
| } | |
| } | |
| } catch (error) { | |
| console.error('Error fetching handle:', error); | |
| if (isMounted.current) { | |
| setHandle(DEFAULT_HANDLE); | |
| } | |
| } finally { | |
| if (isMounted.current) { | |
| setIsLoading(false); | |
| } | |
| } | |
| }; |
Summary by CodeRabbit
Based on the comprehensive pull request summary, here are the concise release notes:
New Features
UI/UX Improvements
Performance
Bug Fixes