Adjust desktop table row styles: tighter gaps and padding#84676
Adjust desktop table row styles: tighter gaps and padding#84676
Conversation
- Reduce gap between table rows from 8px to 4px on desktop - Set row padding to 12px horizontal / 8px vertical on desktop - Reduce gap between table header and first row from 12px to 8px - Changes apply to search results, workspace editor, and expense rules - Mobile styles remain unchanged Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
The failing check is unrelated to this PR. Analysis: The
None of these files are part of this PR's changeset. Evidence: The |
|
🚧 @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.
|
@MelvinBot I meant the gaps in between each table row. It looks like those gaps are still 8px instead of 4px. Can you tighten all gaps between table rows please? It looks like the gaps are tighter on Reports > Expenses, but not the Reports > Reports page. Please make this update everywhere. And it looks like the padding didn't change on Reports > Reports either. Please make those updates to this PR. |
|
@MelvinBot can you also change the minimum height on these table rows to be 52px instead of 64px? For example, in the workspace editor in the categories list, I see a min height of 64px but it should be 52px instead: |
…orts > Reports page) Reduce vertical gap between rows from 8px to 4px (mb2 → mb1) and row padding from 12px to 8px vertical (pv3 → pv2) on desktop, matching the same changes already applied to TransactionListItem and TransactionGroupListItem. Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
|
Updated `ExpenseReportListItem` (Reports > Reports page) to match the tighter desktop styling already applied to the Expenses tab:
Both changes are desktop-only — mobile styles remain unchanged. |
Override minHeight with variables.optionRowHeightCompact (52px) on desktop for TableListItem, ExpenseReportListItem, TransactionListItem, and TransactionGroupListItem. Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
|
Reduced the table row minimum height from 64px to 52px on desktop across all list item components:
Uses the existing |
|
🚧 @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! 🧪🧪
|
|
@MelvinBot some changes you need to make:
|
Match the same desktop-only changes applied to other list item components: - Reduce vertical padding from 12px to 8px (pv3 → pv2) - Set minHeight to 52px (optionRowHeightCompact) - Reduce row gap from 8px to 4px (mb2 → mb1) Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
|
Here's a summary of all desktop table row styling changes now in this PR: Components updated with 52px minHeight, 8px vertical padding, and 4px row gaps on desktop:
All changes are desktop-only — mobile styles remain unchanged. Please trigger a new adhoc build to test. |
|
🚧 @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! 🧪🧪
|
|
@MelvinBot for the Reports > Reports page, it looks like we aren't using We also need those rows to have a minimum height of 52px, and vertical padding of 8px. You still haven't made those changes, can you please implement? |
|
All three changes for the Reports > Reports page (
The screenshot may be from the previous test build (triggered before these changes were pushed). Please trigger a new test build to verify. |
|
@MelvinBot thanks. Can you also make these same exact changes when you are viewing the table on an expense report? So if you are on Reports > Reports and you are viewing an expense report with multiple expenses, we want to make the same changes to that table as well: |
Reduce row gaps from 8px to 4px (gap2 → gap1) and vertical row padding from 12px to 8px (p3 + pv2) on desktop for the transaction table inside expanded expense reports (MoneyRequestReportTransactionList and MoneyRequestReportTransactionItem). Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
|
Applied the same tighter desktop styling to the expense report detail view (the table you see when viewing an expense report with multiple expenses): MoneyRequestReportTransactionList:
MoneyRequestReportTransactionItem:
Mobile styles remain unchanged. |
|
🚧 @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! 🧪🧪
|
|
🚧 @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! 🧪🧪
|
|
@MelvinBot more changes. You need to change the minimum height on these table rows inside of an expense report to be 52px instead of 64px. And can you also remove the top margin from the subgroup header? For instance, when we group the table by category, the group header that has a checkbox and the category name. It has a top margin of 16px currently, and we simply want to get rid of it. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy 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! 🧪🧪
|
|
@MelvinBot please pull main |
…Styles Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
|
Merged main into the branch. The merge completed cleanly with no conflicts. |
|
🚧 @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! 🧪🧪
|
|
@MelvinBot the table header that is shown when you are viewing an actual expense report needs to have an inner padding of 44px. Please do not touch the table headers shown on Reports > Reports or any of those pages, we just need this when viewing an expense report. |
|
The failing check is unrelated to this PR. Analysis: The only failure is in
These tests are about money request field editing permissions ("move expense" logic), which has no connection to this PR's desktop table row style changes. Evidence: This PR modifies styling files (paddings, margins, gaps) across This appears to be a pre-existing test failure on |
Replace pl9 (36px) + pr11 (44px) with ph11 (44px symmetrical) on the table header shown when viewing an actual expense report. Search page headers are unchanged. Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
|
Updated the table header padding for the expense report view:
|
|
@MelvinBot that's not correct, please undo that. We only want this change on the table header above the table of expenses found when you click into an actual expense report from say Reports > Reports. |
The subgroup header (category/tag group headers) should keep their 16px top margin. The tighter styling is only intended for the table column header inside the expense report detail view, which already has the correct styling in MoneyRequestReportTransactionList. Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
|
Restored The tighter styling changes are already applied only to the table column header inside the expense report detail view ( |
|
@MelvinBot please undo your past two commits, they are both incorrect. |
|
Reverted the last two commits:
Both changes have been undone via revert commits. |
|
@MelvinBot this screenshot is from an expense report view, it's what happens when you are actually drilled down into an expense report: For the table header here, the row above the subgroup header and above the expense rows, can you make the inner right padding 44px please? |
Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
|
Updated the table header in the expense report view ( |
|
🚧 @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! 🧪🧪
|
|
@MelvinBot pull in main please |
…Styles Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
|
Merged |
|
🚧 @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! 🧪🧪
|
|
The failing check Analysis: This PR only modifies TypeScript/React component files (styling and padding changes). The failing check is an iOS native build ( Evidence: The PR changes 51 Recommend re-running the failed check. |
2 participants
Explanation of Change
Adjusts table row styles on desktop to create a tighter, more compact table layout across the app. Specifically:
Reduces vertical gaps between table rows from 8px to 4px on desktop (
mb2→mb1), affecting:TransactionListItem)TransactionGroupListItem)TableListItem)Normalizes row padding to 12px horizontal / 8px vertical on desktop:
TransactionGroupListExpanded): changed from 6px to 8px verticalTableListItem): changed from 16px to 12px horizontal, 16px to 8px verticalReduces gap between table header and first row from 12px to 8px (
searchListHeaderContainerStyle.paddingBottom)All changes are desktop-only — mobile styles remain unchanged. Table header paddings are not modified.
Fixed Issues
$
Tests
Offline tests
N/A — these are purely visual style changes with no network dependency.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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
N/A — desktop-only style changes
Android: mWeb Chrome
N/A — desktop-only style changes
iOS: Native
N/A — desktop-only style changes
iOS: mWeb Safari
N/A — desktop-only style changes
MacOS: Chrome / Safari
Screenshots to be added after visual review