Clean up report page styles#71448
Conversation
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@Expensify/design for initial review 🙇🏻👀
Submit tab  
Approve tab  
Pay tab  
Unapproved cash tab  
Expenses tab  
Reports tab
|
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
Did you change the avatar size? We don't need to change anything there, they should stay at 40x40. It looks like our alignment is off here too, the total column isn't aligned from header to table: Maybe you can double check the styles quickly with this Figma file, and then we can run a test build after you fix a few of these items? |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@Expensify/design Thanks for the Figma file; it was really helpful. Please check these screenshots, and if everything looks good, I think we can run test builds. 
|
|
Nice, I will run a test build! |
|
🚧 @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.
|
The arrows are gone from some of the grouping views, like Unreported cash: |
|
On small screens, the text styles here for the From/To don't match Figma. Your PR: Figma: |
|
When we show the second line below a report name or a user name, right now we are setting the line to 20px height: But from Figma, we actually want to keep that line at 16px height (including line height), and add a 4px gap above it: This comment applies to both mobile and desktop btw. |
|
We don't want this section here to have a height of 52px: Rather, it should have a height of 40px and have 8px of margin above it: |
|
@getusha I was unable to reproduce the issues, have I missed something? Monosnap.screencast.2025-10-30.17-12-26.mp4 |
Same, looks like i needed to pull latest commits. |
|
Corner not rounded on selected MoneyRequestTransactionItem
|
|
Category column title not aligned with the categories
|
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Fixed both |
|
@Krishna2323 the border radius is applied here as well, it looks off 
|
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
fixed 🫣 |
|
✋ 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/JS00001 in version: 9.2.42-0 🚀
|
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.2.42-11 🚀
|
|
👋 @Krishna2323 I think something got lost in the shuffle here. The nested expenses rows in the group-by results on mobile are no longer clickable, making it so people can't click to view the expense details. Example of group-by:withdrawal-id below: 
We need to fix that. @dubielzyk-expensify put this together on how it should look for a tappable row with the 
@Krishna2323 - can you get going on a PR for that now, please? I'll create a parent issue for you to comment and then assign to you. Thanks! |
|
@trjExpensify I'll start working on the PR within an hour. Please create tha issue and assign me. Thanks! |
|
Great - comment on the issue and I'll assign you: #76129 Edit: I can assign you. Done! |
|
@getusha @Krishna2323 Please make sure in the future that such "solutions" don't pass review / get merged as it caused another regression (caught too late): Explanation:The 
Note: Besides not being mentioned in the proposal (hacks not mentioned in the proposal should not be added on the fly during PR phase) - this was not the best choice from the PR author / reviewer letting it pass, should've kept the padding and instead remove extra spacing from the arrow icon - this way the regression wouldn't have existed. A better solution here targeting the root cause could've been to keep the original Example DIFFdiff --git a/src/components/MoneyRequestReportView/MoneyRequestReportTransactionItem.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportTransactionItem.tsx
index f4a3a38847a..052c66e45f4 100644
--- a/src/components/MoneyRequestReportView/MoneyRequestReportTransactionItem.tsx
+++ b/src/components/MoneyRequestReportView/MoneyRequestReportTransactionItem.tsx
@@ -86,7 +86,7 @@ function MoneyRequestReportTransactionItem({
const {translate} = useLocalize();
const styles = useThemeStyles();
// eslint-disable-next-line rulesdir/prefer-shouldUseNarrowLayout-instead-of-isSmallScreenWidth
- const {isSmallScreenWidth, isMediumScreenWidth, isLargeScreenWidth, shouldUseNarrowLayout} = useResponsiveLayout();
+ const {isSmallScreenWidth, isMediumScreenWidth, shouldUseNarrowLayout} = useResponsiveLayout();
const theme = useTheme();
const isPendingDelete = isTransactionPendingDelete(transaction);
const pendingAction = getTransactionPendingAction(transaction);
@@ -154,7 +154,7 @@ function MoneyRequestReportTransactionItem({
columns={columns}
areAllOptionalColumnsHidden={areAllOptionalColumnsHidden}
isDisabled={isPendingDelete}
- style={[styles.p3, isLargeScreenWidth && styles.pr0]}
+ style={[styles.p3]}
onButtonPress={() => {
handleOnPress(transaction.transactionID);
}}
diff --git a/src/components/TransactionItemRow/index.tsx b/src/components/TransactionItemRow/index.tsx
index 50571bcabfc..dc87752fdb4 100644
--- a/src/components/TransactionItemRow/index.tsx
+++ b/src/components/TransactionItemRow/index.tsx
@@ -556,7 +556,7 @@ function TransactionItemRow({
{!!isLargeScreenWidth && !!onArrowRightPress && (
<PressableWithFeedback
onPress={() => onArrowRightPress?.()}
- style={[styles.p3Half, styles.pl0half, styles.justifyContentCenter, styles.alignItemsEnd]}
+ style={[styles.p3Half, styles.pl0half, styles.pr0half, styles.justifyContentCenter, styles.alignItemsEnd]}
accessibilityRole={CONST.ROLE.BUTTON}
accessibilityLabel={CONST.ROLE.BUTTON}
>
diff --git a/src/styles/utils/spacing.ts b/src/styles/utils/spacing.ts
index e0c5ec07c68..947ccd3c917 100644
--- a/src/styles/utils/spacing.ts
+++ b/src/styles/utils/spacing.ts
@@ -575,6 +575,10 @@ export default {
paddingRight: 0,
},
+ pr0half: {
+ paddingRight: 2,
+ },
+
pr1: {
paddingRight: 4,
}, |
|
Thanks for the feedback, @ikevin127 — noted. This was a very large PR with many moving parts, so it’s definitely possible that some UI details slipped through. I agree there was a better way to handle the arrow spacing, and I’ll keep an eye out for this pattern in future clean-ups. Regression fixes are part of the process, and I’m always happy to improve my approach. |
| {({hovered}) => { | ||
| return ( | ||
| <Icon | ||
| src={Expensicons.ArrowRight} | ||
| fill={theme.icon} | ||
| additionalStyles={!hovered && styles.opacitySemiTransparent} | ||
| small | ||
| /> | ||
| ); | ||
| }} |
There was a problem hiding this comment.
This change caused issue
we should use correct hover style
Explanation of Change
Fixed Issues
$ #70369
PROPOSAL: #70369 (comment)
Tests
Offline 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_native.mp4
Android: mWeb Chrome
android_chrome.mp4
iOS: Native
ios_native.mp4
iOS: mWeb Safari
ios_safari.mp4
MacOS: Chrome / Safari
web_chrome.mp4
MacOS: Desktop
desktop_app.mp4