[HOLD for 0xmiroslav] [$1000] Web - Subscript avatar not showing in workspace chat

[kbecciv]

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. Login A account with policyExpenseChat beta enabled
2. Invite B to any workspace that has custom workspace avatar
3. On A, Go to B's policy expense chat
   (custom workspace avatar (not default avatar) is key to reproduce)

Expected Result:

Workspace avatar should show as subscript

Actual Result:

No subscript avatar

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.34-1
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
Notes/Photos/Videos: Any additional supporting documentation

bug.2.mov

Recording.3349.mp4

Expensify/Expensify Issue URL:
Issue reported by: @0xmiroslav
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1688056702660869

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0192a425ecc8391c30
• Upwork Job ID: 1679109827603726336
• Last Price Increase: 2023-07-12

[melvin-bot]

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

[melvin-bot]

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

[CortneyOfstad]

Heading OoO for the week (back July 10) so re-assigning this in the meantime 👍

[melvin-bot]

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

[melvin-bot]

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

[jeet-dhandha]

Proposal

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

• Subscript Avatar is not visible when an image is uploaded.

What is the root cause of that problem?

• Root cause is that the imageStyles property is being passed as null to Avatar component in SubscriptAvatar component.

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

• Passing imageStyles as [styles.avataravatarSmall] property to Avatar component fixes the issue.

Code Changes:

function SubscriptAvatar(props) {
   ...rest code
    return (
        ...rest code
                    <Avatar
                        ...rest params
                    />
        ...rest code
                    <Avatar
-                       imageStyles={null}
+                       imageStyles={[styles.avatarSmall]}
                        ...rest params
                    />
        ...rest code
    );
}

After change:

What alternative solutions did you explore? (Optional)

• N/A

[c3024]

Proposal

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

Subscript Avatar is not visible if workspace avatar is changed from default.

What is the root cause of that problem?

In web, Image needs size for proper display. This bug appears only in web and not in native.
This is because second Avatar in SubscriptAvatar is passed imageStyle={null} and this null value causes imageStyle to undefined and style undefined passed to the Image with Image getting only source and onError props.

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

Since we pass imageStyles: null to Avatar only from SubscriptAvatar and default value of imageStyles in Avatar is [] which is also truthy, we can change imageStyle to this line in Avatar

const imageStyle = props.imageStyles ? [StyleUtils.getAvatarStyle(props.size), ...props.imageStyles, StyleUtils.getAvatarBorderRadius(props.size, props.type)] : [StyleUtils.getAvatarStyle(props.size), StyleUtils.getAvatarBorderRadius(props.size, props.type)];

This gives size for web as well and the image appears and is consistent with the default icon styling as well.

Removing this imageStyle altogether from Avatar or changing it to another style affects the iconStyle property in Avatar and the styling for default icon gets affected with right padding getting smaller. Fixing this needs many more styling changes. So I think the above is the simplest solution.

What alternative solutions did you explore? (Optional)

N/A

[0xmiros]

NOTE: this is a follow-up of #20512.
I reported this bug and found solution already while reviewing that PR.
Let me work on this issue, not open for proposals.

[c3024]

If that is the case since, that PR was not merged, they might as well change this in that PR itself and fix it instead of making another report and working again on it separately.

[0xmiros]

@c3024 we already agreed to fix separately. #20512 (comment)

[melvin-bot]

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

[JmillsExpensify]

Slammed with bug reports. I'll close the loop on these today.

[Christinadobrzyn]

@0xmiroslav would you be able to confirm if this GH is the same? #22241

[0xmiros]

@0xmiroslav would you be able to confirm if this GH is the same? #22241

yes, exactly same

[melvin-bot]

@JmillsExpensify Huh... This is 4 days overdue. Who can take care of this?

[JmillsExpensify]

Alright, this one is ready to be triaged!

[melvin-bot]

@JmillsExpensify, @0xmiroslav Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@JmillsExpensify, @0xmiroslav 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

@JmillsExpensify, @0xmiroslav 10 days overdue. Is anyone even seeing these? Hello?

[JmillsExpensify]

Still waiting for payment next steps.

[melvin-bot]

@JmillsExpensify, @0xmiroslav Huh... This is 4 days overdue. Who can take care of this?

[JmillsExpensify]

Are we good to pay this one out yet?

[0xmiros]

Not yet

[melvin-bot]

@JmillsExpensify, @0xmiroslav Huh... This is 4 days overdue. Who can take care of this?

[JmillsExpensify]

Still holding.

[melvin-bot]

@JmillsExpensify, @0xmiroslav Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

@JmillsExpensify, @0xmiroslav Still overdue 6 days?! Let's take care of this!

[melvin-bot]

@JmillsExpensify, @0xmiroslav Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

[JmillsExpensify]

@0xmiroslav should we still hold for payment?

[0xmiros]

Still hold

[melvin-bot]

@JmillsExpensify, @0xmiroslav Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@JmillsExpensify, @0xmiroslav Still overdue 6 days?! Let's take care of this!

[melvin-bot]

@JmillsExpensify, @0xmiroslav 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

[melvin-bot]

@JmillsExpensify, @0xmiroslav 12 days overdue now... This issue's end is nigh!

[JmillsExpensify]

Similar to other issues, I'm going to close this issue since I'm overwhelmed with GH assignments. Please reach out to us when you are ready for payment.