From ad5e16cfa7515f89c553cd73c18594d084d5eb9c Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Mon, 15 Jun 2020 19:24:23 -0300
Subject: [PATCH] Fix site icon size (#8814)

A new `SiteIcon` component has been created for showing icons
representing web3 sites. The icon has a border and background, and it
has a fallback in case no icon is given. This new component accepts a
`size` prop that controls the size of the icon.

The old `IconWithFallback` component had a hard-coded size in the
SCSS styles for the icon, which was being overridden in a few places.
It was difficult to customize, and overly complicated.

The old `IconWithFallback` component is still used, but it's now
simpler. It only handles rendering the underlying `img` for the icon,
or the fallback letter if no image is given.

A separate `IconBorder` component has been created for the border and
white background used. It's solely used by `SiteIcon` for now, but I
intend to use it elsewhere as well, where this same pattern of a white
background is embedded.
---
 .../account-menu/account-menu.component.js    |  4 +-
 .../connected-sites-list.component.js         |  4 +-
 ui/app/components/app/index.scss              |  6 ++-
 .../permissions-connect-header.component.js   |  4 +-
 .../components/ui/icon-border/icon-border.js  | 16 +++++++
 .../ui/icon-border/icon-border.scss           | 10 ++++
 ui/app/components/ui/icon-border/index.js     |  1 +
 .../icon-with-fallback.component.js           | 36 +++++++--------
 .../icon-with-fallback.scss                   |  5 ++
 .../ui/icon-with-fallback/index.scss          | 30 ------------
 ui/app/components/ui/site-icon/index.js       |  1 +
 ui/app/components/ui/site-icon/site-icon.js   | 24 ++++++++++
 .../permissions-connect/redirect/index.scss   | 46 -------------------
 .../permissions-redirect.component.js         | 10 ++--
 14 files changed, 86 insertions(+), 111 deletions(-)
 create mode 100644 ui/app/components/ui/icon-border/icon-border.js
 create mode 100644 ui/app/components/ui/icon-border/icon-border.scss
 create mode 100644 ui/app/components/ui/icon-border/index.js
 create mode 100644 ui/app/components/ui/icon-with-fallback/icon-with-fallback.scss
 delete mode 100644 ui/app/components/ui/icon-with-fallback/index.scss
 create mode 100644 ui/app/components/ui/site-icon/index.js
 create mode 100644 ui/app/components/ui/site-icon/site-icon.js

diff --git a/ui/app/components/app/account-menu/account-menu.component.js b/ui/app/components/app/account-menu/account-menu.component.js
index f6245f304..6409a9cd6 100644
--- a/ui/app/components/app/account-menu/account-menu.component.js
+++ b/ui/app/components/app/account-menu/account-menu.component.js
@@ -8,7 +8,7 @@ import { Menu, Item, Divider, CloseArea } from '../dropdowns/components/menu'
 import { ENVIRONMENT_TYPE_POPUP } from '../../../../../app/scripts/lib/enums'
 import { getEnvironmentType } from '../../../../../app/scripts/lib/util'
 import Identicon from '../../ui/identicon'
-import IconWithFallBack from '../../ui/icon-with-fallback'
+import SiteIcon from '../../ui/site-icon'
 import UserPreferencedCurrencyDisplay from '../user-preferenced-currency-display'
 import { PRIMARY } from '../../../helpers/constants/common'
 import {
@@ -178,7 +178,7 @@ export default class AccountMenu extends Component {
           { iconAndNameForOpenDomain
             ? (
               <div className="account-menu__icon-list">
-                <IconWithFallBack icon={iconAndNameForOpenDomain.icon} name={iconAndNameForOpenDomain.name} />
+                <SiteIcon icon={iconAndNameForOpenDomain.icon} name={iconAndNameForOpenDomain.name} size={32} />
               </div>
             )
             : null
diff --git a/ui/app/components/app/connected-sites-list/connected-sites-list.component.js b/ui/app/components/app/connected-sites-list/connected-sites-list.component.js
index 436b65338..6f6ff4f9a 100644
--- a/ui/app/components/app/connected-sites-list/connected-sites-list.component.js
+++ b/ui/app/components/app/connected-sites-list/connected-sites-list.component.js
@@ -1,6 +1,6 @@
 import React, { Component } from 'react'
 import PropTypes from 'prop-types'
-import IconWithFallBack from '../../ui/icon-with-fallback'
+import SiteIcon from '../../ui/site-icon'
 import { stripHttpSchemes } from '../../../helpers/utils/util'
 
 export default class ConnectedSitesList extends Component {
@@ -28,7 +28,7 @@ export default class ConnectedSitesList extends Component {
         { connectedDomains.map((domain) => (
           <div key={domain.origin} className="connected-sites-list__content-row">
             <div className="connected-sites-list__domain-info">
-              <IconWithFallBack icon={domain.icon} name={domain.name} />
+              <SiteIcon icon={domain.icon} name={domain.name} size={32} />
               <span className="connected-sites-list__domain-name" title={domain.extensionId || domain.origin}>
                 {this.getDomainDisplayName(domain)}
               </span>
diff --git a/ui/app/components/app/index.scss b/ui/app/components/app/index.scss
index 9c8b4a8d5..59088d232 100644
--- a/ui/app/components/app/index.scss
+++ b/ui/app/components/app/index.scss
@@ -90,10 +90,10 @@
 
 @import 'connected-accounts-permissions/index';
 
-@import '../ui/icon-with-fallback/index';
-
 @import '../ui/icon/index';
 
+@import '../ui/icon-with-fallback/icon-with-fallback';
+
 @import '../ui/icon-with-label/index';
 
 @import '../ui/circle-icon/index';
@@ -113,3 +113,5 @@
 @import 'wallet-overview/index';
 
 @import '../ui/account-mismatch-warning/index';
+
+@import '../ui/icon-border/icon-border';
diff --git a/ui/app/components/app/permissions-connect-header/permissions-connect-header.component.js b/ui/app/components/app/permissions-connect-header/permissions-connect-header.component.js
index 46114c8e3..e32f8a0b1 100644
--- a/ui/app/components/app/permissions-connect-header/permissions-connect-header.component.js
+++ b/ui/app/components/app/permissions-connect-header/permissions-connect-header.component.js
@@ -1,6 +1,6 @@
 import PropTypes from 'prop-types'
 import React, { Component } from 'react'
-import IconWithFallBack from '../../ui/icon-with-fallback'
+import SiteIcon from '../../ui/site-icon'
 
 export default class PermissionsConnectHeader extends Component {
   static propTypes = {
@@ -22,7 +22,7 @@ export default class PermissionsConnectHeader extends Component {
 
     return (
       <div className="permissions-connect-header__icon">
-        <IconWithFallBack icon={ icon } name={ iconName } />
+        <SiteIcon icon={ icon } name={ iconName } size={64} />
         <div className="permissions-connect-header__text">{iconName}</div>
       </div>
     )
diff --git a/ui/app/components/ui/icon-border/icon-border.js b/ui/app/components/ui/icon-border/icon-border.js
new file mode 100644
index 000000000..5911cf918
--- /dev/null
+++ b/ui/app/components/ui/icon-border/icon-border.js
@@ -0,0 +1,16 @@
+import React from 'react'
+import PropTypes from 'prop-types'
+
+export default function IconBorder ({ children, size }) {
+  const borderStyle = { height: `${size}px`, width: `${size}px` }
+  return (
+    <div className="icon-border" style={borderStyle}>
+      { children }
+    </div>
+  )
+}
+
+IconBorder.propTypes = {
+  children: PropTypes.node.isRequired,
+  size: PropTypes.number.isRequired,
+}
diff --git a/ui/app/components/ui/icon-border/icon-border.scss b/ui/app/components/ui/icon-border/icon-border.scss
new file mode 100644
index 000000000..6a718c83a
--- /dev/null
+++ b/ui/app/components/ui/icon-border/icon-border.scss
@@ -0,0 +1,10 @@
+
+.icon-border {
+  border-radius: 50%;
+  border: 1px solid #F2F3F4;
+  background: #FFFFFF;
+
+  display: flex;
+  justify-content: center;
+  align-items: center;
+}
diff --git a/ui/app/components/ui/icon-border/index.js b/ui/app/components/ui/icon-border/index.js
new file mode 100644
index 000000000..dea3f116e
--- /dev/null
+++ b/ui/app/components/ui/icon-border/index.js
@@ -0,0 +1 @@
+export { default } from './icon-border'
diff --git a/ui/app/components/ui/icon-with-fallback/icon-with-fallback.component.js b/ui/app/components/ui/icon-with-fallback/icon-with-fallback.component.js
index 13b3e93d7..898861468 100644
--- a/ui/app/components/ui/icon-with-fallback/icon-with-fallback.component.js
+++ b/ui/app/components/ui/icon-with-fallback/icon-with-fallback.component.js
@@ -5,6 +5,7 @@ export default class IconWithFallback extends PureComponent {
   static propTypes = {
     icon: PropTypes.string,
     name: PropTypes.string,
+    size: PropTypes.number.isRequired,
   }
 
   static defaultProps = {
@@ -17,26 +18,21 @@ export default class IconWithFallback extends PureComponent {
   }
 
   render () {
-    const { icon, name } = this.props
+    const { icon, name, size } = this.props
+    const style = { height: `${size}px`, width: `${size}px` }
 
-    return (
-      <div className="icon-with-fallback__identicon-container">
-        <div className="icon-with-fallback__identicon-border" />
-        { !this.state.iconError && icon
-          ? (
-            <img
-              className="icon-with-fallback__identicon"
-              src={icon}
-              onError={() => this.setState({ iconError: true })}
-            />
-          )
-          : (
-            <i className="icon-with-fallback__identicon--default">
-              { name.length ? name.charAt(0).toUpperCase() : '' }
-            </i>
-          )
-        }
-      </div>
-    )
+    return !this.state.iconError && icon
+      ? (
+        <img
+          onError={() => this.setState({ iconError: true })}
+          src={icon}
+          style={style}
+        />
+      )
+      : (
+        <i className="icon-with-fallback__fallback">
+          { name.length ? name.charAt(0).toUpperCase() : '' }
+        </i>
+      )
   }
 }
diff --git a/ui/app/components/ui/icon-with-fallback/icon-with-fallback.scss b/ui/app/components/ui/icon-with-fallback/icon-with-fallback.scss
new file mode 100644
index 000000000..a0cb7cd42
--- /dev/null
+++ b/ui/app/components/ui/icon-with-fallback/icon-with-fallback.scss
@@ -0,0 +1,5 @@
+.icon-with-fallback {
+  &__fallback {
+    color: black;
+  }
+}
diff --git a/ui/app/components/ui/icon-with-fallback/index.scss b/ui/app/components/ui/icon-with-fallback/index.scss
deleted file mode 100644
index 215ea6138..000000000
--- a/ui/app/components/ui/icon-with-fallback/index.scss
+++ /dev/null
@@ -1,30 +0,0 @@
-.icon-with-fallback {
-  &__identicon-container {
-    position: relative;
-    display: flex;
-    justify-content: center;
-    align-items: center;
-    height: 32px;
-    width: 32px;
-  }
-
-  &__identicon-border {
-    height: 32px;
-    width: 32px;
-    border-radius: 50%;
-    border: 1px solid #F2F3F4;
-    position: absolute;
-    background: #FFFFFF;
-  }
-
-  &__identicon {
-    position: relative;
-    width: 24px;
-    height: 24px;
-
-    &--default {
-      position: relative;
-      color: black;
-    }
-  }
-}
diff --git a/ui/app/components/ui/site-icon/index.js b/ui/app/components/ui/site-icon/index.js
new file mode 100644
index 000000000..a68d9779d
--- /dev/null
+++ b/ui/app/components/ui/site-icon/index.js
@@ -0,0 +1 @@
+export { default } from './site-icon'
diff --git a/ui/app/components/ui/site-icon/site-icon.js b/ui/app/components/ui/site-icon/site-icon.js
new file mode 100644
index 000000000..2ddc9f344
--- /dev/null
+++ b/ui/app/components/ui/site-icon/site-icon.js
@@ -0,0 +1,24 @@
+import React from 'react'
+import PropTypes from 'prop-types'
+import IconBorder from '../icon-border'
+import IconWithFallback from '../icon-with-fallback'
+
+export default function SiteIcon ({ icon, name, size }) {
+  const iconSize = Math.floor(size * 0.75)
+  return (
+    <IconBorder size={size}>
+      <IconWithFallback icon={icon} name={name} size={iconSize} />
+    </IconBorder>
+  )
+}
+
+SiteIcon.propTypes = {
+  icon: PropTypes.string,
+  name: PropTypes.string,
+  size: PropTypes.number.isRequired,
+}
+
+SiteIcon.defaultProps = {
+  icon: undefined,
+  name: undefined,
+}
diff --git a/ui/app/pages/permissions-connect/redirect/index.scss b/ui/app/pages/permissions-connect/redirect/index.scss
index 57dbb0fe4..b2cc68554 100644
--- a/ui/app/pages/permissions-connect/redirect/index.scss
+++ b/ui/app/pages/permissions-connect/redirect/index.scss
@@ -33,50 +33,4 @@
     background: white url("/images/permissions-check.svg") no-repeat;
     position: absolute;
   }
-
-  &__identicon, .icon-with-fallback__identicon {
-    width: 32px;
-    height: 32px;
-
-    &--default {
-      background-color: #777A87;
-      color: white;
-      width: 64px;
-      height: 64px;
-      border-radius: 32px;
-      display: flex;
-      align-items: center;
-      justify-content: center;
-      font-weight: bold;
-    }
-  }
-
-  &__identicon-container, .icon-with-fallback__identicon-container {
-    height: auto;
-    position: relative;
-    display: flex;
-    justify-content: center;
-    align-items: center;
-    height: 64px;
-    width: 64px;
-  }
-
-  &__identicon-border, .icon-with-fallback__identicon-border {
-    height: 64px;
-    width: 64px;
-    border-radius: 50%;
-    border: 1px solid white;
-    background: #FFFFFF;
-    box-shadow: 0px 0px 10px rgba(0, 0, 0, 0.25);
-  }
-
-  &__identicon-border {
-    display: flex;
-    justify-content: center;
-    align-items: center;
-  }
-
-  .icon-with-fallback__identicon-border {
-    position: absolute;
-  }
 }
diff --git a/ui/app/pages/permissions-connect/redirect/permissions-redirect.component.js b/ui/app/pages/permissions-connect/redirect/permissions-redirect.component.js
index cbb23d6b1..3795f348c 100644
--- a/ui/app/pages/permissions-connect/redirect/permissions-redirect.component.js
+++ b/ui/app/pages/permissions-connect/redirect/permissions-redirect.component.js
@@ -1,6 +1,6 @@
 import React, { useContext } from 'react'
 import PropTypes from 'prop-types'
-import IconWithFallBack from '../../../components/ui/icon-with-fallback'
+import SiteIcon from '../../../components/ui/site-icon'
 import { I18nContext } from '../../../contexts/i18n'
 
 export default function PermissionsRedirect ({ domainMetadata }) {
@@ -12,16 +12,12 @@ export default function PermissionsRedirect ({ domainMetadata }) {
       <div className="permissions-redirect__result">
         { t('connecting') }
         <div className="permissions-redirect__icons">
-          <IconWithFallBack icon={domainMetadata.icon} name={domainMetadata.name} />
+          <SiteIcon icon={domainMetadata.icon} name={domainMetadata.name} size={64} />
           <div className="permissions-redirect__center-icon">
             <span className="permissions-redirect__check" />
             { renderBrokenLine() }
           </div>
-          <div className="permissions-redirect__identicon-container">
-            <div className="permissions-redirect__identicon-border">
-              <img src="/images/logo/metamask-fox.svg" />
-            </div>
-          </div>
+          <SiteIcon icon="/images/logo/metamask-fox.svg" size={64} />
         </div>
       </div>
     </div>