Add conditional header for WorkspaceCompanyCardTable#80064
Add conditional header for WorkspaceCompanyCardTable#80064carlosmiceli merged 12 commits intoExpensify:mainfrom
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@ikevin127 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f882cbb538
ℹ️ 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".
src/pages/workspace/companyCards/WorkspaceCompanyCardsTable/index.tsx
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
Reviewer Checklist
Screenshots/Videosweb.mov
|
|
Problem: Line 272 uses // Line 272 - New code uses shouldUseNarrowLayout
<View style={shouldUseNarrowLayout && styles.mb5}>
// Line 85 - Existing definition
const shouldUseNarrowTableLayout = shouldUseNarrowLayout || isMediumScreenWidth;Why this matters:
Potential Bug: On medium-width screens (tablets), the margin bottom (styles.mb5) won't be applied to the header buttons when they should be, causing layout inconsistency. Fix: Replace
@Expensify/design Noting that there's a different table breakpoint which goes to header mobile layout earlier than the Screen.Recording.2026-01-22.at.17.39.32.mov |
src/pages/workspace/companyCards/WorkspaceCompanyCardsTable/index.tsx
Outdated
Show resolved
Hide resolved
My vote is on tablets because that's when the search bar tucks underneath and becomes very tall. |
That's what I was advocating for. I might've not used the right words haha 😅 Fine with changing the layout too, but assumed that viewport was rather rare and not worth extra tweaking. Either way I think when things stack we should allow everything to be scrolled away. |
|
@hungvu193 do you think you have all the info you need to make the design updates requested in this PR? |
Yes. I already updated the PR. You can trigger a web build so Design team can verify |
|
🚧 @carlosmiceli 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! 🧪🧪
|
|
Behavior in the latest build seems good to me if we don't want to mess with the layout at all on that weird in between size. cc @Expensify/design for more eyes too |
|
Yeah that looks good to me too. Curious if @shawnborton agree or disagree though |
|
I don't think I have any workspaces with company cards for testing this one - can someone add me to one? Otherwise I trust you both and if you are cool with the changes, that works for me too! |
|
All yours @carlosmiceli 😄 |
|
@hungvu193 there's conflicts 🙇 |
Done |
Done 😄 |
|
@hungvu193 linting now 😆 |
|
@carlosmiceli Done, again 🤣 |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
Thanks for the invite @dubielzyk-expensify ! |
|
🚀 Deployed to staging by https://github.com/carlosmiceli in version: 9.3.10-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.10-6 🚀
|

Explanation of Change
Fixed Issues
$ #78767
PROPOSAL: N/A
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Prerequisite: Your workspace has at least one company card.
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
iOS: mWeb Safari
Screen.Recording.2026-01-21.at.10.06.02.mov
MacOS: Chrome / Safari
Screen.Recording.2026-01-21.at.09.53.58.mov