From 52c02ac02bcb3435ef3c370ff65e82d5f9ed748e Mon Sep 17 00:00:00 2001
From: James Peach <jpeach@cloudflare.com>
Date: Fri, 6 May 2022 15:05:15 +1000
Subject: [PATCH] Don't set the background color on Avatars with pictures.

When an Avatar component has a picture, the image may have a transparent
background. In that case, we don't know whether the generated background
color will work well with the image, or will clash with the image colors.
It's better to not generate a background color so the Avatar image will at
least render with predictable colors.

This fixes #11094.

Signed-off-by: James Peach <jpeach@cloudflare.com>
---
 .changeset/good-beans-knock.md                |  5 +++++
 .../src/components/Avatar/Avatar.test.tsx     | 21 +++++++++++++++++++
 .../src/components/Avatar/Avatar.tsx          | 16 ++++++++++----
 3 files changed, 38 insertions(+), 4 deletions(-)
 create mode 100644 .changeset/good-beans-knock.md

diff --git a/.changeset/good-beans-knock.md b/.changeset/good-beans-knock.md
new file mode 100644
index 0000000000..91de53c128
--- /dev/null
+++ b/.changeset/good-beans-knock.md
@@ -0,0 +1,5 @@
+---
+'@backstage/core-components': patch
+---
+
+Don't set the background color on an Avatar component that has a picture.
diff --git a/packages/core-components/src/components/Avatar/Avatar.test.tsx b/packages/core-components/src/components/Avatar/Avatar.test.tsx
index 1c08d57679..d7bbf712fe 100644
--- a/packages/core-components/src/components/Avatar/Avatar.test.tsx
+++ b/packages/core-components/src/components/Avatar/Avatar.test.tsx
@@ -17,6 +17,7 @@
 import { render } from '@testing-library/react';
 import React from 'react';
 import { Avatar } from './Avatar';
+import { stringToColor } from './utils';
 
 describe('<Avatar />', () => {
   it('renders without exploding', async () => {
@@ -24,4 +25,24 @@ describe('<Avatar />', () => {
 
     expect(getByText('JD')).toBeInTheDocument();
   });
+
+  it('generates a background color', async () => {
+    const bgcolor = stringToColor('John Doe');
+    const { getByText } = render(<Avatar displayName="John Doe" />);
+    expect(getByText('JD')).toHaveStyle(`background-color: ${bgcolor}`);
+  });
+
+  it('does not generate a background color when a picture is given', async () => {
+    const bgcolor = stringToColor('John Doe');
+    const { getByAltText } = render(
+      <Avatar
+        displayName="John Doe"
+        picture="https://backstage.io/test/john-doe.png"
+      />,
+    );
+
+    expect(getByAltText('John Doe')).not.toHaveStyle(
+      `background-color: ${bgcolor}`,
+    );
+  });
 });
diff --git a/packages/core-components/src/components/Avatar/Avatar.tsx b/packages/core-components/src/components/Avatar/Avatar.tsx
index 304ee574d2..af8dabf554 100644
--- a/packages/core-components/src/components/Avatar/Avatar.tsx
+++ b/packages/core-components/src/components/Avatar/Avatar.tsx
@@ -67,15 +67,23 @@ export interface AvatarProps {
 export function Avatar(props: AvatarProps) {
   const { displayName, picture, customStyles } = props;
   const classes = useStyles();
+  let styles = { ...customStyles };
+  // We only calculate the background color if there's not an avatar
+  // picture. If there is a picture, it might have a transparent
+  // background and we don't know whether the calculated background
+  // color will clash.
+  if (!picture) {
+    styles = {
+      backgroundColor: stringToColor(displayName || ''),
+      ...customStyles,
+    };
+  }
   return (
     <MaterialAvatar
       alt={displayName}
       src={picture}
       className={classes.avatar}
-      style={{
-        backgroundColor: stringToColor(displayName || picture || ''),
-        ...customStyles,
-      }}
+      style={styles}
     >
       {displayName && extractInitials(displayName)}
     </MaterialAvatar>