From e8e5af6fcbc03cb94a23141cda0e50a5d7b7252c Mon Sep 17 00:00:00 2001 From: Hyunsu Joo Date: Fri, 15 Mar 2024 02:11:27 +0900 Subject: [PATCH] fix: Close the email side panel upon clicking an open email thread (#4329) * fix: state consistency issue while closing the email thread right drawer (#4205) * Refactored to use useRecoilCallback in RightDrawer open/close hook * - registered an email drawer click outside callback to memorize the thread id when drawer was closed - added a state to memorize then event that triggered right drawer close - added a predicate that checks if event that close email thread right drawer is not the same that the open email thread click event AND that the thread that we want to open is not the thread that is just being closed. --------- Co-authored-by: Lucas Bordeau --- .../emails/components/EmailThreadPreview.tsx | 49 +++++++++++++++-- .../emails/components/EmailThreads.tsx | 27 ++-------- .../activities/emails/hooks/useEmailThread.ts | 38 ++++++++----- .../components/RightDrawerEmailThread.tsx | 21 +++++++- .../state/lastViewableEmailThreadIdState.ts | 8 +++ .../right-drawer/components/RightDrawer.tsx | 20 +++++-- .../right-drawer/hooks/useRightDrawer.ts | 54 +++++++++++++------ .../states/rightDrawerCloseEventsState.ts | 6 +++ 8 files changed, 162 insertions(+), 61 deletions(-) create mode 100644 packages/twenty-front/src/modules/activities/emails/state/lastViewableEmailThreadIdState.ts create mode 100644 packages/twenty-front/src/modules/ui/layout/right-drawer/states/rightDrawerCloseEventsState.ts diff --git a/packages/twenty-front/src/modules/activities/emails/components/EmailThreadPreview.tsx b/packages/twenty-front/src/modules/activities/emails/components/EmailThreadPreview.tsx index 523645b1c..98f5cf004 100644 --- a/packages/twenty-front/src/modules/activities/emails/components/EmailThreadPreview.tsx +++ b/packages/twenty-front/src/modules/activities/emails/components/EmailThreadPreview.tsx @@ -1,7 +1,12 @@ +import { useRef } from 'react'; import styled from '@emotion/styled'; +import { useRecoilCallback } from 'recoil'; import { EmailThreadNotShared } from '@/activities/emails/components/EmailThreadNotShared'; +import { useEmailThread } from '@/activities/emails/hooks/useEmailThread'; +import { emailThreadIdWhenEmailThreadWasClosedState } from '@/activities/emails/state/lastViewableEmailThreadIdState'; import { CardContent } from '@/ui/layout/card/components/CardContent'; +import { useRightDrawer } from '@/ui/layout/right-drawer/hooks/useRightDrawer'; import { GRAY_SCALE } from '@/ui/theme/constants/GrayScale'; import { Avatar } from '@/users/components/Avatar'; import { TimelineThread } from '~/generated/graphql'; @@ -73,19 +78,23 @@ const StyledReceivedAt = styled.div` padding: ${({ theme }) => theme.spacing(0, 1)}; `; +export type EmailThreadVisibility = 'metadata' | 'subject' | 'share_everything'; + type EmailThreadPreviewProps = { divider?: boolean; thread: TimelineThread; - onClick: () => void; - visibility: 'metadata' | 'subject' | 'share_everything'; }; export const EmailThreadPreview = ({ divider, thread, - onClick, - visibility, }: EmailThreadPreviewProps) => { + const cardRef = useRef(null); + + const { openEmailThread } = useEmailThread(); + + const visibility = thread.visibility as EmailThreadVisibility; + const senderNames = thread.firstParticipant.displayName + (thread?.lastTwoParticipants?.[0]?.displayName @@ -104,9 +113,39 @@ export const EmailThreadPreview = ({ false, ]; + const { isSameEventThanRightDrawerClose } = useRightDrawer(); + + const handleThreadClick = useRecoilCallback( + ({ snapshot }) => + (event: React.MouseEvent) => { + const clickJustTriggeredEmailDrawerClose = + isSameEventThanRightDrawerClose(event.nativeEvent); + + const emailThreadIdWhenEmailThreadWasClosed = snapshot + .getLoadable(emailThreadIdWhenEmailThreadWasClosedState()) + .getValue(); + + const canOpen = + thread.visibility === 'share_everything' && + (!clickJustTriggeredEmailDrawerClose || + emailThreadIdWhenEmailThreadWasClosed !== thread.id); + + if (canOpen) { + openEmailThread(thread.id); + } + }, + [ + isSameEventThanRightDrawerClose, + openEmailThread, + thread.id, + thread.visibility, + ], + ); + return ( onClick()} + ref={cardRef} + onClick={(event) => handleThreadClick(event)} divider={divider} visibility={visibility} > diff --git a/packages/twenty-front/src/modules/activities/emails/components/EmailThreads.tsx b/packages/twenty-front/src/modules/activities/emails/components/EmailThreads.tsx index 9d249b4fb..65251d76b 100644 --- a/packages/twenty-front/src/modules/activities/emails/components/EmailThreads.tsx +++ b/packages/twenty-front/src/modules/activities/emails/components/EmailThreads.tsx @@ -8,7 +8,6 @@ import { EmailThreadFetchMoreLoader } from '@/activities/emails/components/Email import { EmailThreadPreview } from '@/activities/emails/components/EmailThreadPreview'; import { TIMELINE_THREADS_DEFAULT_PAGE_SIZE } from '@/activities/emails/constants/Messaging'; import { useEmailThreadStates } from '@/activities/emails/hooks/internal/useEmailThreadStates'; -import { useEmailThread } from '@/activities/emails/hooks/useEmailThread'; import { getTimelineThreadsFromCompanyId } from '@/activities/emails/queries/getTimelineThreadsFromCompanyId'; import { getTimelineThreadsFromPersonId } from '@/activities/emails/queries/getTimelineThreadsFromPersonId'; import { ActivityTargetableObject } from '@/activities/types/ActivityTargetableEntity'; @@ -33,7 +32,6 @@ import { TimelineThread, TimelineThreadsWithTotal, } from '~/generated/graphql'; -import { isDefined } from '~/utils/isDefined'; const StyledContainer = styled.div` display: flex; @@ -58,7 +56,6 @@ export const EmailThreads = ({ }: { entity: ActivityTargetableObject; }) => { - const { openEmailThread } = useEmailThread(); const { enqueueSnackBar } = useSnackBar(); const { getEmailThreadsPageState } = useEmailThreadStates({ @@ -88,9 +85,13 @@ export const EmailThreads = ({ data, loading: firstQueryLoading, fetchMore, - error, } = useQuery(threadQuery, { variables: threadQueryVariables, + onError: (error) => { + enqueueSnackBar(error.message || 'Error loading email threads', { + variant: 'error', + }); + }, }); const fetchMoreRecords = async () => { @@ -141,12 +142,6 @@ export const EmailThreads = ({ } }; - if (isDefined(error)) { - enqueueSnackBar(error.message || 'Error loading email threads', { - variant: 'error', - }); - } - const { totalNumberOfThreads, timelineThreads }: TimelineThreadsWithTotal = data?.[queryName] ?? []; @@ -188,18 +183,6 @@ export const EmailThreads = ({ key={index} divider={index < timelineThreads.length - 1} thread={thread} - onClick={ - thread.visibility === 'share_everything' - ? () => openEmailThread(thread.id) - : () => {} - } - visibility={ - // TODO: Fix typing for visibility - thread.visibility as - | 'metadata' - | 'subject' - | 'share_everything' - } /> ))} diff --git a/packages/twenty-front/src/modules/activities/emails/hooks/useEmailThread.ts b/packages/twenty-front/src/modules/activities/emails/hooks/useEmailThread.ts index bbc6d4c28..a5fca6861 100644 --- a/packages/twenty-front/src/modules/activities/emails/hooks/useEmailThread.ts +++ b/packages/twenty-front/src/modules/activities/emails/hooks/useEmailThread.ts @@ -1,22 +1,36 @@ -import { useSetRecoilState } from 'recoil'; +import { useRecoilCallback } from 'recoil'; import { useOpenEmailThreadRightDrawer } from '@/activities/emails/right-drawer/hooks/useOpenEmailThreadRightDrawer'; import { viewableEmailThreadIdState } from '@/activities/emails/state/viewableEmailThreadIdState'; +import { useRightDrawer } from '@/ui/layout/right-drawer/hooks/useRightDrawer'; +import { isRightDrawerOpenState } from '@/ui/layout/right-drawer/states/isRightDrawerOpenState'; export const useEmailThread = () => { - const setViewableEmailThreadId = useSetRecoilState( - viewableEmailThreadIdState(), - ); - + const { closeRightDrawer } = useRightDrawer(); const openEmailThredRightDrawer = useOpenEmailThreadRightDrawer(); - const openEmailThread = (threadId: string) => { - openEmailThredRightDrawer(); + const openEmailThread = useRecoilCallback( + ({ snapshot, set }) => + (threadId: string) => { + const isRightDrawerOpen = snapshot + .getLoadable(isRightDrawerOpenState()) + .getValue(); - setViewableEmailThreadId(threadId); - }; + const viewableEmailThreadId = snapshot + .getLoadable(viewableEmailThreadIdState()) + .getValue(); - return { - openEmailThread, - }; + if (isRightDrawerOpen && viewableEmailThreadId === threadId) { + set(viewableEmailThreadIdState(), null); + closeRightDrawer(); + return; + } + + openEmailThredRightDrawer(); + set(viewableEmailThreadIdState(), threadId); + }, + [closeRightDrawer, openEmailThredRightDrawer], + ); + + return { openEmailThread }; }; diff --git a/packages/twenty-front/src/modules/activities/emails/right-drawer/components/RightDrawerEmailThread.tsx b/packages/twenty-front/src/modules/activities/emails/right-drawer/components/RightDrawerEmailThread.tsx index a59741d15..c901644f6 100644 --- a/packages/twenty-front/src/modules/activities/emails/right-drawer/components/RightDrawerEmailThread.tsx +++ b/packages/twenty-front/src/modules/activities/emails/right-drawer/components/RightDrawerEmailThread.tsx @@ -1,11 +1,14 @@ -import React from 'react'; import styled from '@emotion/styled'; +import { useRecoilCallback } from 'recoil'; import { EmailLoader } from '@/activities/emails/components/EmailLoader'; import { EmailThreadFetchMoreLoader } from '@/activities/emails/components/EmailThreadFetchMoreLoader'; import { EmailThreadHeader } from '@/activities/emails/components/EmailThreadHeader'; import { EmailThreadMessage } from '@/activities/emails/components/EmailThreadMessage'; import { useRightDrawerEmailThread } from '@/activities/emails/right-drawer/hooks/useRightDrawerEmailThread'; +import { emailThreadIdWhenEmailThreadWasClosedState } from '@/activities/emails/state/lastViewableEmailThreadIdState'; +import { RIGHT_DRAWER_CLICK_OUTSIDE_LISTENER_ID } from '@/ui/layout/right-drawer/constants/RightDrawerClickOutsideListener'; +import { useClickOutsideListener } from '@/ui/utilities/pointer-event/hooks/useClickOutsideListener'; const StyledContainer = styled.div` box-sizing: border-box; @@ -21,6 +24,22 @@ export const RightDrawerEmailThread = () => { const { thread, messages, fetchMoreMessages, loading } = useRightDrawerEmailThread(); + const { useRegisterClickOutsideListenerCallback } = useClickOutsideListener( + RIGHT_DRAWER_CLICK_OUTSIDE_LISTENER_ID, + ); + + useRegisterClickOutsideListenerCallback({ + callbackId: + 'EmailThreadClickOutsideCallBack-' + thread.id ?? 'no-thread-id', + callbackFunction: useRecoilCallback( + ({ set }) => + () => { + set(emailThreadIdWhenEmailThreadWasClosedState(), thread.id); + }, + [thread], + ), + }); + if (!thread) { return null; } diff --git a/packages/twenty-front/src/modules/activities/emails/state/lastViewableEmailThreadIdState.ts b/packages/twenty-front/src/modules/activities/emails/state/lastViewableEmailThreadIdState.ts new file mode 100644 index 000000000..d6404ac51 --- /dev/null +++ b/packages/twenty-front/src/modules/activities/emails/state/lastViewableEmailThreadIdState.ts @@ -0,0 +1,8 @@ +import { createState } from '@/ui/utilities/state/utils/createState'; + +export const emailThreadIdWhenEmailThreadWasClosedState = createState< + string | null +>({ + key: 'emailThreadIdWhenEmailThreadWasClosedState', + defaultValue: null, +}); diff --git a/packages/twenty-front/src/modules/ui/layout/right-drawer/components/RightDrawer.tsx b/packages/twenty-front/src/modules/ui/layout/right-drawer/components/RightDrawer.tsx index ef092669d..1ab9143e9 100644 --- a/packages/twenty-front/src/modules/ui/layout/right-drawer/components/RightDrawer.tsx +++ b/packages/twenty-front/src/modules/ui/layout/right-drawer/components/RightDrawer.tsx @@ -2,10 +2,11 @@ import { useRef } from 'react'; import { useTheme } from '@emotion/react'; import styled from '@emotion/styled'; import { motion } from 'framer-motion'; -import { useRecoilState, useRecoilValue } from 'recoil'; +import { useRecoilCallback, useRecoilState, useRecoilValue } from 'recoil'; import { Key } from 'ts-key-enum'; import { RIGHT_DRAWER_CLICK_OUTSIDE_LISTENER_ID } from '@/ui/layout/right-drawer/constants/RightDrawerClickOutsideListener'; +import { rightDrawerCloseEventState } from '@/ui/layout/right-drawer/states/rightDrawerCloseEventsState'; import { useScopedHotkeys } from '@/ui/utilities/hotkey/hooks/useScopedHotkeys'; import { useClickOutsideListener } from '@/ui/utilities/pointer-event/hooks/useClickOutsideListener'; import { ClickOutsideMode } from '@/ui/utilities/pointer-event/hooks/useListenClickOutside'; @@ -58,9 +59,20 @@ export const RightDrawer = () => { useListenClickOutside({ refs: [rightDrawerRef], - callback: () => { - closeRightDrawer(); - }, + callback: useRecoilCallback( + ({ snapshot, set }) => + (event) => { + const isRightDrawerOpen = snapshot + .getLoadable(isRightDrawerOpenState()) + .getValue(); + + if (isRightDrawerOpen) { + set(rightDrawerCloseEventState(), event); + closeRightDrawer(); + } + }, + [closeRightDrawer], + ), mode: ClickOutsideMode.comparePixels, }); diff --git a/packages/twenty-front/src/modules/ui/layout/right-drawer/hooks/useRightDrawer.ts b/packages/twenty-front/src/modules/ui/layout/right-drawer/hooks/useRightDrawer.ts index a6cb22d7f..fe04ac5a1 100644 --- a/packages/twenty-front/src/modules/ui/layout/right-drawer/hooks/useRightDrawer.ts +++ b/packages/twenty-front/src/modules/ui/layout/right-drawer/hooks/useRightDrawer.ts @@ -1,4 +1,6 @@ -import { useRecoilState, useSetRecoilState } from 'recoil'; +import { useRecoilCallback, useRecoilState } from 'recoil'; + +import { rightDrawerCloseEventState } from '@/ui/layout/right-drawer/states/rightDrawerCloseEventsState'; import { isRightDrawerExpandedState } from '../states/isRightDrawerExpandedState'; import { isRightDrawerOpenState } from '../states/isRightDrawerOpenState'; @@ -6,32 +8,50 @@ import { rightDrawerPageState } from '../states/rightDrawerPageState'; import { RightDrawerPages } from '../types/RightDrawerPages'; export const useRightDrawer = () => { - const [isRightDrawerOpen, setIsRightDrawerOpen] = useRecoilState( - isRightDrawerOpenState(), - ); - const setIsRightDrawerExpanded = useSetRecoilState( - isRightDrawerExpandedState(), + const [isRightDrawerOpen] = useRecoilState(isRightDrawerOpenState()); + + const [rightDrawerPage] = useRecoilState(rightDrawerPageState()); + + const openRightDrawer = useRecoilCallback( + ({ set }) => + (rightDrawerPage: RightDrawerPages) => { + set(rightDrawerPageState(), rightDrawerPage); + set(isRightDrawerExpandedState(), false); + set(isRightDrawerOpenState(), true); + }, + [], ); - const [rightDrawerPage, setRightDrawerPage] = useRecoilState( - rightDrawerPageState(), + const closeRightDrawer = useRecoilCallback( + ({ set }) => + () => { + set(isRightDrawerExpandedState(), false); + set(isRightDrawerOpenState(), false); + }, + [], ); - const openRightDrawer = (rightDrawerPage: RightDrawerPages) => { - setRightDrawerPage(rightDrawerPage); - setIsRightDrawerExpanded(false); - setIsRightDrawerOpen(true); - }; + const isSameEventThanRightDrawerClose = useRecoilCallback( + ({ snapshot }) => + (event: MouseEvent | TouchEvent) => { + const rightDrawerCloseEvent = snapshot + .getLoadable(rightDrawerCloseEventState()) + .getValue(); - const closeRightDrawer = () => { - setIsRightDrawerExpanded(false); - setIsRightDrawerOpen(false); - }; + const isSameEvent = + rightDrawerCloseEvent?.target === event.target && + rightDrawerCloseEvent?.timeStamp === event.timeStamp; + + return isSameEvent; + }, + [], + ); return { rightDrawerPage, isRightDrawerOpen, openRightDrawer, closeRightDrawer, + isSameEventThanRightDrawerClose, }; }; diff --git a/packages/twenty-front/src/modules/ui/layout/right-drawer/states/rightDrawerCloseEventsState.ts b/packages/twenty-front/src/modules/ui/layout/right-drawer/states/rightDrawerCloseEventsState.ts new file mode 100644 index 000000000..66b59ccf0 --- /dev/null +++ b/packages/twenty-front/src/modules/ui/layout/right-drawer/states/rightDrawerCloseEventsState.ts @@ -0,0 +1,6 @@ +import { createState } from '@/ui/utilities/state/utils/createState'; + +export const rightDrawerCloseEventState = createState({ + key: 'rightDrawerCloseEventState', + defaultValue: null, +});