From 2d80900dddd46540d807d8138f1be571ee31ac37 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Wed, 22 May 2024 16:31:27 -0700 Subject: [PATCH 1/9] Hook to track when a value changes based on a deep comparison --- src/hooks/useDeepCompare.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 src/hooks/useDeepCompare.ts diff --git a/src/hooks/useDeepCompare.ts b/src/hooks/useDeepCompare.ts new file mode 100644 index 000000000000..27b7155079ef --- /dev/null +++ b/src/hooks/useDeepCompare.ts @@ -0,0 +1,11 @@ +// useDeepCompare.ts +import { useRef } from 'react'; +import isEqual from 'lodash/isEqual'; + +export default function useDeepCompare(value: T): T | undefined { + const ref = useRef(); + if (!isEqual(value, ref.current)) { + ref.current = value; + } + return ref.current; +} \ No newline at end of file From c4b3d7633c12f3c07281c2b24c943d21231f4fd5 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Wed, 22 May 2024 16:32:04 -0700 Subject: [PATCH 2/9] Update the report memo when the permissions array changes --- src/pages/home/ReportScreen.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 8620d8d4866e..23e16d3bb1dd 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -52,6 +52,7 @@ import ReportActionsView from './report/ReportActionsView'; import ReportFooter from './report/ReportFooter'; import type {ActionListContextType, ReactionListRef, ScrollPosition} from './ReportScreenContext'; import {ActionListContext, ReactionListContext} from './ReportScreenContext'; +import useDeepCompare from '@hooks/useDeepCompare'; type ReportScreenOnyxPropsWithoutParentReportAction = { /** Get modal status */ @@ -168,6 +169,8 @@ function ReportScreen({ const isReportOpenInRHP = useIsReportOpenInRHP(); const {isSmallScreenWidth} = useWindowDimensions(); const shouldUseNarrowLayout = isSmallScreenWidth || isReportOpenInRHP; + const permissions = useDeepCompare(reportProp?.permissions); + /** * Create a lightweight Report so as to keep the re-rendering as light as possible by * passing in only the required props. @@ -215,7 +218,7 @@ function ReportScreen({ isOptimisticReport: reportProp?.isOptimisticReport, lastMentionedTime: reportProp?.lastMentionedTime, avatarUrl: reportProp?.avatarUrl, - permissions: reportProp?.permissions, + permissions, invoiceReceiver: reportProp?.invoiceReceiver, }), [ @@ -256,7 +259,7 @@ function ReportScreen({ reportProp?.isOptimisticReport, reportProp?.lastMentionedTime, reportProp?.avatarUrl, - reportProp?.permissions, + permissions, reportProp?.invoiceReceiver, ], ); From c8f678b66d4ae5e39489a0eb6f044d0072dd6c46 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Wed, 22 May 2024 16:36:47 -0700 Subject: [PATCH 3/9] Add a helpful comment --- src/hooks/useDeepCompare.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/hooks/useDeepCompare.ts b/src/hooks/useDeepCompare.ts index 27b7155079ef..a342636eee82 100644 --- a/src/hooks/useDeepCompare.ts +++ b/src/hooks/useDeepCompare.ts @@ -1,7 +1,20 @@ -// useDeepCompare.ts import { useRef } from 'react'; import isEqual from 'lodash/isEqual'; +/** + * This hook returns a reference to the provided value, + * but only updates that reference if a deep comparison indicates that the value has changed. + * + * This is useful when working with objects or arrays as dependencies to other hooks like `useEffect` or `useMemo`, + * where you want the hook to trigger not just on reference changes, but also when the contents of the object or array change. + * + * @example + * const myArray = // some array + * const deepComparedArray = useDeepCompare(myArray); + * useEffect(() => { + * // This will run not just when myArray is a new array, but also when its contents change. + * }, [deepComparedArray]); + */ export default function useDeepCompare(value: T): T | undefined { const ref = useRef(); if (!isEqual(value, ref.current)) { From 4f772dd7e2bc957ee0e4a7557931ecb5ecd553c4 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Wed, 22 May 2024 16:38:40 -0700 Subject: [PATCH 4/9] More clear name since hook returns a ref --- src/hooks/{useDeepCompare.ts => useDeepCompareRef.ts} | 4 ++-- src/pages/home/ReportScreen.tsx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) rename src/hooks/{useDeepCompare.ts => useDeepCompareRef.ts} (85%) diff --git a/src/hooks/useDeepCompare.ts b/src/hooks/useDeepCompareRef.ts similarity index 85% rename from src/hooks/useDeepCompare.ts rename to src/hooks/useDeepCompareRef.ts index a342636eee82..4869e461d919 100644 --- a/src/hooks/useDeepCompare.ts +++ b/src/hooks/useDeepCompareRef.ts @@ -10,12 +10,12 @@ import isEqual from 'lodash/isEqual'; * * @example * const myArray = // some array - * const deepComparedArray = useDeepCompare(myArray); + * const deepComparedArray = useDeepCompareRef(myArray); * useEffect(() => { * // This will run not just when myArray is a new array, but also when its contents change. * }, [deepComparedArray]); */ -export default function useDeepCompare(value: T): T | undefined { +export default function useDeepCompareRef(value: T): T | undefined { const ref = useRef(); if (!isEqual(value, ref.current)) { ref.current = value; diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 23e16d3bb1dd..06bee0a6a4ee 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -52,7 +52,7 @@ import ReportActionsView from './report/ReportActionsView'; import ReportFooter from './report/ReportFooter'; import type {ActionListContextType, ReactionListRef, ScrollPosition} from './ReportScreenContext'; import {ActionListContext, ReactionListContext} from './ReportScreenContext'; -import useDeepCompare from '@hooks/useDeepCompare'; +import useDeepCompareRef from '@hooks/useDeepCompareRef'; type ReportScreenOnyxPropsWithoutParentReportAction = { /** Get modal status */ @@ -169,7 +169,7 @@ function ReportScreen({ const isReportOpenInRHP = useIsReportOpenInRHP(); const {isSmallScreenWidth} = useWindowDimensions(); const shouldUseNarrowLayout = isSmallScreenWidth || isReportOpenInRHP; - const permissions = useDeepCompare(reportProp?.permissions); + const permissions = useDeepCompareRef(reportProp?.permissions); /** * Create a lightweight Report so as to keep the re-rendering as light as possible by From e29358de2e1c1464d52ec09c19d6b95bc1b34858 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Tue, 28 May 2024 21:01:39 -0700 Subject: [PATCH 5/9] Fix lint --- src/pages/home/ReportScreen.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index d25589ced7ae..5f0be15e0535 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -47,12 +47,12 @@ import type SCREENS from '@src/SCREENS'; import type * as OnyxTypes from '@src/types/onyx'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue'; +import useDeepCompareRef from '@hooks/useDeepCompareRef'; import HeaderView from './HeaderView'; import ReportActionsView from './report/ReportActionsView'; import ReportFooter from './report/ReportFooter'; import type {ActionListContextType, ReactionListRef, ScrollPosition} from './ReportScreenContext'; import {ActionListContext, ReactionListContext} from './ReportScreenContext'; -import useDeepCompareRef from '@hooks/useDeepCompareRef'; type ReportScreenOnyxProps = { /** Tells us if the sidebar has rendered */ From 3db4c963910bc59340259e6478b0fb6387f4092d Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Tue, 4 Jun 2024 09:18:51 -0700 Subject: [PATCH 6/9] Show composer optimistically after user signs in --- src/pages/home/report/ReportFooter.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ReportFooter.tsx b/src/pages/home/report/ReportFooter.tsx index ac56fe916bc9..2349141a6615 100644 --- a/src/pages/home/report/ReportFooter.tsx +++ b/src/pages/home/report/ReportFooter.tsx @@ -26,6 +26,9 @@ import ReportActionCompose from './ReportActionCompose/ReportActionCompose'; import SystemChatReportFooterMessage from './SystemChatReportFooterMessage'; type ReportFooterOnyxProps = { + /** The account that is logged in */ + account: OnyxEntry; + /** Whether to show the compose input */ shouldShowComposeInput: OnyxEntry; @@ -68,6 +71,7 @@ type ReportFooterProps = ReportFooterOnyxProps & { }; function ReportFooter({ + account, lastReportAction, pendingAction, session, @@ -90,7 +94,7 @@ function ReportFooter({ const isAnonymousUser = session?.authTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS; const isSmallSizeLayout = windowWidth - (isSmallScreenWidth ? 0 : variables.sideBarWidth) < variables.anonymousReportFooterBreakpoint; - const hideComposer = !ReportUtils.canUserPerformWriteAction(report, reportNameValuePairs) || blockedFromChat; + const hideComposer = (!ReportUtils.canUserPerformWriteAction(report, reportNameValuePairs) && !account?.isLoading) || blockedFromChat; const canWriteInReport = ReportUtils.canWriteInReport(report); const isSystemChat = ReportUtils.isSystemChat(report); @@ -189,6 +193,9 @@ function ReportFooter({ ReportFooter.displayName = 'ReportFooter'; export default withOnyx({ + account: { + key: ONYXKEYS.ACCOUNT, + }, shouldShowComposeInput: { key: ONYXKEYS.SHOULD_SHOW_COMPOSE_INPUT, initialValue: false, @@ -211,6 +218,7 @@ export default withOnyx({ prevProps.lastReportAction === nextProps.lastReportAction && prevProps.shouldShowComposeInput === nextProps.shouldShowComposeInput && prevProps.isReportReadyForDisplay === nextProps.isReportReadyForDisplay && - lodashIsEqual(prevProps.session, nextProps.session), + lodashIsEqual(prevProps.session, nextProps.session) && + lodashIsEqual(prevProps.account, nextProps.account), ), ); From 1b407392efc7295557b31a7c25f31c37652c4042 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Tue, 4 Jun 2024 10:01:57 -0700 Subject: [PATCH 7/9] Show composer optimistically while loading report --- src/pages/home/ReportScreen.tsx | 1 + src/pages/home/report/ReportFooter.tsx | 17 ++++++++--------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 20a8c753a8e7..2825bfaeb1d0 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -713,6 +713,7 @@ function ReportScreen({ onComposerFocus={() => setIsComposerFocus(true)} onComposerBlur={() => setIsComposerFocus(false)} report={report} + reportMetadata={reportMetadata} reportNameValuePairs={reportNameValuePairs} pendingAction={reportPendingAction} isComposerFullSize={!!isComposerFullSize} diff --git a/src/pages/home/report/ReportFooter.tsx b/src/pages/home/report/ReportFooter.tsx index 2349141a6615..d8bce6b77748 100644 --- a/src/pages/home/report/ReportFooter.tsx +++ b/src/pages/home/report/ReportFooter.tsx @@ -26,9 +26,6 @@ import ReportActionCompose from './ReportActionCompose/ReportActionCompose'; import SystemChatReportFooterMessage from './SystemChatReportFooterMessage'; type ReportFooterOnyxProps = { - /** The account that is logged in */ - account: OnyxEntry; - /** Whether to show the compose input */ shouldShowComposeInput: OnyxEntry; @@ -43,6 +40,8 @@ type ReportFooterProps = ReportFooterOnyxProps & { /** Report object for the current report */ report?: OnyxTypes.Report; + reportMetadata?: OnyxEntry; + reportNameValuePairs?: OnyxEntry; /** The last report action */ @@ -71,11 +70,11 @@ type ReportFooterProps = ReportFooterOnyxProps & { }; function ReportFooter({ - account, lastReportAction, pendingAction, session, report = {reportID: '0'}, + reportMetadata, reportNameValuePairs, shouldShowComposeInput = false, isEmptyChat = true, @@ -94,7 +93,10 @@ function ReportFooter({ const isAnonymousUser = session?.authTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS; const isSmallSizeLayout = windowWidth - (isSmallScreenWidth ? 0 : variables.sideBarWidth) < variables.anonymousReportFooterBreakpoint; - const hideComposer = (!ReportUtils.canUserPerformWriteAction(report, reportNameValuePairs) && !account?.isLoading) || blockedFromChat; + + // If a user just signed in and is viewing a public report, optimistically show the composer while loading the report, since they will have write access when the response comes back. + const showComposerOptimistically = ReportUtils.isPublicRoom(report) && reportMetadata?.isLoadingInitialReportActions; + const hideComposer = (!ReportUtils.canUserPerformWriteAction(report, reportNameValuePairs) && !showComposerOptimistically) || blockedFromChat; const canWriteInReport = ReportUtils.canWriteInReport(report); const isSystemChat = ReportUtils.isSystemChat(report); @@ -193,9 +195,6 @@ function ReportFooter({ ReportFooter.displayName = 'ReportFooter'; export default withOnyx({ - account: { - key: ONYXKEYS.ACCOUNT, - }, shouldShowComposeInput: { key: ONYXKEYS.SHOULD_SHOW_COMPOSE_INPUT, initialValue: false, @@ -219,6 +218,6 @@ export default withOnyx({ prevProps.shouldShowComposeInput === nextProps.shouldShowComposeInput && prevProps.isReportReadyForDisplay === nextProps.isReportReadyForDisplay && lodashIsEqual(prevProps.session, nextProps.session) && - lodashIsEqual(prevProps.account, nextProps.account), + lodashIsEqual(prevProps.reportMetadata, nextProps.reportMetadata), ), ); From 5011a8258a548bb5deeb3689ac92158b630dc608 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Tue, 4 Jun 2024 10:04:29 -0700 Subject: [PATCH 8/9] Prettier --- src/hooks/useDeepCompareRef.ts | 14 +++++++------- src/pages/home/ReportScreen.tsx | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/hooks/useDeepCompareRef.ts b/src/hooks/useDeepCompareRef.ts index 4869e461d919..7511c1516a1d 100644 --- a/src/hooks/useDeepCompareRef.ts +++ b/src/hooks/useDeepCompareRef.ts @@ -1,5 +1,5 @@ -import { useRef } from 'react'; import isEqual from 'lodash/isEqual'; +import {useRef} from 'react'; /** * This hook returns a reference to the provided value, @@ -16,9 +16,9 @@ import isEqual from 'lodash/isEqual'; * }, [deepComparedArray]); */ export default function useDeepCompareRef(value: T): T | undefined { - const ref = useRef(); - if (!isEqual(value, ref.current)) { - ref.current = value; - } - return ref.current; -} \ No newline at end of file + const ref = useRef(); + if (!isEqual(value, ref.current)) { + ref.current = value; + } + return ref.current; +} diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 2825bfaeb1d0..414745270b68 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -21,6 +21,7 @@ import TaskHeaderActionButton from '@components/TaskHeaderActionButton'; import type {CurrentReportIDContextValue} from '@components/withCurrentReportID'; import withCurrentReportID from '@components/withCurrentReportID'; import useAppFocusEvent from '@hooks/useAppFocusEvent'; +import useDeepCompareRef from '@hooks/useDeepCompareRef'; import useIsReportOpenInRHP from '@hooks/useIsReportOpenInRHP'; import useLocalize from '@hooks/useLocalize'; import useNetwork from '@hooks/useNetwork'; @@ -47,7 +48,6 @@ import type SCREENS from '@src/SCREENS'; import type * as OnyxTypes from '@src/types/onyx'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue'; -import useDeepCompareRef from '@hooks/useDeepCompareRef'; import HeaderView from './HeaderView'; import ReportActionsView from './report/ReportActionsView'; import ReportFooter from './report/ReportFooter'; From 1706a7389aeea3323090565c67e0ca882973ef32 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Tue, 4 Jun 2024 17:40:50 -0700 Subject: [PATCH 9/9] Only show optimistic composer for non-anon user --- src/pages/home/report/ReportFooter.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportFooter.tsx b/src/pages/home/report/ReportFooter.tsx index d8bce6b77748..decd8f1b8d56 100644 --- a/src/pages/home/report/ReportFooter.tsx +++ b/src/pages/home/report/ReportFooter.tsx @@ -95,7 +95,7 @@ function ReportFooter({ const isSmallSizeLayout = windowWidth - (isSmallScreenWidth ? 0 : variables.sideBarWidth) < variables.anonymousReportFooterBreakpoint; // If a user just signed in and is viewing a public report, optimistically show the composer while loading the report, since they will have write access when the response comes back. - const showComposerOptimistically = ReportUtils.isPublicRoom(report) && reportMetadata?.isLoadingInitialReportActions; + const showComposerOptimistically = !isAnonymousUser && ReportUtils.isPublicRoom(report) && reportMetadata?.isLoadingInitialReportActions; const hideComposer = (!ReportUtils.canUserPerformWriteAction(report, reportNameValuePairs) && !showComposerOptimistically) || blockedFromChat; const canWriteInReport = ReportUtils.canWriteInReport(report); const isSystemChat = ReportUtils.isSystemChat(report);