Repository navigation
fixed truncated Posted column in Report layout table - #95635
Conversation
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.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![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: 63c55900b9
ℹ️ 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".
| amountColumnSize: isAmountColumnWide ? CONST.SEARCH.TABLE_COLUMN_SIZES.WIDE : CONST.SEARCH.TABLE_COLUMN_SIZES.NORMAL, | ||
| taxAmountColumnSize: isTaxAmountColumnWide ? CONST.SEARCH.TABLE_COLUMN_SIZES.WIDE : CONST.SEARCH.TABLE_COLUMN_SIZES.NORMAL, | ||
| dateColumnSize: shouldShowYearForSomeTransaction ? CONST.SEARCH.TABLE_COLUMN_SIZES.WIDE : CONST.SEARCH.TABLE_COLUMN_SIZES.NORMAL, | ||
| postedColumnSize: shouldShowPostedYearForSomeTransaction ? CONST.SEARCH.TABLE_COLUMN_SIZES.WIDE : CONST.SEARCH.TABLE_COLUMN_SIZES.NORMAL, |
There was a problem hiding this comment.
When any posted date needs the wider 102px cell, this only widens the rendered header/rows; the report table still decides shouldScrollHorizontally and the ScrollView contentContainerStyle from getTableMinWidth(columnsToShow), which has no access to postedColumnSize and always budgets the POSTED column at the fixed narrow amount. On desktop widths where the old table width fits but the widened Posted column does not, the table will not enter the horizontal-scroll path (or the scroll content width will be too small), so the Posted column can still be clipped instead of becoming scrollable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This looks like a pre-existing limitation of the date-column widening rather than something introduced here. getTableMinWidth (SearchUIUtils.ts:6256) budgets a fixed width per column — DATE +48, SUBMITTED/APPROVED/POSTED +72 — with no awareness of the wide 102px variant, so the same gap already applies to Date/Submitted/Approved/Exported, all of which widen to 102px today. In the report layout the Date column already widens (its dateColumnSize was wired before this PR) with the exact same discrepancy. This change just brings Posted to parity with its sibling date columns rather than creating a new scroll-sizing path.
There was a problem hiding this comment.
Making getTableMinWidth size-aware for the whole date-column family would fix it, but I'd prefer to handle that as a separate follow-up than fold it into this focused Posted fix.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 150d5bb0f1
ℹ️ 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".
| const isAmountColumnWide = transactions.some((transaction) => isTransactionAmountTooLong(transaction)); | ||
| const isTaxAmountColumnWide = transactions.some((transaction) => isTransactionTaxAmountTooLong(transaction)); | ||
| const shouldShowYearForSomeTransaction = transactions.some((transaction) => shouldShowTransactionYear(transaction)); | ||
| const shouldShowPostedYearForSomeTransaction = transactions.some((transaction) => shouldShowTransactionPostedYear(transaction)); |
There was a problem hiding this comment.
For report-layout rows with raw card posted dates such as 20260710, TransactionItemRowWide passes transactionItem.posted directly to DateCell; DateCell chooses the year format through DateUtils.doesDateBelongToAPastYear, whose new Date('YYYYMMDD') path yields NaN and therefore selects the year format even for current-year values. This new size predicate returns false for current-year posted dates, so those rows still render MMM d, yyyy in the normal 62px Posted column and remain truncated. Consider matching the renderer's predicate or normalizing posted dates before sizing/rendering.
Useful? React with 👍 / 👎.
|
I think this still needs a code change. @mukhrr What do you think about either normalize |
@brunovjk thanks. I think this makes more sense |
|
Thank you @mukhrr, I tested locally with mocked posted dates. The original issue looks fixed: past-year posted dates like But the Jan 1 edge case still reproduces in a negative timezone. With
|
|
Ah very good catch 👍 let me fix that |
|
@brunovjk I think we are good to proceed |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari
|
brunovjk
left a comment
brunovjk
left a comment
There was a problem hiding this comment.
LGTM.
One note: while testing in a negative timezone, I noticed that when the posted date is Jan 1, 2026, it still renders as Jan 1, 2026 instead of Jan 1, even when all posted dates are in 2026. I don’t think that’s in scope for this PR, since the truncation itself is now fixed, but please let me know if anyone disagrees.
Thanks!
|
@mjasikowski mind merging this? thanks |
|
🚧 mjasikowski has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mjasikowski in version: 9.4.41-0 🚀
|
|
🤖 I reviewed the changes in this PR against the help site articles under Why: This is a visual bug fix, not a behavior or feature change. It widens the Posted column in the Report layout table so full dates that include the year (
What the docs cover: The relevant article, Using Reports in New Expensify, documents how to show/hide, reorder, and save columns (including Posted). It does not describe how date values are rendered or sized within a column, so nothing in it becomes inaccurate because of this change. The fix simply makes the UI match the already-documented behavior — no new feature, setting, tab, or button is introduced that would need documentation. No |
|
Hi @mukhrr. QA team doesn't see the Data column in the Posted column for reports with 2+ expenses. Do we need a special account to test this, or is the feature still behind a beta rollout? REC-20260721140515.mp4 |
@IuliiaHerets i beleive you need a card transactions with Posted value in expenses. You don't need special account or not beta as well |
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.4.41-4 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Fixed Issues
$ #94526
PROPOSAL:
Tests
Precondition:
Verify it's not truncated
Verify that no errors appear in the JS console
Offline tests
QA Steps
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari