Fee card 3 rows fixes (#59)

* Styling and UX improvements to various swaps components Remove unnecessary default values in FeeCard component Add gas customization link to fee card on view quote screen (#40) Add max gas row to fee card on view quote screen Add approve edit limit support to swaps (#41) Update transaction list to support swaps (#42) Update FeeCard props, stories (#53) Co-authored-by: Dan Miller <danjm.com@gmail.com>
MetaMask · Oct 6, 2020 · b468bc5 · b468bc5
1 parent e87dfb9
commit b468bc5
Show file tree

Hide file tree

Showing 5 changed files with 156 additions and 86 deletions.
diff --git a/ui/app/pages/swaps/fee-card/fee-card.js b/ui/app/pages/swaps/fee-card/fee-card.js
@@ -6,17 +6,8 @@ export default function FeeCard ({
   feeRowText,
   primaryFee,
   secondaryFee,
-  thirdRowText,
-  thirdRowLinkText,
-  hideThirdRow = false,
-  onThirdRowClick,
-  thirdRowInfoTooltipText,
-  maxFeeRowInfoTooltipText,
-  primaryMaxFee,
-  secondaryMaxFee,
-  maxFeeRowText,
-  maxFeeRowLinkText,
-  onMaxRowClick,
+  thirdRow,
+  maxFeeRow,
 }) {
   return (
     <div className="fee-card">
@@ -29,58 +20,54 @@ export default function FeeCard ({
           </div>
           <div>
             <div className="fee-card__row-header-secondary--bold">
-              {primaryFee}
+              {primaryFee.fee}
             </div>
             {secondaryFee && (
               <div className="fee-card__row-header-primary--bold">
-                {secondaryFee}
+                {secondaryFee.fee}
               </div>
             )}
           </div>
         </div>
-        <div className="fee-card__row-header" onClick={() => onMaxRowClick && onMaxRowClick()}>
+        <div className="fee-card__row-header" onClick={() => maxFeeRow.onClick()}>
           <div>
             <div className="fee-card__row-header-text">
-              {maxFeeRowText}
+              {maxFeeRow.text}
+            </div>
+            <div className="fee-card__link">
+              {maxFeeRow.linkText}
             </div>
-            {onMaxRowClick && (
-              <div className="fee-card__link">
-                {maxFeeRowLinkText}
-              </div>
-            )}
             <div className="fee-card__row-label">
               <InfoTooltip
                 position="top"
-                contentText={maxFeeRowInfoTooltipText}
+                contentText={maxFeeRow.tooltipText}
               />
             </div>
           </div>
           <div>
             <div className="fee-card__row-header-secondary">
-              {primaryMaxFee}
+              {primaryFee.maxFee}
             </div>
-            {secondaryMaxFee && (
+            {secondaryFee?.maxFee !== undefined && (
               <div className="fee-card__row-header-primary">
-                {secondaryMaxFee}
+                {secondaryFee.maxFee}
               </div>
             )}
           </div>
         </div>
-        {!hideThirdRow && thirdRowText && (
+        {thirdRow && !thirdRow.hide && (
           <div className="fee-card__top-bordered-row">
             <div className="fee-card__row-label">
-              <div className="fee-card__row-text">
-                {thirdRowText}
+              <div className="fee-card__row-header-text">
+                {thirdRow.text}
+              </div>
+              <div className="fee-card__link" onClick={() => thirdRow.onClick()}>
+                {thirdRow.linkText}
               </div>
               <InfoTooltip
                 position="top"
-                contentText={thirdRowInfoTooltipText}
+                contentText={thirdRow.tooltipText}
               />
-              {thirdRowLinkText && (
-                <div className="fee-card__link" onClick={() => onThirdRowClick && onThirdRowClick()}>
-                  {thirdRowLinkText}
-                </div>
-              )}
             </div>
           </div>
         )}
@@ -91,17 +78,25 @@ export default function FeeCard ({
 
 FeeCard.propTypes = {
   feeRowText: PropTypes.string.isRequired,
-  primaryFee: PropTypes.string.isRequired,
-  secondaryFee: PropTypes.string,
-  thirdRowText: PropTypes.string,
-  thirdRowLinkText: PropTypes.string,
-  hideThirdRow: PropTypes.bool,
-  onThirdRowClick: PropTypes.func,
-  thirdRowInfoTooltipText: PropTypes.string,
-  primaryMaxFee: PropTypes.string.isRequired,
-  secondaryMaxFee: PropTypes.string,
-  maxFeeRowText: PropTypes.string.isRequired,
-  maxFeeRowLinkText: PropTypes.string,
-  onMaxRowClick: PropTypes.func,
-  maxFeeRowInfoTooltipText: PropTypes.string,
+  primaryFee: PropTypes.shape({
+    fee: PropTypes.string.isRequired,
+    maxFee: PropTypes.string.isRequired,
+  }).isRequired,
+  secondaryFee: PropTypes.shape({
+    fee: PropTypes.string.isRequired,
+    maxFee: PropTypes.string.isRequired,
+  }),
+  maxFeeRow: PropTypes.shape({
+    text: PropTypes.string.isRequired,
+    linkText: PropTypes.string.isRequired,
+    tooltipText: PropTypes.string.isRequired,
+    onClick: PropTypes.func.isRequired,
+  }).isRequired,
+  thirdRow: PropTypes.shape({
+    text: PropTypes.string.isRequired,
+    linkText: PropTypes.string.isRequired,
+    tooltipText: PropTypes.string.isRequired,
+    onClick: PropTypes.func.isRequired,
+    hide: PropTypes.bool.isRequired,
+  }),
 }
diff --git a/ui/app/pages/swaps/fee-card/fee-card.stories.js b/ui/app/pages/swaps/fee-card/fee-card.stories.js
@@ -11,36 +11,83 @@ export default {
   title: 'FeeCard',
 }
 
-export const WithSecondRow = () => {
+export const WithAllProps = () => {
   return (
     <div style={containerStyle}>
       <FeeCard
-        onFeeRowClick={action('Fee row link clicked')}
         feeRowText={text('feeRowText', 'Network fees')}
-        feeRowLinkText={text('feeRowLinkText', 'Edit')}
-        primaryFee={text('primaryFee', '1 ETH')}
-        secondaryFee={text('secondaryFee', '$300.57')}
-        onSecondRowClick={action('Second row link clicked')}
-        secondRowText={text('secondRowText', 'This calls a contract')}
-        secondRowLinkText={text('secondRowLinkText', 'Learn More')}
+        primaryFee={({
+          fee: text('primaryFee', '1 ETH'),
+          maxFee: text('primaryMaxFee', '2 ETH'),
+        })}
+        secondaryFee={({
+          fee: text('secondaryFee', '100 USD'),
+          maxFee: text('secondaryMaxFee', '200 USD'),
+        })}
+        maxFeeRow={({
+          text: text('maxFeeText', 'Max Fee'),
+          linkText: text('maxFeeLinkText', 'Edit'),
+          tooltipText: text('maxFeeTooltipText', 'Click here to edit.'),
+          onClick: action('Clicked max fee row link'),
+        })}
+        thirdRow={({
+          text: text('thirdRowText', 'Extra Option'),
+          linkText: text('thirdRowLinkText', 'Click Me'),
+          tooltipText: text('thirdRowTooltipText', 'Something happens if you click this'),
+          onClick: action('Clicked third row link'),
+          hide: false,
+        })}
       />
     </div>
   )
 }
 
-export const WithoutSecondRow = () => {
+export const WithoutThirdRow = () => {
   return (
     <div style={containerStyle}>
       <FeeCard
-        onFeeRowClick={action('Fee row link clicked')}
         feeRowText={text('feeRowText', 'Network fees')}
-        feeRowLinkText={text('feeRowLinkText', 'Edit')}
-        primaryFee={text('primaryFee', '1 ETH')}
-        secondaryFee={text('secondaryFee', '$300.57')}
-        onSecondRowClick={action('Second row link clicked')}
-        secondRowText={text('secondRowText', 'This calls a contract')}
-        secondRowLinkText={text('secondRowLinkText', 'Learn More')}
-        hideSecondRow
+        primaryFee={({
+          fee: text('primaryFee', '1 ETH'),
+          maxFee: text('primaryMaxFee', '2 ETH'),
+        })}
+        secondaryFee={({
+          fee: text('secondaryFee', '100 USD'),
+          maxFee: text('secondaryMaxFee', '200 USD'),
+        })}
+        maxFeeRow={({
+          text: text('maxFeeText', 'Max Fee'),
+          linkText: text('maxFeeLinkText', 'Edit'),
+          tooltipText: text('maxFeeTooltipText', 'Click here to edit.'),
+          onClick: action('Clicked max fee row link'),
+        })}
+        thirdRow={({
+          text: text('thirdRowText', 'Extra Option'),
+          linkText: text('thirdRowLinkText', 'Click Me'),
+          tooltipText: text('thirdRowTooltipText', 'Something happens if you click this'),
+          onClick: action('Clicked third row link'),
+          hide: true,
+        })}
+      />
+    </div>
+  )
+}
+
+export const WithOnlyRequiredProps = () => {
+  return (
+    <div style={containerStyle}>
+      <FeeCard
+        feeRowText={text('feeRowText', 'Network fees')}
+        primaryFee={({
+          fee: text('primaryFee', '1 ETH'),
+          maxFee: text('primaryMaxFee', '2 ETH'),
+        })}
+        maxFeeRow={({
+          text: text('maxFeeText', 'Max Fee'),
+          linkText: text('maxFeeLinkText', 'Edit'),
+          tooltipText: text('maxFeeTooltipText', 'Click here to edit.'),
+          onClick: action('Clicked max fee row link'),
+        })}
       />
     </div>
   )

diff --git a/ui/app/pages/swaps/fee-card/index.scss b/ui/app/pages/swaps/fee-card/index.scss
@@ -63,10 +63,6 @@
     }
   }
 
-  &__row-text {
-    margin-right: 0;
-  }
-
   &__row-fee {
     margin-right: 4px;
   }

diff --git a/ui/app/pages/swaps/swaps.util.js b/ui/app/pages/swaps/swaps.util.js
@@ -297,7 +297,7 @@ export function getRenderableGasFeesForQuote (tradeGas, approveGas, gasPrice, cu
   return {
     rawNetworkFees,
     rawEthFee: ethFee,
-    feeinFiat: formattedNetworkFee,
+    feeInFiat: formattedNetworkFee,
     feeInEth: `${ethFee} ETH`,
   }
 }
@@ -308,7 +308,22 @@ export function quotesToRenderableData (quotes, gasPrice, conversionRate, curren
     const sourceValue = calcTokenAmount(sourceAmount, sourceTokenInfo.decimals || 18).toString(10)
     const destinationValue = calcTokenAmount(destinationAmount, destinationTokenInfo.decimals || 18).toPrecision(8)
 
-    const { feeinFiat, rawNetworkFees, rawEthFee, feeInEth } = getRenderableGasFeesForQuote(customGasLimit || gasEstimateWithRefund || decimalToHex(averageGas || 800000), approveGas, gasPrice, currentCurrency, conversionRate)
+    const {
+      feeInFiat,
+      rawNetworkFees,
+      rawEthFee,
+      feeInEth,
+    } = getRenderableGasFeesForQuote(
+      (
+        customGasLimit ||
+        gasEstimateWithRefund ||
+        decimalToHex(averageGas || 800000)
+      ),
+      approveGas,
+      gasPrice,
+      currentCurrency,
+      conversionRate,
+    )
 
     const metaMaskFee = `0.875%`
     const slippageMultiplier = (new BigNumber(100 - slippage)).div(100)
@@ -345,8 +360,8 @@ export function quotesToRenderableData (quotes, gasPrice, conversionRate, curren
       liquiditySource,
       metaMaskFee,
       feeInEth,
-      detailedNetworkFees: `${feeInEth} (${feeinFiat})`,
-      networkFees: feeinFiat,
+      detailedNetworkFees: `${feeInEth} (${feeInFiat})`,
+      networkFees: feeInFiat,
       quoteSource: aggType,
       rawNetworkFees,
       slippage: renderedSlippage,

diff --git a/ui/app/pages/swaps/view-quote/view-quote.js b/ui/app/pages/swaps/view-quote/view-quote.js
@@ -143,8 +143,8 @@ export default function ViewQuote () {
     sourceTokenValue,
   } = renderableDataForUsedQuote
 
-  const { feeinFiat, feeInEth } = getRenderableGasFeesForQuote(usedGasLimit, approveGas, gasPrice, currentCurrency, conversionRate)
-  const { feeinFiat: maxFeeInFiat, feeInEth: maxFeeInEth } = getRenderableGasFeesForQuote(maxGasLimit, approveGas, gasPrice, currentCurrency, conversionRate)
+  const { feeInFiat, feeInEth } = getRenderableGasFeesForQuote(usedGasLimit, approveGas, gasPrice, currentCurrency, conversionRate)
+  const { feeInFiat: maxFeeInFiat, feeInEth: maxFeeInEth } = getRenderableGasFeesForQuote(maxGasLimit, approveGas, gasPrice, currentCurrency, conversionRate)
 
   const tokenCost = (new BigNumber(usedQuote.sourceAmount))
   const ethCost = (new BigNumber(usedQuote.trade.value || 0, 10)).plus((new BigNumber(gasTotalInWeiHex, 16)))
@@ -242,6 +242,15 @@ export default function ViewQuote () {
     ),
   }))
 
+  const thirdRowTextComponent = (
+    <span
+      key="swaps-view-quote-approve-symbol-1"
+      className="view-quote__bold"
+    >
+      {sourceTokenSymbol}
+    </span>
+  )
+
   return (
     <div className="view-quote">
       <div className="view-quote__content">
@@ -305,20 +314,28 @@ export default function ViewQuote () {
           })}
         >
           <FeeCard
-            onThirdRowClick={onFeeCardThirdRowClickHandler}
-            onMaxRowClick={onFeeCardMaxRowClickHandler}
-            secondaryFee={feeinFiat}
-            primaryFee={feeInEth}
-            secondaryMaxFee={maxFeeInFiat}
-            primaryMaxFee={maxFeeInEth}
             feeRowText={t('swapEstimatedNetworkFee')}
-            maxFeeRowText={t('swapMaxNetworkFees')}
-            maxFeeRowLinkText={t('edit')}
-            maxFeeRowInfoTooltipText={t('swapMaxNetworkFeeInfo')}
-            thirdRowText={t('swapThisWillAllowApprove', [<span key="swaps-view-quote-approve-symbol-1" className="view-quote__bold">{sourceTokenSymbol}</span>])}
-            thirdRowLinkText={t('swapEditLimit')}
-            hideThirdRow={!approveTxParams || (balanceError && !warningHidden)}
-            thirdRowInfoTooltipText={t('swapEnableDescription', [sourceTokenSymbol])}
+            primaryFee={({
+              fee: feeInEth,
+              maxFee: maxFeeInEth,
+            })}
+            secondaryFee={({
+              fee: feeInFiat,
+              maxFee: maxFeeInFiat,
+            })}
+            maxFeeRow={({
+              text: t('swapMaxNetworkFees'),
+              linkText: t('edit'),
+              tooltipText: t('swapMaxNetworkFeeInfo'),
+              onClick: onFeeCardMaxRowClickHandler,
+            })}
+            thirdRow={({
+              text: t('swapThisWillAllowApprove', [thirdRowTextComponent]),
+              linkText: t('swapEditLimit'),
+              tooltipText: t('swapEnableDescription', [sourceTokenSymbol]),
+              onClick: onFeeCardThirdRowClickHandler,
+              hide: !approveTxParams || (balanceError && !warningHidden),
+            })}
           />
         </div>
       </div>