Repository navigation
Feedbacks - #87
Feedbacks#87
Conversation
* feedbacks * feedbacks * address autocomplete for web * map on web implementation
pushed changes
|
Warning Rate limit exceeded@dev-rathore has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces several enhancements across multiple components, focusing on notification management, text truncation, and UI improvements. Key changes include adding a Changes
Sequence DiagramsequenceDiagram
participant User
participant Component
participant NotificationContext
participant Firestore
User->>Component: Trigger Notification
Component->>NotificationContext: createNotification()
NotificationContext->>Firestore: Add Notification Document
NotificationContext->>Component: Notification Created
Component->>NotificationContext: sendNotification()
NotificationContext->>User: Notification Sent
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: 3
🧹 Nitpick comments (13)
components/card/collaboration-details/invitation-card.tsx (1)
97-99: Consider adding visual indication for truncated text.The implementation looks good and matches the truncation length used in other cards. Since the text is truncated and clickable (wrapped in Pressable), consider adding a visual indicator (e.g., ellipsis or "Read more" link) when text is actually truncated.
<CardDescription - text={truncateText(data.profile?.content?.about || "", 160)} + text={`${truncateText(data.profile?.content?.about || "", 160)}${ + (data.profile?.content?.about?.length || 0) > 160 ? ' <a href="#">Read more</a>' : '' + }`} />components/menu/index.tsx (1)
57-57: Extract magic number and improve null handling.While the implementation is good, consider these improvements:
- Extract the truncation length (120) to a constant for better maintainability.
- Add null coalescing for better error handling.
+const BRAND_ABOUT_MAX_LENGTH = 120; + // In the component: -{truncateText(selectedBrand?.profile?.about, 120)} +{truncateText(selectedBrand?.profile?.about ?? "", BRAND_ABOUT_MAX_LENGTH)}types/Notification.ts (1)
8-15: Consider tightening type definitions
The optionaldata?: { [key: string]: any }property enables flexibility but reduces type safety. If you have known fields for notifications, consider a stricter schema for better clarity and maintainability.components/map/index.tsx (1)
23-29: Minimize redundant map re-centering and manage callback dependencies
- Calling both
setCenterandpanToon line 23 can be redundant; consider using only one for a smoother user experience.- If you intend to recalculate the callback when
locationchanges, ensure that updating location does not cause excessive re-renders.- Specifying both
centeranddefaultCentercan be redundant. Typically,centertakes precedence if provided.Refactor example:
- event.map?.setCenter(event.detail.latLng as any); event.map?.panTo(event.detail.latLng as any);Use either
setCenterorpanToconsistently, or handle them conditionally to avoid conflicting animations.Also applies to: 33-35, 41-41, 43-44, 46-46, 48-52
components/collaboration-card/card-components/CollaborationStats.tsx (1)
65-66: Consider improving the currency format.The current format
${CURRENCY}. ${value}includes a period after the currency which might look unconventional. Consider removing the period for a cleaner display:- ? `${CURRENCY}. ${props.budget.min}` - : `${CURRENCY}. ${props.budget.min} - ${CURRENCY}. ${props.budget.max}` + ? `${CURRENCY} ${props.budget.min}` + : `${CURRENCY} ${props.budget.min} - ${CURRENCY} ${props.budget.max}`contexts/notification-context.provider.tsx (1)
27-31: Consider using an enum for userType.The userType parameter would benefit from being more strictly typed using an enum to prevent potential errors from typos or invalid values.
enum UserType { MANAGERS = "managers", USERS = "users" } createNotification: ( userId: string, notification: INotifications, userType: UserType = UserType.MANAGERS, ) => Promise<void>;components/collaboration/create-collaboration/AddressAutocomplete.tsx (3)
42-42: Consider using a more specific type for the web ref.The web ref is typed as
anywhich loses type safety. Consider using the correct type from thereact-google-autocompletelibrary.-const mapInputRefWeb = useRef<any>(null); +const mapInputRefWeb = useRef<HTMLInputElement>(null);
74-74: Add type assertion for location type.The type assertion
as stringcould be replaced with proper type checking to ensure type safety.-type: collaboration.location?.type as string, +type: typeof collaboration.location?.type === 'string' ? collaboration.location.type : 'Remote',
87-94: Consider extracting shared styles.The style object is duplicated between web and native implementations. Consider extracting it into a shared constant.
+const sharedStyles = (theme: Theme) => ({ + backgroundColor: Colors(theme).background, + borderRadius: 4, + borderColor: Colors(theme).primary, + borderWidth: 1, + color: Colors(theme).text, +}); -style={{ - backgroundColor: Colors(theme).background, - padding: 10, - borderRadius: 4, - borderColor: Colors(theme).primary, - borderWidth: 1, - color: Colors(theme).text, -}} +style={{ + ...sharedStyles(theme), + padding: 10, +}}hooks/request/use-applications.tsx (1)
109-122: Consider consolidating notification data.The notification data is duplicated between
createNotificationandsendNotificationcalls. Consider extracting the shared data into a constant.+const notificationData = { + title: "Application Accepted", + description: `Your application for ${data.collaboration.name} has been accepted`, + data: { + collaborationId: data.collaboration.id, + }, +}; createNotification( influencerApplication.application.userId, { - data: { - collaborationId: data.collaboration.id, - }, - description: `Your application for ${data.collaboration.name} has been accepted`, + ...notificationData, isRead: false, timeStamp: Date.now(), - title: "Application Accepted", type: "application-accepted", }, "users" ); sendNotification( { users: [influencerApplication.application.userId], }, { - data: { - collaborationId: influencerApplication.application.collaborationId, - }, notification: { - title: "Application Accepted", - description: `Your application for ${data.collaboration.name} has been accepted`, + ...notificationData }, }, );components/create-collaboration/screen-one.tsx (1)
59-70: Consider adding currency formatting utility.While using the CURRENCY constant improves consistency, consider creating a utility function for currency formatting that handles all cases.
+const formatCurrency = (amount: number) => `${CURRENCY} ${convertToKUnits(amount)}`; -return `${CURRENCY}. ${convertToKUnits(Math.max(collaboration.budget?.min, collaboration.budget?.max))}`; +return formatCurrency(Math.max(collaboration.budget?.min || 0, collaboration.budget?.max || 0)); -return `${CURRENCY}. ${convertToKUnits(collaboration.budget?.min || 0)}`; +return formatCurrency(collaboration.budget?.min || 0); -return `${CURRENCY}. ${convertToKUnits(collaboration.budget?.min || 0)}-${convertToKUnits(collaboration.budget?.max || 500)}`; +return `${formatCurrency(collaboration.budget?.min || 0)}-${formatCurrency(collaboration.budget?.max || 500)}`;components/BottomSheetActions.tsx (1)
78-102: Consider consolidating notification calls to avoid duplicate messages.The code currently sends two notifications for the same event:
createNotificationfor storing in FirestoresendNotificationfor push notificationThis could lead to duplicate notifications for users.
Consider consolidating these into a single method that handles both storage and push notification:
- createNotification( - cardId.influencerID, - { - data: { - collaborationId: data.collaboration.id, - }, - description: `Your application for ${data.collaboration.name} has been accepted`, - isRead: false, - timeStamp: Date.now(), - title: "Application Accepted", - type: "application-accepted", - - }, - "users" - ); - - sendNotification( - { - users: [cardId.influencerID], - }, - { - notification: { - title: "Application Accepted", - description: `Your application for ${data.collaboration.name} has been accepted`, - }, - }, - ); + // Single method to handle both storage and push notification + const notificationData = { + title: "Application Accepted", + description: `Your application for ${data.collaboration.name} has been accepted`, + data: { + collaborationId: data.collaboration.id, + }, + type: "application-accepted", + }; + + await handleNotification(cardId.influencerID, notificationData);components/collaboration/collaboration-details/OverviewTabContent.tsx (1)
418-419: Empty callback functions should include TODO comments.The empty callback functions
onMapRegionChangeandonFormattedAddressChangemight need implementation in the future.Consider adding TODO comments to explain why these are empty:
- onMapRegionChange={(region) => { }} - onFormattedAddressChange={(address) => { }} + onMapRegionChange={(region) => { /* TODO: Implement map region change handler */ }} + onFormattedAddressChange={(address) => { /* TODO: Implement address change handler */ }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
components/BottomSheetActions.tsx(4 hunks)components/card/collaboration-details/application-card.tsx(2 hunks)components/card/collaboration-details/invitation-card.tsx(2 hunks)components/channel/channel-list.tsx(1 hunks)components/collaboration-card/card-components/CollaborationStats.tsx(2 hunks)components/collaboration/collaboration-details/OverviewTabContent.tsx(5 hunks)components/collaboration/create-collaboration/AddressAutocomplete.tsx(1 hunks)components/collaboration/create-collaboration/CreateCollaborationMap.tsx(1 hunks)components/create-collaboration/index.tsx(0 hunks)components/create-collaboration/screen-one.tsx(2 hunks)components/create-collaboration/screen-two.tsx(3 hunks)components/map/index.tsx(1 hunks)components/menu/index.tsx(2 hunks)components/proposals/Applications.tsx(2 hunks)components/ui/card/secondary/card-description.tsx(1 hunks)contexts/notification-context.provider.tsx(5 hunks)hooks/request/use-applications.tsx(3 hunks)package.json(3 hunks)styles/CollaborationDetails.styles.tsx(1 hunks)types/Notification.ts(1 hunks)
💤 Files with no reviewable changes (1)
- components/create-collaboration/index.tsx
🔇 Additional comments (13)
styles/CollaborationDetails.styles.tsx (1)
42-42: Great improvement to text readability!The addition of
lineHeight: 22creates optimal spacing for the 14px font size (approximately 1.57x ratio), which enhances text readability and visual comfort.components/ui/card/secondary/card-description.tsx (1)
40-40: Style change looks good, but verify HTML rendering with truncated content.The marginTop addition provides consistent spacing. However, since this component uses RenderHTML and the text prop is being truncated in parent components, ensure that the truncation doesn't break HTML tags, which could lead to rendering issues.
Run this script to check for potential HTML tag truncation issues:
✅ Verification successful
HTML truncation implementation is safe for this use case
The marginTop style change is good, and after analyzing the codebase:
- Content being rendered comes from controlled sources (profile data)
- Truncation happens at generous lengths (120-160 characters)
- RenderHTML component is resilient to malformed HTML and provides fallback rendering
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for HTML tags in the truncated text to verify proper handling rg -l '<[^>]+>' | xargs rg 'truncateText.*?["`].*?<.*?>'Length of output: 58
Script:
#!/bin/bash # Find truncation implementation and HTML content handling echo "=== Searching for truncation implementations ===" rg -A 3 "truncate.*Text|truncate.*Html" echo -e "\n=== Searching for RenderHTML usage ===" rg -A 3 "RenderHTML|renderHtml" echo -e "\n=== Searching for HTML content preparation ===" rg -A 3 "html.*content|HTML.*content"Length of output: 7616
components/card/collaboration-details/application-card.tsx (1)
94-94: Implementation is consistent with invitation card.The truncation implementation matches the invitation card, which is good for consistency. Consider applying the same visual indication for truncated text as suggested for the invitation card.
components/proposals/Applications.tsx (2)
6-6: Importingrouter
No issues found. Good job integratingrouterfor navigation.
8-8: Inclusion ofonSnapshot
Adopting the Firestore real-time listener (onSnapshot) is an excellent move for live updates.components/channel/channel-list.tsx (1)
12-12: LGTM! Clean removal of unused group functionality.The cleanup of unused imports and state management improves code maintainability while preserving the core channel listing and search functionality.
components/collaboration/create-collaboration/CreateCollaborationMap.tsx (1)
75-78: LGTM! Improved type safety in map region handling.The explicit construction of the location object with latitude and longitude properties ensures type safety and prevents potential undefined values.
contexts/notification-context.provider.tsx (1)
Line range hint
18-41: Standardize notification types.The codebase is mixing
INotificationsandNotificationtypes. Consider consolidating these into a single type to improve consistency.Let's verify the usage of these types across the codebase:
components/create-collaboration/screen-two.tsx (1)
232-236: LGTM! Clean integration of the new AddressAutocomplete component.The implementation correctly passes all required props and maintains the existing functionality.
components/BottomSheetActions.tsx (2)
41-44: LGTM! Notification context usage is properly structured.The destructuring of notification context methods is clean and follows best practices.
276-285: Verify the collaboration link format.The hardcoded URL format
https://creators.trendly.pro/collaboration/${cardId}might need verification to ensure it matches your production environment.✅ Verification successful
The collaboration link format is correctly implemented ✅
The URL format
https://creators.trendly.pro/collaboration/${cardId}is consistent with the application's domain structure, where different subdomains serve specific purposes (creators for collaboration features, be for backend, brands for brand-specific features).🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other instances of collaboration URLs to verify format consistency rg -g '!*.{json,md}' "creators\.trendly\.pro/collaboration" .Length of output: 172
Script:
#!/bin/bash # Search for environment files and URL configurations fd -e env -e config.ts -e config.js # Search for any trendly.pro URLs rg -g '!*.{json,md}' "trendly\.pro" # Search for URL utility functions or constants rg -g '!*.{json,md}' -i "(baseUrl|apiUrl|BASE_URL|API_URL|const.*url)"Length of output: 2860
components/collaboration/collaboration-details/OverviewTabContent.tsx (1)
320-331: LGTM! Currency display is properly standardized.The implementation correctly handles both single value and range displays using the CURRENCY constant.
package.json (1)
47-47: Verify dependency version compatibility with Expo 51.The project uses Expo 51.0.21, but some dependency versions might need verification:
- expo-notifications downgrade to 0.28.19 might be incompatible
- expo-clipboard 7.0.1 and react-google-autocomplete 2.7.4 need version compatibility check
Also applies to: 57-57, 67-67
✅ Verification successful
All dependency versions are compatible with Expo SDK 51
- expo-clipboard@7.0.1 is compatible (patch version update from 7.0.0)
- expo-notifications@0.28.19 is compatible (stable version within 0.28.x range)
- react-google-autocomplete version is independent of Expo SDK as it's not an Expo package
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check Expo compatibility matrix for these packages curl -s https://raw.githubusercontent.com/expo/expo/sdk-51/packages/expo/bundledNativeModules.json | jq '.[] | select(.name | contains("clipboard") or contains("notifications"))'Length of output: 247
Script:
#!/bin/bash # Check Expo SDK 51 dependencies curl -s https://raw.githubusercontent.com/expo/expo/sdk-51/packages/expo/package.json | jq '.dependencies["expo-clipboard"], .dependencies["expo-notifications"]' # Check expo-clipboard package curl -s https://raw.githubusercontent.com/expo/expo/main/packages/expo-clipboard/package.json | jq '.version' # Check expo-notifications package curl -s https://raw.githubusercontent.com/expo/expo/main/packages/expo-notifications/package.json | jq '.version'Length of output: 414
Script:
#!/bin/bash # Check expo-clipboard README for compatibility info curl -s https://raw.githubusercontent.com/expo/expo/sdk-51/packages/expo-clipboard/README.md # Check expo-notifications README for compatibility info curl -s https://raw.githubusercontent.com/expo/expo/sdk-51/packages/expo-notifications/README.md # Check SDK 51 release notes curl -s https://raw.githubusercontent.com/expo/expo/sdk-51/CHANGELOG.mdLength of output: 79052
Script:
#!/bin/bash # Check Expo SDK 51 documentation for these packages curl -s "https://docs.expo.dev/versions/v51.0.0/sdk/clipboard/" curl -s "https://docs.expo.dev/versions/v51.0.0/sdk/notifications/"Length of output: 65667
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
Chores