Repository navigation
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
fix: Show empty state instead of No results when default view hides all rows
…port-actions # Conflicts: # src/components/ParentNavigationSubtitle.tsx # src/libs/ReportActionsUtils.ts # src/libs/actions/OnyxDerived/configs/visibleReportActions.ts # tests/unit/OnyxDerived/visibleReportActionsTest.ts # tests/unit/OnyxDerivedTest.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
This targets the Onyx store bump in Expensify#98956 and builds on the unmerged
useOnyxStatefrom react-native-onyx.VISIBLE_REPORT_ACTIONSis an OnyxDerived map from reportID to the cached visibility of each of its report actions.isReportActionVisibleuses it to skip the full visibility check, and falls back to that check when an action has no cached entry. The engine updates the changed report on every report action write for the whole life of the app and writes the whole map back into Onyx and to disk each time, even though most readers only need one report.This removes the derived value. The visibility compute moves unchanged into a selector with a cache per report, keyed on the report's actions object and the signed-in accountID. Readers that need one report (the LHN row, the report and money request report action lists, the parent navigation subtitle, the linked action guard and the report action context menu) use
useVisibleReportActions, which depends only on that report's report actions key and the session. The LHN row also passes its transaction thread, so it subscribes to both reports. Writes to any other report don't run their selector at all. The Search surfaces (autocomplete, the router warmer, the search selector and the share tab) need every report and useuseAllVisibleReportActions. It walks the collection once per write for all of them together, reuses the cached visibility of every report that didn't change, and returns the previous map when nothing visible changed. Both hooks read the store live, so the Search snapshot never applies, as before. The hooks return the same{[reportID]: visibility}shape as before, so the utils keep their signatures; the type is renamed toVisibleReportActions. The config, its key and its type are removed, along with its outdated comment about a report dependency. The persisted row is left on disk, as with theTODOSremoval.A new reassure test mounts 50 real LHN rows, or the Search router warmer, over up to 2,050 reports with actions, unchanged on the base and on this branch. Render counts are the same in every scenario, and a probe of
getOptionDatashows that a write to one report re-renders only the row that shows it, on both. Ten comment writes take 240 ms instead of 288 ms with only LHN rows mounted, 318 ms instead of 355 ms with the Search consumer mounted, and 264 ms instead of 312 ms with nothing mounted (medians of 6 alternating runs). The difference is the engine's compute and its write of the whole map, which no longer happen.Fixed Issues
$ https://github.com/callstack-internal/expensify-issues/issues/3081
PROPOSAL:
Tests
Offline tests
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari