Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Separate ReportScreenContext to prevent cyclic re-renders #25948

Closed
wants to merge 5 commits into from
Closed

Separate ReportScreenContext to prevent cyclic re-renders #25948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3be5e69
Separate ReportScreenContext to prevent cyclic re-renders
ospfranco Aug 25, 2023
6125c38
Get rid of full list re-render when marking messages as unread
ospfranco Aug 28, 2023
be7ea56
Linting
ospfranco Aug 28, 2023
caed994
Merge branch 'main' into osp/ReportScreen-optimizations
ospfranco Aug 29, 2023
37926ee
Merge branch 'main' into osp/ReportScreen-optimizations
ospfranco Aug 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/components/Reactions/ReportActionItemEmojiReactions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import EmojiReactionsPropTypes from './EmojiReactionsPropTypes';
import Tooltip from '../Tooltip';
import ReactionTooltipContent from './ReactionTooltipContent';
import * as EmojiUtils from '../../libs/EmojiUtils';
import ReportScreenContext from '../../pages/home/ReportScreenContext';
import {ReactionListContext} from '../../pages/home/ReportScreenContext';

const propTypes = {
emojiReactions: EmojiReactionsPropTypes,
Expand Down Expand Up @@ -41,7 +41,7 @@ const defaultProps = {
};

function ReportActionItemEmojiReactions(props) {
const {reactionListRef} = useContext(ReportScreenContext);
const reactionListRef = useContext(ReactionListContext);
const popoverReactionListAnchor = useRef(null);
let totalReactionCount = 0;

Expand Down
4 changes: 2 additions & 2 deletions src/hooks/useReportScrollManager/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {useContext, useCallback} from 'react';
import ReportScreenContext from '../../pages/home/ReportScreenContext';
import {ActionListContext} from '../../pages/home/ReportScreenContext';

function useReportScrollManager() {
const {flatListRef} = useContext(ReportScreenContext);
const flatListRef = useContext(ActionListContext);

/**
* Scroll to the provided index. On non-native implementations we do not want to scroll when we are scrolling because
Expand Down
4 changes: 2 additions & 2 deletions src/hooks/useReportScrollManager/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {useContext, useCallback} from 'react';
import ReportScreenContext from '../../pages/home/ReportScreenContext';
import {ActionListContext} from '../../pages/home/ReportScreenContext';

function useReportScrollManager() {
const {flatListRef} = useContext(ReportScreenContext);
const flatListRef = useContext(ActionListContext);

/**
* Scroll to the provided index.
Expand Down
177 changes: 87 additions & 90 deletions src/pages/home/ReportScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import getIsReportFullyVisible from '../../libs/getIsReportFullyVisible';
import MoneyRequestHeader from '../../components/MoneyRequestHeader';
import MoneyReportHeader from '../../components/MoneyReportHeader';
import * as ComposerActions from '../../libs/actions/Composer';
import ReportScreenContext from './ReportScreenContext';
import {ActionListContext, ReactionListContext} from './ReportScreenContext';
import TaskHeaderActionButton from '../../components/TaskHeaderActionButton';
import DragAndDropProvider from '../../components/DragAndDrop/Provider';
import usePrevious from '../../hooks/usePrevious';
Expand Down Expand Up @@ -285,105 +285,102 @@ function ReportScreen({
}, [route, report, errors, fetchReportIfNeeded, prevReport.reportID]);

return (
<ReportScreenContext.Provider
value={{
flatListRef,
reactionListRef,
}}
>
<ScreenWrapper
style={screenWrapperStyle}
shouldEnableKeyboardAvoidingView={isTopMostReportId}
>
<FullPageNotFoundView
shouldShow={(!report.reportID && !report.isLoadingReportActions && !isLoading) || shouldHideReport}
subtitleKey="notFound.noAccess"
shouldShowCloseButton={false}
shouldShowBackButton={isSmallScreenWidth}
onBackButtonPress={Navigation.goBack}
shouldShowLink={false}
<ActionListContext.Provider value={flatListRef}>
<ReactionListContext.Provider value={reactionListRef}>
<ScreenWrapper
style={screenWrapperStyle}
shouldEnableKeyboardAvoidingView={isTopMostReportId}
>
<OfflineWithFeedback
pendingAction={addWorkspaceRoomOrChatPendingAction}
errors={addWorkspaceRoomOrChatErrors}
shouldShowErrorMessages={false}
needsOffscreenAlphaCompositing
<FullPageNotFoundView
shouldShow={(!report.reportID && !report.isLoadingReportActions && !isLoading) || shouldHideReport}
subtitleKey="notFound.noAccess"
shouldShowCloseButton={false}
shouldShowBackButton={isSmallScreenWidth}
onBackButtonPress={Navigation.goBack}
shouldShowLink={false}
>
{headerView}
{ReportUtils.isTaskReport(report) && isSmallScreenWidth && ReportUtils.isOpenTaskReport(report) && (
<View style={[styles.borderBottom]}>
<View style={[styles.appBG, styles.pl0]}>
<View style={[styles.ph5, styles.pb3]}>
<TaskHeaderActionButton report={report} />
<OfflineWithFeedback
pendingAction={addWorkspaceRoomOrChatPendingAction}
errors={addWorkspaceRoomOrChatErrors}
shouldShowErrorMessages={false}
needsOffscreenAlphaCompositing
>
{headerView}
{ReportUtils.isTaskReport(report) && isSmallScreenWidth && ReportUtils.isOpenTaskReport(report) && (
<View style={[styles.borderBottom]}>
<View style={[styles.appBG, styles.pl0]}>
<View style={[styles.ph5, styles.pb3]}>
<TaskHeaderActionButton report={report} />
</View>
</View>
</View>
</View>
)}
</OfflineWithFeedback>
{Boolean(accountManagerReportID) && ReportUtils.isConciergeChatReport(report) && isBannerVisible && (
<Banner
containerStyles={[styles.mh4, styles.mt4, styles.p4, styles.bgDark]}
textStyles={[styles.colorReversed]}
text={translate('reportActionsView.chatWithAccountManager')}
onClose={dismissBanner}
onPress={chatWithAccountManager}
shouldShowCloseButton
/>
)}
<DragAndDropProvider isDisabled={!isReportReadyForDisplay}>
<View
style={[styles.flex1, styles.justifyContentEnd, styles.overflowHidden]}
onLayout={(event) => {
// Rounding this value for comparison because they can look like this: 411.9999694824219
const newSkeletonViewContainerHeight = Math.round(event.nativeEvent.layout.height);

// The height can be 0 if the component unmounts - we are not interested in this value and want to know how much space it
// takes up so we can set the skeleton view container height.
if (newSkeletonViewContainerHeight === 0) {
return;
}
setSkeletonViewContainerHeight(newSkeletonViewContainerHeight);
}}
>
{isReportReadyForDisplay && !isLoadingInitialReportActions && !isLoading && (
<ReportActionsView
reportActions={reportActions}
report={report}
isComposerFullSize={isComposerFullSize}
parentViewHeight={skeletonViewContainerHeight}
policy={policy}
/>
)}

{/* Note: The report should be allowed to mount even if the initial report actions are not loaded. If we prevent rendering the report while they are loading then
we'll unnecessarily unmount the ReportActionsView which will clear the new marker lines initial state. */}
{(!isReportReadyForDisplay || isLoadingInitialReportActions || isLoading) && <ReportActionsSkeletonView containerHeight={skeletonViewContainerHeight} />}

{isReportReadyForDisplay && (
<>
<ReportFooter
pendingAction={addWorkspaceRoomOrChatPendingAction}
isOffline={network.isOffline}
</OfflineWithFeedback>
{Boolean(accountManagerReportID) && ReportUtils.isConciergeChatReport(report) && isBannerVisible && (
<Banner
containerStyles={[styles.mh4, styles.mt4, styles.p4, styles.bgDark]}
textStyles={[styles.colorReversed]}
text={translate('reportActionsView.chatWithAccountManager')}
onClose={dismissBanner}
onPress={chatWithAccountManager}
shouldShowCloseButton
/>
)}
<DragAndDropProvider isDisabled={!isReportReadyForDisplay}>
<View
style={[styles.flex1, styles.justifyContentEnd, styles.overflowHidden]}
onLayout={(event) => {
// Rounding this value for comparison because they can look like this: 411.9999694824219
const newSkeletonViewContainerHeight = Math.round(event.nativeEvent.layout.height);

// The height can be 0 if the component unmounts - we are not interested in this value and want to know how much space it
// takes up so we can set the skeleton view container height.
if (newSkeletonViewContainerHeight === 0) {
return;
}
setSkeletonViewContainerHeight(newSkeletonViewContainerHeight);
}}
>
{isReportReadyForDisplay && !isLoadingInitialReportActions && !isLoading && (
<ReportActionsView
reportActions={reportActions}
report={report}
isComposerFullSize={isComposerFullSize}
onSubmitComment={onSubmitComment}
policies={policies}
parentViewHeight={skeletonViewContainerHeight}
policy={policy}
/>
</>
)}
)}

{!isReportReadyForDisplay && (
<ReportFooter
shouldDisableCompose
isOffline={network.isOffline}
/>
)}
</View>
</DragAndDropProvider>
</FullPageNotFoundView>
</ScreenWrapper>
</ReportScreenContext.Provider>
{/* Note: The report should be allowed to mount even if the initial report actions are not loaded. If we prevent rendering the report while they are loading then
we'll unnecessarily unmount the ReportActionsView which will clear the new marker lines initial state. */}
{(!isReportReadyForDisplay || isLoadingInitialReportActions || isLoading) && <ReportActionsSkeletonView containerHeight={skeletonViewContainerHeight} />}

{isReportReadyForDisplay && (
<>
<ReportFooter
pendingAction={addWorkspaceRoomOrChatPendingAction}
isOffline={network.isOffline}
reportActions={reportActions}
report={report}
isComposerFullSize={isComposerFullSize}
onSubmitComment={onSubmitComment}
policies={policies}
/>
</>
)}

{!isReportReadyForDisplay && (
<ReportFooter
shouldDisableCompose
isOffline={network.isOffline}
/>
)}
</View>
</DragAndDropProvider>
</FullPageNotFoundView>
</ScreenWrapper>
</ReactionListContext.Provider>
</ActionListContext.Provider>
);
}

