Skip to content
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

[$500] Request Money - Money Request button text is not center. #29759

Closed
6 tasks done
kbecciv opened this issue Oct 17, 2023 · 34 comments
Closed
6 tasks done

[$500] Request Money - Money Request button text is not center. #29759

kbecciv opened this issue Oct 17, 2023 · 34 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@kbecciv
Copy link

kbecciv commented Oct 17, 2023

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.3.85.0
Reproducible in staging?: y
Reproducible in production?: y
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: @yh-0218
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1697479665208989

Action Performed:

  1. Open any report.
  2. Click <+> on the left of composer.
  3. Manual input
  4. Click next button

Expected Result:

Button text must be align center.

Actual Result:

Button text is not align center.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Android: Native

Screenshot 2023-10-17 at 12 00 27 AM

Android: mWeb Chrome

Screenshot 2023-10-16 at 9 47 14 PM

iOS: Native

Screenshot 2023-10-16 at 10 04 57 PM

iOS: mWeb Safari

Screenshot 2023-10-16 at 9 43 08 PM

MacOS: Chrome / Safari
Screen.Recording.2023-10-16.at.9.08.39.PM.mov

image (12)

MacOS: Desktop

Screenshot 2023-10-16 at 9 53 15 PM

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0146a50c0ecd80150f
  • Upwork Job ID: 1714253661573099520
  • Last Price Increase: 2023-10-17
@kbecciv kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 17, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2023

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot melvin-bot bot changed the title Request Money - Money Request button text is not center. [$500] Request Money - Money Request button text is not center. Oct 17, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2023

Job added to Upwork: https://www.upwork.com/jobs/~0146a50c0ecd80150f

@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 17, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2023

Triggered auto assignment to Contributor-plus team member for initial proposal review - @narefyev91 (External)

@kbecciv
Copy link
Author

kbecciv commented Oct 17, 2023

Proposal by: @yh-0218
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1697479665208989

Proposal

Please re-state the problem that we are trying to solve in this issue.

Money Request button text is not center.

What is the root cause of that problem?

Button have padding: 8px 10px 8px 18px;


Paddings that define for button is not correct.

What changes do you think we should make in order to solve the problem?

We need to make padding correctly on this

