[HOLD for payment 2024-02-14] [$500] A border is being displayed on the second image of the group chat in the search model

[kavimuru]

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

---

Version Number: v1.4.24-7
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: Applause internal team
Slack conversation:

Action Performed:

1. Create group chat and send a message
2. Go to the search model
3. Hover over the first group chat
4. Notice only second image displayed with border

Expected Result:

A border should not displayed on second image of the first group chat in the search model

Actual Result:

A border is being displayed on the second image of the group chat in the search model

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

Add any screenshot/video evidence

Bug6339897_1705050171836.2024-01-12_00-29-44.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0135be644839e7389d
• Upwork Job ID: 1745742363732340736
• Last Price Increase: 2024-01-12
• Automatic offers:
• bernhardoj | Contributor | 28128985

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0135be644839e7389d

[melvin-bot]

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

[melvin-bot]

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

[bernhardoj]

Proposal

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

The second avatar of a group chat shows a border while hovered.

What is the root cause of that problem?

The border color is actually doesn't match the background color when we hover over a focused option.

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

We need to prevent the border from being applied when the option is focused.

App/src/components/OptionRow.tsx

Lines 209 to 221 in b6a18b1

	<SubscriptAvatar
	mainAvatar={option.icons[0]}
	secondaryAvatar={option.icons[1]}
	backgroundColor={hovered ? hoveredBackgroundColor : subscriptColor}
	size={CONST.AVATAR_SIZE.DEFAULT}
	/>
	) : (
	<MultipleAvatars
	icons={option.icons}
	size={CONST.AVATAR_SIZE.DEFAULT}
	secondAvatarStyle={[StyleUtils.getBackgroundAndBorderStyle(hovered ? hoveredBackgroundColor : subscriptColor)]}
	shouldShowTooltip={showTitleTooltip && OptionsListUtils.shouldOptionShowTooltip(option)}
	/>

(hovered && !optionIsFocused)

just like we did in OptionRowLHN

[dukenv0307]

Proposal

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

A border is being displayed on the second image of the group chat in the search model

What is the root cause of that problem?

We have a border width and border of the subscription avatar here.

App/src/components/SubscriptAvatar.tsx

Lines 89 to 90 in b6a18b1

	StyleUtils.getAvatarBorderWidth(isSmall ? CONST.AVATAR_SIZE.SMALL_SUBSCRIPT : CONST.AVATAR_SIZE.SUBSCRIPT),
	StyleUtils.getBorderColorStyle(backgroundColor ?? theme.componentBG),

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

We should remove this border style.

App/src/components/SubscriptAvatar.tsx

Lines 89 to 90 in b6a18b1

	StyleUtils.getAvatarBorderWidth(isSmall ? CONST.AVATAR_SIZE.SMALL_SUBSCRIPT : CONST.AVATAR_SIZE.SUBSCRIPT),
	StyleUtils.getBorderColorStyle(backgroundColor ?? theme.componentBG),

What alternative solutions did you explore? (Optional)

NA

[dragnoir]

Proposal

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

A border is being displayed on the second image of the group chat in the search model

What is the root cause of that problem?

OptionRow, src/components/OptionRow.tsx, is calculating the hoveredBackgroundColor

App/src/components/OptionRow.tsx

Line 145 in a3088d5

const hoveredBackgroundColor = hoveredStyle?.backgroundColor ? (hoveredStyle.backgroundColor as string) : backgroundColor;

The other components like SubscriptAvatar and MultipleAvatars are using the value hoveredBackgroundColor to set the border color:

The issue is that the hoveredBackgroundColor is not updated when the Row is focused

If we remove the border style, when hovering, there will be issues. BEcause now there's always a border === to the background color and it gives a feeling that there's transparency

When no hovered, border === background color
when hovered, border === new background color

When focused, we need to set background color to the focusBAckground color to adjust the border color.

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

on OptionRow, update here:

if (optionIsFocused) {
  subscriptColor  =  focusedBackgroundColor;
+  hoveredBackgroundColor  =  focusedBackgroundColor;
}

We are updating the value of hoveredBackgroundColor to be equal to focusedBackgroundColor when optionIsFocused is true.

Commit: src/components/OptionRow.tsx

POC

17.mp4

[melvin-bot]

@slafortune, @thesahindia Whoops! This issue is 2 days overdue. Let's get this updated quick!

[thesahindia]

@bernhardoj's proposal looks good to me!

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[dragnoir]

@thesahindia did you checked my proposal? Can I pls ask how you picked the best proposal?

[dragnoir]

@iwiznia I would like to respectfully request if you could @thesahindia kindly revisit my proposal. I believe it addresses some crucial points that could enhance our project's functionality and efficiency. Here are my thoughts

• prevent the border from being applied is a workaround and not solving the core issue, the component <SubscriptAvatar> and other components interact with the props that set the color. The issue here is that this last value is wrong, so I thionk fixing the value is better than trying to igonore it
• Hover Interaction Consistency: It's important to maintain interaction cues like hover effects. Currently, the hover or focus elements blend with the background color, which could be problematic if the background color changes in the future. Therefore, adjusting the border color to maintain the hover interaction seems like a more sustainable approach.
• Global Usability of and and others: These components are not just limited to our use case in the OptionRow but could be utilized in various scenarios. Thus, it's essential to keep the logic global and adaptable, instead of circumventing the hover effect on focused elements. This approach ensures that we cater to all potential use cases, not just the ones where the hover color matches the focus background.
• Comprehensive Solution to Component Issues: The solution in the selected proposal only addresses a specific issue with , while the problem also affects other components like . I believe that resolving the root cause in the code would be more beneficial than just focusing on a single aspect of the hover effect across multiple components.

I've detailed these points in my proposal, along with a proof-of-concept video and branch code. I would greatly appreciate any feedback or explanations on why the current workaround might be considered a preferable solution. Your insights would be extremely valuable.

Thank you for taking the time to consider my request.

[bernhardoj]

@dragnoir hi, I don't see how your proposal is different from mine.

backgroundColor={(hovered && !optionIsFocused) ? hoveredBackgroundColor : subscriptColor}

vs

if (optionIsFocused) {
  subscriptColor  =  focusedBackgroundColor;
+  hoveredBackgroundColor  =  focusedBackgroundColor;
}

Yes, it's written differently, but the core concept is the same.

Note: Before submitting a proposal on an issue, be sure to read any other existing proposals. ALL NEW PROPOSALS MUST BE DIFFERENT FROM EXISTING PROPOSALS. The difference should be important, meaningful or considerable.

[dragnoir]

@bernhardoj my proposal is 100% different from yours.

We need to prevent the border from being applied when the option is focused.

Your solution is about preventing the border from being applied.

My solution is to allow border BUT with the right value

(hovered && !optionIsFocused)

Your solution will need custom conditions for each style on all the componets that OptionRow uses. Wish effect even the components behavior in future uses.

My solution is one line of code that fix oll withgout editing components logic and keeping the same logic in all the components I mentioned.

As you mentioned:

The border color is actually doesn't match the background color when we hover over a focused option.

But your proposal is about ignoring the issues, hiding it. You said it, doesn't match, so it's better to match it

My solution will match the border to the background color.

[bernhardoj]

We need to prevent the border from being applied when the option is focused.

Your solution is about preventing the border from being applied.

I think there is a misunderstanding on how I word it. The issue here is that the hovered border color is applied when it shouldn't, so the solution is simply preventing that when it's focused just like we did in OptionRowLHN (which I put this ref on my solution too).

You can compare the results between mine and your solution and you will notice there will be no difference. The difference is in how we write the code and you are proposing to make it DRY. Making it DRY is something that can be done in the PR stage.

Let's wait for the team to reply.

[dragnoir]

@bernhardoj again our solutions are totally different.

• You haven't catched that focusedBackgroundColor is the right value to use
• You are not updating the value of hoveredBackgroundColor before the return
• and lot's I already mentioned.

Yes I agree, let's wee what @iwiznia @slafortune @thesahindia will decide and I will respect their decition.

[slafortune]

@iwiznia @thesahindia just wanted to verify that all has been considered and we can move forward with this

[iwiznia]

Hmmm @thesahindia what's your take here?

[thesahindia]

@dragnoir, I don't know if you tested the solution correctly because I am not seeing that issue. Here's what I got -

@iwiznia, I am not seeing any major reason for not going with the first proposal that @bernhardoj proposed. I will leave the final decision to you.

[dragnoir]

@thesahindia can you pls share the bransh you tested with or the code difference you created? I am 100% sure of what I didi, I shared the code diff, screenshots and lots of explanation. At least pls share the bransh you tested with.

[melvin-bot]

@iwiznia, @slafortune, @thesahindia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[bernhardoj]

PR is ready

cc: @thesahindia

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• Fix the second avatar border color doesn't match the background when hovered in search page #35354

If no regressions arise, payment will be issued on 2024-02-14. 🎊

For reference, here are some details about the assignees on this issue:

• @bernhardoj requires payment automatic offer (Contributor)
• @thesahindia requires payment through NewDot Manual Requests

[melvin-bot]

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:

• [@thesahindia] The PR that introduced the bug has been identified. Link to the PR:
• [@thesahindia] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@thesahindia] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@thesahindia] Determine if we should create a regression test for this bug.
• [@thesahindia] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@slafortune] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[slafortune]

@thesahindia can you complete the checklist above? Thanks!

[slafortune]

@bernhardoj - automatic offer (Contributor) Paid $500

@thesahindia requires payment through NewDot Manual Requests - $500
Waiting on checklist.

[thesahindia]

Test case steps -

1. Open a group chat and send any message
2. Open search page
3. The first item on the search page should be the group chat. Hover over it.
4. Verify the border color of the second avatar still matches with the background color
5. Press the arrow down key once to move the focused option to the next item
6. Repeat step 3-4

[slafortune]

@thesahindia requires payment through NewDot Manual Requests - $500

[JmillsExpensify]

$500 approved for @thesahindia