From fdf3e9d2d79f636ec32c6d8def155c4aac5378b8 Mon Sep 17 00:00:00 2001
From: George Marshall <george.marshall@consensys.net>
Date: Wed, 15 Jun 2022 13:16:53 -0700
Subject: [PATCH] Fix/14856 network settings page (#14857)

* Removing other features code

* Improving acccessibility of check icon

* Fixing add network button footer in popover view

* Fixing unit tests
---
 .../app/dropdowns/network-dropdown.js         |   2 +-
 .../app/dropdowns/network-dropdown.test.js    |   2 +-
 .../app/network-display/network-display.js    |   4 +-
 .../ui/color-indicator/color-indicator.js     |   2 +-
 ui/components/ui/text-field/index.scss        |   6 +
 ui/helpers/constants/design-system.js         |   1 +
 ui/pages/settings/networks-tab/index.scss     | 249 ++++++------------
 .../networks-form/networks-form.js            |   1 +
 .../networks-list-item/networks-list-item.js  |  12 +-
 .../networks-list/networks-list.js            |   2 +-
 .../settings/networks-tab/networks-tab.js     |   8 +-
 11 files changed, 114 insertions(+), 175 deletions(-)

diff --git a/ui/components/app/dropdowns/network-dropdown.js b/ui/components/app/dropdowns/network-dropdown.js
index 48af4be2c..33ac6af44 100644
--- a/ui/components/app/dropdowns/network-dropdown.js
+++ b/ui/components/app/dropdowns/network-dropdown.js
@@ -174,7 +174,7 @@ class NetworkDropdown extends Component {
             <div className="network-check__transparent">✓</div>
           )}
           <ColorIndicator
-            color={opts.isLocalHost ? 'localhost' : COLORS.TEXT_MUTED}
+            color={opts.isLocalHost ? 'localhost' : COLORS.ICON_MUTED}
             size={SIZES.LG}
             type={ColorIndicator.TYPES.FILLED}
           />
