-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Share an email thread to workspace members chip and dropdown (#4199) #5640
base: main
Are you sure you want to change the base?
Changes from all commits
4f3345f
fa523a5
49ef2a7
408abf0
a890f5b
97dcab2
b6ff717
069ee17
74e5028
31c535d
8209fbb
3a409d1
cbabdfa
5e0e316
4074fed
0f08608
6ad1047
df594e5
0e94dc5
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 |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import { MessageThread } from '@/activities/emails/types/MessageThread'; | ||
import { SharedDropdownMenu } from '@/ui/layout/dropdown/components/SharedDropdownMenu'; | ||
|
||
export const EmailThreadMembersChip = ({ | ||
messageThread, | ||
}: { | ||
messageThread: MessageThread | null; | ||
}) => { | ||
const renderChip = () => { | ||
if (!messageThread) { | ||
return null; | ||
} | ||
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. Avoid using optional chaining ( 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. This is correct, we will be fixing this in our next commit. |
||
const isEveryone = messageThread?.everyone; | ||
const numberOfMessageThreadMembers = | ||
messageThread.messageThreadMember.length; | ||
switch (isEveryone) { | ||
case false: | ||
if (numberOfMessageThreadMembers === 1) { | ||
return ( | ||
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. The 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. Label already has a default value (empty string). |
||
<SharedDropdownMenu | ||
label="Private" | ||
messageThreadMembers={messageThread.messageThreadMember} | ||
/> | ||
); | ||
} else { | ||
return ( | ||
<SharedDropdownMenu | ||
messageThreadMembers={messageThread.messageThreadMember} | ||
/> | ||
); | ||
} | ||
case true: | ||
return ( | ||
<SharedDropdownMenu | ||
label="Everyone" | ||
messageThreadMembers={messageThread.messageThreadMember} | ||
everyone={true} | ||
/> | ||
); | ||
default: | ||
return null; | ||
} | ||
}; | ||
|
||
return <>{renderChip()} </>; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import { useEffect } from 'react'; | ||
import styled from '@emotion/styled'; | ||
import { useRecoilCallback } from 'recoil'; | ||
import { useRecoilCallback, useSetRecoilState } from 'recoil'; | ||
|
||
import { CustomResolverFetchMoreLoader } from '@/activities/components/CustomResolverFetchMoreLoader'; | ||
import { EmailLoader } from '@/activities/emails/components/EmailLoader'; | ||
|
@@ -10,6 +11,7 @@ import { useRightDrawerEmailThread } from '@/activities/emails/right-drawer/hook | |
import { emailThreadIdWhenEmailThreadWasClosedState } from '@/activities/emails/states/lastViewableEmailThreadIdState'; | ||
import { EmailThreadMessage as EmailThreadMessageType } from '@/activities/emails/types/EmailThreadMessage'; | ||
import { RIGHT_DRAWER_CLICK_OUTSIDE_LISTENER_ID } from '@/ui/layout/right-drawer/constants/RightDrawerClickOutsideListener'; | ||
import { messageThreadState } from '@/ui/layout/right-drawer/states/messageThreadState'; | ||
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. Move the declaration of setMessageThread above the useEffect hook for better readability. 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. We will apply this change as well. |
||
import { useClickOutsideListener } from '@/ui/utilities/pointer-event/hooks/useClickOutsideListener'; | ||
|
||
const StyledContainer = styled.div` | ||
|
@@ -34,8 +36,17 @@ const getVisibleMessages = (messages: EmailThreadMessageType[]) => | |
}); | ||
|
||
export const RightDrawerEmailThread = () => { | ||
const setMessageThread = useSetRecoilState(messageThreadState); | ||
|
||
const { thread, messages, fetchMoreMessages, loading } = | ||
useRightDrawerEmailThread(); | ||
const visibleMessages = getVisibleMessages(messages); | ||
useEffect(() => { | ||
if (!visibleMessages[0]?.messageThread) { | ||
return; | ||
} | ||
setMessageThread(visibleMessages[0]?.messageThread); | ||
}); | ||
|
||
const { useRegisterClickOutsideListenerCallback } = useClickOutsideListener( | ||
RIGHT_DRAWER_CLICK_OUTSIDE_LISTENER_ID, | ||
|
@@ -60,7 +71,6 @@ export const RightDrawerEmailThread = () => { | |
return null; | ||
} | ||
|
||
const visibleMessages = getVisibleMessages(messages); | ||
const visibleMessagesCount = visibleMessages.length; | ||
const is5OrMoreMessages = visibleMessagesCount >= 5; | ||
const firstMessages = visibleMessages.slice( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import { MessageThreadMember } from '@/activities/emails/types/MessageThreadMember'; | ||
|
||
export type MessageThread = { | ||
id: string; | ||
everyone: boolean; | ||
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. Consider renaming 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. We think everyone is fine for this case. |
||
messageThreadMember: MessageThreadMember[]; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import { WorkspaceMember } from '@/workspace-member/types/WorkspaceMember'; | ||
|
||
export type MessageThreadMember = { | ||
workspaceMember: WorkspaceMember; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
import styled from '@emotion/styled'; | ||
import { useRecoilValue } from 'recoil'; | ||
|
||
import { EmailThreadMembersChip } from '@/activities/emails/components/EmailThreadMembersChip'; | ||
import { messageThreadState } from '@/ui/layout/right-drawer/states/messageThreadState'; | ||
|
||
const StyledButtonContainer = styled.div` | ||
align-items: center; | ||
display: flex; | ||
flex-direction: row; | ||
margin-left: ${({ theme }) => theme.spacing(3)}; | ||
`; | ||
|
||
export const MessageThreadMembersBar = () => { | ||
const messageThread = useRecoilValue(messageThreadState); | ||
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. Handle the case where 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. This is already handled. |
||
return ( | ||
<StyledButtonContainer> | ||
<EmailThreadMembersChip messageThread={messageThread} /> | ||
</StyledButtonContainer> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
import { useTheme } from '@emotion/react'; | ||
import { offset } from '@floating-ui/react'; | ||
import { | ||
Avatar, | ||
Chip, | ||
ChipVariant, | ||
IconChevronDown, | ||
IconPlus, | ||
IconUserCircle, | ||
MultiChip, | ||
} from 'twenty-ui'; | ||
|
||
import { MessageThreadMember } from '@/activities/emails/types/MessageThreadMember'; | ||
import { Dropdown } from '@/ui/layout/dropdown/components/Dropdown'; | ||
import { DropdownMenu } from '@/ui/layout/dropdown/components/DropdownMenu'; | ||
import { DropdownMenuItemsContainer } from '@/ui/layout/dropdown/components/DropdownMenuItemsContainer'; | ||
import { DropdownMenuSeparator } from '@/ui/layout/dropdown/components/DropdownMenuSeparator'; | ||
import { useDropdown } from '@/ui/layout/dropdown/hooks/useDropdown'; | ||
import { MenuItem } from '@/ui/navigation/menu-item/components/MenuItem'; | ||
import { MenuItemSelectAvatar } from '@/ui/navigation/menu-item/components/MenuItemSelectAvatar'; | ||
import { getImageAbsoluteURIOrBase64 } from '~/utils/image/getImageAbsoluteURIOrBase64'; | ||
|
||
export const SharedDropdownMenu = ({ | ||
messageThreadMembers, | ||
label = '', | ||
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. Consider adding a type check for 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. We already use Typescript that says that |
||
everyone = false, | ||
}: { | ||
messageThreadMembers: MessageThreadMember[] | null; | ||
label?: string; | ||
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. Handle the case where 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. We followed the same approach as with the other components, so we don't believe this is necessary. |
||
everyone?: boolean; | ||
}) => { | ||
const messageThreadMembersAvatarUrls = messageThreadMembers?.map( | ||
(member) => member.workspaceMember.avatarUrl, | ||
); | ||
|
||
const messageThreadMembersNames = messageThreadMembers?.map( | ||
(member) => member.workspaceMember?.name.firstName, | ||
); | ||
|
||
const theme = useTheme(); | ||
|
||
const { closeDropdown } = useDropdown('message-thread-share'); | ||
|
||
const handleAddParticipantClick = () => { | ||
closeDropdown(); | ||
}; | ||
|
||
const handleParticipantClick = () => { | ||
closeDropdown(); | ||
}; | ||
|
||
return ( | ||
<Dropdown | ||
dropdownId={'message-thread-share'} | ||
clickableComponent={ | ||
everyone ? ( | ||
<Chip | ||
label="Everyone" | ||
variant={ChipVariant.Highlighted} | ||
leftComponent={<IconUserCircle size={theme.icon.size.md} />} | ||
rightComponent={<IconChevronDown size={theme.icon.size.sm} />} | ||
/> | ||
) : messageThreadMembers?.length === 1 ? ( | ||
<Chip | ||
label={label} | ||
variant={ChipVariant.Highlighted} | ||
leftComponent={ | ||
<Avatar | ||
avatarUrl={getImageAbsoluteURIOrBase64( | ||
messageThreadMembersAvatarUrls?.[0], | ||
)} | ||
entityId={messageThreadMembers?.[0].workspaceMember.id} | ||
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. Consider extracting the avatar rendering logic into a separate function to improve readability. 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. We don't think that this is necessary for this case. |
||
placeholder={messageThreadMembersNames?.[0]} | ||
size="md" | ||
type={'rounded'} | ||
/> | ||
} | ||
rightComponent={<IconChevronDown size={theme.icon.size.sm} />} | ||
/> | ||
) : ( | ||
<MultiChip | ||
names={messageThreadMembersNames ?? []} | ||
RightIcon={IconChevronDown} | ||
avatarUrls={ | ||
(messageThreadMembersAvatarUrls?.filter(Boolean) as string[]) ?? | ||
[] | ||
} | ||
/> | ||
) | ||
} | ||
dropdownComponents={ | ||
<DropdownMenu width="160px" z-index={offset(1)}> | ||
<DropdownMenuItemsContainer> | ||
{messageThreadMembers?.map((member) => ( | ||
<MenuItemSelectAvatar | ||
key={member.workspaceMember.id} | ||
selected={false} | ||
testId="menu-item" | ||
onClick={handleParticipantClick} | ||
text={ | ||
member.workspaceMember.name.firstName + | ||
' ' + | ||
member.workspaceMember.name.lastName | ||
} | ||
avatar={ | ||
<Avatar | ||
avatarUrl={getImageAbsoluteURIOrBase64( | ||
member.workspaceMember.avatarUrl, | ||
)} | ||
entityId={member.workspaceMember.id} | ||
placeholder={member.workspaceMember.name.firstName} | ||
size="md" | ||
type={'rounded'} | ||
/> | ||
} | ||
/> | ||
))} | ||
<DropdownMenuSeparator /> | ||
<MenuItem | ||
LeftIcon={IconPlus} | ||
onClick={handleAddParticipantClick} | ||
text="Add participant" | ||
/> | ||
</DropdownMenuItemsContainer> | ||
</DropdownMenu> | ||
} | ||
dropdownHotkeyScope={{ | ||
scope: 'message-thread-share', | ||
}} | ||
/> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
import { createState } from 'twenty-ui'; | ||
|
||
import { MessageThread } from '@/activities/emails/types/MessageThread'; | ||
|
||
export const messageThreadState = createState<MessageThread | null>({ | ||
key: 'messageThreadState', | ||
defaultValue: null, | ||
}); |
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.
Consider renaming
isEveryone
toisVisibleToEveryone
for better clarity.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.
We don't feel the need for this change, as we wanted the variable to be consistent with the database table.