-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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.
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
fix: show existing reports while loading #25159
Changes from 16 commits
4da6777
f36ca39
d8a65a2
de433a4
01dda2f
503af34
b35a98f
8058d02
e8322ca
7ffa793
bce3738
e73cc36
261b811
2734584
7c0f96b
52070c8
54bfb83
2d85e65
243844e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,17 +66,23 @@ const defaultProps = { | |
function SidebarLinksData({isFocused, allReportActions, betas, chatReports, currentReportID, insets, isLoadingReportData, isSmallScreenWidth, onLinkClick, policies, priorityMode}) { | ||
const {translate} = useLocalize(); | ||
|
||
const reportIDsRef = useRef([]); | ||
const reportIDsRef = useRef(null); | ||
const isLoading = SessionUtils.didUserLogInDuringSession() && isLoadingReportData; | ||
const optionListItems = useMemo(() => { | ||
const reportIDs = SidebarUtils.getOrderedReportIDs(currentReportID, chatReports, betas, policies, priorityMode, allReportActions); | ||
if (deepEqual(reportIDsRef.current, reportIDs)) { | ||
return reportIDsRef.current; | ||
} | ||
reportIDsRef.current = reportIDs; | ||
return reportIDs; | ||
}, [allReportActions, betas, chatReports, currentReportID, policies, priorityMode]); | ||
|
||
const isLoading = SessionUtils.didUserLogInDuringSession() && isLoadingReportData; | ||
// We need to update existing reports only once while loading because they are updated several times during loading and causes this reguression: https://github.com/Expensify/App/issues/24596#issuecomment-1681679531 | ||
if ((isLoading && !reportIDsRef.current) || (_.isEmpty(reportIDsRef.current) && currentReportID)) { | ||
reportIDsRef.current = reportIDs; | ||
} | ||
if (!isLoading) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think you can move this to 78 line too like if (long condition || !isLoading) |
||
reportIDsRef.current = reportIDs; | ||
} | ||
return reportIDsRef.current || []; | ||
}, [allReportActions, betas, chatReports, currentReportID, policies, priorityMode, isLoading]); | ||
|
||
return ( | ||
<View | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,9 @@ const propTypes = { | |
}), | ||
}), | ||
|
||
/** Recent waypoints array */ | ||
recentWaypoints: PropTypes.arrayOf(PropTypes.object), | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bad conflicts resolution? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was the source of lint error. I resolved the conflicts There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will you take a look again? |
||
/** The optimistic transaction for this request */ | ||
transaction: transactionPropTypes, | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated