Skip to content
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.

Sign up for GitHub

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

BA-2060: leave group chat #176

Merged
merged 2 commits into from
Jan 27, 2025
+317 −126
Merged

BA-2060: leave group chat #176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16ea36d
feat: leave group
pt-tsl Jan 9, 2025
853a950
fix: PR fixes
pt-tsl Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions packages/components/modules/messages/ChatRoom/ChatRoomHeader/ChatRoomOptions/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { FC } from 'react'

import { MenuItem, MenuList, Typography } from '@mui/material'

import { ChatRoomOptionsProps } from './types'

const ChatRoomOptions: FC<ChatRoomOptionsProps> = ({
onArchiveClicked,
onDetailsClicked,
onLeaveClicked,
}) => (
<MenuList>
{/* TODO: Implement archive room functionality */}
<MenuItem onClick={onArchiveClicked}>
<Typography variant="body2">Archive Chat</Typography>
</MenuItem>
<MenuItem onClick={onDetailsClicked}>
<Typography variant="body2">Group Details</Typography>
</MenuItem>
<MenuItem onClick={onLeaveClicked}>
<Typography variant="body2" color="error">
Leave Group
</Typography>
</MenuItem>
</MenuList>
)

export default ChatRoomOptions
5 changes: 5 additions & 0 deletions packages/components/modules/messages/ChatRoom/ChatRoomHeader/ChatRoomOptions/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export interface ChatRoomOptionsProps {
onArchiveClicked: () => void
onDetailsClicked: () => void
onLeaveClicked: () => void
}
Comment on lines +1 to +5
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add type safety for chat room types

The interface should include properties to distinguish between group and direct chats, as some options (like leaving) are only applicable to group chats. Also consider adding admin permission check.

 export interface ChatRoomOptionsProps {
+  isGroupChat: boolean
+  isAdmin?: boolean
   onArchiveClicked: () => void
   onDetailsClicked: () => void
   onLeaveClicked: () => void
 }
📝 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.

Suggested change
export interface ChatRoomOptionsProps {
onArchiveClicked: () => void
onDetailsClicked: () => void
onLeaveClicked: () => void
}
export interface ChatRoomOptionsProps {
isGroupChat: boolean
isAdmin?: boolean
onArchiveClicked: () => void
onDetailsClicked: () => void
onLeaveClicked: () => void
}

117 changes: 80 additions & 37 deletions packages/components/modules/messages/ChatRoom/ChatRoomHeader/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,72 +1,115 @@
import { FC } from 'react'
import { FC, useState } from 'react'

import { useCurrentProfile } from '@baseapp-frontend/authentication'
import {
AvatarWithPlaceholder,
IconButton,
Iconify,
Popover,
ThreeDotsIcon,
usePopover,
useResponsive,
} from '@baseapp-frontend/design-system'

import { Box, Typography } from '@mui/material'
import { useFragment } from 'react-relay'

import LeaveGroupDialog from '../../__shared__/LeaveGroupDialog'
import { useChatRoom } from '../../context'
import { TitleFragment } from '../../graphql/fragments/Title'
import { getParticipantCountString, useNameAndAvatar } from '../../utils'
import ChatRoomOptions from './ChatRoomOptions'
import { BackButtonContainer, ChatHeaderContainer, ChatTitleContainer } from './styled'
import { ChatRoomHeaderProps } from './types'

