From 91187dcf82af5ac38534f1a685c626145f6b47f5 Mon Sep 17 00:00:00 2001 From: Gaurav Khanna Date: Tue, 10 Sep 2024 07:33:08 -0700 Subject: [PATCH] Fix note linked text in timeline view (in dark mode) (#6944) This PR Fixes https://github.com/twentyhq/twenty/issues/6942 Other improvements : - Fetch activities (note and task) title only when loading timeline, so we don't always have a clickable title. - Fixed IconButton width regression. --------- Co-authored-by: Lucas Bordeau --- .../components/EventRow.tsx | 5 ++- .../components/TimelineActivities.tsx | 2 +- ...s => useLinkedObjectObjectMetadataItem.ts} | 2 +- .../hooks/useLinkedObjectsTitle.ts | 43 +++++++++++++++++++ ...ctivities.tsx => useTimelineActivities.ts} | 18 ++++++-- .../activity/components/EventRowActivity.tsx | 41 ++++++++++++------ .../types/TimelineActivityLinkedObject.ts | 1 + ...lterTimelineActivityByLinkedObjectTypes.ts | 12 ++++++ .../hooks/useCombinedFindManyRecords.ts | 5 ++- .../ui/input/button/components/IconButton.tsx | 4 +- 10 files changed, 107 insertions(+), 26 deletions(-) rename packages/twenty-front/src/modules/activities/timelineActivities/hooks/{useLinkedObject.ts => useLinkedObjectObjectMetadataItem.ts} (86%) create mode 100644 packages/twenty-front/src/modules/activities/timelineActivities/hooks/useLinkedObjectsTitle.ts rename packages/twenty-front/src/modules/activities/timelineActivities/hooks/{useTimelineActivities.tsx => useTimelineActivities.ts} (71%) create mode 100644 packages/twenty-front/src/modules/activities/timelineActivities/types/TimelineActivityLinkedObject.ts create mode 100644 packages/twenty-front/src/modules/activities/timelineActivities/utils/filterTimelineActivityByLinkedObjectTypes.ts diff --git a/packages/twenty-front/src/modules/activities/timelineActivities/components/EventRow.tsx b/packages/twenty-front/src/modules/activities/timelineActivities/components/EventRow.tsx index d920ec38f0..e046316132 100644 --- a/packages/twenty-front/src/modules/activities/timelineActivities/components/EventRow.tsx +++ b/packages/twenty-front/src/modules/activities/timelineActivities/components/EventRow.tsx @@ -3,7 +3,8 @@ import { useContext } from 'react'; import { useRecoilValue } from 'recoil'; import { TimelineActivityContext } from '@/activities/timelineActivities/contexts/TimelineActivityContext'; -import { useLinkedObject } from '@/activities/timelineActivities/hooks/useLinkedObject'; + +import { useLinkedObjectObjectMetadataItem } from '@/activities/timelineActivities/hooks/useLinkedObjectObjectMetadataItem'; import { EventIconDynamicComponent } from '@/activities/timelineActivities/rows/components/EventIconDynamicComponent'; import { EventRowDynamicComponent } from '@/activities/timelineActivities/rows/components/EventRowDynamicComponent'; import { TimelineActivity } from '@/activities/timelineActivities/types/TimelineActivity'; @@ -100,7 +101,7 @@ export const EventRow = ({ const { labelIdentifierValue } = useContext(TimelineActivityContext); const beautifiedCreatedAt = beautifyPastDateRelativeToNow(event.createdAt); - const linkedObjectMetadataItem = useLinkedObject( + const linkedObjectMetadataItem = useLinkedObjectObjectMetadataItem( event.linkedObjectMetadataId, ); diff --git a/packages/twenty-front/src/modules/activities/timelineActivities/components/TimelineActivities.tsx b/packages/twenty-front/src/modules/activities/timelineActivities/components/TimelineActivities.tsx index b94921254f..bbda464681 100644 --- a/packages/twenty-front/src/modules/activities/timelineActivities/components/TimelineActivities.tsx +++ b/packages/twenty-front/src/modules/activities/timelineActivities/components/TimelineActivities.tsx @@ -46,7 +46,7 @@ export const TimelineActivities = ({ const isTimelineActivitiesEmpty = !timelineActivities || timelineActivities.length === 0; - if (loading && isTimelineActivitiesEmpty) { + if (loading) { return ; } diff --git a/packages/twenty-front/src/modules/activities/timelineActivities/hooks/useLinkedObject.ts b/packages/twenty-front/src/modules/activities/timelineActivities/hooks/useLinkedObjectObjectMetadataItem.ts similarity index 86% rename from packages/twenty-front/src/modules/activities/timelineActivities/hooks/useLinkedObject.ts rename to packages/twenty-front/src/modules/activities/timelineActivities/hooks/useLinkedObjectObjectMetadataItem.ts index b628b56907..9f230bef62 100644 --- a/packages/twenty-front/src/modules/activities/timelineActivities/hooks/useLinkedObject.ts +++ b/packages/twenty-front/src/modules/activities/timelineActivities/hooks/useLinkedObjectObjectMetadataItem.ts @@ -3,7 +3,7 @@ import { useRecoilValue } from 'recoil'; import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState'; import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; -export const useLinkedObject = (id: string) => { +export const useLinkedObjectObjectMetadataItem = (id: string) => { const objectMetadataItems: ObjectMetadataItem[] = useRecoilValue( objectMetadataItemsState, ); diff --git a/packages/twenty-front/src/modules/activities/timelineActivities/hooks/useLinkedObjectsTitle.ts b/packages/twenty-front/src/modules/activities/timelineActivities/hooks/useLinkedObjectsTitle.ts new file mode 100644 index 0000000000..2df7bc03a4 --- /dev/null +++ b/packages/twenty-front/src/modules/activities/timelineActivities/hooks/useLinkedObjectsTitle.ts @@ -0,0 +1,43 @@ +import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; +import { useCombinedFindManyRecords } from '@/object-record/multiple-objects/hooks/useCombinedFindManyRecords'; +import { isNonEmptyArray } from '@sniptt/guards'; + +export const useLinkedObjectsTitle = (linkedObjectIds: string[]) => { + const { loading } = useCombinedFindManyRecords({ + skip: !isNonEmptyArray(linkedObjectIds), + operationSignatures: [ + { + objectNameSingular: CoreObjectNameSingular.Task, + variables: { + filter: { + id: { + in: linkedObjectIds ?? [], + }, + }, + }, + fields: { + id: true, + title: true, + }, + }, + { + objectNameSingular: CoreObjectNameSingular.Note, + variables: { + filter: { + id: { + in: linkedObjectIds ?? [], + }, + }, + }, + fields: { + id: true, + title: true, + }, + }, + ], + }); + + return { + loading, + }; +}; diff --git a/packages/twenty-front/src/modules/activities/timelineActivities/hooks/useTimelineActivities.tsx b/packages/twenty-front/src/modules/activities/timelineActivities/hooks/useTimelineActivities.ts similarity index 71% rename from packages/twenty-front/src/modules/activities/timelineActivities/hooks/useTimelineActivities.tsx rename to packages/twenty-front/src/modules/activities/timelineActivities/hooks/useTimelineActivities.ts index 843586660b..fb65053c15 100644 --- a/packages/twenty-front/src/modules/activities/timelineActivities/hooks/useTimelineActivities.tsx +++ b/packages/twenty-front/src/modules/activities/timelineActivities/hooks/useTimelineActivities.ts @@ -1,3 +1,4 @@ +import { useLinkedObjectsTitle } from '@/activities/timelineActivities/hooks/useLinkedObjectsTitle'; import { TimelineActivity } from '@/activities/timelineActivities/types/TimelineActivity'; import { ActivityTargetableObject } from '@/activities/types/ActivityTargetableEntity'; import { getActivityTargetObjectFieldIdName } from '@/activities/utils/getActivityTargetObjectFieldIdName'; @@ -19,10 +20,10 @@ export const useTimelineActivities = ( }); const { - records: TimelineActivities, - loading, + records: timelineActivities, + loading: loadingTimelineActivities, fetchMoreRecords, - } = useFindManyRecords({ + } = useFindManyRecords({ objectNameSingular: CoreObjectNameSingular.TimelineActivity, filter: { [targetableObjectFieldIdName]: { @@ -38,8 +39,17 @@ export const useTimelineActivities = ( fetchPolicy: 'cache-and-network', }); + const activityIds = timelineActivities + .filter((timelineActivity) => timelineActivity.name.match(/note|task/i)) + .map((timelineActivity) => timelineActivity.linkedRecordId); + + const { loading: loadingLinkedObjectsTitle } = + useLinkedObjectsTitle(activityIds); + + const loading = loadingTimelineActivities || loadingLinkedObjectsTitle; + return { - timelineActivities: TimelineActivities as TimelineActivity[], + timelineActivities, loading, fetchMoreRecords, }; diff --git a/packages/twenty-front/src/modules/activities/timelineActivities/rows/activity/components/EventRowActivity.tsx b/packages/twenty-front/src/modules/activities/timelineActivities/rows/activity/components/EventRowActivity.tsx index 30c13a7214..1c1f34e43a 100644 --- a/packages/twenty-front/src/modules/activities/timelineActivities/rows/activity/components/EventRowActivity.tsx +++ b/packages/twenty-front/src/modules/activities/timelineActivities/rows/activity/components/EventRowActivity.tsx @@ -1,5 +1,4 @@ import styled from '@emotion/styled'; -import { useRecoilState } from 'recoil'; import { useOpenActivityRightDrawer } from '@/activities/hooks/useOpenActivityRightDrawer'; import { @@ -8,15 +7,21 @@ import { StyledEventRowItemColumn, } from '@/activities/timelineActivities/rows/components/EventRowDynamicComponent'; import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; -import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState'; +import { useGetRecordFromCache } from '@/object-record/cache/hooks/useGetRecordFromCache'; +import { isNonEmptyString } from '@sniptt/guards'; type EventRowActivityProps = EventRowDynamicComponentProps; const StyledLinkedActivity = styled.span` + color: ${({ theme }) => theme.font.color.primary}; cursor: pointer; text-decoration: underline; `; +export const StyledEventRowItemText = styled.span` + color: ${({ theme }) => theme.font.color.primary}; +`; + export const EventRowActivity = ({ event, authorFullName, @@ -30,9 +35,21 @@ export const EventRowActivity = ({ throw new Error('Could not find linked record id for event'); } - const [activityInStore] = useRecoilState( - recordStoreFamilyState(event.linkedRecordId), - ); + const getActivityFromCache = useGetRecordFromCache({ + objectNameSingular, + recordGqlFields: { + id: true, + title: true, + }, + }); + + const activityInStore = getActivityFromCache(event.linkedRecordId); + + const activityTitle = isNonEmptyString(activityInStore?.title) + ? activityInStore?.title + : isNonEmptyString(event.linkedRecordCachedName) + ? event.linkedRecordCachedName + : 'Untitled'; const openActivityRightDrawer = useOpenActivityRightDrawer({ objectNameSingular, @@ -44,15 +61,11 @@ export const EventRowActivity = ({ {`${eventAction} a related ${eventObject}`} - {activityInStore ? ( - openActivityRightDrawer(event.linkedRecordId)} - > - {event.linkedRecordCachedName} - - ) : ( - {event.linkedRecordCachedName} - )} + openActivityRightDrawer(event.linkedRecordId)} + > + {activityTitle} + ); }; diff --git a/packages/twenty-front/src/modules/activities/timelineActivities/types/TimelineActivityLinkedObject.ts b/packages/twenty-front/src/modules/activities/timelineActivities/types/TimelineActivityLinkedObject.ts new file mode 100644 index 0000000000..9fd28f828f --- /dev/null +++ b/packages/twenty-front/src/modules/activities/timelineActivities/types/TimelineActivityLinkedObject.ts @@ -0,0 +1 @@ +export type TimelineActivityLinkedObject = 'note' | 'task'; diff --git a/packages/twenty-front/src/modules/activities/timelineActivities/utils/filterTimelineActivityByLinkedObjectTypes.ts b/packages/twenty-front/src/modules/activities/timelineActivities/utils/filterTimelineActivityByLinkedObjectTypes.ts new file mode 100644 index 0000000000..455ceca01c --- /dev/null +++ b/packages/twenty-front/src/modules/activities/timelineActivities/utils/filterTimelineActivityByLinkedObjectTypes.ts @@ -0,0 +1,12 @@ +import { TimelineActivity } from '@/activities/timelineActivities/types/TimelineActivity'; +import { TimelineActivityLinkedObject } from '@/activities/timelineActivities/types/TimelineActivityLinkedObject'; + +export const filterTimelineActivityByLinkedObjectTypes = + (linkedObjectTypes: TimelineActivityLinkedObject[]) => + (timelineActivity: TimelineActivity) => { + return linkedObjectTypes.some((linkedObjectType) => { + const linkedObjectPartInName = timelineActivity.name.split('.')[0]; + + return linkedObjectPartInName.includes(linkedObjectType); + }); + }; diff --git a/packages/twenty-front/src/modules/object-record/multiple-objects/hooks/useCombinedFindManyRecords.ts b/packages/twenty-front/src/modules/object-record/multiple-objects/hooks/useCombinedFindManyRecords.ts index 95385c3777..b147b5535f 100644 --- a/packages/twenty-front/src/modules/object-record/multiple-objects/hooks/useCombinedFindManyRecords.ts +++ b/packages/twenty-front/src/modules/object-record/multiple-objects/hooks/useCombinedFindManyRecords.ts @@ -11,13 +11,13 @@ export const useCombinedFindManyRecords = ({ skip = false, }: { operationSignatures: RecordGqlOperationSignature[]; - skip: boolean; + skip?: boolean; }) => { const findManyQuery = useGenerateCombinedFindManyRecordsQuery({ operationSignatures, }); - const { data } = useQuery( + const { data, loading } = useQuery( findManyQuery ?? EMPTY_QUERY, { skip, @@ -35,5 +35,6 @@ export const useCombinedFindManyRecords = ({ return { result: resultWithoutConnection, + loading, }; }; diff --git a/packages/twenty-front/src/modules/ui/input/button/components/IconButton.tsx b/packages/twenty-front/src/modules/ui/input/button/components/IconButton.tsx index 421962c878..e5e83551cd 100644 --- a/packages/twenty-front/src/modules/ui/input/button/components/IconButton.tsx +++ b/packages/twenty-front/src/modules/ui/input/button/components/IconButton.tsx @@ -1,6 +1,6 @@ -import React from 'react'; import { css, useTheme } from '@emotion/react'; import styled from '@emotion/styled'; +import React from 'react'; import { IconComponent } from 'twenty-ui'; export type IconButtonSize = 'medium' | 'small'; @@ -233,7 +233,7 @@ const StyledButton = styled.button< white-space: nowrap; - width: ${({ size }) => (size === 'small' ? '24px' : '32px')}; + min-width: ${({ size }) => (size === 'small' ? '24px' : '32px')}; &:focus { outline: none;