-
Notifications
You must be signed in to change notification settings - Fork 3k
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 9 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,21 @@ 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; | ||
if (isLoading && !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]); | ||
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. @s-alves10 can you add some comments trying to say why we using this ref and notes on each of the cases you have there? It is just not very easy to understand why we have this complexity 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. Comments added |
||
|
||
return ( | ||
<View | ||
|
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.
you can just use
style: {styles.flex1}
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