-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Remove onyx.connect from DraftCommentUtils #70409
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
Changes from all commits
1bb121d
29de46e
7f67e2b
6a10149
178792b
9b22733
44b77ea
8db51eb
226af7c
046c912
e36b95e
fd11513
73a33c8
edb9524
4c7ff24
6e47d79
59dc622
e05275f
3884307
104f226
52c0c5d
a71b87f
32f5741
53cab16
abfbd93
cd1a50e
00dc392
2f521e6
6db7638
bcc85e5
ad8bb86
dea6dc1
a1489ec
23ea45b
c8e5abd
c556132
ff33459
4162b83
9fee042
ff5d3cc
64185bb
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 |
|---|---|---|
|
|
@@ -71,11 +71,10 @@ function SidebarOrderedReportsContextProvider({ | |
| const [transactions, {sourceValue: transactionsUpdates}] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION, {canBeMissing: true}); | ||
| const [transactionViolations, {sourceValue: transactionViolationsUpdates}] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, {canBeMissing: true}); | ||
| const [reportNameValuePairs, {sourceValue: reportNameValuePairsUpdates}] = useOnyx(ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS, {canBeMissing: true}); | ||
| const [, {sourceValue: reportsDraftsUpdates}] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, {canBeMissing: true}); | ||
| const [reportsDrafts, {sourceValue: reportsDraftsUpdates}] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, {canBeMissing: true}); | ||
| const [betas] = useOnyx(ONYXKEYS.BETAS, {canBeMissing: true}); | ||
| const [reportAttributes] = useOnyx(ONYXKEYS.DERIVED.REPORT_ATTRIBUTES, {selector: reportsSelector, canBeMissing: true}); | ||
| const [currentReportsToDisplay, setCurrentReportsToDisplay] = useState<ReportsToDisplayInLHN>({}); | ||
|
|
||
| const {shouldUseNarrowLayout} = useResponsiveLayout(); | ||
| const {accountID} = useCurrentUserPersonalDetails(); | ||
| const currentReportIDValue = useCurrentReportID(); | ||
|
|
@@ -152,25 +151,26 @@ function SidebarOrderedReportsContextProvider({ | |
| const shouldDoIncrementalUpdate = updatedReports.length > 0 && Object.keys(currentReportsToDisplay).length > 0; | ||
| let reportsToDisplay = {}; | ||
| if (shouldDoIncrementalUpdate) { | ||
| reportsToDisplay = SidebarUtils.updateReportsToDisplayInLHN( | ||
| currentReportsToDisplay, | ||
| chatReports, | ||
| updatedReports, | ||
| derivedCurrentReportID, | ||
| priorityMode === CONST.PRIORITY_MODE.GSD, | ||
| reportsToDisplay = SidebarUtils.updateReportsToDisplayInLHN({ | ||
| displayedReports: currentReportsToDisplay, | ||
| reports: chatReports, | ||
| updatedReportsKeys: updatedReports, | ||
| currentReportId: derivedCurrentReportID, | ||
| isInFocusMode: priorityMode === CONST.PRIORITY_MODE.GSD, | ||
| betas, | ||
| policies, | ||
| transactionViolations, | ||
| reportNameValuePairs, | ||
| reportAttributes, | ||
| ); | ||
| draftComments: reportsDrafts, | ||
| }); | ||
| } else { | ||
| reportsToDisplay = SidebarUtils.getReportsToDisplayInLHN( | ||
| derivedCurrentReportID, | ||
| chatReports, | ||
| betas, | ||
| policies, | ||
| priorityMode, | ||
| reportsDrafts, | ||
| transactionViolations, | ||
| reportNameValuePairs, | ||
| reportAttributes, | ||
|
|
@@ -180,19 +180,20 @@ function SidebarOrderedReportsContextProvider({ | |
| return reportsToDisplay; | ||
| // Rule disabled intentionally — triggering a re-render on currentReportsToDisplay would cause an infinite loop | ||
| // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps | ||
| }, [getUpdatedReports, chatReports, derivedCurrentReportID, priorityMode, betas, policies, transactionViolations, reportNameValuePairs, reportAttributes]); | ||
| }, [getUpdatedReports, chatReports, derivedCurrentReportID, priorityMode, betas, policies, transactionViolations, reportNameValuePairs, reportAttributes, reportsDrafts]); | ||
|
|
||
| const deepComparedReportsToDisplayInLHN = useDeepCompareRef(reportsToDisplayInLHN); | ||
| const deepComparedReportsDrafts = useDeepCompareRef(reportsDrafts); | ||
|
|
||
| useEffect(() => { | ||
| setCurrentReportsToDisplay(reportsToDisplayInLHN); | ||
| }, [reportsToDisplayInLHN]); | ||
|
|
||
| const getOrderedReportIDs = useCallback( | ||
| () => SidebarUtils.sortReportsToDisplayInLHN(deepComparedReportsToDisplayInLHN ?? {}, priorityMode, localeCompare, reportNameValuePairs, reportAttributes), | ||
| () => SidebarUtils.sortReportsToDisplayInLHN(deepComparedReportsToDisplayInLHN ?? {}, priorityMode, localeCompare, deepComparedReportsDrafts, reportNameValuePairs, reportAttributes), | ||
| // Rule disabled intentionally - reports should be sorted only when the reportsToDisplayInLHN changes | ||
| // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps | ||
| [reportsToDisplayInLHN, localeCompare], | ||
| [deepComparedReportsToDisplayInLHN, localeCompare, deepComparedReportsDrafts], | ||
|
Contributor
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. Why do we need to add deepComparedReportsDrafts to dependency list? It is intentional to keep reportsToDisplayInLHN and localeCompare only in the dependency list (see detail here: #63980)
Contributor
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. @kubabutkiewicz Kindly bump
Contributor
Author
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. Yes I am on it, earlier without
Contributor
Author
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. Ok, I was able to test this, so without |
||
| ); | ||
|
|
||
| const orderedReportIDs = useMemo(() => getOrderedReportIDs(), [getOrderedReportIDs]); | ||
|
|
||
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.