From 6a85f5a6ff95d522bb8db8162328bb8f97c12781 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Fri, 29 Mar 2024 21:43:21 +0000 Subject: [PATCH] Revert "Merge pull request #38722 from rayane-djouah/Feat-Redesign-thread-ancestry" This reverts commit 903fe8ad901cae0a5ae0d62f74324922559f9ee5, reversing changes made to d6fc25a20ddf4547e779944f60faa269d013293c. --- assets/images/thread.svg | 3 -- src/components/AvatarWithDisplayName.tsx | 1 - src/components/Icon/Expensicons.ts | 2 - src/components/ParentNavigationSubtitle.tsx | 7 +-- src/languages/en.ts | 1 - src/languages/es.ts | 1 - src/libs/ReportUtils.ts | 7 ++- src/pages/ReportDetailsPage.tsx | 1 - src/pages/home/HeaderView.tsx | 1 - src/pages/home/report/RepliesDivider.tsx | 36 -------------- src/pages/home/report/ReportActionItem.tsx | 2 +- .../report/ReportActionItemParentAction.tsx | 15 ++---- src/pages/home/report/ReportActionsList.tsx | 1 - .../report/ReportActionsListItemRenderer.tsx | 5 -- src/pages/home/report/ThreadDivider.tsx | 48 ------------------- src/styles/index.ts | 10 +--- src/styles/utils/index.ts | 4 +- tests/unit/ReportUtilsTest.js | 31 ++++++++++-- 18 files changed, 43 insertions(+), 133 deletions(-) delete mode 100644 assets/images/thread.svg delete mode 100644 src/pages/home/report/RepliesDivider.tsx delete mode 100644 src/pages/home/report/ThreadDivider.tsx diff --git a/assets/images/thread.svg b/assets/images/thread.svg deleted file mode 100644 index 3b8f334fafdd..000000000000 --- a/assets/images/thread.svg +++ /dev/null @@ -1,3 +0,0 @@ - - - diff --git a/src/components/AvatarWithDisplayName.tsx b/src/components/AvatarWithDisplayName.tsx index f6afb4dae2d6..396c10151fbf 100644 --- a/src/components/AvatarWithDisplayName.tsx +++ b/src/components/AvatarWithDisplayName.tsx @@ -141,7 +141,6 @@ function AvatarWithDisplayName({ )} diff --git a/src/components/Icon/Expensicons.ts b/src/components/Icon/Expensicons.ts index 1fcf0d07276c..5191d2012b05 100644 --- a/src/components/Icon/Expensicons.ts +++ b/src/components/Icon/Expensicons.ts @@ -141,7 +141,6 @@ import Sync from '@assets/images/sync.svg'; import Tag from '@assets/images/tag.svg'; import Task from '@assets/images/task.svg'; import Tax from '@assets/images/tax.svg'; -import Thread from '@assets/images/thread.svg'; import ThreeDots from '@assets/images/three-dots.svg'; import ThumbsUp from '@assets/images/thumbs-up.svg'; import Transfer from '@assets/images/transfer.svg'; @@ -231,7 +230,6 @@ export { Folder, Tag, Tax, - Thread, Gallery, Gear, Globe, diff --git a/src/components/ParentNavigationSubtitle.tsx b/src/components/ParentNavigationSubtitle.tsx index d36a2e93f5b3..3109453ca6b0 100644 --- a/src/components/ParentNavigationSubtitle.tsx +++ b/src/components/ParentNavigationSubtitle.tsx @@ -15,14 +15,11 @@ type ParentNavigationSubtitleProps = { /** parent Report ID */ parentReportID?: string; - /** parent Report Action ID */ - parentReportActionID?: string; - /** PressableWithoutFeedack additional styles */ pressableStyles?: StyleProp; }; -function ParentNavigationSubtitle({parentNavigationSubtitleData, parentReportActionID, parentReportID = '', pressableStyles}: ParentNavigationSubtitleProps) { +function ParentNavigationSubtitle({parentNavigationSubtitleData, parentReportID = '', pressableStyles}: ParentNavigationSubtitleProps) { const styles = useThemeStyles(); const {workspaceName, reportName} = parentNavigationSubtitleData; @@ -31,7 +28,7 @@ function ParentNavigationSubtitle({parentNavigationSubtitleData, parentReportAct return ( { - Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(parentReportID, parentReportActionID)); + Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(parentReportID)); }} accessibilityLabel={translate('threads.parentNavigationSummary', {reportName, workspaceName})} role={CONST.ROLE.LINK} diff --git a/src/languages/en.ts b/src/languages/en.ts index d30b62fc50b3..39a3d582e9de 100755 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -2415,7 +2415,6 @@ export default { hiddenMessage: '[Hidden message]', }, threads: { - thread: 'Thread', replies: 'Replies', reply: 'Reply', from: 'From', diff --git a/src/languages/es.ts b/src/languages/es.ts index 30e4717bdf57..fb57abb3eabb 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -2907,7 +2907,6 @@ export default { hiddenMessage: '[Mensaje oculto]', }, threads: { - thread: 'Hilo', replies: 'Respuestas', reply: 'Respuesta', from: 'De', diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 9fa28535a7a7..4733457164a3 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -423,6 +423,7 @@ type Ancestor = { report: Report; reportAction: ReportAction; shouldDisplayNewMarker: boolean; + shouldHideThreadDividerLine: boolean; }; type AncestorIDs = { @@ -5477,7 +5478,7 @@ function shouldDisableThread(reportAction: OnyxEntry, reportID: st ); } -function getAllAncestorReportActions(report: Report | null | undefined): Ancestor[] { +function getAllAncestorReportActions(report: Report | null | undefined, shouldHideThreadDividerLine: boolean): Ancestor[] { if (!report) { return []; } @@ -5487,6 +5488,7 @@ function getAllAncestorReportActions(report: Report | null | undefined): Ancesto // Store the child of parent report let currentReport = report; + let currentUnread = shouldHideThreadDividerLine; while (parentReportID) { const parentReport = getReport(parentReportID); @@ -5501,11 +5503,14 @@ function getAllAncestorReportActions(report: Report | null | undefined): Ancesto report: currentReport, reportAction: parentReportAction, shouldDisplayNewMarker: isParentReportActionUnread, + // We should hide the thread divider line if the previous ancestor action is unread + shouldHideThreadDividerLine: currentUnread, }); parentReportID = parentReport?.parentReportID; parentReportActionID = parentReport?.parentReportActionID; if (!isEmptyObject(parentReport)) { currentReport = parentReport; + currentUnread = isParentReportActionUnread; } } diff --git a/src/pages/ReportDetailsPage.tsx b/src/pages/ReportDetailsPage.tsx index 80563fcf7b1b..f06b40af8851 100644 --- a/src/pages/ReportDetailsPage.tsx +++ b/src/pages/ReportDetailsPage.tsx @@ -252,7 +252,6 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD )} diff --git a/src/pages/home/HeaderView.tsx b/src/pages/home/HeaderView.tsx index a6ddbc7bfb95..8aa2b855176c 100644 --- a/src/pages/home/HeaderView.tsx +++ b/src/pages/home/HeaderView.tsx @@ -272,7 +272,6 @@ function HeaderView({report, personalDetails, parentReport, parentReportAction, )} diff --git a/src/pages/home/report/RepliesDivider.tsx b/src/pages/home/report/RepliesDivider.tsx deleted file mode 100644 index deac38596c99..000000000000 --- a/src/pages/home/report/RepliesDivider.tsx +++ /dev/null @@ -1,36 +0,0 @@ -import React from 'react'; -import {View} from 'react-native'; -import Icon from '@components/Icon'; -import * as Expensicons from '@components/Icon/Expensicons'; -import Text from '@components/Text'; -import useLocalize from '@hooks/useLocalize'; -import useTheme from '@hooks/useTheme'; -import useThemeStyles from '@hooks/useThemeStyles'; -import variables from '@styles/variables'; - -type RepliesDividerProps = { - /** Whether we should hide thread divider line */ - shouldHideThreadDividerLine: boolean; -}; - -function RepliesDivider({shouldHideThreadDividerLine}: RepliesDividerProps) { - const styles = useThemeStyles(); - const theme = useTheme(); - const {translate} = useLocalize(); - - return ( - - - {translate('threads.replies')} - {!shouldHideThreadDividerLine && } - - ); -} - -RepliesDivider.displayName = 'RepliesDivider'; -export default RepliesDivider; diff --git a/src/pages/home/report/ReportActionItem.tsx b/src/pages/home/report/ReportActionItem.tsx index 2716fedcf59a..958f19ba336f 100644 --- a/src/pages/home/report/ReportActionItem.tsx +++ b/src/pages/home/report/ReportActionItem.tsx @@ -860,7 +860,7 @@ function ReportActionItem({ checkIfContextMenuActive={toggleContextMenuFromActiveReportAction} setIsEmojiPickerActive={setIsEmojiPickerActive} /> - + ReportActions.clearAllRelatedReportActionErrors(report.reportID, action)} // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing diff --git a/src/pages/home/report/ReportActionItemParentAction.tsx b/src/pages/home/report/ReportActionItemParentAction.tsx index bf34a833b081..9e7f8afa507f 100644 --- a/src/pages/home/report/ReportActionItemParentAction.tsx +++ b/src/pages/home/report/ReportActionItemParentAction.tsx @@ -13,9 +13,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import type * as OnyxTypes from '@src/types/onyx'; import AnimatedEmptyStateBackground from './AnimatedEmptyStateBackground'; -import RepliesDivider from './RepliesDivider'; import ReportActionItem from './ReportActionItem'; -import ThreadDivider from './ThreadDivider'; type ReportActionItemParentActionProps = { /** Flag to show, hide the thread divider line */ @@ -39,9 +37,6 @@ type ReportActionItemParentActionProps = { /** Report actions belonging to the report's parent */ parentReportAction: OnyxEntry; - - /** Whether we should display "Replies" divider */ - shouldDisplayReplyDivider: boolean; }; function ReportActionItemParentAction({ @@ -51,7 +46,6 @@ function ReportActionItemParentAction({ parentReportAction, index = 0, shouldHideThreadDividerLine = false, - shouldDisplayReplyDivider, }: ReportActionItemParentActionProps) { const styles = useThemeStyles(); const StyleUtils = useStyleUtils(); @@ -67,7 +61,7 @@ function ReportActionItemParentAction({ onyxSubscribe({ key: `${ONYXKEYS.COLLECTION.REPORT}${ancestorReportID}`, callback: () => { - setAllAncestors(ReportUtils.getAllAncestorReportActions(report)); + setAllAncestors(ReportUtils.getAllAncestorReportActions(report, shouldHideThreadDividerLine)); }, }), ); @@ -75,7 +69,7 @@ function ReportActionItemParentAction({ onyxSubscribe({ key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${ancestorReportID}`, callback: () => { - setAllAncestors(ReportUtils.getAllAncestorReportActions(report)); + setAllAncestors(ReportUtils.getAllAncestorReportActions(report, shouldHideThreadDividerLine)); }, }), ); @@ -101,9 +95,8 @@ function ReportActionItemParentAction({ errorRowStyles={[styles.ml10, styles.mr2]} onClose={() => Report.navigateToConciergeChatAndDeleteReport(ancestor.report.reportID)} > - Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.parentReportID ?? '', ancestor.reportAction.reportActionID))} + onPress={() => Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.reportID))} parentReportAction={parentReportAction} report={ancestor.report} reportActions={reportActions} @@ -114,9 +107,9 @@ function ReportActionItemParentAction({ shouldDisplayNewMarker={ancestor.shouldDisplayNewMarker} index={index} /> + {!ancestor.shouldHideThreadDividerLine && } ))} - {shouldDisplayReplyDivider && } ); } diff --git a/src/pages/home/report/ReportActionsList.tsx b/src/pages/home/report/ReportActionsList.tsx index d1b9c420b0af..215b7c97de57 100644 --- a/src/pages/home/report/ReportActionsList.tsx +++ b/src/pages/home/report/ReportActionsList.tsx @@ -532,7 +532,6 @@ function ReportActionsList({ mostRecentIOUReportActionID={mostRecentIOUReportActionID} shouldHideThreadDividerLine={shouldHideThreadDividerLine} shouldDisplayNewMarker={shouldDisplayNewMarker(reportAction, index)} - shouldDisplayReplyDivider={sortedReportActions.length > 1} /> ), [ diff --git a/src/pages/home/report/ReportActionsListItemRenderer.tsx b/src/pages/home/report/ReportActionsListItemRenderer.tsx index 263415d90c39..bee07ae66a37 100644 --- a/src/pages/home/report/ReportActionsListItemRenderer.tsx +++ b/src/pages/home/report/ReportActionsListItemRenderer.tsx @@ -40,9 +40,6 @@ type ReportActionsListItemRendererProps = { /** Linked report action ID */ linkedReportActionID?: string; - - /** Whether we should display "Replies" divider */ - shouldDisplayReplyDivider: boolean; }; function ReportActionsListItemRenderer({ @@ -57,7 +54,6 @@ function ReportActionsListItemRenderer({ shouldHideThreadDividerLine, shouldDisplayNewMarker, linkedReportActionID = '', - shouldDisplayReplyDivider, }: ReportActionsListItemRendererProps) { const shouldDisplayParentAction = reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED && ReportUtils.isChatThread(report) && !ReportActionsUtils.isTransactionThread(parentReportAction); @@ -131,7 +127,6 @@ function ReportActionsListItemRenderer({ return shouldDisplayParentAction ? ( - Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(ancestor?.report?.parentReportID ?? '', ancestor.reportAction.reportActionID))} - accessibilityLabel={translate('threads.thread')} - role={CONST.ROLE.BUTTON} - style={[styles.flexRow, styles.alignItemsCenter, styles.gap1]} - > - - {translate('threads.thread')} - - {!ancestor.shouldDisplayNewMarker && } - - ); -} - -ThreadDivider.displayName = 'ThreadDivider'; -export default ThreadDivider; diff --git a/src/styles/index.ts b/src/styles/index.ts index d6dbb539e44d..3ed62e23ecf9 100644 --- a/src/styles/index.ts +++ b/src/styles/index.ts @@ -1898,6 +1898,7 @@ const styles = (theme: ThemeColors) => fontFamily: FontUtils.fontFamily.platform.EXP_NEUE, lineHeight: variables.lineHeightXLarge, maxWidth: '100%', + ...cursor.cursorAuto, ...whiteSpace.preWrap, ...wordBreak.breakWord, }, @@ -2742,8 +2743,7 @@ const styles = (theme: ThemeColors) => height: 1, backgroundColor: theme.border, flexGrow: 1, - marginLeft: 8, - marginRight: 20, + marginHorizontal: 20, }, unreadIndicatorText: { @@ -2754,12 +2754,6 @@ const styles = (theme: ThemeColors) => textTransform: 'capitalize', }, - threadDividerText: { - fontFamily: FontUtils.fontFamily.platform.EXP_NEUE, - fontSize: variables.fontSizeSmall, - textTransform: 'capitalize', - }, - flipUpsideDown: { transform: `rotate(180deg)`, }, diff --git a/src/styles/utils/index.ts b/src/styles/utils/index.ts index a3357b8982a1..0acc6bb976ee 100644 --- a/src/styles/utils/index.ts +++ b/src/styles/utils/index.ts @@ -1451,7 +1451,7 @@ const createStyleUtils = (theme: ThemeColors, styles: ThemeStyles) => ({ /** * Generate the styles for the ReportActionItem wrapper view. */ - getReportActionItemStyle: (isHovered = false, isClickable = false): ViewStyle => + getReportActionItemStyle: (isHovered = false): ViewStyle => // TODO: Remove this "eslint-disable-next" once the theme switching migration is done and styles are fully typed (GH Issue: https://github.com/Expensify/App/issues/27337) // eslint-disable-next-line @typescript-eslint/no-unsafe-return ({ @@ -1462,7 +1462,7 @@ const createStyleUtils = (theme: ThemeColors, styles: ThemeStyles) => ({ : // Warning: Setting this to a non-transparent color will cause unread indicator to break on Android theme.transparent, opacity: 1, - ...(isClickable ? styles.cursorPointer : styles.cursorInitial), + ...styles.cursorInitial, }), /** diff --git a/tests/unit/ReportUtilsTest.js b/tests/unit/ReportUtilsTest.js index ffd5c9147dc0..7b563d46b7eb 100644 --- a/tests/unit/ReportUtilsTest.js +++ b/tests/unit/ReportUtilsTest.js @@ -747,13 +747,34 @@ describe('ReportUtils', () => { it('should return correctly all ancestors of a thread report', () => { const resultAncestors = [ - {report: reports[1], reportAction: reportActions[0], shouldDisplayNewMarker: false}, - {report: reports[2], reportAction: reportActions[1], shouldDisplayNewMarker: false}, - {report: reports[3], reportAction: reportActions[2], shouldDisplayNewMarker: false}, - {report: reports[4], reportAction: reportActions[3], shouldDisplayNewMarker: false}, + {report: reports[1], reportAction: reportActions[0], shouldDisplayNewMarker: false, shouldHideThreadDividerLine: false}, + {report: reports[2], reportAction: reportActions[1], shouldDisplayNewMarker: false, shouldHideThreadDividerLine: false}, + {report: reports[3], reportAction: reportActions[2], shouldDisplayNewMarker: false, shouldHideThreadDividerLine: false}, + {report: reports[4], reportAction: reportActions[3], shouldDisplayNewMarker: false, shouldHideThreadDividerLine: false}, ]; - expect(ReportUtils.getAllAncestorReportActions(reports[4])).toEqual(resultAncestors); + expect(ReportUtils.getAllAncestorReportActions(reports[4], false)).toEqual(resultAncestors); + }); + + it('should hide thread divider line of the nearest ancestor if the first action of thread report is unread', () => { + const allAncestors = ReportUtils.getAllAncestorReportActions(reports[4], true); + expect(allAncestors.reverse()[0].shouldHideThreadDividerLine).toBe(true); + }); + + it('should hide thread divider line of the previous ancestor and display unread marker of the current ancestor if the current ancestor action is unread', () => { + let allAncestors = ReportUtils.getAllAncestorReportActions(reports[4], false); + expect(allAncestors[0].shouldHideThreadDividerLine).toBe(false); + expect(allAncestors[1].shouldDisplayNewMarker).toBe(false); + + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}2`, { + lastReadTime: '2024-02-01 04:42:28.001', + }) + .then(() => waitForBatchedUpdates()) + .then(() => { + allAncestors = ReportUtils.getAllAncestorReportActions(reports[4], false); + expect(allAncestors[0].shouldHideThreadDividerLine).toBe(true); + expect(allAncestors[1].shouldDisplayNewMarker).toBe(true); + }); }); }); });