[HOLD for payment 2023-04-18] [$4000] Emoji reaction button sizes are not 28px

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Go to any chat
2. Add a reaction to any message
3. Verify the size of the emoji

Expected Result:

Should be 28px

Actual Result:

This is smaller and appears to be 24px

Workaround:

unknown

Platforms:

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

• [] Android / native
• [] Android / Chrome
• [] iOS / native
• [] iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.2.81-1
Reproducible in staging?: y
Reproducible in production?: New feature
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
Notes/Photos/Videos:

Expensify/Expensify Issue URL:
Issue reported by: @shawnborton
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1678368054498269

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011ebe3645f6da2f17
• Upwork Job ID: 1635326392868700160
• Last Price Increase: 2023-03-30

[MelvinBot]

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

[MelvinBot]

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

[stitesExpensify]

FYI @isabelastisser I will take this on internally tomorrow

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~011ebe3645f6da2f17

[isabelastisser]

Not overdue, internal!!

[MelvinBot]

Triggered auto assignment to Contributor Plus for review of internal employee PR - @sobitneupane (Internal)

[isabelastisser]

Internal!

[stitesExpensify]

Sorry for the delay on this one, ECX is taking up 99% of my time!

[stitesExpensify]

In fact, let's just make this external so that it gets done faster since I clearly don't have the time right now

[MelvinBot]

Current assignee @isabelastisser is eligible for the External assigner, not assigning anyone new.

[MelvinBot]

Current assignee @sobitneupane is eligible for the External assigner, not assigning anyone new.

[MelvinBot]

Current assignee @stitesExpensify is eligible for the External assigner, not assigning anyone new.

[Pujan92]

Proposal

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

Reaction bubble size is not of 28px, it is of 24px

What is the root cause of that problem?

Currently 20px lineHeight is given to the emoji and 2px as vertical padding which sums it to the 24px

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

App/src/styles/styles.js

Line 2962 in c40148b

paddingVertical: 2,

Update vertical padding to 4 which makes the bubble 28px.

[daraksha-dk]

Proposal

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

Emoji reaction button sizes are not 28px

What is the root cause of that problem?

This is a feature request or an improvement.

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

We need to update the font-size and line-height of the emoji reaction buttons and adjust padding accordingly.
Will need to confirm the exact values from design team.
Font-size can be 15px or 16px and lineHeight/padding will be updated accordingly.

This is how it will look for font-size:15px and lineHeight:24px

And obviously, the "number" next to the "emoji reaction" and "emoji button" will have the updated styles to match this

[isabelastisser]

Reviewing this now.

[isabelastisser]

Sorry about the payment error @daraksha-dk and @sobitneupane. I see that @daraksha-dk was assigned the issue on April 6, and the PR was merged on April 10.

Merged PR within 3 business days of assignment - 50% bonus

Bonus sent in Upwork!

[isabelastisser]

Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

https://github.com/Expensify/Expensify/issues/278042

[daraksha-dk]

@isabelastisser Thanks for the bonus but the issue is actually priced at $4000 as per the GH and in Upwork it is showing as $1000. Let me know if I'm mistaken in any way.

[isabelastisser]

We kept the price at $1000, context in expensify-bugs: https://expensify.slack.com/archives/C049HHMV9SM/p1680192019041749

[daraksha-dk]

@isabelastisser It was initially believed that the issue was simple enough, but as new regressions were discovered, it required a significant amount of research and effort to resolve. While I ultimately concur with the decision made by the team, I can't help but feel that it is somewhat unjust.

cc: @stitesExpensify @shawnborton

[daraksha-dk]

@mallenexpensify Please share your thoughts on this as I've seen you resolving similar issues. Thanks!

[isabelastisser]

I understand @daraksha-dk ! @mallenexpensify @shawnborton and @stitesExpensify , please discuss!

[mallenexpensify]

Reopening pending review.

We kept the price at $1000, context in expensify-bugs: https://expensify.slack.com/archives/C049HHMV9SM/p1680192019041749

This was a recommendation from another contributor so we shouldn't treat that as binding.

