feature/BA-2725 profile summary drawer #301
Conversation
rf2tsl
requested review from
anicioalexandre and
priscilladeroode
as code owners
|
WalkthroughAdds contact-details support for single chats: extends ChatRoomProvider state and setters, updates hooks/fragments to include profile fields, adds ProfileSummary UI and left-panel routing, and wires ChatRoomHeader/Options to open the profile summary. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ChatRoomOptions
participant ChatRoomHeader
participant useChatRoomStore as Store
participant ChatRoomsComponent
participant ProfileSummary
User->>ChatRoomOptions: Click "Contact Details"
ChatRoomOptions->>ChatRoomHeader: invoke onContactDetailsClicked
ChatRoomHeader->>Store: setSingleChatProfileDetails({pk,name,username,imageUrl,biography})
ChatRoomHeader->>Store: setLeftPanelContent(LEFT_PANEL_CONTENT.profileSummary)
ChatRoomOptions->>ChatRoomOptions: close popover
Store-->>ChatRoomsComponent: leftPanelContent = profileSummary
ChatRoomsComponent->>ProfileSummary: render ProfileSummaryComponent
ProfileSummary->>Store: read singleChatProfileDetails
ProfileSummary-->>User: display profile (avatar, name, username, bio, actions)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
Tip 📝 Customizable high-level summaries are now available!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
packages/components/modules/messages/web/ProfileSummary/Body/types.ts (1)
3-10: Minor: PropsWithChildren extension appears unused.The BodyProps interface extends PropsWithChildren, but the Body component (Body/index.tsx) doesn't render
children. Consider removing this extension unless children support is planned for the future.packages/components/modules/messages/web/ProfileSummary/Body/index.tsx (1)
61-65: Minor: Consider hiding "About" section when biography is empty.When
biographyis null or empty, the "About" heading still renders with no content below it, which may look incomplete.- <Subheader> - <Typography variant="subtitle2" color="text.primary"> - About - </Typography> - </Subheader> - <Subheader> - <Typography variant="caption" color="text.secondary"> - {biography} - </Typography> - </Subheader> + {biography && ( + <> + <Subheader> + <Typography variant="subtitle2" color="text.primary"> + About + </Typography> + </Subheader> + <Subheader> + <Typography variant="caption" color="text.secondary"> + {biography} + </Typography> + </Subheader> + </> + )}packages/components/modules/messages/web/ProfileSummary/index.tsx (1)
12-28: Minor: Simplify redundant nullish coalescing.Line 24 uses
pk ?? undefined, which is redundant sincepkis alreadynumber | undefined. Ifpkis undefined, it stays undefined; if it's a number, it's passed as-is.<Body name={name} avatar={imageUrl ?? ''} username={username ?? ''} biography={biography ?? ''} - pk={pk ?? undefined} + pk={pk} />packages/components/modules/messages/web/ProfileSummary/Body/styled.tsx (1)
13-20: Consider flexible row heights for better accessibility.The fixed
22pxgrid rows may not accommodate font scaling or longer content. Consider usingautoormin-contentforgridTemplateRowsto ensure the layout adapts to content size and respects user font preferences.Apply this diff to make rows flexible:
export const TitleContainer = styled(Box)(() => ({ width: '100%', textAlign: 'center', display: 'grid', justifyItems: 'center', - gridTemplateRows: '22px 22px', + gridTemplateRows: 'auto auto', gap: '8px', }))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
packages/components/modules/messages/common/context/ChatRoomProvider/constants.ts(1 hunks)packages/components/modules/messages/common/context/ChatRoomProvider/index.tsx(1 hunks)packages/components/modules/messages/common/context/ChatRoomProvider/types.ts(2 hunks)packages/components/modules/messages/common/graphql/fragments/RoomTitle.ts(1 hunks)packages/components/modules/messages/common/utils.ts(2 hunks)packages/components/modules/messages/web/AllChatRoomsList/ChatRoomItem/index.tsx(2 hunks)packages/components/modules/messages/web/ChatRoom/ChatRoomHeader/ChatRoomOptions/index.tsx(1 hunks)packages/components/modules/messages/web/ChatRoom/ChatRoomHeader/ChatRoomOptions/types.ts(1 hunks)packages/components/modules/messages/web/ChatRoom/ChatRoomHeader/index.tsx(4 hunks)packages/components/modules/messages/web/ChatRoomsComponent/constants.ts(1 hunks)packages/components/modules/messages/web/ChatRoomsComponent/index.tsx(4 hunks)packages/components/modules/messages/web/ChatRoomsComponent/types.ts(2 hunks)packages/components/modules/messages/web/MessagesList/__storybook__/stories.tsx(1 hunks)packages/components/modules/messages/web/ProfileSummary/Body/index.tsx(1 hunks)packages/components/modules/messages/web/ProfileSummary/Body/styled.tsx(1 hunks)packages/components/modules/messages/web/ProfileSummary/Body/types.ts(1 hunks)packages/components/modules/messages/web/ProfileSummary/Header/index.tsx(1 hunks)packages/components/modules/messages/web/ProfileSummary/Header/styled.tsx(1 hunks)packages/components/modules/messages/web/ProfileSummary/Header/types.ts(1 hunks)packages/components/modules/messages/web/ProfileSummary/index.tsx(1 hunks)packages/components/modules/messages/web/ProfileSummary/types.ts(1 hunks)packages/design-system/components/web/icons/ProfileNoCircleIcon/index.tsx(1 hunks)packages/design-system/components/web/icons/index.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-04-09T22:06:40.026Z
Learnt from: davidbermudez-tsl
Repo: silverlogic/baseapp-frontend PR: 209
File: packages/components/modules/content-feed/web/ContentFeed/types.ts:1-1
Timestamp: 2025-04-09T22:06:40.026Z
Learning: The empty ContentFeedProps interface in packages/components/modules/content-feed/web/ContentFeed/types.ts will be populated with properties in a future PR rather than being converted to a type alias.
Applied to files:
packages/components/modules/messages/web/ProfileSummary/Header/types.tspackages/components/modules/messages/web/ProfileSummary/Body/types.ts
📚 Learning: 2025-10-29T13:48:38.345Z
Learnt from: nossila
Repo: silverlogic/baseapp-frontend PR: 300
File: packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx:57-59
Timestamp: 2025-10-29T13:48:38.345Z
Learning: In packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx, the full page reload via window.location.reload() when switching profiles is intentional and required due to websockets connections and data loading patterns that don't yet support reactive profile switching. This is a known technical debt that will be addressed with a larger refactoring in the future.
Applied to files:
packages/components/modules/messages/web/ProfileSummary/types.tspackages/components/modules/messages/web/ProfileSummary/index.tsxpackages/components/modules/messages/web/ChatRoomsComponent/index.tsxpackages/components/modules/messages/web/ChatRoom/ChatRoomHeader/index.tsx
🧬 Code graph analysis (8)
packages/components/modules/messages/web/ChatRoomsComponent/types.ts (1)
packages/components/modules/messages/web/ProfileSummary/types.ts (1)
ProfileSummaryProps(1-3)
packages/components/modules/messages/web/ProfileSummary/index.tsx (1)
packages/components/modules/messages/web/ProfileSummary/types.ts (1)
ProfileSummaryProps(1-3)
packages/components/modules/messages/web/ProfileSummary/Body/index.tsx (2)
packages/components/modules/messages/web/ProfileSummary/Body/types.ts (1)
BodyProps(3-10)packages/components/modules/messages/web/ProfileSummary/Body/styled.tsx (4)
HeaderContainer(4-11)TitleContainer(13-20)SubheaderContainer(22-27)Subheader(29-32)
packages/components/modules/messages/web/AllChatRoomsList/ChatRoomItem/index.tsx (1)
packages/components/modules/messages/common/utils.ts (1)
useChatroomDetails(62-67)
packages/components/modules/messages/web/ChatRoomsComponent/index.tsx (1)
packages/components/modules/messages/web/ChatRoomsComponent/constants.ts (1)
LEFT_PANEL_CONTENT(1-8)
packages/components/modules/messages/web/ProfileSummary/Header/index.tsx (2)
packages/components/modules/messages/web/ProfileSummary/Header/types.ts (1)
HeaderProps(3-7)packages/components/modules/messages/web/ProfileSummary/Header/styled.tsx (1)
ProfileSummaryHeaderContainer(4-15)
packages/components/modules/messages/common/context/ChatRoomProvider/index.tsx (2)
packages/components/modules/messages/common/context/ChatRoomProvider/types.ts (1)
ChatRoomState(1-12)packages/components/modules/messages/common/context/ChatRoomProvider/constants.ts (1)
INITIAL_CHAT_ROOM_STATE(3-14)
packages/components/modules/messages/web/ChatRoom/ChatRoomHeader/index.tsx (2)
packages/components/modules/messages/common/utils.ts (1)
useChatroomDetails(62-67)packages/components/modules/messages/web/ChatRoomsComponent/constants.ts (1)
LEFT_PANEL_CONTENT(1-8)
🔇 Additional comments (20)
packages/design-system/components/web/icons/index.ts (1)
31-31: LGTM!The export follows the established pattern and maintains alphabetical ordering.
packages/components/modules/messages/web/ProfileSummary/Header/styled.tsx (1)
1-15: LGTM! Clean styled component implementation.The grid layout with fixed 24px columns on the sides is well-suited for icon buttons, and the responsive padding adjustment for small screens is appropriate.
packages/components/modules/messages/web/ProfileSummary/Header/types.ts (1)
1-7: LGTM! Well-structured type definitions.The use of
Omit<IconifyProps, 'ref'>is good practice to prevent ref conflicts when spreading icon props.packages/components/modules/messages/common/context/ChatRoomProvider/constants.ts (1)
6-13: LGTM! State expansion aligns with feature requirements.The new state fields appropriately initialize with
undefinedfor profile details and0for left panel content (matchingLEFT_PANEL_CONTENT.chatRoomList). Components consumingsingleChatProfileDetailswill need to handle undefined values appropriately.packages/components/modules/messages/web/ProfileSummary/types.ts (1)
1-3: LGTM! Clean and focused type definition.The type correctly defines the required callback for the ProfileSummary component.
packages/components/modules/messages/web/ChatRoomsComponent/constants.ts (1)
7-7: LGTM! Consistent addition to the panel content enum.The new
profileSummary: 5value follows the existing sequential pattern and integrates cleanly with the left panel navigation flow.packages/components/modules/messages/web/AllChatRoomsList/ChatRoomItem/index.tsx (1)
24-24: LGTM! Improved hook consistency.The refactoring from
useNameAndAvatartouseChatroomDetailsaligns with the expanded profile data requirements throughout the messaging system while maintaining backward compatibility with the existing{ title, avatar }destructuring pattern.packages/components/modules/messages/web/ChatRoom/ChatRoomHeader/ChatRoomOptions/types.ts (1)
8-8: LGTM! Consistent callback addition.The new
onContactDetailsClickedcallback follows the established naming pattern and integrates cleanly with the existing options interface.packages/components/modules/messages/web/MessagesList/__storybook__/stories.tsx (1)
13-14: LGTM! Appropriate mock implementations.The no-op setters correctly align with the expanded
UseChatRoomAPI, and the underscore-prefixed parameters properly indicate intentionally unused values in the mock store.packages/components/modules/messages/common/graphql/fragments/RoomTitle.ts (1)
12-20: LGTM! GraphQL fragment properly extended.The additional profile fields (pk, biography, urlPath) align well with the ProfileSummary drawer requirements. The image dimension increase to 128×128 matches the component's needs.
packages/components/modules/messages/web/ChatRoomsComponent/types.ts (1)
36-37: LGTM! Type definitions follow established patterns.The ProfileSummary props follow the same pattern as other optional component props in this interface.
packages/components/modules/messages/common/utils.ts (2)
31-60: LGTM! Utility functions properly extended.The renaming from
useRoomNameAndAvatartouseSingleChatDetailsbetter reflects the function's purpose. All branches (deleted user, normal user) correctly handle the new fields.
62-67: LGTM! Function rename improves clarity.The rename from
useNameAndAvatartouseChatroomDetailsis more descriptive and accurately reflects that it handles both single and group chats.packages/components/modules/messages/web/ProfileSummary/Body/index.tsx (1)
45-54: Incomplete feature: "Add contact to a group" has no handler.The "Add contact to a group" button lacks an
onClickhandler. Is this feature planned for a future PR?If this is incomplete, consider either:
- Removing it until the feature is ready
- Adding
disabled={true}to indicate it's not yet functional- Implementing the handler in this PR
Also, fix the aria-label mismatch:
<IconButton size="small" - aria-label="edit group chat" + aria-label="add contact to a group" sx={{ maxWidth: 'fit-content', gap: '8px' }} >packages/components/modules/messages/web/ProfileSummary/Header/index.tsx (1)
11-28: LGTM! Clean header implementation.The Header component is well-structured with appropriate defaults and clear accessibility labels.
packages/components/modules/messages/web/ProfileSummary/index.tsx (1)
13-14: Verify: Handling of null singleChatProfileDetails.When
singleChatProfileDetailsis null or undefined, the destructuring will still work (all values become undefined), and the Body component will render with empty/undefined values. This may be acceptable, but verify this matches the intended UX when profile data is unavailable.Consider whether a fallback UI (error state or empty state message) would be better than rendering the profile drawer with all empty fields.
packages/components/modules/messages/web/ChatRoom/ChatRoomHeader/ChatRoomOptions/index.tsx (1)
20-22: Verify: Should "Contact Details" be restricted to single chats?The concern is valid based on code evidence. "Group Details" and "Leave Group" are already gated with
{isGroup ? ... : null}, but "Contact Details" renders unconditionally. Given the PR objectives specify "one-on-one chats" and ProfileSummaryComponent has no group-aware logic, consider adding the same conditional gate:{!isGroup ? <MenuItem onClick={onContactDetailsClicked}>...</MenuItem> : null}.packages/components/modules/messages/common/context/ChatRoomProvider/index.tsx (1)
20-21: LGTM! Clean Zustand setter implementations.Both setters follow standard Zustand patterns. The spread operator on line 21 correctly ensures a new object reference for proper reactivity.
packages/components/modules/messages/web/ChatRoom/ChatRoomHeader/index.tsx (1)
166-169: LGTM! Contact details handler correctly wired.The handler properly closes the popover and navigates to the profile summary view using the shared state management.
packages/components/modules/messages/web/ChatRoomsComponent/index.tsx (1)
35-35: LGTM! State migration and ProfileSummary integration look correct.The migration from local
useStateto the global Zustand store is implemented correctly:
- Proper destructuring of
leftPanelContentandsetLeftPanelContentfromuseChatRoom()- New
ProfileSummaryComponentprop follows the existing pattern for other panel components- The profileSummary case correctly wires the back button to return to the chat room list
Also applies to: 42-42, 102-107
| singleChatProfileDetails?: { | ||
| pk: number | undefined | ||
| name: string | null | undefined | ||
| username: string | null | undefined | ||
| imageUrl: string | null | undefined | ||
| biography?: string | null | undefined | ||
| } |
There was a problem hiding this comment.
Fix type inconsistency between state and setter.
There's a mismatch between the state type and the setter parameter:
- State allows
name: string | null | undefined(lines 7-9) - Setter parameter only allows
name: string | undefined(line 23)
This inconsistency can cause issues:
- Type errors at call sites when passing potentially-null values
- Confusion about whether null is a valid value
- The state could still receive null via
setChatRoom, bypassing the setter's type restriction
Apply this diff to align the types:
setSingleChatProfileDetails: (details: {
pk: number | undefined
- name: string | undefined
- username: string | undefined
- imageUrl: string | undefined
+ name: string | null | undefined
+ username: string | null | undefined
+ imageUrl: string | null | undefined
biography?: string | undefined
}) => voidOr, if nulls should never be allowed, update the state type to remove | null.
Also applies to: 21-27
🤖 Prompt for AI Agents
In packages/components/modules/messages/common/context/ChatRoomProvider/types.ts
around lines 5-11 (and similarly for lines 21-27), the state type for
singleChatProfileDetails permits null for fields like
name/username/imageUrl/biography but the setter parameter does not, causing a
type mismatch; resolve by making the setter parameter types mirror the state
(add | null to name, username, imageUrl, biography and any other nullable
fields) OR remove | null from the state type if null should never be
allowed—ensure both the state and setter signatures use the exact same nullable
union types.
| useEffect(() => { | ||
| if (!isGroup) { | ||
| setSingleChatProfileDetails({ | ||
| pk: pk ?? undefined, | ||
| name: title ?? '', | ||
| username: path ?? '', | ||
| imageUrl: avatar ?? '', | ||
| biography: biography ?? '', | ||
| }) | ||
| } | ||
| }, [pk, title, path, avatar, biography]) |
There was a problem hiding this comment.
Fix missing dependencies in useEffect.
The effect is missing isGroup and setSingleChatProfileDetails in its dependency array. This violates the exhaustive-deps rule and can cause:
- Stale closures if
setSingleChatProfileDetailschanges - The effect not re-running when
isGrouptoggles - Unpredictable behavior when room type changes
Apply this diff to fix the dependencies:
useEffect(() => {
if (!isGroup) {
setSingleChatProfileDetails({
pk: pk ?? undefined,
name: title ?? '',
username: path ?? '',
imageUrl: avatar ?? '',
biography: biography ?? '',
})
}
- }, [pk, title, path, avatar, biography])
+ }, [pk, title, path, avatar, biography, isGroup, setSingleChatProfileDetails])📝 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 (!isGroup) { | |
| setSingleChatProfileDetails({ | |
| pk: pk ?? undefined, | |
| name: title ?? '', | |
| username: path ?? '', | |
| imageUrl: avatar ?? '', | |
| biography: biography ?? '', | |
| }) | |
| } | |
| }, [pk, title, path, avatar, biography]) | |
| useEffect(() => { | |
| if (!isGroup) { | |
| setSingleChatProfileDetails({ | |
| pk: pk ?? undefined, | |
| name: title ?? '', | |
| username: path ?? '', | |
| imageUrl: avatar ?? '', | |
| biography: biography ?? '', | |
| }) | |
| } | |
| }, [pk, title, path, avatar, biography, isGroup, setSingleChatProfileDetails]) |
🤖 Prompt for AI Agents
In packages/components/modules/messages/web/ChatRoom/ChatRoomHeader/index.tsx
around lines 51 to 61, the useEffect dependency array is incomplete: add isGroup
and setSingleChatProfileDetails to the dependency list so the effect re-runs
when room type or the setter changes and to satisfy exhaustive-deps; update the
dependency array accordingly to include [pk, title, path, avatar, biography,
isGroup, setSingleChatProfileDetails].
| const formattedUsername = username ? username.replace(/^\/+/, '') : '' | ||
| const profilePath = username ?? `/profile/${pk}` |
There was a problem hiding this comment.
Fix: Handle undefined pk in profilePath fallback.
If both username and pk are undefined/null, profilePath will be /profile/undefined, causing a broken link.
- const formattedUsername = username ? username.replace(/^\/+/, '') : ''
- const profilePath = username ?? `/profile/${pk}`
+ const formattedUsername = username ? username.replace(/^\/+/, '') : ''
+ const profilePath = username ?? (pk ? `/profile/${pk}` : undefined)Then add a guard to disable the "Go to profile" button when profilePath is undefined:
<IconButton
size="small"
aria-label="go to profile"
onClick={() => window.open(profilePath, '_blank')}
sx={{ maxWidth: 'fit-content', gap: '8px' }}
+ disabled={!profilePath}
>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/components/modules/messages/web/ProfileSummary/Body/index.tsx around
lines 17 to 18, the fallback builds `/profile/${pk}` even when pk is undefined
which yields `/profile/undefined`; change the logic so profilePath is undefined
if neither username nor pk exist (e.g., set profilePath to the cleaned username
if present, otherwise to `/profile/${pk}` only when pk is defined, else
undefined). After that, guard the "Go to profile" button by disabling or hiding
it when profilePath is undefined so it cannot render a broken link.
| <TypographyWithEllipsis variant="body2" color="text.secondary"> | ||
| {`@${formattedUsername}`} | ||
| </TypographyWithEllipsis> |
There was a problem hiding this comment.
Minor: Display empty state when username is missing.
When username is empty or undefined, the component displays @ which looks incomplete. Consider showing a placeholder or omitting the username row entirely.
- <TypographyWithEllipsis variant="body2" color="text.secondary">
- {`@${formattedUsername}`}
- </TypographyWithEllipsis>
+ {formattedUsername && (
+ <TypographyWithEllipsis variant="body2" color="text.secondary">
+ {`@${formattedUsername}`}
+ </TypographyWithEllipsis>
+ )}📝 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.
| <TypographyWithEllipsis variant="body2" color="text.secondary"> | |
| {`@${formattedUsername}`} | |
| </TypographyWithEllipsis> | |
| {formattedUsername && ( | |
| <TypographyWithEllipsis variant="body2" color="text.secondary"> | |
| {`@${formattedUsername}`} | |
| </TypographyWithEllipsis> | |
| )} |
🤖 Prompt for AI Agents
In packages/components/modules/messages/web/ProfileSummary/Body/index.tsx around
lines 27 to 29, the code renders `@${formattedUsername}` unconditionally which
yields a lone "@" when username is missing; update the render to handle
empty/undefined usernames by either omitting the entire TypographyWithEllipsis
row when formattedUsername is falsy, or rendering a clear placeholder (e.g.,
"Username unavailable" or "—") instead of `@`. Implement the check around the
JSX so the UI shows a sensible empty state rather than just "@".
| const ProfileNoCircleIcon: FC<SvgIconProps> = ({ sx, ...props }) => ( | ||
| <SvgIcon sx={{ fontSize: 24, color: 'grey.800', ...sx }} {...props}> | ||
| <svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||
| <path | ||
| d="M12 11.1998C13.1 11.1998 14.2 10.7998 15 9.9998C16.7 8.2998 16.7 5.5998 15 3.9998C14.2 3.1998 13.1 2.7998 12 2.7998C10.9 2.7998 9.79995 3.1998 8.99995 3.9998C7.29995 5.6998 7.29995 8.3998 8.99995 9.9998C9.79995 10.7998 10.9 11.1998 12 11.1998ZM10.1 5.0998C10.6 4.5998 11.3 4.2998 12 4.2998C12.7 4.2998 13.4 4.5998 13.9 5.0998C15 6.1998 15 7.8998 13.9 8.9998C12.9 9.9998 11.1 9.9998 9.99995 8.9998C8.99995 7.8998 8.99995 6.0998 10.1 5.0998Z" | ||
| fill="#637381" | ||
| /> | ||
| <path | ||
| d="M12 12.7998C7.69995 12.7998 3.19995 14.8998 3.19995 18.4998V19.4998C3.19995 20.4998 3.99995 21.2998 4.99995 21.2998H19C20 21.2998 20.7999 20.4998 20.7999 19.4998V18.4998C20.7999 14.8998 16.3 12.7998 12 12.7998ZM19.2 19.4998C19.2 19.5998 19.1 19.6998 19 19.6998H4.99995C4.89995 19.6998 4.79995 19.5998 4.79995 19.4998V18.4998C4.79995 15.8998 8.59995 14.2998 12 14.2998C15.5 14.2998 19.2 15.8998 19.2 18.4998V19.4998Z" | ||
| fill="#637381" | ||
| /> | ||
| </svg> | ||
| </SvgIcon> | ||
| ) |
There was a problem hiding this comment.
Remove nested SVG and hard-coded fill colors.
The component has two issues:
- Nested SVG inside SvgIcon: MUI's
SvgIconalready renders an<svg>element. Nesting another<svg>inside creates redundant elements and prevents proper rendering. - Hard-coded fill colors: The
fill="#637381"in the paths prevents the icon from inheriting colors via thecolorprop or parent theme, limiting reusability.
Apply this diff to fix both issues:
const ProfileNoCircleIcon: FC<SvgIconProps> = ({ sx, ...props }) => (
<SvgIcon sx={{ fontSize: 24, color: 'grey.800', ...sx }} {...props}>
- <svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">
- <path
- d="M12 11.1998C13.1 11.1998 14.2 10.7998 15 9.9998C16.7 8.2998 16.7 5.5998 15 3.9998C14.2 3.1998 13.1 2.7998 12 2.7998C10.9 2.7998 9.79995 3.1998 8.99995 3.9998C7.29995 5.6998 7.29995 8.3998 8.99995 9.9998C9.79995 10.7998 10.9 11.1998 12 11.1998ZM10.1 5.0998C10.6 4.5998 11.3 4.2998 12 4.2998C12.7 4.2998 13.4 4.5998 13.9 5.0998C15 6.1998 15 7.8998 13.9 8.9998C12.9 9.9998 11.1 9.9998 9.99995 8.9998C8.99995 7.8998 8.99995 6.0998 10.1 5.0998Z"
- fill="#637381"
- />
- <path
- d="M12 12.7998C7.69995 12.7998 3.19995 14.8998 3.19995 18.4998V19.4998C3.19995 20.4998 3.99995 21.2998 4.99995 21.2998H19C20 21.2998 20.7999 20.4998 20.7999 19.4998V18.4998C20.7999 14.8998 16.3 12.7998 12 12.7998ZM19.2 19.4998C19.2 19.5998 19.1 19.6998 19 19.6998H4.99995C4.89995 19.6998 4.79995 19.5998 4.79995 19.4998V18.4998C4.79995 15.8998 8.59995 14.2998 12 14.2998C15.5 14.2998 19.2 15.8998 19.2 18.4998V19.4998Z"
- fill="#637381"
- />
- </svg>
+ <path
+ d="M12 11.1998C13.1 11.1998 14.2 10.7998 15 9.9998C16.7 8.2998 16.7 5.5998 15 3.9998C14.2 3.1998 13.1 2.7998 12 2.7998C10.9 2.7998 9.79995 3.1998 8.99995 3.9998C7.29995 5.6998 7.29995 8.3998 8.99995 9.9998C9.79995 10.7998 10.9 11.1998 12 11.1998ZM10.1 5.0998C10.6 4.5998 11.3 4.2998 12 4.2998C12.7 4.2998 13.4 4.5998 13.9 5.0998C15 6.1998 15 7.8998 13.9 8.9998C12.9 9.9998 11.1 9.9998 9.99995 8.9998C8.99995 7.8998 8.99995 6.0998 10.1 5.0998Z"
+ />
+ <path
+ d="M12 12.7998C7.69995 12.7998 3.19995 14.8998 3.19995 18.4998V19.4998C3.19995 20.4998 3.99995 21.2998 4.99995 21.2998H19C20 21.2998 20.7999 20.4998 20.7999 19.4998V18.4998C20.7999 14.8998 16.3 12.7998 12 12.7998ZM19.2 19.4998C19.2 19.5998 19.1 19.6998 19 19.6998H4.99995C4.89995 19.6998 4.79995 19.5998 4.79995 19.4998V18.4998C4.79995 15.8998 8.59995 14.2998 12 14.2998C15.5 14.2998 19.2 15.8998 19.2 18.4998V19.4998Z"
+ />
</SvgIcon>
)With this change, the icon will:
- Render correctly without nested SVG elements
- Inherit the color from the
colorprop or defaultgrey.800from thesxprop - Work seamlessly with MUI's theming system
📝 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 ProfileNoCircleIcon: FC<SvgIconProps> = ({ sx, ...props }) => ( | |
| <SvgIcon sx={{ fontSize: 24, color: 'grey.800', ...sx }} {...props}> | |
| <svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> | |
| <path | |
| d="M12 11.1998C13.1 11.1998 14.2 10.7998 15 9.9998C16.7 8.2998 16.7 5.5998 15 3.9998C14.2 3.1998 13.1 2.7998 12 2.7998C10.9 2.7998 9.79995 3.1998 8.99995 3.9998C7.29995 5.6998 7.29995 8.3998 8.99995 9.9998C9.79995 10.7998 10.9 11.1998 12 11.1998ZM10.1 5.0998C10.6 4.5998 11.3 4.2998 12 4.2998C12.7 4.2998 13.4 4.5998 13.9 5.0998C15 6.1998 15 7.8998 13.9 8.9998C12.9 9.9998 11.1 9.9998 9.99995 8.9998C8.99995 7.8998 8.99995 6.0998 10.1 5.0998Z" | |
| fill="#637381" | |
| /> | |
| <path | |
| d="M12 12.7998C7.69995 12.7998 3.19995 14.8998 3.19995 18.4998V19.4998C3.19995 20.4998 3.99995 21.2998 4.99995 21.2998H19C20 21.2998 20.7999 20.4998 20.7999 19.4998V18.4998C20.7999 14.8998 16.3 12.7998 12 12.7998ZM19.2 19.4998C19.2 19.5998 19.1 19.6998 19 19.6998H4.99995C4.89995 19.6998 4.79995 19.5998 4.79995 19.4998V18.4998C4.79995 15.8998 8.59995 14.2998 12 14.2998C15.5 14.2998 19.2 15.8998 19.2 18.4998V19.4998Z" | |
| fill="#637381" | |
| /> | |
| </svg> | |
| </SvgIcon> | |
| ) | |
| const ProfileNoCircleIcon: FC<SvgIconProps> = ({ sx, ...props }) => ( | |
| <SvgIcon sx={{ fontSize: 24, color: 'grey.800', ...sx }} {...props}> | |
| <path | |
| d="M12 11.1998C13.1 11.1998 14.2 10.7998 15 9.9998C16.7 8.2998 16.7 5.5998 15 3.9998C14.2 3.1998 13.1 2.7998 12 2.7998C10.9 2.7998 9.79995 3.1998 8.99995 3.9998C7.29995 5.6998 7.29995 8.3998 8.99995 9.9998C9.79995 10.7998 10.9 11.1998 12 11.1998ZM10.1 5.0998C10.6 4.5998 11.3 4.2998 12 4.2998C12.7 4.2998 13.4 4.5998 13.9 5.0998C15 6.1998 15 7.8998 13.9 8.9998C12.9 9.9998 11.1 9.9998 9.99995 8.9998C8.99995 7.8998 8.99995 6.0998 10.1 5.0998Z" | |
| /> | |
| <path | |
| d="M12 12.7998C7.69995 12.7998 3.19995 14.8998 3.19995 18.4998V19.4998C3.19995 20.4998 3.99995 21.2998 4.99995 21.2998H19C20 21.2998 20.7999 20.4998 20.7999 19.4998V18.4998C20.7999 14.8998 16.3 12.7998 12 12.7998ZM19.2 19.4998C19.2 19.5998 19.1 19.6998 19 19.6998H4.99995C4.89995 19.6998 4.79995 19.5998 4.79995 19.4998V18.4998C4.79995 15.8998 8.59995 14.2998 12 14.2998C15.5 14.2998 19.2 15.8998 19.2 18.4998V19.4998Z" | |
| /> | |
| </SvgIcon> | |
| ) |
🤖 Prompt for AI Agents
In packages/design-system/components/web/icons/ProfileNoCircleIcon/index.tsx
around lines 5-18, remove the nested <svg> so the paths are direct children of
MUI's SvgIcon, move the svg attributes (viewBox="0 0 24 24") to the SvgIcon,
remove width/height and xmlns, and replace the hard-coded fill="#637381" on both
<path> elements with fill="currentColor" (or remove the fill so the icon
inherits color), ensuring the SvgIcon keeps sx={{ fontSize: 24, color:
'grey.800', ...sx }} and spreads {...props} so the icon inherits theme colors
and props correctly.
|
| editGroupChat: 3, | ||
| groupDetails: 4, | ||
| profileSummary: 5, | ||
| } as const |
There was a problem hiding this comment.
lets move this closer to the useChatRoom/ChatRoomProvider context, since we moved this logic there
| replace?: boolean | undefined, | ||
| ) => void | ||
| resetChatRoom: () => void | ||
| setLeftPanelContent: (content: number) => void |
There was a problem hiding this comment.
lets use:
export type LeftPanelContentValues = ValueOf<typeof LEFT_PANEL_CONTENT> (move this type over to here)
instead of number
|
|
||
| useEffect(() => { | ||
| if (!isGroup) { | ||
| setSingleChatProfileDetails({ |
There was a problem hiding this comment.
I think that the best approach here would be the ProfileSummary itself calling its own Fragment to get the data it needs and then register this fragment under GroupDetailsQuery defined at ChatRoomsComponent.
By doing that we dont need to store anything in this state provider and we keep in sync with the relay philosophy in which we specify the data requirements of a component by using fragments
| <Suspense | ||
| fallback={ | ||
| <> | ||
| <Header onBackButtonClicked={onBackButtonClicked} /> |
| <svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||
| <path | ||
| d="M12 11.1998C13.1 11.1998 14.2 10.7998 15 9.9998C16.7 8.2998 16.7 5.5998 15 3.9998C14.2 3.1998 13.1 2.7998 12 2.7998C10.9 2.7998 9.79995 3.1998 8.99995 3.9998C7.29995 5.6998 7.29995 8.3998 8.99995 9.9998C9.79995 10.7998 10.9 11.1998 12 11.1998ZM10.1 5.0998C10.6 4.5998 11.3 4.2998 12 4.2998C12.7 4.2998 13.4 4.5998 13.9 5.0998C15 6.1998 15 7.8998 13.9 8.9998C12.9 9.9998 11.1 9.9998 9.99995 8.9998C8.99995 7.8998 8.99995 6.0998 10.1 5.0998Z" | ||
| fill="#637381" |
There was a problem hiding this comment.
we usually change the fill to currentColor to allow it to be reused with different colors (only works with icons with only one color)
https://github.com/silverlogic/baseapp-frontend/blob/master/packages/design-system/components/web/icons/README.md
| ProfileSummaryComponent = DefaultProfileSummary, | ||
| }) => { | ||
| const isUpToMd = useResponsive('up', 'md') | ||
| const [leftPanelContent, setLeftPanelContent] = useState<LeftPanelContentValues>( |
Acceptance Criteria
As a user in a one-on-one chat, I want to open a profile summary drawer when I click the other participant’s name or avatar, so that I can quickly view their details without leaving the chat.
Acceptance Criteria
Clicking the participant’s name or avatar should trigger the opening of a profile summary drawer component from the side of the screen.
Given I open the hamburger menu on the top right, I should see a contact details option. When I click it, it should open the profile summary drawer as well.
The profile summary drawer must retrieve and display:
Display name
Avatar
username
Bio
The drawer component should be able to be closed
The drawer should be displayed while keeping the conversation visible and usable.
Data for the profile summary must be fetched from the backend using the correct profile ID associated with the current conversation.
The profile summary drawer should include a clearly visible “Go to Profile” action that navigates to the participant’s full profile page.
It should open a new tab
The navigation to the profile page should open the correct identity for the participant
Implementation should be responsive for Desktop, tablet and mobile view
Guardrails
No animation is needed for the drawer
Design Link: https://www.figma.com/design/XRD6wSl1m8Kz6XUcAy5CLp/BaseApp---WEB?node-id=8109-145277&t=U2v8Soqr9Bh1UMZw-0
Approvd
https://app.approvd.io//projects/BA/stories/43040 2
Summary by CodeRabbit