-
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 wrong scrolling in Search list when navigating by keyboard #43490
Changes from 4 commits
9da4042
c41ca39
a97b14d
8575219
c8904da
f50473d
5f1a9be
eca4735
bd00445
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 |
---|---|---|
|
@@ -116,9 +116,11 @@ function getReportSections(data: OnyxTypes.SearchResults['data']): ReportListIte | |
if (key.startsWith(ONYXKEYS.COLLECTION.REPORT)) { | ||
const value = {...data[key]}; | ||
const reportKey = `${ONYXKEYS.COLLECTION.REPORT}${value.reportID}`; | ||
const transactions = reportIDToTransactions[reportKey]?.transactions ?? []; | ||
|
||
reportIDToTransactions[reportKey] = { | ||
...value, | ||
transactions: reportIDToTransactions[reportKey]?.transactions ?? [], | ||
transactions, | ||
}; | ||
} else if (key.startsWith(ONYXKEYS.COLLECTION.TRANSACTION)) { | ||
const transactionItem = {...data[key]}; | ||
|
@@ -155,7 +157,7 @@ function getReportSections(data: OnyxTypes.SearchResults['data']): ReportListIte | |
} | ||
} | ||
|
||
return Object.values(reportIDToTransactions); | ||
return Object.values(reportIDToTransactions).filter((item) => item.transactions?.length !== 0); | ||
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. We started filtering this on the backend, so we shouldn't have to do it in the frontend anymore. 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. Fair enough but I found such cases that fallen into this if: https://github.com/Expensify/App/blob/main/src/components/SelectionList/Search/ReportListItem.tsx#L82 or the other one. At lest for me I can see empty list elements. 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 put up a fix for this, so we should be able to remove the filter once my fix is deployed.
Kicu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
const searchTypeToItemMap: SearchTypeToItemMap = { | ||
|
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.
NAB This feels brittle since if the height changes we would introduce the bug again. That being said, I'm not sure how to solve it this.
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.
Exactly same for me. We could in some way extract the same variable from styles perhaps and use, but still you would have to change it in two places.
I agree that its brittle but I see no other simple solution. padding affects item height, and correct item height has to be pushed down to RN List