fix:78318: Hide Table.Header when isEmptyResult is true#79472
fix:78318: Hide Table.Header when isEmptyResult is true#79472carlosmiceli merged 3 commits intoExpensify:mainfrom
Conversation
|
@hungvu193 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] |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
src/components/Table/TableHeader.tsx
Outdated
| const hasActiveFilters = filterConfig | ||
| ? Object.keys(activeFilters).some((key) => { | ||
| const filterValue = activeFilters[key]; | ||
| const defaultValue = filterConfig?.[key]?.default; | ||
| return filterValue !== defaultValue; | ||
| }) | ||
| : false; | ||
|
|
||
| const hasSearchString = activeSearchString.trim().length > 0; | ||
| const isEmptyResult = filteredAndSortedData.length === 0 && originalDataLength > 0 && (hasSearchString || hasActiveFilters); |
There was a problem hiding this comment.
Do you think we can move this logic into TableContext and reuse it everywhere else since it's duplciate logic?
There was a problem hiding this comment.
Yes, I think it's possible, because we define the context here:
App/src/components/Table/Table.tsx
Lines 179 to 192 in ef24438
Therefore, I believe we can define this data here and then pass it, but in Header.Body we will still have to retrieve most of these values, as they are used not only to define the empty state. I will update the PR within a few hours.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-01-13.at.22.41.00.movAndroid: mWeb ChromeScreen.Recording.2026-01-13.at.22.41.00.moviOS: HybridAppScreen.Recording.2026-01-13.at.22.19.46.moviOS: mWeb SafariScreen.Recording.2026-01-13.at.22.10.49.movMacOS: Chrome / SafariScreen.Recording.2026-01-13.at.22.07.38.mov |
|
@joekaufmanexpensify do you need to review it too? |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Makes sense from a product perspective 👍
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/carlosmiceli in version: 9.3.1-0 🚀
|
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.3.1-1 🚀
|
Explanation of Change
This PR fixes an issue where the table header remained visible when a search query returned no results
Fixed Issues
$#78318
PROPOSAL:#78318 (comment)
Tests
Precondition:
Workspace has company cards.
Offline tests
Same as tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.ScrollViewcomponent to make it scrollable when more elements are added to the page.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
78318-android-native.mov
Android: mWeb Chrome
78318-android-web.mov
iOS: Native
78318-ios-native.mov
iOS: mWeb Safari
78318-ios-web.mov
MacOS: Chrome / Safari
78318-web.mov