-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[CP Staging] Fix action buttons in search row #61903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CP Staging] Fix action buttons in search row #61903
Conversation
🚧 @mountiny has triggered a test app build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
🚧 @trjExpensify has triggered a test app build. You can view the workflow run here. |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
Noticed a couple of things here on the build:
2025-05-13_22-47-09.mp4 |
@DylanDylann @srikarparsi One of you needs to 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] |
I'm already looking at it. |
Locally it looks fine, the views are blue and not on a green background. I'll check in a moment what the style differences are and why it looks like that on your side: Screen.Recording.2025-05-14.at.10.50.26.movWhen I ran the ad-hoc version in my browser, the "View" button on the light theme looked exactly as expected. We can double-check to make sure this isn't a false positive. Screen.Recording.2025-05-14.at.11.03.46.mov |
|
@sumo-slonik Is this ready to review again? |
It's already fixed. |
#61977 - if this is fixed by this PR, let's add it to the OP. 👍 |
🚧 @trjExpensify has triggered a test app build. You can view the workflow run here. |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
Feeling good to me from an alignment standpoint 👍 |
@DylanDylann
It aligns with what we agreed upon here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm seeing a weird behavior where the action isn't correct. In this case I should see Submit
, but I see Pay
in the Expense report
page
Screen.Recording.2025-05-14.at.10.00.44.AM.mov
Haha, aren't we all. So it transpires that bug is on production, and it was raised here 2 weeks ago. It's languishing in proposal review, but I bumped it today and it's on my radar now. 👿 |
Well, in that case this looks good then. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2025-05-14.at.23.40.30.mov |
@trjExpensify @sumo-slonik @shawnborton On the small screen, the alignment is still slightly off. Do you think we need to adjust it? ![]() |
Nice catch, yea we should fix that |
Minor above UI problem on mobile screen. The rest looks good to me |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SWM is already out of office and this change looks minor compared to the benefits we get from this fix already
…akowski/transaction-component-in-search [CP Staging] Fix action buttons in search row (cherry picked from commit dd867c3) (cherry-picked to staging by francoisl)
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Cherry-picked to staging by https://github.com/francoisl in version: 9.1.45-18 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
Explanation of Change
Fixed Issues
$ #61895
$ #61977
PROPOSAL:
Tests
Steps from #61977
It only reproduces on desktop web and desktop
Navigate to Search → Report Expense in the application.
Check the following elements on the report table:
Confirm that all elements are correctly displayed and functioning as expected.
Offline tests
Unnecessary
QA Steps
Same as test
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectioncanBeMissing
param foruseOnyx
toggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
MacOS: Chrome / Safari
Screen.Recording.2025-05-13.at.13.48.10.mov
MacOS: Desktop
Screen.Recording.2025-05-13.at.13.53.38.mov