-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[HOLD for payment 2024-05-22] Bottom button on missing padding #40827
Comments
Triggered auto assignment to @trjExpensify ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Bottom button is missing padding on some card related pages. What is the root cause of that problem?FormAlertWithSubmitButton doesn't have padding set at the below places. App/src/pages/settings/Wallet/ReportCardLostPage.tsx Lines 182 to 189 in 6b18ee9
App/src/pages/settings/Wallet/ReportCardLostPage.tsx Lines 200 to 205 in 6b18ee9
App/src/pages/settings/Wallet/ReportVirtualCardFraudPage.tsx Lines 75 to 81 in 6b18ee9
App/src/pages/settings/Wallet/TransferBalancePage.tsx Lines 210 to 221 in 6b18ee9
What changes do you think we should make in order to solve the problem?Wrap FormAlertWithSubmitButton inside a View and add styles.mh5, styles.mb5. Example:
For transfer balance page, View is already there. We just need to add the styles. In the same way, update at all the mentioned places. |
ProposalPlease re-state the problem that we are trying to solve in this issue.Bottom button on missing padding What is the root cause of that problem?We are missing
What changes do you think we should make in order to solve the problem?We will pass
AdditionallyThe confirm task page also seems to be missing the bottom padding: We will pass App/src/pages/tasks/NewTaskPage.tsx Line 194 in 6b18ee9
|
@lukemorawski it seems like #38039 caused a regression in a few other places. Could you please take a look at this issue and work on a fix? cc @abdulrahuman5196 |
@luacmartins sure, though I need to add some fake cards, as I don't have access to those pages now. |
What's the latest on this one? |
I've been asked to take over this issue. I've pushed latest changes to the branch that was previously created and uploaded screenshots with fixed screens |
Thanks! This regression has been live for a couple of weeks now, so I would love to get this cleaned up ASAP. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.73-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-05-22. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
@mollfpr was the C+ on this one! Checklist when you're ready. |
Payment Summary
BugZero Checklist (@trjExpensify)
|
Bump for the checklist! |
@mollfpr could you please complete the checklist? |
Melv only listens to me, Carlos. |
https://github.com/Expensify/App/pull/38039/files
We already have the check when updating a generic component, we should do the test diligently.
I don't think we need to add a new test case here. |
Alrighty, thanks. Confirming payment summary as follows: $250 to @mollfpr for the C+ review. Go ahead and submit an expense. Closing! |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 1.4.64-3
Reproducible in staging?: y
Reproducible in production?: couldn't check
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @Jonvrebdubielzyk
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1713832272368439
Action Performed:
Expected Result:
On step 5 I would expect the footer button to have correct padding of 20px on both sides and bottom
Actual Result:
On step 5 it didn't have any padding. See screenshots in thread
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
View all open jobs on GitHub
Issue Owner
Current Issue Owner: @trjExpensifyThe text was updated successfully, but these errors were encountered: