[HOLD for payment 2023-07-20] [HOLD for payment 2023-07-17] [$1000] Chat - Area outside the group avatar is clickable

[lanitochka17]

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

---

Issue found when executing PR #21402

Action Performed:

1. Go to staging.new.expensify.com
2. Create a group of five
3. Resize the conversation view to small screen
4. Tap outside the group avatar

Expected Result:

The area outside the group avatar is not clickable

Actual Result:

The area outside the group avatar is clickable

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.3.38.3

Reproducible in staging?: Yes

Reproducible in production?: No

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: Any additional supporting documentation

Bug6120101_bandicam_2023-07-08_04-10-13-252.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016720a203167c26a6
• Upwork Job ID: 1678527009111691264
• Last Price Increase: 2023-07-10

[OSBotify]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @stitesExpensify (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[allroundexperts]

Proposal

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

Area outside the group avatar is clickable

What is the root cause of that problem?

The horizontal padding included here is considered to be part of the Pressable when clicking on the avatar group.

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

We need to replace the padding here with margins.

If we want to keep on using the paddings, then we have to shift them to the wrapping component instead.

What alternative solutions did you explore? (Optional)

None

[DrLoopFall]

Proposal

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

The area outside the group avatar is clickable

What is the root cause of that problem?

There are actually two problems here,

1. We are using padding here, which expands the clickable area
   
   

   App/src/pages/home/report/ReportActionItemCreated.js

   Line 76 in 60f4817

   style={[styles.ph5, styles.pb3, styles.alignSelfStart]}

2. We are using left for positioning/shifting avatars but the PressableWithoutFeedback itself is not shifted, which causes it to be clickable in both the original and shifted positions.
   
   

   App/src/styles/StyleUtils.js

   Line 906 in 60f4817

   left: -(overlapSize * index),

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

Make individual avatars/row clickable.

Remove pressable from here

App/src/pages/home/report/ReportActionItemCreated.js

Line 74 in 60f4817

<PressableWithoutFeedback

Wrap this with PressableWithoutFeedback

App/src/components/MultipleAvatars.js

Line 157 in 60f4817

<UserDetailsTooltip

or this

App/src/components/MultipleAvatars.js

Line 152 in 60f4817

<View

We add onPress prop in MultipleAvatars

What alternative solutions did you explore? (Optional)

Replace paddings with margins.

- style={[styles.ph5, styles.pb3, styles.alignSelfStart]} 
+ style={[styles.mh5, styles.mb3, styles.alignSelfStart]} 

Use margin-left instead of left for positioning the avatar.

- left: -(overlapSize * index)
+ marginLeft: index ? -overlapSize : 0

We also need to update(or remove) the width

App/src/components/MultipleAvatars.js

Lines 138 to 144 in 60f4817

	if (props.icons.length > 4) {
	// Width of overlapping avatars + border space
	width = oneAvatarSize.width * 3 + oneAvatarBorderWidth * 8;
	} else {
	// one avatar width + overlaping avatar sizes + border space
	width = oneAvatarSize.width + overlapSize * 2 * (props.icons.length - 1) + oneAvatarBorderWidth * (props.icons.length * 2);
	}

NOTE: In alternate solution this gap is still clickable.

[situchan]

Proposal

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

Chat - welcome area outside the group avatar is clickable

What is the root cause of that problem?

There are 2 root causes.

• red space comes from wrong style in PressableWithoutFeedback of ReportActionItemCreated
• green right space comes from wrong calculation of avatarContainerStyles width. It was old logic to treat count of avatars as 4 if more than 4 but after [HOLD for payment 2023-07-17] [$2000] Feature Request: Update header for 6 person DMs to show all avatars #15929, they're wrapped into 2 lines so count is now Math.ceil(icon length / 2). In above example, count is 3.

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

Based on above analysis,

• replace padding with margin
  

  App/src/pages/home/report/ReportActionItemCreated.js

  Line 76 in 60f4817

  style={[styles.ph5, styles.pb3, styles.alignSelfStart]}

After fix:

• refactor width calculation logic: get max length of row

        const length = avatarRows.length > 1 ? Math.max(avatarRows[0].length, avatarRows[1].length) : avatarRows[0].length;
        const width = oneAvatarSize.width + overlapSize * 2 * (length - 1) + oneAvatarBorderWidth * (length * 2);

After fix:

[stitesExpensify]

Okay so everyone so far has pointed to this line as the culprit, but as far as I can tell that hasn't been updated in 6 months. What happened in this staging deploy that caused things to break since it's working properly on production?

[stitesExpensify]

Was it just something that was missed by this pr maybe? https://github.com/Expensify/App/pull/20512/files

[situchan]

@stitesExpensify it was missed by #15929

[situchan]

In my proposal, first issue existed long time. 2nd issue was caused by #15929

[DrLoopFall]

@stitesExpensify I think the width was set as a workaround for the issue described in my proposal (the 2nd issue)

We are using left for positioning/shifting avatars but the PressableWithoutFeedback itself is not shifted, which causes it to be clickable in both the original and shifted positions.

If we use margin-left instead, we can remove the width, which may prevent any future issues.

[stitesExpensify]

Hm okay. I think that your proposal makes sense @situchan. @puneetlath @WikusKriek what do you think since it looks like this PR is what caused the issue #15929

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~016720a203167c26a6

[melvin-bot]

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

[DrLoopFall]

Hi @stitesExpensify, @puneetlath and @Christinadobrzyn,
May I know why @situchan's proposal was selected over my proposal, as my proposal fixes the actual root cause of the issue, not fixing some workaround.

Also, @situchan's proposal is almost a duplicate of my proposal.

[situchan]

@DrLoopFall please check PR. It was already merged.
Calculating container width correctly is already existing logic and it's not a workaround, and not dupe of your proposal.
If you think this logic should be refactored, please feel free to raise discussion on slack.

[DrLoopFall]

@situchan, I didn't say your proposed logic is a workaround, I said the solution you provided was fixing a workaround that was already there. So I proposed to totally remove that workaround and fix it in the root itself.

@stitesExpensify I think the width was set as a workaround for the issue described in my proposal (the 2nd issue)

We are using left for positioning/shifting avatars but the PressableWithoutFeedback itself is not shifted, which causes it to be clickable in both the original and shifted positions.

If we use margin-left instead, we can remove the width, which may prevent any future issues.

[puneetlath]

@DrLoopFall I didn't totally follow your proposal and since it was a deploy blocker, I went with @situchan's since I understood it. It's definitely possible that your solution is better.

It looks like the solution we implemented created a bug. Perhaps you can post your proposal on this issue and we can discuss further? #22632

[DrLoopFall]

@puneetlath Thank you,
I've submitted a proposal there, as I already said using left was causing the problem, please have a look there.

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.39-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix clickable area of multiple avatars component #22595

If no regressions arise, payment will be issued on 2023-07-20. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

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

[Christinadobrzyn]

Reassigning as I'm going ooo. @sonialiap This is just waiting for payment based on #22445 (comment) Thanks!

[sonialiap]

Paid ✔️