Revert "[Perf] Scope REPORT_ACTIONS subscription to per-row level in LHNOptionsList "#86651
Conversation
melvin-bot
bot
requested review from
srikarparsi and
trjExpensify
and removed request for
a team
|
@srikarparsi Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea46233abb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| reportActions, | ||
| visibleReportActionsData, | ||
| itemOneTransactionThreadReport?.reportID, | ||
| ); |
There was a problem hiding this comment.
Passing itemOneTransactionThreadReport?.reportID drops the transaction-thread context whenever the child report object is not yet hydrated in reports, even though reportAttributes already contains oneTransactionThreadReportID. In that state, getLastVisibleActionIncludingTransactionThread() ignores thread actions and can show stale/missing LHN preview text (and related metadata) until hydration completes. Use the derived thread ID directly instead of reading it from the optional report object.
Useful? React with 👍 / 👎.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
PR doesn’t need product input as a perf PR. Unassigning and unsubscribing myself. |
|
@srikarparsi looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
Straight revert |
|
🚧 @srikarparsi has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
4 participants
Reverts #84262 because even if it reduces the LHN rerenders when the report action collection changes, it might introduce performance issues when scrolling the LHN. See this for more details.