Repository navigation
Dev - #66
Dev#66
Conversation
* redesign application and invitation cards
pushed new contract screens
* repurpose messaging screen * accept application changes * added phases * invitationId as influencerId * fixed signup signin * fix: bug on brand creation * minor changes * minor changes * minor changes * minor changes
* fix: accept application * fix: managerId
* fix: initial redirections * scheme change * router navigate fix * router navigate fix
* redirection changes * redirection changes
* added no brands protected screen and shimmer ui * shimmer ui update * added loading indicator
image component and other fixes
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces a new React functional component Changes
Sequence DiagramsequenceDiagram
participant User
participant BrandProtectedScreen
participant Firestore
User->>BrandProtectedScreen: Authenticate
BrandProtectedScreen->>Firestore: Fetch Brands
alt Brands Found
Firestore-->>BrandProtectedScreen: Return Brand Data
BrandProtectedScreen->>User: Render Content
else No Brands
BrandProtectedScreen->>User: Redirect to Onboarding
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
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: 3
🔭 Outside diff range comments (2)
contexts/auth-context.provider.tsx (1)
Line range hint
183-189: Consider extracting API URL to environment variables.The hardcoded API URL should be moved to environment variables for better configuration management.
+// In .env file +CHAT_API_URL=https://be.trendly.pro/api/v1/chat/auth +// In the code - await fetch("https://be.trendly.pro/api/v1/chat/auth", { + await fetch(process.env.CHAT_API_URL, {components/profile/index.tsx (1)
Line range hint
59-94: Consider handling potential race conditions in profile update.The
updateProfilefunction updates both the profile image and user data. If the image upload fails, the profile data might still be updated, leading to inconsistency.Consider wrapping the updates in a transaction:
try { + const batch = writeBatch(FirestoreDB); let updatedImageURL = manager.profileImage || ""; if (imageToUpload) { // ... image upload logic ... updatedImageURL = await getDownloadURL(storageRef); } - await updateDoc(managerRef, { + batch.update(managerRef, { profileImage: updatedImageURL, name: name || manager.name, }); if (selectedBrand) { const brandMemberRef = doc( FirestoreDB, "brands", selectedBrand.id, "members", manager.id ); - await updateDoc(brandMemberRef, { + batch.update(brandMemberRef, { role, }); } + await batch.commit(); setImageToUpload(""); Toaster.success("Profile updated successfully"); fetchRole();
🧹 Nitpick comments (19)
layouts/protected/brand-protected-screen.tsx (1)
69-92: Consider component reusabilityThe
LoadingScreencomponent could be moved to a shared components directory since loading states are commonly needed across the app.Consider moving this to
@/components/ui/loading-screenfor better reusability.components/shimmers/explore-influencer-shimmer.tsx (1)
56-62: Consider extracting bottom navigation items count to a constant.The magic number 5 in the Array constructor could be extracted to a constant for better maintainability.
+const BOTTOM_NAV_ITEMS_COUNT = 5; + const ExploreInfluencerShimmer = () => { // ... <View style={styles.bottomNav}> - {[...Array(5)].map((_, index) => ( + {[...Array(BOTTOM_NAV_ITEMS_COUNT)].map((_, index) => ( <Shimmer key={index} width={56} height={48} /> ))} </View>app/index.tsx (1)
13-19: Consider adding loading and error handling for the splash image.While the implementation is correct, consider adding:
resizeModeprop to control how the image scales- Error handling for failed image loads
<Image source={require("@/assets/images/splash.png")} + resizeMode="contain" + onError={(e) => console.warn('Error loading splash image:', e.nativeEvent.error)} style={{ width: '100%', height: '100%', }} />components/ui/screen-header/index.tsx (1)
34-34: Fix the handleAction function syntax.The function has inconsistent syntax (semicolon instead of closing brace).
- }; + }components/collaboration/collaboration-details/modal/ManagerModal.tsx (1)
43-54: Remove duplicate styling in ImageComponent.The component has overlapping size definitions:
size="medium"prop- Explicit width/height in style prop
<ImageComponent url={managerImage} altText={managerName} shape="circle" size="medium" - style={{ - width: 120, - height: 120, - borderRadius: 240, - }} initials={managerName} initialsSize={40} />components/brand-profile/members-card/index.tsx (1)
50-56: Improve the altText for better accessibility.The altText "Image" is too generic. Consider using a more descriptive text that includes the manager's name.
<ImageComponent size="small" shape="circle" initials={manager.name} url={manager.profileImage || ""} - altText="Image" + altText={`Profile picture of ${manager.name}`} />components/contract-card/ContractHeader.tsx (2)
Line range hint
5-5: Remove unused importThe
Imageimport from 'react-native' is no longer used after switching toImageComponent.-import { Image, Pressable } from "react-native"; +import { Pressable } from "react-native";
50-55: Improve accessibility and remove redundant styling
- The altText "Image" is not descriptive enough for screen readers. Consider using something more meaningful like
{username}'s profile picture.- The
borderRadiusstyle may conflict withshape="circle"prop.<ImageComponent url={userImage} - altText="Image" + altText={`${username}'s profile picture`} shape="circle" initials={username} size="small" style={{ width: 40, height: 40, - borderRadius: 20, }} />components/collaboration-card/card-components/CollaborationHeader.tsx (2)
Line range hint
5-5: Remove unused importThe
Imageimport from 'react-native' is no longer used after switching toImageComponent.-import { Image, Pressable } from "react-native"; +import { Pressable } from "react-native";
51-55: Fix inconsistent styling and improve accessibility
- There's a mismatch between
shape="circle"andborderRadius: 5- The altText should be more descriptive
- Consider adding
initialsprop for fallback display<ImageComponent url={brandImage} - altText="Image" + altText={`${brandName}'s brand logo`} shape="circle" size="small" + initials={brandName} style={{ width: 40, height: 40, - borderRadius: 5, }} />components/collaboration/collaboration-details/modal/BrandModal.tsx (2)
Line range hint
1-1: Remove unused importThe
Imageimport is no longer used after switching toImageComponent.-import { Image, Pressable, Linking } from "react-native"; +import { Pressable, Linking } from "react-native";
46-50: Add fallback display and improve accessibilityConsider adding
initialsprop for fallback display when image fails to load, and make the altText more descriptive.<ImageComponent url={brand.image} - altText="Brand Image" + altText={`${brand.name}'s brand logo`} shape="square" size="medium" + initials={brand.name} style={{components/menu/index.tsx (3)
Line range hint
44-47: Standardize image component usageFor consistency, replace the brand logo Image component with ImageComponent as well.
-<Image - source={imageUrl(selectedBrand?.image)} - style={styles.avatarBrandImage} -/> +<ImageComponent + url={selectedBrand?.image || ""} + altText={`${selectedBrand?.name}'s brand logo`} + shape="square" + size="medium" + initials={selectedBrand?.name} + style={styles.avatarBrandImage} +/>
94-99: Improve accessibility descriptionThe altText "Image" is not descriptive enough for screen readers.
<ImageComponent url={manager?.profileImage || ""} initials={manager?.name} shape="circle" size="small" - altText="Image" + altText={`${manager?.name}'s profile picture`} style={styles.avatar} />
Line range hint
1-1: Consider creating image component usage guidelinesTo maintain consistency across the codebase, consider:
- Creating documentation for ImageComponent usage patterns
- Establishing guidelines for:
- Meaningful altText construction
- When to use different shapes and sizes
- Style handling with shape props
- Required props for different use cases (profile pictures vs logos)
- Adding prop-types or TypeScript interfaces to enforce these guidelines
This will help maintain consistency as the component is adopted across the codebase.
components/InfluencerCard.tsx (1)
150-154: Consider memoizing the carousel data transformation.The attachment processing is performed on every render.
Consider using useMemo:
+const carouselData = useMemo(() => + influencer.profile?.attachments?.map((attachment) => + processRawAttachment(attachment) + ) || [], + [influencer.profile?.attachments] +); -data={ - influencer.profile?.attachments?.map((attachment) => - processRawAttachment(attachment) - ) || [] -} +data={carouselData}components/contracts/ActionContainer.tsx (1)
217-222: Move inline styles to StyleSheet.Both ImageComponent implementations use identical inline styles. This should be moved to the StyleSheet for better maintainability.
Apply this refactor:
const styles = StyleSheet.create({ // ... existing styles ... + profileImage: { + width: 40, + height: 40, + borderRadius: 20, + }, }); -style={{ width: 40, height: 40, borderRadius: 20 }} +style={styles.profileImage}Also applies to: 271-276
components/collaboration/collaboration-details/OverviewTabContent.tsx (1)
190-194: Consolidate image styles in StyleSheet.Both ImageComponent implementations use inline styles that could be moved to the StyleSheet. Additionally, consider creating reusable size constants.
Apply this refactor:
+const IMAGE_SIZES = { + BRAND_LOGO: { + width: 40, + height: 40, + borderRadius: 5, + }, + PROFILE: { + width: 40, + height: 40, + borderRadius: 20, + }, +}; const styles = stylesFn(theme); +const localStyles = StyleSheet.create({ + brandLogo: IMAGE_SIZES.BRAND_LOGO, + profileImage: IMAGE_SIZES.PROFILE, +}); -style={{ width: 40, height: 40, borderRadius: 5 }} +style={localStyles.brandLogo} -style={{ width: 40, height: 40, borderRadius: 20 }} +style={localStyles.profileImage}Also applies to: 449-454
components/contracts/AddMemberModal.tsx (1)
115-120: Consider improving type safety and accessibility.While the ImageComponent implementation is good, there are a few suggestions for improvement:
Type Safety:
- Consider adding proper typing for the item parameter instead of using 'any'
- Add fallback handling for undefined profileImage
Accessibility:
- Make the altText more descriptive by including the member's name
Here's a suggested improvement:
- <ImageComponent - size="small" - url={item.item.profileImage} - initials={item.item.name} - altText="Profile Image" - /> + <ImageComponent + size="small" + url={item.item.profileImage} + initials={item.item.name} + altText={`Profile image of ${item.item.name}`} + />Also, consider adding proper TypeScript interfaces:
interface Member { profileImage?: string; name: string; email: string; managerId: string; } interface RenderItemProps { item: Member; index: number; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
app/(main)/(drawer)/_layout.tsx(1 hunks)app/(main)/(screens)/(onboarding)/onboarding-your-brand.tsx(4 hunks)app/index.tsx(1 hunks)components/InfluencerCard.tsx(7 hunks)components/brand-profile/members-card/index.tsx(2 hunks)components/collaboration-card/card-components/CollaborationHeader.tsx(2 hunks)components/collaboration/collaboration-details/OverviewTabContent.tsx(3 hunks)components/collaboration/collaboration-details/modal/BrandModal.tsx(2 hunks)components/collaboration/collaboration-details/modal/ManagerModal.tsx(2 hunks)components/contract-card/ContractHeader.tsx(2 hunks)components/contracts/ActionContainer.tsx(3 hunks)components/contracts/AddMemberModal.tsx(2 hunks)components/menu/index.tsx(2 hunks)components/profile/index.tsx(4 hunks)components/shimmers/explore-influencer-shimmer.tsx(1 hunks)components/ui/screen-header/index.tsx(3 hunks)constants/Colors.ts(2 hunks)contexts/auth-context.provider.tsx(2 hunks)layouts/protected/brand-protected-screen.tsx(1 hunks)layouts/protected/index.ts(1 hunks)shared-uis(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- shared-uis
🧰 Additional context used
🪛 Biome (1.9.4)
layouts/protected/brand-protected-screen.tsx
[error] 56-56: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (13)
app/(main)/(drawer)/_layout.tsx (1)
11-28: LGTM! Clean integration of BrandProtectedScreenThe wrapper implementation looks good and maintains the existing drawer functionality while adding brand protection.
layouts/protected/index.ts (1)
1-3: LGTM! Clean export implementationThe barrel file pattern is properly implemented.
constants/Colors.ts (1)
22-23: LGTM! Consistent color definitionsThe shimmer colors are properly defined for both themes and follow the existing pattern.
Also applies to: 38-39
components/shimmers/explore-influencer-shimmer.tsx (1)
12-67: LGTM! Well-structured shimmer implementation.The component is well-organized and follows React best practices. The shimmer effects are appropriately sized for each UI element, providing a good loading state experience.
app/(main)/(screens)/(onboarding)/onboarding-your-brand.tsx (2)
45-56: Good improvement in error handling!The error handling has been enhanced by properly setting
isSubmittingto false when validation fails.
63-66: Consider adding error handling for image upload.The image upload process should include error handling to manage potential failures.
- imageUrl = await uploadImageBytes( - blob, - `brands/${brandData.name}-${Date.now()}` - ); + try { + imageUrl = await uploadImageBytes( + blob, + `brands/${brandData.name}-${Date.now()}` + ); + } catch (error) { + Toaster.error('Failed to upload brand image'); + setIsSubmitting(false); + return; + }✅ Verification successful
Error handling is necessary for Firebase Storage operations
The
uploadImageBytesfunction uses Firebase Storage operations that can fail due to network issues, permissions, or storage quota. The suggested error handling:
- Aligns with existing error handling patterns in the component
- Uses the available
Toasterutility- Properly manages the submission state
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find uploadImageBytes implementation ast-grep --pattern 'export $_ function uploadImageBytes' # Search for Toaster utility ast-grep --pattern 'export $_ Toaster' # Look at the full component file to understand the context cat "app/(main)/(screens)/(onboarding)/onboarding-your-brand.tsx"Length of output: 3803
Script:
#!/bin/bash # Search for uploadImageBytes implementation in contexts fd "firebase-storage.context" --exec cat {}Length of output: 1523
contexts/auth-context.provider.tsx (1)
250-252: LGTM! Clean function signature formatting.The function signature formatting improves readability.
app/index.tsx (1)
1-11: LGTM! Clean implementation of centered container.The styling implementation correctly centers the content using flex properties.
components/ui/screen-header/index.tsx (1)
44-64: LGTM! Accessible back button implementation.The back button implementation:
- Uses Pressable for better touch handling
- Properly implements the FontAwesomeIcon
- Has appropriate spacing and transparency
components/profile/index.tsx (1)
163-169: LGTM! Clean implementation of ImageComponent.The ImageComponent implementation with fallback initials and proper alt text improves accessibility and user experience.
components/InfluencerCard.tsx (2)
81-86: LGTM! Well-structured ImageComponent implementation.The ImageComponent implementation with proper size, shape, and accessibility attributes enhances the user interface.
98-100: Verify the social media handle fallback.The fallback value "influencer-handle" might not be the best user experience.
Consider handling empty social media data more gracefully:
-{influencer.socials?.[0] || "influencer-handle"} +{influencer.socials?.[0] || `@${influencer.name.toLowerCase().replace(/\s+/g, '')}`}✅ Verification successful
Verified: The suggested social media handle fallback is appropriate
The suggestion to use
@${influencer.name.toLowerCase().replace(/\s+/g, '')}as fallback is valid because:
- The
namefield is required in the data structure- It follows the established handle format seen in the codebase
- It provides more meaningful information than the static "influencer-handle"
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Search for type definitions of influencer ast-grep --pattern 'interface $_ { $$$ socials?: $_ $$$ }' # Search for type definitions with name field ast-grep --pattern 'interface $_ { $$$ name: $_ $$$ }' # Look for other usages of social handles rg "socials\?\.\[" -A 2 -B 2 # Search for any existing handle generation patterns rg -i "\.replace\(/\s+/g,\s*['\"]" -A 2 -B 2Length of output: 2516
Script:
#!/bin/bash # Search for influencer type/interface definitions ast-grep --pattern 'type Influencer = { $$$ }' ast-grep --pattern 'interface Influencer { $$$ }' # Look for usages of influencer.name rg "influencer\.name" -A 2 -B 2 # Search for files that might contain influencer types fd "influencer.*\.(ts|tsx)" --type fLength of output: 2394
Script:
#!/bin/bash # Check contents of influencer type files cat constants/Influencer.ts cat constants/Influencers.ts # Double check for any type definitions in the codebase ast-grep --pattern 'export type $_ = { name: $_ socials?: $_ $$$ }'Length of output: 3445
components/contracts/AddMemberModal.tsx (1)
23-23: LGTM! Clean import of the shared ImageComponent.The import follows the project's component organization pattern, using the shared UI components effectively.
| useEffect(() => { | ||
| if (manager && manager.id) { | ||
| setLoading(true); | ||
| fetchBrands(manager.id); | ||
| } | ||
| }, [manager]); |
There was a problem hiding this comment.
Add error handling for async operations
The fetchBrands call lacks error handling, which could leave users stuck in a loading state if the Firebase query fails.
Add try-catch block:
useEffect(() => {
if (manager?.id) {
setLoading(true);
- fetchBrands(manager.id);
+ fetchBrands(manager.id).catch((error) => {
+ console.error('Failed to fetch brands:', error);
+ setLoading(false);
+ });
}
}, [manager]);📝 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 (manager && manager.id) { | |
| setLoading(true); | |
| fetchBrands(manager.id); | |
| } | |
| }, [manager]); | |
| useEffect(() => { | |
| if (manager?.id) { | |
| setLoading(true); | |
| fetchBrands(manager.id).catch((error) => { | |
| console.error('Failed to fetch brands:', error); | |
| setLoading(false); | |
| }); | |
| } | |
| }, [manager]); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 56-56: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| const fetchBrands = async ( | ||
| managerId: string | ||
| ): Promise<void> => { | ||
| const brandsCollection = collection(FirestoreDB, "brands"); | ||
|
|
||
| const brandsSnapshot = await getDocs(brandsCollection); | ||
|
|
||
| for (const brandDoc of brandsSnapshot.docs) { | ||
| const brandId = brandDoc.id; | ||
|
|
||
| const membersCollection = collection(FirestoreDB, `brands/${brandId}/members`); | ||
| const membersQuery = query(membersCollection, where("managerId", "==", managerId)); | ||
| const membersSnapshot = await getDocs(membersQuery); | ||
|
|
||
| if (!membersSnapshot.empty) { | ||
| setLoading(false); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| // If the manager has no brands, redirect to onboarding | ||
| router.replace({ | ||
| pathname: "/onboarding-your-brand", | ||
| params: { | ||
| firstBrand: "true", | ||
| }, | ||
| }); | ||
| setLoading(false); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize Firestore query performance
The current implementation fetches all brands and then queries their members collections individually, which could be inefficient as the number of brands grows.
Consider restructuring the data model or query to reduce the number of reads:
- const brandsCollection = collection(FirestoreDB, "brands");
- const brandsSnapshot = await getDocs(brandsCollection);
-
- for (const brandDoc of brandsSnapshot.docs) {
- const brandId = brandDoc.id;
-
- const membersCollection = collection(FirestoreDB, `brands/${brandId}/members`);
- const membersQuery = query(membersCollection, where("managerId", "==", managerId));
- const membersSnapshot = await getDocs(membersQuery);
-
- if (!membersSnapshot.empty) {
- setLoading(false);
- return;
- }
- }
+ // Create a collection group query across all members subcollections
+ const membersQuery = query(
+ collectionGroup(FirestoreDB, 'members'),
+ where('managerId', '==', managerId),
+ limit(1)
+ );
+ const membersSnapshot = await getDocs(membersQuery);
+
+ if (!membersSnapshot.empty) {
+ setLoading(false);
+ return;
+ }📝 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 fetchBrands = async ( | |
| managerId: string | |
| ): Promise<void> => { | |
| const brandsCollection = collection(FirestoreDB, "brands"); | |
| const brandsSnapshot = await getDocs(brandsCollection); | |
| for (const brandDoc of brandsSnapshot.docs) { | |
| const brandId = brandDoc.id; | |
| const membersCollection = collection(FirestoreDB, `brands/${brandId}/members`); | |
| const membersQuery = query(membersCollection, where("managerId", "==", managerId)); | |
| const membersSnapshot = await getDocs(membersQuery); | |
| if (!membersSnapshot.empty) { | |
| setLoading(false); | |
| return; | |
| } | |
| } | |
| // If the manager has no brands, redirect to onboarding | |
| router.replace({ | |
| pathname: "/onboarding-your-brand", | |
| params: { | |
| firstBrand: "true", | |
| }, | |
| }); | |
| setLoading(false); | |
| }; | |
| const fetchBrands = async ( | |
| managerId: string | |
| ): Promise<void> => { | |
| // Create a collection group query across all members subcollections | |
| const membersQuery = query( | |
| collectionGroup(FirestoreDB, 'members'), | |
| where('managerId', '==', managerId), | |
| limit(1) | |
| ); | |
| const membersSnapshot = await getDocs(membersQuery); | |
| if (!membersSnapshot.empty) { | |
| setLoading(false); | |
| return; | |
| } | |
| // If the manager has no brands, redirect to onboarding | |
| router.replace({ | |
| pathname: "/onboarding-your-brand", | |
| params: { | |
| firstBrand: "true", | |
| }, | |
| }); | |
| setLoading(false); | |
| }; |
| role: "Manager", | ||
| }) | ||
| .then(() => { | ||
| router.replace({ | ||
| pathname: "/onboarding-get-started", | ||
| params: { | ||
| brandId: docRef.id, | ||
| firstBrand: firstBrand === "true" ? "true" : "false", | ||
| }, | ||
| }); | ||
| setSelectedBrand(brandData as Brand); | ||
| }) | ||
| .catch((error) => { | ||
| Toaster.error("Error creating brand"); | ||
| }) | ||
| .finally(() => { | ||
| setIsSubmitting(false); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a transaction for atomic operations.
The brand creation and manager role assignment should be atomic operations to maintain data consistency.
- await setDoc(managerRef, {
- managerId: user.id,
- role: "Manager",
- })
+ try {
+ await FirestoreDB.runTransaction(async (transaction) => {
+ transaction.set(docRef, brand);
+ transaction.set(managerRef, {
+ managerId: user.id,
+ role: "Manager",
+ });
+ });
+
+ router.replace({
+ pathname: "/onboarding-get-started",
+ params: {
+ brandId: docRef.id,
+ firstBrand: firstBrand === "true" ? "true" : "false",
+ },
+ });
+ setSelectedBrand(brandData as Brand);
+ } catch (error) {
+ Toaster.error("Error creating brand");
+ } finally {
+ setIsSubmitting(false);
+ }Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
New Features
BrandProtectedScreencomponent to manage brand data loading for authenticated usersLoadingScreencomponent to display loading indicators during data fetchingChores