feat: introduce extractDisplayInfo for Avatar/AvatarStack components#3244
feat: introduce extractDisplayInfo for Avatar/AvatarStack components#3244arnautov-anton wants to merge 4 commits into
Conversation
6096b6d to
856bacd
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAvatar rendering now supports explicit initials and context-provided display extraction. Core components, participant lists, typing indicators, and ChannelDetail views use customizable ChangesAvatar customization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant View
participant ComponentContext
participant extractDisplayInfo
participant Avatar
View->>ComponentContext: resolve Avatar and extractor
View->>extractDisplayInfo: provide user data
extractDisplayInfo-->>View: return avatar display props
View->>Avatar: render contextual avatar
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Size Change: +863 B (+0.1%) Total Size: 881 kB 📦 View Changed
ℹ️ View Unchanged
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3244 +/- ##
==========================================
+ Coverage 85.17% 85.21% +0.03%
==========================================
Files 505 506 +1
Lines 15779 15795 +16
Branches 5008 5023 +15
==========================================
+ Hits 13440 13459 +19
+ Misses 2339 2336 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/components/Avatar/utils.ts (1)
3-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove commented-out code.
Please remove the commented-out code
// || user?.id,to keep the codebase clean. If you intend to fall back toid, uncomment it; otherwise, remove it entirely.♻️ Proposed refactor
export const extractDisplayInfo: NonNullable< ComponentContextValue['extractDisplayInfo'] > = ({ user }) => ({ id: user?.id, imageUrl: user?.image, - userName: user?.name, // || user?.id, + userName: user?.name, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Avatar/utils.ts` around lines 3 - 9, Remove the commented-out fallback `// || user?.id,` from the `userName` property in `extractDisplayInfo`; leave the existing `user?.name` behavior unchanged.src/components/TextareaComposer/SuggestionList/MentionItem/UserItem.tsx (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove unused import.
The
UserResponsetype is imported but never used in this file. Removing it will keep the imports clean and address the linter warning.♻️ Proposed refactor
-import type { UserResponse, UserSuggestion } from 'stream-chat'; +import type { UserSuggestion } from 'stream-chat';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/TextareaComposer/SuggestionList/MentionItem/UserItem.tsx` at line 3, Remove the unused UserResponse type from the import in UserItem.tsx, leaving only the UserSuggestion type required by the file.Source: Linters/SAST tools
src/components/Poll/PollOptionSelector.tsx (1)
106-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBe explicit when mapping to the context callback.
Passing the iteration item directly to
mapcan be risky if the context consumer expects strictly{ user }without the other properties present onPollVote(likeidorcreated_at), or if they rely on the second parameter (index). For consistency with other components and safer contract adherence, wrap the callback to explicitly pass only the expected payload.♻️ Proposed refactor
- .map(extractDisplayInfo), - [displayAvatarCount, extractDisplayInfo, latest_votes_by_option, option.id], + .map((vote) => extractDisplayInfo({ user: vote.user })), + [displayAvatarCount, extractDisplayInfo, latest_votes_by_option, option.id],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Poll/PollOptionSelector.tsx` around lines 106 - 107, Update the mapping logic in PollOptionSelector’s vote display computation to wrap extractDisplayInfo in an explicit callback, passing only the expected user payload rather than the full PollVote item and avoiding reliance on map’s index argument. Preserve the existing dependency list and resulting display data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Avatar/Avatar.tsx`:
- Around line 65-76: Update the Avatar component’s prop destructuring to remove
initials from the object spread before rendering the div, and use the resulting
DOM-safe props object in place of rest. Also remove the redundant trim call when
assigning avatarImageAlt, since nameString is already trimmed.
In
`@src/plugins/ChannelDetail/Views/ChannelMembersView/ChannelMembersAddView.tsx`:
- Around line 68-76: Prevent unstable component-slot recreation by moving
extractDisplayInfo calls inside the useMemo factory for
ChannelMembersAddView.tsx lines 68-76, ChannelMembersBrowseView.tsx lines 65-73,
and PinnedMessagesView.tsx lines 90-98. Update the respective dependency arrays
to [Avatar, extractDisplayInfo, user, user.online], [Avatar, extractDisplayInfo,
user, user?.online], and [Avatar, extractDisplayInfo, message.user], preserving
each slot’s existing avatar behavior.
---
Nitpick comments:
In `@src/components/Avatar/utils.ts`:
- Around line 3-9: Remove the commented-out fallback `// || user?.id,` from the
`userName` property in `extractDisplayInfo`; leave the existing `user?.name`
behavior unchanged.
In `@src/components/Poll/PollOptionSelector.tsx`:
- Around line 106-107: Update the mapping logic in PollOptionSelector’s vote
display computation to wrap extractDisplayInfo in an explicit callback, passing
only the expected user payload rather than the full PollVote item and avoiding
reliance on map’s index argument. Preserve the existing dependency list and
resulting display data.
In `@src/components/TextareaComposer/SuggestionList/MentionItem/UserItem.tsx`:
- Line 3: Remove the unused UserResponse type from the import in UserItem.tsx,
leaving only the UserSuggestion type required by the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6c4e6f7b-6546-4bdc-8b55-ef98b08f31e8
📒 Files selected for processing (23)
src/components/Avatar/Avatar.tsxsrc/components/Avatar/utils.tssrc/components/ChannelListItem/utils.tsxsrc/components/Dialog/components/ContextMenu.tsxsrc/components/Message/MessageRepliesCountButton.tsxsrc/components/Message/MessageUI.tsxsrc/components/Poll/PollOptionSelector.tsxsrc/components/Poll/PollVote.tsxsrc/components/Reactions/MessageReactionsDetail.tsxsrc/components/Search/SearchResults/SearchResultItem.tsxsrc/components/TextareaComposer/SuggestionList/MentionItem/UserItem.tsxsrc/components/Threads/ThreadList/ThreadListItemUI.tsxsrc/components/Threads/ThreadList/__tests__/ThreadListItemUI.test.tsxsrc/components/TypingIndicator/TypingIndicator.tsxsrc/components/TypingIndicator/hooks/useDebouncedTypingActive.tssrc/context/ComponentContext.tsxsrc/plugins/ChannelDetail/Views/ChannelMediaView/ChannelMediaView.tsxsrc/plugins/ChannelDetail/Views/ChannelMembersView/ChannelMembersAddView.tsxsrc/plugins/ChannelDetail/Views/ChannelMembersView/ChannelMembersBrowseView.tsxsrc/plugins/ChannelDetail/Views/ChannelMembersView/__tests__/ChannelMembersAddView.test.tsxsrc/plugins/ChannelDetail/Views/ChannelMembersView/__tests__/ChannelMembersBrowseView.test.tsxsrc/plugins/ChannelDetail/Views/PinnedMessagesView/PinnedMessagesView.tsxsrc/plugins/ChannelDetail/Views/PinnedMessagesView/__tests__/PinnedMessagesView.test.tsx
| const displayInfo = extractDisplayInfo({ user }); | ||
|
|
||
| const LeadingSlot = useMemo( | ||
| () => | ||
| function MemberAvatar() { | ||
| return ( | ||
| <Avatar | ||
| imageUrl={user.image} | ||
| isOnline={user.online} | ||
| size='md' | ||
| userName={displayName} | ||
| /> | ||
| ); | ||
| return <Avatar {...displayInfo} isOnline={user.online} size='md' />; | ||
| }, | ||
| [displayName, user.image, user.online], | ||
| [Avatar, displayInfo, user.online], | ||
| ); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔴 Critical | ⚡ Quick win
Unmount/remount loop caused by unstable dependency in component slot.
extractDisplayInfo returns a new object on every call. Computing displayInfo during render and passing it into the useMemo dependency array causes the array to change on every render. Consequently, the inline functional component (LeadingSlot) is recreated on every render, forcing React to continuously unmount and remount the avatar slot component, causing layout thrashing and loss of DOM state.
To resolve this, move the displayInfo computation inside the useMemo factory function so the slot component is only recreated when its actual dependencies (user, Avatar, extractDisplayInfo) change.
src/plugins/ChannelDetail/Views/ChannelMembersView/ChannelMembersAddView.tsx#L68-L76: MovedisplayInfocomputation inside theuseMemofactory function and update dependencies to[Avatar, extractDisplayInfo, user, user.online].src/plugins/ChannelDetail/Views/ChannelMembersView/ChannelMembersBrowseView.tsx#L65-L73: MovedisplayInfocomputation inside theuseMemofactory function and update dependencies to[Avatar, extractDisplayInfo, user, user?.online].src/plugins/ChannelDetail/Views/PinnedMessagesView/PinnedMessagesView.tsx#L90-L98: MovedisplayInfocomputation inside theuseMemofactory function and update dependencies to[Avatar, extractDisplayInfo, message.user].
🚀 Proposed fixes
ChannelMembersAddView.tsx
- const displayInfo = extractDisplayInfo({ user });
-
const LeadingSlot = useMemo(
- () =>
- function MemberAvatar() {
- return <Avatar {...displayInfo} isOnline={user.online} size='md' />;
- },
- [Avatar, displayInfo, user.online],
+ () => {
+ const displayInfo = extractDisplayInfo({ user });
+ return function MemberAvatar() {
+ return <Avatar {...displayInfo} isOnline={user.online} size='md' />;
+ };
+ },
+ [Avatar, extractDisplayInfo, user, user.online],
);ChannelMembersBrowseView.tsx
- const displayInfo = extractDisplayInfo({ user: user ?? undefined });
-
const LeadingSlot = useMemo(
- () =>
- function MemberAvatar() {
- return <Avatar {...displayInfo} isOnline={user?.online} size='md' />;
- },
- [Avatar, displayInfo, user],
+ () => {
+ const displayInfo = extractDisplayInfo({ user: user ?? undefined });
+ return function MemberAvatar() {
+ return <Avatar {...displayInfo} isOnline={user?.online} size='md' />;
+ };
+ },
+ [Avatar, extractDisplayInfo, user, user?.online],
);PinnedMessagesView.tsx
- const displayInfo = extractDisplayInfo({ user: message.user ?? undefined });
-
const LeadingSlot = useMemo(
- () =>
- function MessageAuthorAvatar() {
- return <Avatar {...displayInfo} size='md' />;
- },
- [Avatar, displayInfo],
+ () => {
+ const displayInfo = extractDisplayInfo({ user: message.user ?? undefined });
+ return function MessageAuthorAvatar() {
+ return <Avatar {...displayInfo} size='md' />;
+ };
+ },
+ [Avatar, extractDisplayInfo, message.user],
);📝 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 displayInfo = extractDisplayInfo({ user }); | |
| const LeadingSlot = useMemo( | |
| () => | |
| function MemberAvatar() { | |
| return ( | |
| <Avatar | |
| imageUrl={user.image} | |
| isOnline={user.online} | |
| size='md' | |
| userName={displayName} | |
| /> | |
| ); | |
| return <Avatar {...displayInfo} isOnline={user.online} size='md' />; | |
| }, | |
| [displayName, user.image, user.online], | |
| [Avatar, displayInfo, user.online], | |
| ); | |
| const LeadingSlot = useMemo( | |
| () => { | |
| const displayInfo = extractDisplayInfo({ user }); | |
| return function MemberAvatar() { | |
| return <Avatar {...displayInfo} isOnline={user.online} size='md' />; | |
| }; | |
| }, | |
| [Avatar, extractDisplayInfo, user, user.online], | |
| ); |
📍 Affects 3 files
src/plugins/ChannelDetail/Views/ChannelMembersView/ChannelMembersAddView.tsx#L68-L76(this comment)src/plugins/ChannelDetail/Views/ChannelMembersView/ChannelMembersBrowseView.tsx#L65-L73src/plugins/ChannelDetail/Views/PinnedMessagesView/PinnedMessagesView.tsx#L90-L98
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/plugins/ChannelDetail/Views/ChannelMembersView/ChannelMembersAddView.tsx`
around lines 68 - 76, Prevent unstable component-slot recreation by moving
extractDisplayInfo calls inside the useMemo factory for
ChannelMembersAddView.tsx lines 68-76, ChannelMembersBrowseView.tsx lines 65-73,
and PinnedMessagesView.tsx lines 90-98. Update the respective dependency arrays
to [Avatar, extractDisplayInfo, user, user.online], [Avatar, extractDisplayInfo,
user, user?.online], and [Avatar, extractDisplayInfo, message.user], preserving
each slot’s existing avatar behavior.
856bacd to
d947464
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/TextareaComposer/SuggestionList/MentionItem/UserItem.tsx`:
- Around line 35-44: Stabilize the displayInfo value used by the LeadingSlot
useMemo in UserItem by memoizing the extractDisplayInfo({ user: entity }) result
with the appropriate stable dependencies. Keep LeadingSlot dependent on this
memoized value so the UserItemAvatar component is not recreated on every render.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 348ecba8-13ad-4e57-ae0e-835b788390f1
📒 Files selected for processing (23)
src/components/Avatar/Avatar.tsxsrc/components/Avatar/utils.tssrc/components/ChannelListItem/utils.tsxsrc/components/Dialog/components/ContextMenu.tsxsrc/components/Message/MessageRepliesCountButton.tsxsrc/components/Message/MessageUI.tsxsrc/components/Poll/PollOptionSelector.tsxsrc/components/Poll/PollVote.tsxsrc/components/Reactions/MessageReactionsDetail.tsxsrc/components/Search/SearchResults/SearchResultItem.tsxsrc/components/TextareaComposer/SuggestionList/MentionItem/UserItem.tsxsrc/components/Threads/ThreadList/ThreadListItemUI.tsxsrc/components/Threads/ThreadList/__tests__/ThreadListItemUI.test.tsxsrc/components/TypingIndicator/TypingIndicator.tsxsrc/components/TypingIndicator/hooks/useDebouncedTypingActive.tssrc/context/ComponentContext.tsxsrc/plugins/ChannelDetail/Views/ChannelMediaView/ChannelMediaView.tsxsrc/plugins/ChannelDetail/Views/ChannelMembersView/ChannelMembersAddView.tsxsrc/plugins/ChannelDetail/Views/ChannelMembersView/ChannelMembersBrowseView.tsxsrc/plugins/ChannelDetail/Views/ChannelMembersView/__tests__/ChannelMembersAddView.test.tsxsrc/plugins/ChannelDetail/Views/ChannelMembersView/__tests__/ChannelMembersBrowseView.test.tsxsrc/plugins/ChannelDetail/Views/PinnedMessagesView/PinnedMessagesView.tsxsrc/plugins/ChannelDetail/Views/PinnedMessagesView/__tests__/PinnedMessagesView.test.tsx
🚧 Files skipped from review as they are similar to previous changes (20)
- src/plugins/ChannelDetail/Views/PinnedMessagesView/tests/PinnedMessagesView.test.tsx
- src/plugins/ChannelDetail/Views/ChannelMembersView/tests/ChannelMembersBrowseView.test.tsx
- src/plugins/ChannelDetail/Views/ChannelMembersView/tests/ChannelMembersAddView.test.tsx
- src/components/Threads/ThreadList/tests/ThreadListItemUI.test.tsx
- src/components/Message/MessageRepliesCountButton.tsx
- src/components/TypingIndicator/hooks/useDebouncedTypingActive.ts
- src/components/TypingIndicator/TypingIndicator.tsx
- src/components/Poll/PollOptionSelector.tsx
- src/context/ComponentContext.tsx
- src/components/Dialog/components/ContextMenu.tsx
- src/plugins/ChannelDetail/Views/PinnedMessagesView/PinnedMessagesView.tsx
- src/components/Reactions/MessageReactionsDetail.tsx
- src/plugins/ChannelDetail/Views/ChannelMembersView/ChannelMembersAddView.tsx
- src/components/Poll/PollVote.tsx
- src/components/Threads/ThreadList/ThreadListItemUI.tsx
- src/components/ChannelListItem/utils.tsx
- src/plugins/ChannelDetail/Views/ChannelMembersView/ChannelMembersBrowseView.tsx
- src/components/Avatar/Avatar.tsx
- src/plugins/ChannelDetail/Views/ChannelMediaView/ChannelMediaView.tsx
- src/components/Message/MessageUI.tsx
a688999 to
04956d7
Compare
04956d7 to
54cb7b7
Compare
🎯 Goal
Unify how the Avatar/AvatarStack information is being constructed through the common and replaceable utility
extractDisplayInfofunction.Summary by CodeRabbit
initialsprop override for avatars.alttext, with the explicitinitialsoverride taking priority.