Skip to content

Commit

Permalink
Merge pull request #21227 from bernhardoj/fix/20847-scroll-manager
Browse files Browse the repository at this point in the history
Fix crash when edit a message after coming back from a child report
  • Loading branch information
amyevans authored Jun 26, 2023
2 parents d1b3869 + 90dee15 commit c261901
Show file tree
Hide file tree
Showing 14 changed files with 254 additions and 171 deletions.
4 changes: 2 additions & 2 deletions src/components/Reactions/ReportActionItemReactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import * as Report from '../../libs/actions/Report';
import Tooltip from '../Tooltip';
import ReactionTooltipContent from './ReactionTooltipContent';
import * as EmojiUtils from '../../libs/EmojiUtils';
import ReactionListRefContext from '../../pages/home/report/ReactionList/ReactionListRefContext';
import ReportScreenContext from '../../pages/home/ReportScreenContext';

const propTypes = {
/**
Expand Down Expand Up @@ -47,7 +47,7 @@ const defaultProps = {
};

function ReportActionItemReactions(props) {
const reactionListRef = useContext(ReactionListRefContext);
const {reactionListRef} = useContext(ReportScreenContext);
const popoverReactionListAnchor = useRef(null);
const reactionsWithCount = _.filter(props.reactions, (reaction) => reaction.users.length > 0);

Expand Down
36 changes: 36 additions & 0 deletions src/hooks/useReportScrollManager/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import {useContext} from 'react';
import ReportScreenContext from '../../pages/home/ReportScreenContext';

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

/**
* Scroll to the provided index. On non-native implementations we do not want to scroll when we are scrolling because
* we are editing a comment.
*
* @param {Object} index
* @param {Boolean} isEditing
*/
const scrollToIndex = (index, isEditing) => {
if (!flatListRef.current || isEditing) {
return;
}

flatListRef.current.scrollToIndex(index);
};

/**
* Scroll to the bottom of the flatlist.
*/
const scrollToBottom = () => {
if (!flatListRef.current) {
return;
}

flatListRef.current.scrollToOffset({animated: false, offset: 0});
};

return {ref: flatListRef, scrollToIndex, scrollToBottom};
}

export default useReportScrollManager;
34 changes: 34 additions & 0 deletions src/hooks/useReportScrollManager/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import {useContext} from 'react';
import ReportScreenContext from '../../pages/home/ReportScreenContext';

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

/**
* Scroll to the provided index.
*
* @param {Object} index
*/
const scrollToIndex = (index) => {
if (!flatListRef.current) {
return;
}

flatListRef.current.scrollToIndex(index);
};

/**
* Scroll to the bottom of the flatlist.
*/
const scrollToBottom = () => {
if (!flatListRef.current) {
return;
}

flatListRef.current.scrollToOffset({animated: false, offset: 0});
};

return {ref: flatListRef, scrollToIndex, scrollToBottom};
}

export default useReportScrollManager;
22 changes: 9 additions & 13 deletions src/libs/ReportScrollManager/index.js
Original file line number Diff line number Diff line change
@@ -1,34 +1,30 @@
import React from 'react';

// This ref is created using React.createRef here because this function is used by a component that doesn't have access
// to the original ref.
const flatListRef = React.createRef();