diff --git a/ui/components/app/dropdowns/network-dropdown.test.js b/ui/components/app/dropdowns/network-dropdown.test.js
index d1a330fce..5067988ca 100644
--- a/ui/components/app/dropdowns/network-dropdown.test.js
+++ b/ui/components/app/dropdowns/network-dropdown.test.js
@@ -79,7 +79,7 @@ describe('Network Dropdown', () => {
       let found = false;
       while (!found) {
         if (
-          _wrapper.find(ColorIndicator).at(i).prop('color') === 'text-muted'
+          _wrapper.find(ColorIndicator).at(i).prop('color') === 'icon-muted'
         ) {
           i += 1;
         } else {
diff --git a/ui/components/app/network-display/network-display.js b/ui/components/app/network-display/network-display.js
index 6e3d6aabb..63af71f90 100644
--- a/ui/components/app/network-display/network-display.js
+++ b/ui/components/app/network-display/network-display.js
@@ -48,9 +48,7 @@ export default function NetworkDisplay({
         >
           <ColorIndicator
             color={
-              networkType === NETWORK_TYPE_RPC
-                ? COLORS.ICON_DEFAULT
-                : networkType
+              networkType === NETWORK_TYPE_RPC ? COLORS.ICON_MUTED : networkType
             }
             size={indicatorSize}
             type={ColorIndicator.TYPES.FILLED}
diff --git a/ui/components/ui/color-indicator/color-indicator.js b/ui/components/ui/color-indicator/color-indicator.js
index 00cf3932d..455f7d53f 100644
--- a/ui/components/ui/color-indicator/color-indicator.js
+++ b/ui/components/ui/color-indicator/color-indicator.js
@@ -6,7 +6,7 @@ import { COLORS, SIZES } from '../../../helpers/constants/design-system';
 export default function ColorIndicator({
   size = SIZES.SM,
   type = 'outlined',
-  color = COLORS.ICON_DEFAULT,
+  color = COLORS.ICON_MUTED,
   borderColor,
   iconClassName,
 }) {
diff --git a/ui/components/ui/text-field/index.scss b/ui/components/ui/text-field/index.scss
index a33cf5630..2626c345b 100644
--- a/ui/components/ui/text-field/index.scss
+++ b/ui/components/ui/text-field/index.scss
@@ -2,3 +2,9 @@
 .MuiInputBase-input {
   color: var(--color-text-default) !important;
 }
+
+.MuiFormHelperText-root.Mui-error {
+  @include H7;
+
+  color: var(--color-error-default) !important;
+}
diff --git a/ui/helpers/constants/design-system.js b/ui/helpers/constants/design-system.js
index 1809ea31c..26ac390af 100644
--- a/ui/helpers/constants/design-system.js
+++ b/ui/helpers/constants/design-system.js
@@ -11,6 +11,7 @@ export const COLORS = {
   TEXT_ALTERNATIVE: 'text-alternative',
   TEXT_MUTED: 'text-muted',
   ICON_DEFAULT: 'icon-default',
+  ICON_ALTERNATIVE: 'text-alternative',
   ICON_MUTED: 'icon-muted',
   BORDER_DEFAULT: 'border-default',
   BORDER_MUTED: 'border-muted',
diff --git a/ui/pages/settings/networks-tab/index.scss b/ui/pages/settings/networks-tab/index.scss
index e26db2772..f10554839 100644
--- a/ui/pages/settings/networks-tab/index.scss
+++ b/ui/pages/settings/networks-tab/index.scss
@@ -7,31 +7,34 @@
   }
 
   &__content {
-    display: flex;
-    height: 100%;
-    max-width: 779px;
-    justify-content: space-between;
-
-    &__custom-image {
-      border: 1px solid var(--color-border-default);
-      margin-inline-start: 8px;
+    &--with-networks-list-popup-footer {
+      // padding-bottom required to offset content from fixed footer: &__networks-list-popup-footer
+      padding-bottom: 80px;
+      // isolation contains stacking context(z-index) of &__networks-list-popup-footer
+      isolation: isolate;
     }
 
-    &__check-icon {
-      margin-inline-end: 10px;
-      color: var(--color-success-default);
+    @media screen and (min-width: $break-large) {
+      display: grid;
+      grid-template-columns: 1fr 1fr;
+    }
 
-      &__transparent {
-        color: transparent;
-        width: 16px;
-        margin-inline-end: 10px;
-      }
+    &__icon-check {
+      max-width: 24px;
+      flex: 0 0 24px;
+    }
+
+    &__custom-image {
+      border: 1px solid var(--color-border-muted);
     }
 
     &__icon-with-fallback {
-      padding: 0 1px 2px 2px;
+      display: flex;
+      align-items: center;
+      padding: 0;
+      max-width: 24px;
+      flex: 0 0 24px;
       color: var(--color-text-alternative);
-      margin-inline-start: 8px;
 
       @each $variant, $color in design-system.$color-map {
         &--color-#{$variant} {
@@ -40,160 +43,90 @@
         }
       }
     }
-
-    @media screen and (max-width: $break-small) {
-      margin-top: 0;
-      flex-direction: column;
-      overflow-x: hidden;
-      align-items: center;
-    }
   }
 
   &__body {
-    padding-right: 24px;
-    height: 100%;
     display: flex;
     flex-direction: column;
 
-    @media screen and (max-width: $break-small) {
-      padding: 0;
+    @media screen and (min-width: $break-large) {
+      margin-inline-end: 16px;
+      margin-bottom: 24px;
     }
   }
 
   &__subheader {
-    @include H4;
+    display: none;
 
-    padding: 16px 4px;
-    border-bottom: 1px solid var(--color-border-default);
-    height: 72px;
-    align-items: center;
-    display: flex;
-    flex-flow: row nowrap;
-  }
+    @media screen and (min-width: $break-large) {
+      @include H4;
 
-  &__subheader--break {
-    margin-inline-start: 10px;
+      padding: 16px 4px;
+      border-bottom: 1px solid var(--color-border-default);
+      height: 72px;
+      align-items: center;
+      display: flex;
+      gap: 8px;
+      flex-flow: row nowrap;
+    }
   }
 
   &__sub-header-text {
     @include H4;
 
     color: var(--color-text-muted);
-    margin-right: 10px;
   }
 
   &__network-form {
-    display: flex;
-    flex: 1 0;
-    flex-direction: column;
-    justify-content: space-between;
-    max-height: 465px;
-    max-width: 400px;
-    margin-top: 24px;
-    padding-inline-start: 24px;
+    padding: 16px 24px;
+
+    @media screen and (min-width: $break-large) {
+      padding: 16px;
+    }
 
     .page-container__footer {
       border-top: none;
 
-      @media screen and (max-width: $break-small) {
-        width: 93%;
-      }
-
       footer {
         padding: 10px 0;
       }
     }
-
-    @media screen and (max-width: $break-small) {
-      display: flex;
-      flex: auto;
-      max-width: 100%;
-      max-height: 100%;
-      align-items: center;
-      width: 90%;
-      margin-top: 10px;
-      padding: 0;
-    }
-  }
-
-  &__add-network-form {
-    display: flex;
-    flex-direction: column;
-    max-height: 465px;
-  }
-
-  &__network-form-body {
-    display: grid;
-    grid-template-columns: 100%;
-    width: 95%;
-
-    &__view-only {
-      width: 100%;
-    }
-  }
-
-  &__add-network-form-body {
-    display: grid;
-    grid-template-columns: 48% 48%;
-    column-gap: 5%;
-    margin-top: 24px;
-    width: 100%;
   }
 
   &__networks-list {
-    flex: 0.5 0 auto;
-    max-width: 350px;
-    border-right: 1px solid var(--color-border-muted);
+    display: flex;
+    flex-direction: column;
+    gap: 16px;
+    padding: 16px 24px;
+
+    @media screen and (min-width: $break-large) {
+      padding: 16px;
+      min-width: 160px; // Allow network form to take priority in the grid on resize
+      border-right: 1px solid var(--color-border-muted);
+    }
 
     &__custom-search-network {
-      margin-top: 24px;
-    }
+      @include H6;
 
-    .MuiInput-input {
-      font-size: 14px;
-
-      @media screen and (max-width: $break-small) {
-        font-size: 12px;
+      @media screen and (min-width: $break-large) {
+        @include Paragraph;
       }
     }
-
-    .MuiTextField-root {
-      padding-inline-end: 16px;
-
-      #search-networks-helper-text {
-        color: var(--color-text-alternative);
-      }
-
-      @media screen and (max-width: $break-small) {
-        padding: 0 24px 0 24px;
-      }
-    }
-
-    &__label {
-      @media screen and (max-width: $break-small) {
-        margin-inline-start: 58px;
-      }
-    }
-
-    @media screen and (max-width: $break-small) {
-      flex: 1;
-      overflow-y: auto;
-      max-width: 100vw;
-      width: 100vw;
-      margin-top: 0;
-    }
   }
 
   &__networks-list-popup-footer {
-    width: 100%;
     display: flex;
     justify-content: center;
-    padding-top: 23px;
-    padding-bottom: 23px;
+    padding: 16px 24px;
     border-top: 1px solid var(--color-border-default);
+    position: fixed;
+    bottom: 0;
+    left: 0;
+    width: 100%;
+    background: var(--color-background-default);
 
-    .button {
-      width: 178px;
+    @media screen and (min-width: $break-large) {
+      display: none;
     }
   }
 
@@ -218,39 +151,16 @@
 
   &__networks-list-item {
     display: flex;
-    padding: 12px 16px 12px 0;
-    position: relative;
-    align-items: center;
-    width: 311px;
-
-    .color-indicator {
-      &:hover {
-        cursor: pointer;
-      }
-
-      @media screen and (max-width: $break-small) {
-        margin: 0 4px 0 20px;
-      }
-    }
-
-    @media screen and (max-width: $break-small) {
-      padding: 12px 0 12px 24px;
-      max-width: 351px;
-    }
-  }
-
-  &__networks-list-item:last-of-type {
-    @media screen and (max-width: $break-small) {
-      border-bottom: none;
-    }
+    gap: 4px;
+    padding-top: 4px;
+    padding-bottom: 4px;
   }
 
   &__networks-list-name {
     @include Paragraph;
 
-    margin-left: 11px;
+    margin-left: 8px;
     color: var(--color-text-alternative);
-    width: 70%;
     text-overflow: ellipsis;
     white-space: nowrap;
     overflow: hidden;
@@ -291,34 +201,43 @@
   &__network-form-footer {
     display: flex;
     flex-flow: row nowrap;
-    padding: 0.75rem 0;
-    width: 95%;
-
-    @media screen and (max-width: $break-small) {
-      width: 93%;
-    }
+    padding: 12px 0;
 
     .btn--rounded {
       white-space: nowrap;
     }
 
     .btn-secondary {
-      margin-right: 0.375rem;
+      margin-left: auto;
     }
 
     .btn-primary {
-      margin-left: 0.375rem;
+      margin-left: 16px;
     }
 
     .btn-danger {
-      margin-right: 3.75rem;
+      margin-right: 16px;
+    }
+  }
+
+  &__add-network-form {
+    padding: 16px 24px;
+    grid-column: span 2; // spread both columns of grid layout
+
+    @media screen and (min-width: $break-large) {
+      max-width: 400px; // but only expand to 400px
+      padding: 16px;
+    }
+
+    &__alert {
+      margin-top: 0;
     }
   }
 
   &__add-network-form-footer {
     display: flex;
     flex-flow: row;
-    padding: 0.75rem 0;
+    padding: 12px 0;
     width: 60%;
 
     .btn-secondary {
diff --git a/ui/pages/settings/networks-tab/networks-form/networks-form.js b/ui/pages/settings/networks-tab/networks-form/networks-form.js
index 402437c86..ef846c638 100644
--- a/ui/pages/settings/networks-tab/networks-form/networks-form.js
+++ b/ui/pages/settings/networks-tab/networks-form/networks-form.js
@@ -551,6 +551,7 @@ const NetworksForm = ({
           iconFillColor="var(--color-warning-default)"
           useIcon
           withRightButton
+          className="networks-tab__add-network-form__alert"
         />
       ) : null}
       <div
diff --git a/ui/pages/settings/networks-tab/networks-list-item/networks-list-item.js b/ui/pages/settings/networks-tab/networks-list-item/networks-list-item.js
index 5f41df97c..b6b13089c 100644
--- a/ui/pages/settings/networks-tab/networks-list-item/networks-list-item.js
+++ b/ui/pages/settings/networks-tab/networks-list-item/networks-list-item.js
@@ -73,9 +73,17 @@ const NetworksListItem = ({
       }}
     >
       {isCurrentRpcTarget ? (
-        <IconCheck color="var(--color-success-default)" />
+        <IconCheck
+          className="networks-tab__content__icon-check"
+          color="var(--color-success-default)"
+          aria-label={t('active')}
+        />
       ) : (
-        <div className="networks-tab__content__check-icon__transparent">✓</div>
+        <IconCheck
+          className="networks-tab__content__icon-check"
+          color="transparent"
+          aria-hidden="true"
+        />
       )}
       {network.chainId in CHAIN_ID_TO_NETWORK_IMAGE_URL_MAP ? (
         <Identicon
diff --git a/ui/pages/settings/networks-tab/networks-list/networks-list.js b/ui/pages/settings/networks-tab/networks-list/networks-list.js
index c875a6427..56c12834b 100644
--- a/ui/pages/settings/networks-tab/networks-list/networks-list.js
+++ b/ui/pages/settings/networks-tab/networks-list/networks-list.js
@@ -66,7 +66,7 @@ const NetworksList = ({
       {searchQuery === '' && (
         <Typography
           variant={TYPOGRAPHY.H6}
-          margin={[6, 0, 0, 9]}
+          marginTop={4}
           color={COLORS.TEXT_ALTERNATIVE}
           className="networks-tab__networks-list__label"
         >
diff --git a/ui/pages/settings/networks-tab/networks-tab.js b/ui/pages/settings/networks-tab/networks-tab.js
index 8958b6b61..c73787566 100644
--- a/ui/pages/settings/networks-tab/networks-tab.js
+++ b/ui/pages/settings/networks-tab/networks-tab.js
@@ -1,5 +1,6 @@
 import React, { useEffect } from 'react';
 import PropTypes from 'prop-types';
+import classnames from 'classnames';
 import { useLocation } from 'react-router-dom';
 import { useDispatch, useSelector } from 'react-redux';
 import { useI18nContext } from '../../../hooks/useI18nContext';
@@ -92,7 +93,12 @@ const NetworksTab = ({ addNewNetwork }) => {
       {isFullScreen ? (
         <NetworksFormSubheader addNewNetwork={addNewNetwork} />
       ) : null}
-      <div className="networks-tab__content">
+      <div
+        className={classnames('networks-tab__content', {
+          'networks-tab__content--with-networks-list-popup-footer':
+            !isFullScreen && !shouldRenderNetworkForm,
+        })}
+      >
         {addNewNetwork ? (
           <NetworksForm
             networksToRender={networksToRender}