@daraksha-dk, can you please outline what you think if fair compensation and why? (there are 80+ comments on this issue and I'm wanting to avoid going down a 🐰🕳 ). Please include timelines if they're relevant and also details on any regressions (and... links to both). Sorry to task ya with more work, I just want to make sure, once a final decision is made, we're doing it based on all the accurate details.

[daraksha-dk]

Thanks @mallenexpensify, I'm providing a summary of things that happened

• At first, we believed that this was a straightforward problem that could be resolved with just one or two lines of style modification. As a result, the proposal selection process was completed by C+ and CME here - Initial Review Completed [at this time the price was already $2000]

• On the same day, another contributor identified potential regressions that would occur if we proceed with the present approach. As a result, it was necessary to refactor certain sections of the code to prevent any such regressions.- New regressions found

• Screenshots with regressions were asked to get them validated from design team and it was shared here - Screenshots 1

• I suggested that we remove * sizeScale usage from our code base, which means a code refactor was needed - @stitesExpensify agreed with the suggestion here

• Although @shawnborton initially approved, I noticed potential confusion and decided to express my concern by providing a summary of the changes that would be made if we chose not to do the refactor - Screenshots 2

• It was also agreed by @shawnborton that the multipliers were unnecessary and recommended that precise values should be utilized instead. Subsequently, screenshots containing the proposed values were requested and shared - Screenshots 3 [at this point the price doubled to $4000]

• After one more screenshot we agreed to move forward with my latest proposal - Green signal from @sobitneupane

• @stitesExpensify came back from OOO and also gave the green signal

As you can see that I was going to be assigned when the issue was at $2000 but as the regressions were discovered I didn't get assigned and anyone with a better solution could have gotten it. My understanding is that all the contributors are dedicating significant time and effort to addressing issues, providing thoughtful responses, and refining their proposals in order to increase the likelihood of being selected and the compensation is being offered as an incentive for this hard work. This appears to be consistently followed in almost all the cases and this case should be no different. However, ultimately, I will respect and agree to the team's decision on how this matter should be handled like I said earlier!

[mallenexpensify]

Thanks for the details @daraksha-dk , a couple more questions

• Was there a regression from your PR after it was deployed to production? It doesn't appear so. (sidenote, CONTRIBUTING.md is getting update now to reflect the below which wasn't crystal-clear in the previous version)

If the PR causes a regression at any point within the regression period (starting when the code is merged and ending 7 days after being deployed to production), contributors are not eligible for the 50% bonus."

• Was there any other basis for the decision for the $1000 payment besides the link to the slack post here?

I'm going to be OOO most of the next few days, apologies in advance if I don't get back to this til Monday (I'll try though)

[daraksha-dk]

@mallenexpensify

Was there a regression from your PR after it was deployed to production?

There are no regressions till now linked to my PR

Was there any other basis for the decision for the $1000 payment besides the link to the slack post here?

As far as I know, there isn't any other justification for this decision. If there had been a separate discussion on this matter, I believe it would have been mentioned either on GH or in the OP's Slack thread (and price would have been updated then).

Thanks for looking into it!

[mallenexpensify]

Thanks @daraksha-dk, can you elaborate on

There are no regressions till now

If there are any regressions since the PR was deployed to production, can you link them? I'm not finding any.

[daraksha-dk]

@mallenexpensify Sorry if it wasn't clear, I just meant that we haven't found/caught any regressions due to my PR.

[isabelastisser]

@mallenexpensify is OOO, waiting until next week.

[isabelastisser]

Not overdue!

[mallenexpensify]

Thanks @daraksha-dk, I can't see a reason why they job price shouldn't be $4000, as we didn't revert it back to $2000 when it auto-doubled. And the 50% would be on top of that so total amount should be $6000.

@isabelastisser @stitesExpensify , is there something I'm missing? (outside of the Slack comment suggesting a different price?)

Sidenotes....

• We're not auto-doubling job prices anymore so unexpected increases shouldn't happen anymore.

the compensation is being offered as an incentive for this hard work.

One clarification I'd make here is, as much as we appreciate the hard work, compensation is strictly being offered to fix bugs, regardless of how much work it might take.

[isabelastisser]

Thanks for the explanation, @mallenexpensify ! :) @stitesExpensify please let me know if I can move forward with the payment increase in Upwork now.

[daraksha-dk]

bump @stitesExpensify

[stitesExpensify]

@isabelastisser yep let's go ahead and move forward with payment.

[isabelastisser]

Payment updated in Upwork! Thanks for the discussion and help, team!

[daraksha-dk]

@isabelastisser Appreciate it, however, I believe the total payment should be $6000 as indicated here. Let me know if I'm mistaken

[mallenexpensify]

Closing the loop, @daraksha-dk is paid in full