Expand Down
6 changes: 4 additions & 2 deletions src/pages/home/ReportScreenContext.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import {createContext} from 'react';

const ReportScreenContext = createContext();
export default ReportScreenContext;
const ActionListContext = createContext();
const ReactionListContext = createContext();

export {ActionListContext, ReactionListContext};
4 changes: 2 additions & 2 deletions src/pages/home/report/ReportActionItem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ import * as PersonalDetailsUtils from '../../../libs/PersonalDetailsUtils';
import ReportActionItemBasicMessage from './ReportActionItemBasicMessage';
import * as store from '../../../libs/actions/ReimbursementAccount/store';
import * as BankAccounts from '../../../libs/actions/BankAccounts';
import ReportScreenContext from '../ReportScreenContext';
import {ReactionListContext} from '../ReportScreenContext';
import Permissions from '../../../libs/Permissions';

const propTypes = {
Expand Down Expand Up @@ -127,7 +127,7 @@ function ReportActionItem(props) {
const [isContextMenuActive, setIsContextMenuActive] = useState(ReportActionContextMenu.isActiveReportAction(props.action.reportActionID));
const [isHidden, setIsHidden] = useState(false);
const [moderationDecision, setModerationDecision] = useState(CONST.MODERATION.MODERATOR_DECISION_APPROVED);
const {reactionListRef} = useContext(ReportScreenContext);
const reactionListRef = useContext(ReactionListContext);
const textInputRef = useRef();
const popoverAnchorRef = useRef();
const downloadedPreviews = useRef([]);
Expand Down
23 changes: 11 additions & 12 deletions src/pages/home/report/ReportActionsList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ function ReportActionsList({
const readActionSkipped = useRef(false);
const reportActionSize = useRef(sortedReportActions.length);

// Considering that renderItem is enclosed within a useCallback, marking it as "read" twice will retain the value as "true," preventing the useCallback from re-executing.
// However, if we create and listen to an object, it will lead to a new useCallback execution.
const [messageManuallyMarked, setMessageManuallyMarked] = useState({read: false});
// This state is used to force a re-render when the user manually marks a message as unread
// by using a timestamp you can force re-renders without having to worry about if another message was marked as unread before
const [messageManuallyMarkedUnread, setMessageManuallyMarkedUnread] = useState(0);
const [isFloatingMessageCounterVisible, setIsFloatingMessageCounterVisible] = useState(false);
const animatedStyles = useAnimatedStyle(() => ({
opacity: opacity.value,
Expand Down Expand Up @@ -163,15 +163,14 @@ function ReportActionsList({

useEffect(() => {
const didManuallyMarkReportAsUnread = report.lastReadTime < DateUtils.getDBTime() && ReportUtils.isUnread(report);
if (!didManuallyMarkReportAsUnread) {
setMessageManuallyMarked({read: false});
return;
if (didManuallyMarkReportAsUnread) {
// Clearing the current unread marker so that it can be recalculated
currentUnreadMarker.current = null;
setMessageManuallyMarkedUnread(new Date().getTime());
} else {
setMessageManuallyMarkedUnread(0);
}

// Clearing the current unread marker so that it can be recalculated
currentUnreadMarker.current = null;
setMessageManuallyMarked({read: true});

// We only care when a new lastReadTime is set in the report
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [report.lastReadTime]);
Expand Down Expand Up @@ -230,7 +229,7 @@ function ReportActionsList({
const isCurrentMessageUnread = isMessageUnread(reportAction, report.lastReadTime);
shouldDisplayNewMarker = isCurrentMessageUnread && !isMessageUnread(nextMessage, report.lastReadTime);

if (!messageManuallyMarked.read) {
if (!messageManuallyMarkedUnread) {
shouldDisplayNewMarker = shouldDisplayNewMarker && reportAction.actorAccountID !== Report.getCurrentUserAccountID();
}
const canDisplayMarker = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < userActiveSince.current : true;
Expand Down Expand Up @@ -272,7 +271,7 @@ function ReportActionsList({
/>
);
},
[report, hasOutstandingIOU, sortedReportActions, mostRecentIOUReportActionID, messageManuallyMarked],
[report, hasOutstandingIOU, sortedReportActions, mostRecentIOUReportActionID, messageManuallyMarkedUnread],
);

// Native mobile does not render updates flatlist the changes even though component did update called.
Expand Down
8 changes: 3 additions & 5 deletions src/pages/home/report/ReportActionsView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import * as ReportActionsUtils from '../../../libs/ReportActionsUtils';
import reportPropTypes from '../../reportPropTypes';
import PopoverReactionList from './ReactionList/PopoverReactionList';
import getIsReportFullyVisible from '../../../libs/getIsReportFullyVisible';
import ReportScreenContext from '../ReportScreenContext';
import {ReactionListContext} from '../ReportScreenContext';

const propTypes = {
/** The report currently being looked at */
Expand Down Expand Up @@ -54,10 +54,8 @@ const defaultProps = {
};

function ReportActionsView(props) {
const context = useContext(ReportScreenContext);

useCopySelectionHelper();

const reactionListRef = useContext(ReactionListContext);
const didLayout = useRef(false);
const didSubscribeToReportTypingEvents = useRef(false);
const hasCachedActions = useRef(_.size(props.reportActions) > 0);
Expand Down Expand Up @@ -189,7 +187,7 @@ function ReportActionsView(props) {
loadMoreChats={loadMoreChats}
policy={props.policy}
/>
<PopoverReactionList ref={context.reactionListRef} />
<PopoverReactionList ref={reactionListRef} />
</>
);
}
Expand Down
Loading