-
Notifications
You must be signed in to change notification settings - Fork 0
Improve mobile perfomance #2721
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
Conversation
✅ Deploy Preview for preview-common ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MeyerPV
Here is Calude's review of this pr:
Performance Improvements:
The use of useMemo and useCallback hooks in various components is a good practice. It helps prevent unnecessary re-renders and optimizes performance, especially for complex components or those that deal with large datasets.
The refactoring of getLastMessage in ChatChannelItem.tsx to use useMemo is a good optimization, especially if chatChannel.lastMessage doesn't change frequently.
Code Quality and Readability:
The extraction of TreeItemTriggerContent into a separate component is a good move. It improves code organization and makes the TreeItemTrigger component more manageable.
The consistent use of useMemo for complex calculations or object creations (e.g., in DesktopCommonDropdown.tsx) improves code predictability and performance.
Best Practices:
The use of TypeScript throughout the codebase is commendable. It adds type safety and improves code maintainability.
The consistent use of functional components with hooks aligns with modern React best practices.
Suggestions for Improvement:
In CommonFeed.tsx, the FeedLayoutTopContent memo could potentially be moved to a separate component to further improve code organization.
Consider using custom hooks for some of the logic in larger components like CommonFeedComponent to improve readability and reusability.
Potential Issues:
In useInboxItems.ts, make sure that all dependencies are correctly listed in the dependency array of useCallback to avoid stale closures.
Overall Structure:
The PR shows a consistent effort to optimize performance and improve code structure across multiple components, which is commendable.
Documentation:
Consider adding more inline comments for complex logic, especially in components like CommonFeedComponent where there's a lot happening.
Overall, this PR demonstrates a good understanding of React best practices and a focus on performance optimization. The changes are generally positive and should improve the overall quality and maintainability of the codebase.
https://daostack.notion.site/Improve-mobile-performance-1224be59420a41cda63bbd30e7f550a3?pvs=74
What was changed?