Add compact UI styles for search reports and expenses tables #86283
Add compact UI styles for search reports and expenses tables #86283Krishna2323 wants to merge 27 commits intoExpensify:mainfrom
Conversation
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…in-height, and bottom border separation Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
…ages Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…height and border separators Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@codex review |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…Radius and remove duplicate type Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@codex review |
![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: 5343876a87
ℹ️ 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".
| case CONST.SEARCH.TABLE_COLUMNS.AVATAR: | ||
| columnWidth = {...getWidthStyle(variables.w40), ...styles.alignItemsCenter}; | ||
| columnWidth = {...getWidthStyle(variables.w28), ...styles.alignItemsCenter}; |
There was a problem hiding this comment.
Changing TABLE_COLUMNS.AVATAR to 28px introduces overflow in desktop grouped views that still render 40px assets. CardListItemHeader uses ReportActionAvatars without a size prop (so it stays at default avatar size), and WithdrawalIDListItemHeader uses bank icons sized via iconSizeExtraLarge; both are wider than 28px. In Group by Card / Group by Withdrawal tables this causes icon overlap into neighboring columns and misaligned cell content. Keep this column at 40px or downsize those icon/avatar renderers to 28px in those views.
Useful? React with 👍 / 👎.
|
🚧 @shawnborton has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
This is looking good! For skeletons, can we make it so that the top of the skeletons doesn't have rounded edges and has a top border? We might also want to adjust the skeleton elements to more closely match the rows now (update button width, add space on the right where arrow goes so buttons line up, etc) |
|
On Reports > Reports, can you fix the logic so that the amount column uses the correct width? I had to do that in my POC here. Similar, for Reports > Expenses, can you fix the logic so that the date column uses the correct width (smaller)? I also did that in my POC. |
|
For these types of grouped rows here: Check that against my POC. I had to adjust the vertical padding of 6px down to 4px. Ideally you want those rows to also be 56px when they are not expanded. |
|
For the Expensify Card view, it would be nice to use a 28x28 avatar and also fix the date clumn width logic: |
|
Nested table rows have weird styles when selected - let's get rid of any border radius. |
|
Not sure if this has always been like this but I would love it if this spinner was vertically centered. If that exists on prod, feel free to ignore for now. |
|
Same comment on these rows, let's get them down to 56px (change vertical padding of 6px to 4px). 
|
|
I think @dubielzyk-expensify had wanted a min height of 56px on all rows, not 52px: |
1 similar comment
|
I think @dubielzyk-expensify had wanted a min height of 56px on all rows, not 52px: |
|
I think that's all I have for now, great work on this one! |
…dths, skeleton alignment, and spinner centering Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…re isReportActionEntry block Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@codex review |
|
The bank logos are not rendering correctly: |
|
Unapproved cash rows (basically all grouping header rows) are showing as 60px instead of 56px: When I said change the vertical padding to 4px, I think we ONLY want to do that for the header rows. For the children rows, it looks too small: So let's bump that up to 8px vertical padding there: |
|
The withdrawn column on Expensify Card page doesn't have the correct width logic applied, the columns are wider than they need to be: |
|
On Reports > Expenses, the Date table header cell is too wide compared to the cells below: |
|
Okay, no further comments at this time :) Feel free to make another round of updates and then we can test again. |
JS00001
left a comment
JS00001
left a comment
There was a problem hiding this comment.
Code looks good to me, but is there any way that we can make these styles reusable so we can use them across the other tables?
| shouldAnimate | ||
| fixedNumItems={shouldShowLoadingMoreItems ? 5 : 1} | ||
| reasonAttributes={showPendingExpensePlaceholder ? pendingExpenseReasonAttributes : loadMoreSkeletonReasonAttributes} | ||
| isLoadMore |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
… used Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@Krishna2323 ready for another round of testing? |
|
🚧 @shawnborton 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! 🧪🧪
|
|
Date column header width on Reports > Expenses is still not correct: |
|
Unapproved cash row height is still off, it's 60px but it should be 56px: |
|
Withdrawn column is still too wide: |
|
Sometimes the grouping rows behave a little strangely... if I expand them, sometimes the bottom border isn't there. If I collapse them, sometimes I see a 1px white gap. I feel like whatever CleanShot.2026-03-27.at.10.13.51.mp4 |
|
@shawnborton Sorry, I forgot to mention that this wasn’t ready for another review yet. I’ve fixed some of the previous bugs, but not all of them. Resolving conflicts now... |
…ct-table-styles Made-with: Cursor # Conflicts: # src/components/Search/SearchList/ListItem/CardListItem.tsx # src/components/SelectionList/BaseSelectionList.tsx # src/components/SelectionList/ListItem/ListItemRenderer.tsx # src/components/SelectionList/ListItem/TableListItem.tsx
|
Remaning bugs/improvements:
|
| keyExtractor={keyExtractor} | ||
| onScroll={onScroll} | ||
| showsVerticalScrollIndicator | ||
| showsVerticalScrollIndicator={false} |
There was a problem hiding this comment.
@shawnborton Hiding the scrollbar to fix this issue will reintroduce #78849. This also happens on staging, should we tackle it in a separate issue, or do you have another idea?
There was a problem hiding this comment.
Hmm, fair. I think the only way to solve this is going to be to keep the header of the table fixed/absolute positioned within the same view as the scroll view. Happy to do this separate, cc @JS00001 @luacmartins for a second opinion too.
| sortOrder={sortOrder} | ||
| // In GroupBy views, disable flex expansion for Total columns so Expenses column gets more space | ||
| shouldRemoveTotalColumnFlex={!!groupBy && !isExpenseReportView} | ||
| shouldRemoveTotalColumnFlex={!!groupBy || isExpenseReportView} |
| numberOfLines={2} | ||
| style={[styles.lineHeightLarge, styles.preWrap, styles.justifyContentCenter]} | ||
| numberOfLines={1} | ||
| style={[styles.lineHeightLarge, styles.justifyContentCenter]} |
There was a problem hiding this comment.
Make sure that removing preWrap doesn't re-introduce bug what it fixed.
| numberOfLines={shouldUseNarrowLayout ? 1 : 2} | ||
| style={[shouldUseNarrowLayout ? styles.lh20 : styles.lineHeightLarge, shouldUseNarrowLayout ? styles.pre : styles.preWrap, styles.justifyContentCenter, styles.flex1]} | ||
| numberOfLines={1} | ||
| style={[shouldUseNarrowLayout ? styles.lh20 : styles.lineHeightLarge, shouldUseNarrowLayout && styles.pre, styles.justifyContentCenter, styles.flex1]} |
| numberOfLines={2} | ||
| style={[styles.lineHeightLarge, styles.preWrap, styles.justifyContentCenter]} | ||
| numberOfLines={1} | ||
| style={[styles.lineHeightLarge, styles.justifyContentCenter]} |
…ld row padding to 8px Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
@shawnborton I can't test this because I don't card expenses but looking at the code the issue seems to be on main also, could you please check and confirm? The width of withdrawn column is fixed to 96px. |
Investigated the intermittent bottom border issue extensively. It appears to be a rendering edge case related to how the browser composites |
I don't think it should be fixed though, it should use the same logic as other date columns where it uses a smaller width for dates with no year, and a larger width with dates that have a year. Again, check my POC - I had it fixed there and it helps the table feel more compact. |
5 participants
Explanation of Change
Fixed Issues
$ #86202
PROPOSAL:
Tests
Expenses Tab (TransactionListItem)
theme.bordercolortheme.buttonHoveredBGReports Tab (ExpenseReportListItem)
Tasks (TaskListItem)
type:task)Grouped Search — Group Header (TransactionGroupListItem)
theme.buttonHoveredBGGrouped Search — Expanded Rows (TransactionGroupListExpanded)
pv1Half(6px)borderNone)Member Avatar (MemberListItemHeader)
Arrow Button (TransactionItemRow)
pv2) with no extra horizontal paddingMobile Regression Checks (resize browser to <1024px)
mb2gaps between themtransactionGroupListItemStylepaddingVertical remains 6pxOffline tests
QA Steps
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