const ChatRoomHeader: FC<ChatRoomHeaderProps> = ({
participantsCount,
roomTitleRef,
onDisplayGroupDetailsClicked,
roomId,
}) => {
const roomHeader = useFragment(TitleFragment, roomTitleRef)
const [open, setOpen] = useState(false)

const isUpToMd = useResponsive('up', 'md')
const { resetChatRoom } = useChatRoom()

const { title, avatar } = useNameAndAvatar(roomHeader)
const members = getParticipantCountString(participantsCount)
const popover = usePopover()
const { currentProfile } = useCurrentProfile()

const onChatRoomOptionsClicked = (e: React.MouseEvent<HTMLButtonElement>) => {
e.stopPropagation()
popover.onOpen(e)
}

return (
<ChatHeaderContainer>
{isUpToMd ? (
<div />
) : (
<BackButtonContainer>
<IconButton
aria-label="return to chat room list"
onClick={resetChatRoom}
sx={{ maxWidth: 'fit-content' }}
>
<Iconify icon="eva:arrow-ios-back-fill" width={24} />
</IconButton>
</BackButtonContainer>
)}
<ChatTitleContainer
onClick={roomHeader.isGroup ? onDisplayGroupDetailsClicked : undefined}
isClickable={roomHeader.isGroup}
>
<AvatarWithPlaceholder
className="self-start justify-self-center"
width={32}
height={32}
src={avatar}
sx={{ border: 'none', alignSelf: 'center' }}
/>
<Box>
<Typography component="span" variant="subtitle2" sx={{ float: 'left', clear: 'left' }}>
{title}
</Typography>
{roomHeader.isGroup && (
<Typography component="span" variant="caption" sx={{ float: 'left', clear: 'left' }}>
{members}
<>
<LeaveGroupDialog
open={open}
onClose={() => setOpen(false)}
profileId={currentProfile?.id}
roomId={roomId}
/>
<ChatHeaderContainer>
{isUpToMd ? (
<div />
) : (
<BackButtonContainer>
<IconButton
aria-label="return to chat room list"
onClick={resetChatRoom}
sx={{ maxWidth: 'fit-content' }}
>
<Iconify icon="eva:arrow-ios-back-fill" width={24} />
</IconButton>
</BackButtonContainer>
)}
<ChatTitleContainer
onClick={roomHeader.isGroup ? onDisplayGroupDetailsClicked : undefined}
isClickable={roomHeader.isGroup}
>
<AvatarWithPlaceholder
className="self-start justify-self-center"
width={32}
height={32}
src={avatar}
sx={{ border: 'none', alignSelf: 'center' }}
/>
<Box>
<Typography component="span" variant="subtitle2" sx={{ float: 'left', clear: 'left' }}>
{title}
</Typography>
)}
</Box>
</ChatTitleContainer>
</ChatHeaderContainer>
{roomHeader.isGroup && (
<Typography component="span" variant="caption" sx={{ float: 'left', clear: 'left' }}>
{members}
</Typography>
)}
</Box>
<Box>
<IconButton onClick={onChatRoomOptionsClicked} aria-label="Show chatroom options">
<ThreeDotsIcon sx={{ fontSize: '24px' }} />
</IconButton>
<Popover
open={popover.open}
onClose={popover.onClose}
slotProps={{
root: { onClick: (e: React.MouseEvent<HTMLDivElement>) => e.stopPropagation() },
}}
>
<ChatRoomOptions
onArchiveClicked={() => {}}
onDetailsClicked={() =>
roomHeader.isGroup ? onDisplayGroupDetailsClicked() : undefined
}
onLeaveClicked={() => setOpen(true)}
/>
</Popover>
</Box>
</ChatTitleContainer>
</ChatHeaderContainer>
</>
)
}

Expand Down
2 changes: 1 addition & 1 deletion packages/components/modules/messages/ChatRoom/ChatRoomHeader/styled.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const ChatTitleContainer = styled(Box)<ChatTitleContainerProps>(
alignItems: 'center',
display: 'grid',
gap: theme.spacing(1.5),
gridTemplateColumns: 'min-content 1fr',
gridTemplateColumns: 'min-content 1fr min-content',
height: '56px',
padding: 0,
width: '100%',
Expand Down
3 changes: 2 additions & 1 deletion packages/components/modules/messages/ChatRoom/ChatRoomHeader/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import { TitleFragment$key } from '../../../../__generated__/TitleFragment.graph
export interface ChatRoomHeaderProps {
participantsCount: number
roomTitleRef: TitleFragment$key
onDisplayGroupDetailsClicked: () => void
onDisplayGroupDetailsClicked: VoidFunction
roomId?: string
Comment on lines +8 to +9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Make roomId required for leave group functionality.

Since the room ID is essential for the leave group feature, this property should be required rather than optional. This ensures type safety and prevents potential runtime errors when implementing the leave group functionality.

-  roomId?: string
+  roomId: string
📝 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.

Suggested change
onDisplayGroupDetailsClicked: VoidFunction
roomId?: string
onDisplayGroupDetailsClicked: VoidFunction
roomId: string

}

export interface ChatTitleContainerProps extends BoxProps {
Expand Down
1 change: 1 addition & 0 deletions packages/components/modules/messages/ChatRoom/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const ChatRoom: FC<ChatRoomProps> = ({
participantsCount={chatRoom.participantsCount}
roomTitleRef={chatRoom}
onDisplayGroupDetailsClicked={onDisplayGroupDetailsClicked}
roomId={roomId}
/>
<ChatBodyContainer>
<MessagesList roomRef={chatRoom} {...MessagesListProps} />
Expand Down
4 changes: 2 additions & 2 deletions packages/components/modules/messages/GroupDetails/GroupDetailsHeader/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export interface GroupDetailsHeaderProps {
onBackButtonClicked: () => void
onEditButtonClicked?: () => void
onBackButtonClicked: VoidFunction
onEditButtonClicked?: VoidFunction
shouldDisplayEditButton: boolean
}
12 changes: 7 additions & 5 deletions packages/components/modules/messages/GroupDetails/ProfileCard/AdminOptionsMenu/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,18 @@ import { MenuItem, MenuList, Typography } from '@mui/material'
import { AdminOptionsProps } from './types'

const AdminOptionsMenu: FC<AdminOptionsProps> = ({
isAdmin,
isMe,
onViewProfileClicked,
onToggleAdminClicked,
onRemoveClicked,
}) => (
<MenuList>
<MenuItem onClick={onToggleAdminClicked} disabled={isMe}>
<Typography variant="body2">{isAdmin ? 'Make normal user' : 'Make Admin'}</Typography>
<MenuItem onClick={onViewProfileClicked}>
<Typography variant="body2">View Profile</Typography>
</MenuItem>
<MenuItem onClick={onRemoveClicked} disabled={isMe}>
<MenuItem onClick={onToggleAdminClicked}>
<Typography variant="body2">Make Admin</Typography>
</MenuItem>
Comment on lines +16 to +18
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Restore permission checks for admin actions.

The removal of isAdmin checks could lead to permission issues. The "Make Admin" option should only be available to admins.

-    <MenuItem onClick={onToggleAdminClicked}>
+    {isAdmin && (
+      <MenuItem onClick={onToggleAdminClicked}>
         <Typography variant="body2">Make Admin</Typography>
-    </MenuItem>
+      </MenuItem>
+    )}

Committable suggestion skipped: line range outside the PR's diff.

<MenuItem onClick={onRemoveClicked}>
<Typography variant="body2" color="error">
Remove
</Typography>
Expand Down
7 changes: 3 additions & 4 deletions packages/components/modules/messages/GroupDetails/ProfileCard/AdminOptionsMenu/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
export interface AdminOptionsProps {
isAdmin: boolean
isMe: boolean
onToggleAdminClicked: () => void
onRemoveClicked: () => void
onViewProfileClicked: VoidFunction
onToggleAdminClicked: VoidFunction
onRemoveClicked: VoidFunction
Comment on lines 1 to +4
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Reconsider removal of permission flags

  1. The removal of isAdmin and isMe flags could lead to incorrect access control. These flags are crucial for conditional rendering and permission checks.
  2. The onRemoveClicked function name is ambiguous - it could mean either removing a member or leaving the group.
 export interface AdminOptionsProps {
+  isAdmin: boolean
+  isMe: boolean
   onViewProfileClicked: VoidFunction
   onToggleAdminClicked: VoidFunction
-  onRemoveClicked: VoidFunction
+  onRemoveMemberClicked: VoidFunction
 }
📝 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.

Suggested change
export interface AdminOptionsProps {
isAdmin: boolean
isMe: boolean
onToggleAdminClicked: () => void
onRemoveClicked: () => void
onViewProfileClicked: VoidFunction
onToggleAdminClicked: VoidFunction
onRemoveClicked: VoidFunction
export interface AdminOptionsProps {
isAdmin: boolean
isMe: boolean
onViewProfileClicked: VoidFunction
onToggleAdminClicked: VoidFunction
onRemoveMemberClicked: VoidFunction
}

}
26 changes: 26 additions & 0 deletions packages/components/modules/messages/GroupDetails/ProfileCard/MemberOptionsMenu/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { FC } from 'react'

import { MenuItem, MenuList, Typography } from '@mui/material'

import { MemberOptionsMenuProps } from './types'

const MemberOptionsMenu: FC<MemberOptionsMenuProps> = ({
isMe,
onViewProfileClicked,
onRemoveClicked,
}) => (
<MenuList>
<MenuItem onClick={onViewProfileClicked}>
<Typography variant="body2">View Profile</Typography>
</MenuItem>
{isMe && (
<MenuItem onClick={onRemoveClicked}>
<Typography variant="body2" color="error">
Leave Group
</Typography>
</MenuItem>
)}
</MenuList>
)

export default MemberOptionsMenu
5 changes: 5 additions & 0 deletions packages/components/modules/messages/GroupDetails/ProfileCard/MemberOptionsMenu/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export interface MemberOptionsMenuProps {
isMe: boolean
onViewProfileClicked: VoidFunction
onRemoveClicked: VoidFunction
}
Comment on lines +1 to +5
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding admin permission check

The interface should include an isAdmin property to properly handle permission-based menu options, especially since the PR comments mention concerns about admin-only functionality in the backend.

 export interface MemberOptionsMenuProps {
   isMe: boolean
+  isAdmin: boolean
   onViewProfileClicked: VoidFunction
   onRemoveClicked: VoidFunction
 }
📝 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.

Suggested change
export interface MemberOptionsMenuProps {
isMe: boolean
onViewProfileClicked: VoidFunction
onRemoveClicked: VoidFunction
}
export interface MemberOptionsMenuProps {
isMe: boolean
isAdmin: boolean
onViewProfileClicked: VoidFunction
onRemoveClicked: VoidFunction
}

39 changes: 33 additions & 6 deletions packages/components/modules/messages/GroupDetails/ProfileCard/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { useFragment } from 'react-relay'
import { ProfileItemFragment$key } from '../../../../__generated__/ProfileItemFragment.graphql'
import { ProfileItemFragment } from '../../../profiles/graphql/queries/ProfileItem'
import AdminOptionsMenu from './AdminOptionsMenu'
import MemberOptionsMenu from './MemberOptionsMenu'
import { ADMIN_LABEL, CHAT_ROOM_PARTICIPANT_ROLES } from './constants'
import { MainContainer } from './styled'
import { ProfileCardProps } from './types'
Expand All @@ -34,12 +35,43 @@ const ProfileCard: FC<ProfileCardProps> = ({
const showMenu = hasAdminPermissions
const { currentProfile } = useCurrentProfile()
const popover = usePopover()
const isMe = currentProfile?.id === id

const handleRemoveClicked = () => {
popover.onClose()
initiateRemoval(id, name)
}

const renderMenuItems = () => {
if (isMe) {
return (
<MemberOptionsMenu
isMe={isMe}
onViewProfileClicked={popover.onClose /* TODO: Add functionality */}
onRemoveClicked={handleRemoveClicked}
/>
)
}

if (!isMe && hasAdminPermissions) {
return (
<AdminOptionsMenu
onViewProfileClicked={popover.onClose /* TODO: Add functionality */}
onToggleAdminClicked={popover.onClose /* TODO: Add functionality */}
onRemoveClicked={handleRemoveClicked}
/>
)
}

return (
<MemberOptionsMenu
isMe={isMe}
onViewProfileClicked={popover.onClose /* TODO: Add functionality */}
onRemoveClicked={handleRemoveClicked}
/>
)
}

return (
<MainContainer key={`profile-${id}`}>
<AvatarWithPlaceholder
Expand Down Expand Up @@ -83,12 +115,7 @@ const ProfileCard: FC<ProfileCardProps> = ({
<ThreeDotsIcon sx={{ fontSize: '24px' }} />
</IconButton>
<Popover open={popover.open} onClose={popover.onClose}>
<AdminOptionsMenu
isAdmin={showAdminLabel}
isMe={id === currentProfile?.id}
onToggleAdminClicked={popover.onClose /* TODO: Add functionality */}
onRemoveClicked={handleRemoveClicked}
/>
{renderMenuItems()}
</Popover>
</Box>
) : (
Expand Down
Loading
Loading