-
Notifications
You must be signed in to change notification settings - Fork 2
feature/BA-2725 profile summary drawer #301
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,14 @@ | ||
| export type ChatRoomState = { | ||
| id?: string | ||
| participants?: (string | null | undefined)[] | ||
| leftPanelContent?: number | ||
| singleChatProfileDetails?: { | ||
| pk: number | undefined | ||
| name: string | null | undefined | ||
| username: string | null | undefined | ||
| imageUrl: string | null | undefined | ||
| biography?: string | null | undefined | ||
| } | ||
| } | ||
|
|
||
| type ChatRoomFunctions = { | ||
|
|
@@ -9,6 +17,14 @@ type ChatRoomFunctions = { | |
| replace?: boolean | undefined, | ||
| ) => void | ||
| resetChatRoom: () => void | ||
| setLeftPanelContent: (content: number) => void | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets use: |
||
| setSingleChatProfileDetails: (details: { | ||
| pk: number | undefined | ||
| name: string | undefined | ||
| username: string | undefined | ||
| imageUrl: string | undefined | ||
| biography?: string | undefined | ||
| }) => void | ||
| } | ||
|
|
||
| export type UseChatRoom = ChatRoomState & ChatRoomFunctions | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| import { FC, useState } from 'react' | ||||||||||||||||||||||||||||||||||||||||||||||
| import { FC, useEffect, useState } from 'react' | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| import { useCurrentProfile } from '@baseapp-frontend/authentication' | ||||||||||||||||||||||||||||||||||||||||||||||
| import { AvatarWithPlaceholder } from '@baseapp-frontend/design-system/components/web/avatars' | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -20,10 +20,11 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||
| getParticipantCountString, | ||||||||||||||||||||||||||||||||||||||||||||||
| useArchiveChatRoomMutation, | ||||||||||||||||||||||||||||||||||||||||||||||
| useChatRoom, | ||||||||||||||||||||||||||||||||||||||||||||||
| useChatroomDetails, | ||||||||||||||||||||||||||||||||||||||||||||||
| useCheckIsAdmin, | ||||||||||||||||||||||||||||||||||||||||||||||
| useNameAndAvatar, | ||||||||||||||||||||||||||||||||||||||||||||||
| } from '../../../common' | ||||||||||||||||||||||||||||||||||||||||||||||
| import { RoomTitleFragment } from '../../../common/graphql/fragments/RoomTitle' | ||||||||||||||||||||||||||||||||||||||||||||||
| import { LEFT_PANEL_CONTENT } from '../../ChatRoomsComponent/constants' | ||||||||||||||||||||||||||||||||||||||||||||||
| import LeaveGroupDialog from '../../__shared__/LeaveGroupDialog' | ||||||||||||||||||||||||||||||||||||||||||||||
| import ChatRoomOptions from './ChatRoomOptions' | ||||||||||||||||||||||||||||||||||||||||||||||
| import { BackButtonContainer, ChatHeaderContainer, ChatTitleContainer } from './styled' | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -37,14 +38,28 @@ const ChatRoomHeader: FC<ChatRoomHeaderProps> = ({ | |||||||||||||||||||||||||||||||||||||||||||||
| roomId, | ||||||||||||||||||||||||||||||||||||||||||||||
| }) => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const roomHeader = useFragment(TitleFragment, roomTitleRef) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const [open, setOpen] = useState(false) | ||||||||||||||||||||||||||||||||||||||||||||||
| const { currentProfile } = useCurrentProfile() | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const isUpToMd = useResponsive('up', 'md') | ||||||||||||||||||||||||||||||||||||||||||||||
| const { resetChatRoom } = useChatRoom() | ||||||||||||||||||||||||||||||||||||||||||||||
| const { resetChatRoom, setSingleChatProfileDetails, setLeftPanelContent } = useChatRoom() | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const { isGroup } = roomHeader | ||||||||||||||||||||||||||||||||||||||||||||||
| const { title, avatar } = useNameAndAvatar(roomHeader) | ||||||||||||||||||||||||||||||||||||||||||||||
| const { title, avatar, path, pk, biography } = useChatroomDetails(roomHeader) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||
| if (!isGroup) { | ||||||||||||||||||||||||||||||||||||||||||||||
| setSingleChatProfileDetails({ | ||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that the best approach here would be the |
||||||||||||||||||||||||||||||||||||||||||||||
| pk: pk ?? undefined, | ||||||||||||||||||||||||||||||||||||||||||||||
| name: title ?? '', | ||||||||||||||||||||||||||||||||||||||||||||||
| username: path ?? '', | ||||||||||||||||||||||||||||||||||||||||||||||
| imageUrl: avatar ?? '', | ||||||||||||||||||||||||||||||||||||||||||||||
| biography: biography ?? '', | ||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| }, [pk, title, path, avatar, biography]) | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+51
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix missing dependencies in useEffect. The effect is missing
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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const { participants } = useFragment<RoomTitleFragment$key>(RoomTitleFragment, roomHeader) | ||||||||||||||||||||||||||||||||||||||||||||||
| const { isSoleAdmin } = useCheckIsAdmin(participants as MembersListFragment$data['participants']) | ||||||||||||||||||||||||||||||||||||||||||||||
| const members = getParticipantCountString(participantsCount) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -148,6 +163,10 @@ const ChatRoomHeader: FC<ChatRoomHeaderProps> = ({ | |||||||||||||||||||||||||||||||||||||||||||||
| popover.onClose() | ||||||||||||||||||||||||||||||||||||||||||||||
| setOpen(true) | ||||||||||||||||||||||||||||||||||||||||||||||
| }} | ||||||||||||||||||||||||||||||||||||||||||||||
| onContactDetailsClicked={() => { | ||||||||||||||||||||||||||||||||||||||||||||||
| popover.onClose() | ||||||||||||||||||||||||||||||||||||||||||||||
| setLeftPanelContent(LEFT_PANEL_CONTENT.profileSummary) | ||||||||||||||||||||||||||||||||||||||||||||||
| }} | ||||||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||||||
| </Popover> | ||||||||||||||||||||||||||||||||||||||||||||||
| </Box> | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,4 +4,5 @@ export const LEFT_PANEL_CONTENT = { | |
| createGroupChat: 2, | ||
| editGroupChat: 3, | ||
| groupDetails: 4, | ||
| profileSummary: 5, | ||
| } as const | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets move this closer to the |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| 'use client' | ||
|
|
||
| import { FC, useState } from 'react' | ||
| import { FC } from 'react' | ||
|
|
||
| import { useResponsive } from '@baseapp-frontend/design-system/hooks/web' | ||
|
|
||
|
|
@@ -13,10 +13,11 @@ import ChatRoom from '../ChatRoom' | |
| import DefaultGroupChatCreate from '../GroupChatCreate' | ||
| import DefaultGroupChatDetails from '../GroupChatDetails' | ||
| import DefaultGroupChatEdit from '../GroupChatEdit' | ||
| import DefaultProfileSummary from '../ProfileSummary' | ||
| import DefaultSingleChatCreate from '../SingleChatCreate' | ||
| import { LEFT_PANEL_CONTENT } from './constants' | ||
| import { ChatRoomContainer, ChatRoomsContainer, ChatRoomsListContainer } from './styled' | ||
| import { ChatRoomsComponentProps, LeftPanelContentValues } from './types' | ||
| import { ChatRoomsComponentProps } from './types' | ||
|
|
||
| const ChatRoomsComponent: FC<ChatRoomsComponentProps> = ({ | ||
| chatRoomsQueryData, | ||
|
|
@@ -31,15 +32,14 @@ const ChatRoomsComponent: FC<ChatRoomsComponentProps> = ({ | |
| GroupChatEditComponentProps = {}, | ||
| SingleChatCreateComponent = DefaultSingleChatCreate, | ||
| SingleChatCreateComponentProps = {}, | ||
| ProfileSummaryComponent = DefaultProfileSummary, | ||
| }) => { | ||
| const isUpToMd = useResponsive('up', 'md') | ||
| const [leftPanelContent, setLeftPanelContent] = useState<LeftPanelContentValues>( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice move! 🎉 |
||
| LEFT_PANEL_CONTENT.chatRoomList, | ||
| ) | ||
|
|
||
| const [groupDetailsQueryRef, loadGroupDetailsQuery] = | ||
| useQueryLoader<GroupDetailsQueryType>(GroupDetailsQuery) | ||
| const { id: selectedRoom } = useChatRoom() | ||
|
|
||
| const { id: selectedRoom, leftPanelContent, setLeftPanelContent } = useChatRoom() | ||
|
|
||
| const displayGroupDetails = () => { | ||
| if (selectedRoom) { | ||
|
|
@@ -99,6 +99,12 @@ const ChatRoomsComponent: FC<ChatRoomsComponentProps> = ({ | |
| {...SingleChatCreateComponentProps} | ||
| /> | ||
| ) | ||
| case LEFT_PANEL_CONTENT.profileSummary: | ||
| return ( | ||
| <ProfileSummaryComponent | ||
| onBackButtonClicked={() => setLeftPanelContent(LEFT_PANEL_CONTENT.chatRoomList)} | ||
| /> | ||
| ) | ||
| default: | ||
| return ( | ||
| <AllChatRoomsListComponent | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,84 @@ | ||||||||||||||||||
| import { FC } from 'react' | ||||||||||||||||||
|
|
||||||||||||||||||
| import { CircledAvatar } from '@baseapp-frontend/design-system/components/web/avatars' | ||||||||||||||||||
| import { IconButton } from '@baseapp-frontend/design-system/components/web/buttons' | ||||||||||||||||||
| import { | ||||||||||||||||||
| NewGroupIcon, | ||||||||||||||||||
| ProfileNoCircleIcon, | ||||||||||||||||||
| } from '@baseapp-frontend/design-system/components/web/icons' | ||||||||||||||||||
| import { TypographyWithEllipsis } from '@baseapp-frontend/design-system/components/web/typographies' | ||||||||||||||||||
|
|
||||||||||||||||||
| import { Box, Divider, Typography } from '@mui/material' | ||||||||||||||||||
|
|
||||||||||||||||||
| import { | ||||||||||||||||||
| ButtonContainer, | ||||||||||||||||||
| HeaderContainer, | ||||||||||||||||||
| Subheader, | ||||||||||||||||||
| SubheaderContainer, | ||||||||||||||||||
| TitleContainer, | ||||||||||||||||||
| } from './styled' | ||||||||||||||||||
| import { BodyProps } from './types' | ||||||||||||||||||
|
|
||||||||||||||||||
| const Body: FC<BodyProps> = ({ avatar, avatarSize = 144, biography, username, name, pk }) => { | ||||||||||||||||||
| const formattedUsername = username ? username.replace(/^\/+/, '') : '' | ||||||||||||||||||
| const profilePath = username ?? `/profile/${pk}` | ||||||||||||||||||
|
Comment on lines
+23
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix: Handle undefined pk in profilePath fallback. If both - 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 <IconButton
size="small"
aria-label="go to profile"
onClick={() => window.open(profilePath, '_blank')}
sx={{ maxWidth: 'fit-content', gap: '8px' }}
+ disabled={!profilePath}
>
🤖 Prompt for AI Agents |
||||||||||||||||||
| return ( | ||||||||||||||||||
| <Box sx={{ display: 'grid', gridTemplateRows: 'auto 1fr' }}> | ||||||||||||||||||
| <HeaderContainer> | ||||||||||||||||||
| <CircledAvatar src={avatar} width={avatarSize} height={avatarSize} hasError={false} /> | ||||||||||||||||||
| <TitleContainer> | ||||||||||||||||||
| <TypographyWithEllipsis variant="subtitle1" color="text.primary"> | ||||||||||||||||||
| {name} | ||||||||||||||||||
| </TypographyWithEllipsis> | ||||||||||||||||||
| <TypographyWithEllipsis variant="body2" color="text.secondary"> | ||||||||||||||||||
| {`@${formattedUsername}`} | ||||||||||||||||||
| </TypographyWithEllipsis> | ||||||||||||||||||
|
Comment on lines
+33
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: Display empty state when username is missing. When - <TypographyWithEllipsis variant="body2" color="text.secondary">
- {`@${formattedUsername}`}
- </TypographyWithEllipsis>
+ {formattedUsername && (
+ <TypographyWithEllipsis variant="body2" color="text.secondary">
+ {`@${formattedUsername}`}
+ </TypographyWithEllipsis>
+ )}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| </TitleContainer> | ||||||||||||||||||
| </HeaderContainer> | ||||||||||||||||||
| <SubheaderContainer> | ||||||||||||||||||
| <ButtonContainer> | ||||||||||||||||||
| <IconButton | ||||||||||||||||||
| size="small" | ||||||||||||||||||
| aria-label="go to profile" | ||||||||||||||||||
| onClick={() => window.open(profilePath, '_blank')} | ||||||||||||||||||
| sx={{ maxWidth: 'fit-content', gap: '8px' }} | ||||||||||||||||||
| > | ||||||||||||||||||
| <ProfileNoCircleIcon sx={{ fontSize: '18px' }} /> | ||||||||||||||||||
| <Typography variant="subtitle2" color="text.primary"> | ||||||||||||||||||
| Go to profile | ||||||||||||||||||
| </Typography> | ||||||||||||||||||
| </IconButton> | ||||||||||||||||||
|
|
||||||||||||||||||
| <IconButton | ||||||||||||||||||
| size="small" | ||||||||||||||||||
| aria-label="edit group chat" | ||||||||||||||||||
| sx={{ maxWidth: 'fit-content', gap: '8px' }} | ||||||||||||||||||
| > | ||||||||||||||||||
| <NewGroupIcon sx={{ fontSize: '18px', color: 'text.primary' }} /> | ||||||||||||||||||
| <Typography variant="subtitle2" color="text.primary"> | ||||||||||||||||||
| Add contact to a group | ||||||||||||||||||
| </Typography> | ||||||||||||||||||
| </IconButton> | ||||||||||||||||||
| </ButtonContainer> | ||||||||||||||||||
| <Subheader> | ||||||||||||||||||
| <Typography variant="subtitle2" color="text.primary"> | ||||||||||||||||||
| About | ||||||||||||||||||
| </Typography> | ||||||||||||||||||
| </Subheader> | ||||||||||||||||||
| <Divider /> | ||||||||||||||||||
| <Subheader> | ||||||||||||||||||
| <Typography variant="caption" color="text.secondary"> | ||||||||||||||||||
| {biography?.split('\n').map((line, index) => ( | ||||||||||||||||||
| <span key={index}> | ||||||||||||||||||
| {line} | ||||||||||||||||||
| {index < biography.split('\n').length - 1 && <br />} | ||||||||||||||||||
| </span> | ||||||||||||||||||
| ))} | ||||||||||||||||||
| </Typography> | ||||||||||||||||||
| </Subheader> | ||||||||||||||||||
| </SubheaderContainer> | ||||||||||||||||||
| </Box> | ||||||||||||||||||
| ) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| export default Body | ||||||||||||||||||
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.
Fix type inconsistency between state and setter.
There's a mismatch between the state type and the setter parameter:
name: string | null | undefined(lines 7-9)name: string | undefined(line 23)This inconsistency can cause issues:
setChatRoom, bypassing the setter's type restrictionApply 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