buttonLarge: {

What alternative solutions did you explore? (Optional)

Current button (edited)

@yh-0218
Copy link
Contributor

yh-0218 commented Oct 17, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

Money Request button text is not center.

What is the root cause of that problem?

Button have padding: 8px 10px 8px 18px;


Paddings that define for button is not correct.

What changes do you think we should make in order to solve the problem?

We need to make padding correctly on this

buttonLarge: {

What alternative solutions did you explore? (

@narefyev91
Copy link
Contributor

Proposal from @yh-0218 looks good to me #29759 (comment)
🎀 👀 🎀 C+ reviewed

@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2023

Triggered auto assignment to @cristipaval, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 17, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2023

📣 @yh-0218 You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2023

📣 @yh-0218 We're missing your Upwork ID to automatically send you an offer for the Reporter role.
Once you apply to the Upwork job, your Upwork ID will be stored and you will be automatically hired for future jobs!

@yh-0218
Copy link
Contributor

yh-0218 commented Oct 17, 2023

Hello @narefyev91 PR is ready for review. Could you review this

@yh-0218
Copy link
Contributor

yh-0218 commented Oct 17, 2023

Hi, @cristipaval PR is ready for review.

@yh-0218
Copy link
Contributor

yh-0218 commented Oct 18, 2023

Hello, @narefyev91 On this step, what do I need to do?

@narefyev91
Copy link
Contributor

Hello, @narefyev91 On this step, what do I need to do?

Hey - nothing, we need Expensify engineer to approve and merge you PR - i already tag @cristipaval - if he will not be able - i will ping someone from Expensify

@Christinadobrzyn
Copy link
Contributor

Since the PR is in review, I'm going to move this issue to weekly. Feel free to let me know if you'd prefer it remain daily.

@Christinadobrzyn Christinadobrzyn added Weekly KSv2 and removed Daily KSv2 labels Oct 18, 2023
cristipaval added a commit that referenced this issue Oct 18, 2023
fixed money request button text center issue #29759
@Pujan92
Copy link
Contributor

Pujan92 commented Oct 19, 2023

Found a regression from this issue PR and reported here, Left padding of 18px is given to align the left icon correctly.

@narefyev91
Copy link
Contributor

I will say it's not a regression for this PR.
All Buttons were broken based on this PR which added not correct padding for all buttons https://github.com/Expensify/App/pull/12455/files
To fix issue which you linked @Pujan92 padding should be applied to icon and not to the whole button container

@yh-0218
Copy link
Contributor

yh-0218 commented Oct 19, 2023

@Pujan92 @narefyev91 I will create New PR soon

@Pujan92
Copy link
Contributor

Pujan92 commented Oct 19, 2023

I agree partially that the margin left needs to provide to an icon for correct alignment @narefyev91, but as we have updated the padding left in this PR I think that icon button case needs to be tested here only.

@Pujan92
Copy link
Contributor

Pujan92 commented Oct 19, 2023

@Pujan92 @narefyev91 I will create New PR soon

I think you need to wait for raising the PR untill that issue gets created by testing team.

@yh-0218
Copy link
Contributor

yh-0218 commented Oct 19, 2023

Got it. @Pujan92

@melvin-bot
Copy link

melvin-bot bot commented Oct 19, 2023

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@yh-0218
Copy link
Contributor

yh-0218 commented Oct 26, 2023

hi, @narefyev91 when can this task be completed?

@narefyev91
Copy link
Contributor

@yh-0218 Hey! I think it should be close after 2 days from today

@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Oct 27, 2023

HI! I was assigned this payment GH to pay @narefyev91 but I feel like it's a dupe of the payments needed for this job

@narefyev91 can you confirm if you should be paid here and also in this GH? Answer here - #29759 (comment)

Payment plan for review:

Issue Reporter: $50 @yh-0218 (Paid in Upwork)
Contributor: $250 (50% reduction because of regression @yh-0218) (Paid in Upwork)
Contributor+: @narefyev91 Paid here #29861 (comment)

Eligible for 50% #urgency bonus? N - based on #29759 (comment)

Upwork job is here.

@Christinadobrzyn
Copy link
Contributor

I'm not sure why the Payment notification didn't happen for this PR

@cristipaval Is this ready to pay based on #29759 (comment)

@narefyev91
Copy link
Contributor

HI! I was assigned this payment GH to pay @narefyev91 but I feel like it's a dupe of the payments needed for this job

@narefyev91 can you confirm if you should be paid here and also in this GH?

Payment plan for review:

Issue Reporter: $50 @yh-0218 Contributor: $500 + $250 bonus @yh-0218 Contributor+: $500 + $250 bonus @narefyev91

Eligible for 50% #urgency bonus? Y - @yh-0218 created PR on October 17th merged on Oct 18th

Upwork job is here. @yh-0218 and @narefyev91 can you apply to the job if the above breakdown makes sense?

Hey @Christinadobrzyn! I'm from expert agency Callstack - i think it should be included in a separate invoice. Thanks!

@cristipaval
Copy link
Contributor

cristipaval commented Oct 30, 2023

@Christinadobrzyn, the C+ here is from Callstack
The payments due here are the reporting bonus and the money for the contributor who fixed the issue (@yh-0218). No bonus applicable here, since there was a regression after the fix

@Christinadobrzyn
Copy link
Contributor

Thanks @cristipaval and @narefyev91 - updated the payment outline here - #29759 (comment)

I'll keep this open and monitor for @yh-0218 to accept the Upwork offer!

@yh-0218
Copy link
Contributor

yh-0218 commented Oct 30, 2023

hi, @Christinadobrzyn so my payment is $300?

@Christinadobrzyn
Copy link
Contributor

hi @yh-0218 yes, that's correct! You'll be paid as the reporter and as the contributor.

@Christinadobrzyn
Copy link
Contributor

I paid you in Upwork based on this payment structure - #29759 (comment)

Let me know if you have any questions!

@melvin-bot
Copy link

melvin-bot bot commented Oct 31, 2023

@cristipaval @narefyev91 @Christinadobrzyn @yh-0218 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Oct 31, 2023
@Christinadobrzyn
Copy link
Contributor

ah sorry, this can be closed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

6 participants