/**
* Scroll to the provided index. On non-native implementations we do not want to scroll when we are scrolling because
* we are editing a comment.
*
* @param {Object} ref
* @param {Object} index
* @param {Boolean} isEditing
*/
function scrollToIndex(index, isEditing) {
if (isEditing) {
function scrollToIndex(ref, index, isEditing) {
if (!ref.current || isEditing) {
return;
}

flatListRef.current.scrollToIndex(index);
ref.current.scrollToIndex(index);
}

/**
* Scroll to the bottom of the flatlist.
*
* @param {Object} ref
*/
function scrollToBottom() {
if (!flatListRef.current) {
function scrollToBottom(ref) {
if (!ref.current) {
return;
}

flatListRef.current.scrollToOffset({animated: false, offset: 0});
ref.current.scrollToOffset({animated: false, offset: 0});
}

export {flatListRef, scrollToIndex, scrollToBottom};
export {scrollToIndex, scrollToBottom};
24 changes: 12 additions & 12 deletions src/libs/ReportScrollManager/index.native.js
Original file line number Diff line number Diff line change
@@ -1,28 +1,28 @@
import React from 'react';

// This ref is created using React.createRef here because this function is used by a component that doesn't have access
// to the original ref.
const flatListRef = React.createRef();

/**
* Scroll to the provided index.
*
* @param {Object} ref
* @param {Object} index
*/
function scrollToIndex(index) {
flatListRef.current.scrollToIndex(index);
function scrollToIndex(ref, index) {
if (!ref.current) {
return;
}

ref.current.scrollToIndex(index);
}

/**
* Scroll to the bottom of the flatlist.
*
* @param {Object} ref
*/
function scrollToBottom() {
if (!flatListRef.current) {
function scrollToBottom(ref) {
if (!ref.current) {
return;
}

flatListRef.current.scrollToOffset({animated: false, offset: 0});
ref.current.scrollToOffset({animated: false, offset: 0});
}

export {flatListRef, scrollToIndex, scrollToBottom};
export {scrollToIndex, scrollToBottom};
7 changes: 7 additions & 0 deletions src/libs/actions/IOU.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import DateUtils from '../DateUtils';
import TransactionUtils from '../TransactionUtils';
import * as ErrorUtils from '../ErrorUtils';
import * as UserUtils from '../UserUtils';
import * as Report from './Report';

const chatReports = {};
const iouReports = {};
Expand Down Expand Up @@ -427,6 +428,7 @@ function requestMoney(report, amount, currency, payeeEmail, payeeAccountID, part
);
resetMoneyRequestInfo();
Navigation.dismissModal(chatReport.reportID);
Report.notifyNewAction(chatReport.reportID, payeeAccountID);
}

/**
Expand Down Expand Up @@ -733,6 +735,7 @@ function splitBill(participants, currentUserLogin, currentUserAccountID, amount,

resetMoneyRequestInfo();
Navigation.dismissModal();
Report.notifyNewAction(groupData.chatReportID, currentUserAccountID);
}

/**
Expand Down Expand Up @@ -763,6 +766,7 @@ function splitBillAndOpenReport(participants, currentUserLogin, currentUserAccou

resetMoneyRequestInfo();
Navigation.dismissModal(groupData.chatReportID);
Report.notifyNewAction(groupData.chatReportID, currentUserAccountID);
}

/**
Expand Down Expand Up @@ -1227,6 +1231,7 @@ function sendMoneyElsewhere(report, amount, currency, comment, managerID, recipi

resetMoneyRequestInfo();
Navigation.dismissModal(params.chatReportID);
Report.notifyNewAction(params.chatReportID, managerID);
}

/**
Expand All @@ -1244,6 +1249,7 @@ function sendMoneyWithWallet(report, amount, currency, comment, managerID, recip

resetMoneyRequestInfo();
Navigation.dismissModal(params.chatReportID);
Report.notifyNewAction(params.chatReportID, managerID);
}

/**
Expand All @@ -1261,6 +1267,7 @@ function sendMoneyViaPaypal(report, amount, currency, comment, managerID, recipi

resetMoneyRequestInfo();
Navigation.dismissModal(params.chatReportID);
Report.notifyNewAction(params.chatReportID, managerID);

asyncOpenURL(Promise.resolve(), buildPayPalPaymentUrl(amount, recipient.payPalMeAddress, currency));
}
Expand Down
30 changes: 18 additions & 12 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,21 @@ function subscribeToNewActionEvent(reportID, callback) {
};
}

/**
* Notify the ReportActionsView that a new comment has arrived
*
* @param {String} reportID
* @param {Number} accountID
* @param {String} reportActionID
*/
function notifyNewAction(reportID, accountID, reportActionID) {
if (reportID !== newActionSubscriber.reportID) {
return;
}
const isFromCurrentUser = accountID === currentUserAccountID;
newActionSubscriber.callback(isFromCurrentUser, reportActionID);
}

/**
* Add up to two report actions to a report. This method can be called for the following situations:
*
Expand Down Expand Up @@ -307,12 +322,7 @@ function addActions(reportID, text = '', file) {
successData,
failureData,
});

// Notify the ReportActionsView that a new comment has arrived
if (reportID === newActionSubscriber.reportID) {
const isFromCurrentUser = lastAction.actorAccountID === currentUserAccountID;
newActionSubscriber.callback(isFromCurrentUser, lastAction.reportActionID);
}
notifyNewAction(reportID, lastAction.actorAccountID, lastAction.reportActionID);
}

/**
Expand Down Expand Up @@ -1458,12 +1468,7 @@ function showReportActionNotification(reportID, action) {
Navigation.navigate(ROUTES.getReportRoute(reportID));
},
});

// Notify the ReportActionsView that a new comment has arrived
if (reportID === newActionSubscriber.reportID) {
const isFromCurrentUser = action.actorAccountID === currentUserAccountID;
newActionSubscriber.callback(isFromCurrentUser, action.reportActionID);
}
notifyNewAction(reportID, action.actorAccountID, action.reportActionID);
}

/**
Expand Down Expand Up @@ -1853,6 +1858,7 @@ export {
clearPolicyRoomNameErrors,
clearIOUError,
subscribeToNewActionEvent,
notifyNewAction,
showReportActionNotification,
addEmojiReaction,
removeEmojiReaction,
Expand Down
Loading

0 comments on commit c261901

Please sign in to comment.