[HOLD for Regression Test] [$1000] Emoji button on hover background doesn't have proper padding (background touches the icon border) in the dark theme.

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Open any chat
2. Hover on the emoji button on the composer,
3. Check the grey hover background

Expected Result:

Grey hover background should have proper padding

Actual Result:

Hover background doesn't have horizontal padding and it touches the icon.

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• Desktop App

Version Number: 1.2.33-2
Reproducible in staging?: y
Reproducible in production?: new UI
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: @mananjadhav
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1669722656145379

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0116174b9a06c6ad84
• Upwork Job ID: 1598772700294258688

[melvin-bot]

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

[Puneet-here]

Proposal

Remove margin: 3

App/src/styles/styles.js

Line 1503 in 58c22bc

margin: 3,

instead pass

marginVertical: 3,
paddingHorizontal: 3,

If we want to make it round then increase the borderRadius (variables.buttonBorderRadius can be used) and increase the paddingHorizontal (it should be 5 or 6)

[Pujan92]

Proposal

Update the below style changes to highlight only the emoji icon.

App/src/styles/styles.js

Lines 1499 to 1505 in 58c22bc

	chatItemEmojiButton: {
	alignSelf: 'flex-end',
	borderRadius: 6,
	height: 32,
	margin: 3,
	justifyContent: 'center',
	},

chatItemEmojiButton: {
      alignSelf: 'center',
      borderRadius: '50%',
      margin: 3,
},

[mananjadhav]

I reported this bug and it still needs to be triaged.

@Pujan92 I don't think we should be removing the hover background. This is added for all the icons I suppose.

Tagging @Expensify/design as well.

[Pujan92]

Ok, In that case I think the below properties work well. As we are aligning it to the center with alignSelf we can remove other(height, justifyContent) properties.

    chatItemEmojiButton: {
        alignSelf: 'center',
        margin: 2,
        padding: 6,
        borderRadius: 6,
    },

[shawnborton]

Can you pull main and test again? We just made our buttons round, so I think we would want the emoji button to be round and the exact same size as the send button as well.

[Puneet-here]

@shawnborton, is this expected?

[shawnborton]

Ah apologies, once this one is merged then let's test again

[mananjadhav]

@shawnborton do you want to fix this one as a part of that PR? I reviewed the PR and I can confirm it is not fixed in that PR.?

[shawnborton]

Hmm I think that PR is about to be merged, so I'll let @grgia decide: either we can make a quick follow up issue, or we could squeeze this change into that PR before it gets merged.

[Puneet-here]

Hey @shawnborton, we already have this issue for emoji button and I also have a proposal.

Here's the result

[shawnborton]

Okay cool, that also works for me. cc @greg-schroeder so we can hire @Puneet-here for this one

[grgia]

I'll go ahead and move this forward and assign @Puneet-here

Just a heads up for the future,

Per the Contribution Guide
"Issues that have not had the External label applied have not yet been approved for implementation. This means, if you propose a solution to an issue without the External label (which you are allowed to do) it is possible that the issue will be fixed internally. If the External label has not yet been applied, Expensify has the right to use your proposal to fix said issue, without providing compensation for your solution. This process covers the very rare instance where we need or want to fix an issue internally."

[melvin-bot]

Current assignee @greg-schroeder is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0116174b9a06c6ad84

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

@shawnborton, @greg-schroeder, @rushatgabhane, @grgia, @Puneet-here Whoops! This issue is 2 days overdue. Let's get this updated quick!

[JmillsExpensify]

@greg-schroeder Can we get a regression test on this one? It's going on three weeks, so it'd be great to get that step crossed off. Let's close this issue out.

[melvin-bot]

@shawnborton, @greg-schroeder, @rushatgabhane, @grgia, @Puneet-here Still overdue 6 days?! Let's take care of this!

[JmillsExpensify]

@greg-schroeder Do you need help getting the regression test done on this one? I'm also going to reach out to you via DM to make sure you don't have something wonky going on with notifications.

[melvin-bot]

@shawnborton, @greg-schroeder, @rushatgabhane, @grgia, @Puneet-here 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

[melvin-bot]

@shawnborton, @greg-schroeder, @rushatgabhane, @grgia, @Puneet-here 10 days overdue. I'm getting more depressed than Marvin.

[melvin-bot]

This issue has not been updated in over 14 days. @shawnborton, @greg-schroeder, @rushatgabhane, @grgia, @Puneet-here eroding to Weekly issue.

[mallenexpensify]

@greg-schroeder , guessing you didn't see this cuz the Reviewing label was applied and it wasn't going/showing as overdue. Check here for instructions for the regression test steps https://stackoverflow.com/c/expensify/questions/14574

[greg-schroeder]

Okay working on regression steps

[shawnborton]

Not overdue

[greg-schroeder]

Final step is regression test, working on that this week

[grgia]

Not overdue

[shawnborton]

Not overdue

[greg-schroeder]

Posted in slack re: regression here: https://expensify.slack.com/archives/C01SKUP7QR0/p1675082181227879

[greg-schroeder]

Per slack, uniform padding/spacing covered by design guidelines: https://expensify.slack.com/archives/C01SKUP7QR0/p1675088862292759?thread_ts=1675082181.227879&cid=C01SKUP7QR0