Fix/button base ellipsis support (#18205)

* ButtonBase ellipsis update Update ui/components/multichain/account-picker/index.js Co-authored-by: Garrett Bear <[email protected]> Update ui/components/multichain/account-picker/index.js Co-authored-by: Garrett Bear <[email protected]> Update ui/components/multichain/account-picker/index.js Co-authored-by: Garrett Bear <[email protected]> Update ui/components/multichain/account-picker/index.js Co-authored-by: Garrett Bear <[email protected]> * buttonbase updates to fix ellipsis * multichain support * remove multichain * code cleanup * clean up * component clean up * span update * fix snapshots * fix snapshot * Updating ButtonBase to reduce html to a minimum but ensure all functionality still works (#18210) * fix color and disable * remove unused css * Update ui/components/component-library/button-base/README.mdx Co-authored-by: George Marshall <[email protected]> * fix e2e test from button update * update e2e test from button base update --------- Co-authored-by: David Walsh <[email protected]> Co-authored-by: George Marshall <[email protected]>
MetaMask · Mar 22, 2023 · de4cf0a · de4cf0a
1 parent d3026e7
commit de4cf0a
Show file tree

Hide file tree

Showing 18 changed files with 131 additions and 147 deletions.
diff --git a/test/e2e/tests/add-account.spec.js b/test/e2e/tests/add-account.spec.js
@@ -242,7 +242,7 @@ describe('Add account', function () {
 
         // enter private key',
         await driver.fill('#private-key-box', testPrivateKey);
-        await driver.clickElement({ text: 'Import', tag: 'span' });
+        await driver.clickElement({ text: 'Import', tag: 'button' });
 
         // should show the correct account name
         const importedAccountName = await driver.findElement(

diff --git a/test/e2e/tests/failing-contract.spec.js b/test/e2e/tests/failing-contract.spec.js
@@ -63,7 +63,7 @@ describe('Failing contract interaction ', function () {
         // dismiss warning and confirm the transaction
         await driver.clickElement({
           text: 'I want to proceed anyway',
-          tag: 'span',
+          tag: 'button',
         });
         await driver.clickElement({ text: 'Confirm', tag: 'button' });
         await driver.waitUntilXWindowHandles(2);
@@ -149,7 +149,7 @@ describe('Failing contract interaction on non-EIP1559 network', function () {
         // dismiss warning and confirm the transaction
         await driver.clickElement({
           text: 'I want to proceed anyway',
-          tag: 'span',
+          tag: 'button',
         });
         await driver.clickElement({ text: 'Confirm', tag: 'button' });
         await driver.waitUntilXWindowHandles(2);

diff --git a/test/e2e/tests/from-import-ui.spec.js b/test/e2e/tests/from-import-ui.spec.js
@@ -212,7 +212,7 @@ describe('MetaMask Import UI', function () {
 
         // enter private key',
         await driver.fill('#private-key-box', testPrivateKey1);
-        await driver.clickElement({ text: 'Import', tag: 'span' });
+        await driver.clickElement({ text: 'Import', tag: 'button' });
 
         // should show the correct account name
         const importedAccountName = await driver.findElement(
@@ -239,7 +239,7 @@ describe('MetaMask Import UI', function () {
         await driver.clickElement({ text: 'Import account', tag: 'div' });
         // enter private key
         await driver.fill('#private-key-box', testPrivateKey2);
-        await driver.clickElement({ text: 'Import', tag: 'span' });
+        await driver.clickElement({ text: 'Import', tag: 'button' });
 
         // should see new account in account menu
         const importedAccount2Name = await driver.findElement(
@@ -330,7 +330,7 @@ describe('MetaMask Import UI', function () {
 
         await driver.fill('#json-password-box', 'foobarbazqux');
 
-        await driver.clickElement({ text: 'Import', tag: 'span' });
+        await driver.clickElement({ text: 'Import', tag: 'button' });
 
         // should show the correct account name
         const importedAccountName = await driver.findElement(
@@ -392,7 +392,7 @@ describe('MetaMask Import UI', function () {
 
         // enter private key',
         await driver.fill('#private-key-box', testPrivateKey);
-        await driver.clickElement({ text: 'Import', tag: 'span' });
+        await driver.clickElement({ text: 'Import', tag: 'button' });
 
         // error should occur
         await driver.waitForSelector({

diff --git a/ui/components/component-library/button-base/README.mdx b/ui/components/component-library/button-base/README.mdx
@@ -102,7 +102,9 @@ import { ButtonBase } from '../../component-library';
 
 When an `externalLink` prop is passed it will change the element to an anchor(`a`) tag and add the `target="_blank"` and `rel="noopener noreferrer"` attributes.
 
+<Canvas>
   <Story id="components-componentlibrary-buttonbase--external-link" />
+</Canvas>
 
 ```jsx
 import { ButtonBase } from '../../component-library';
@@ -168,7 +170,7 @@ import { ICON_NAMES } from '../icon';
 
 ### RTL
 
-For RTL language support use the `textProps` prop to pass a `textDirection` prop.
+For RTL language support use the `textDirection` prop.
 
 <Canvas>
   <Story id="components-componentlibrary-buttonbase--rtl" />
@@ -187,11 +189,28 @@ import { ButtonBase, ICON_NAMES } from '../../component-library';
   <ButtonBase
     startIconName={ICON_NAMES.ADD_SQUARE}
     endIconName={ICON_NAMES.ARROW_2_RIGHT}
-    textProps={{
-      textDirection: TEXT_DIRECTIONS.RIGHT_TO_LEFT,
-    }}
+    textDirection={TEXT_DIRECTIONS.RIGHT_TO_LEFT}
   >
     Button Demo
   </ButtonBase>
 </>;
 ```
+
+### Ellipsis
+
+Use the boolean `ellipsis` prop to change the if the `ButtonBase` component to have an ellipsis.
+
+Note: this should only be used for dynamic/user generated content or addresses. Generally, button text should be succinct and only contain one or two words.
+
+<Canvas>
+  <Story id="components-componentlibrary-buttonbase--ellipsis" />
+</Canvas>
+
+```jsx
+import { ButtonBase } from '../../component-library';
+
+<Box style={{ width: 180 }}>
+  <ButtonBase>This is long text example without ellipsis</ButtonBase>
+  <ButtonBase ellipsis>This is long text example with ellipsis</ButtonBase>
+</Box>;
+```
diff --git a/ui/components/component-library/button-base/__snapshots__/button-base.test.js.snap b/ui/components/component-library/button-base/__snapshots__/button-base.test.js.snap
@@ -3,32 +3,24 @@
 exports[`ButtonBase should render anchor element correctly by href and externalLink, href target and rel exist 1`] = `
 <div>
   <a
-    class="box mm-button-base mm-button-base--size-md box--padding-right-4 box--padding-left-4 box--display-inline-flex box--flex-direction-row box--justify-content-center box--align-items-center box--color-text-default box--background-color-background-alternative box--rounded-pill"
+    class="box mm-text mm-button-base mm-button-base--size-md mm-text--body-md mm-text--color-text-default box--padding-right-4 box--padding-left-4 box--display-inline-flex box--flex-direction-row box--justify-content-center box--align-items-center box--background-color-background-alternative box--rounded-pill"
     data-testid="button-base"
     href="https://www.test.com/"
     rel="noopener noreferrer"
     target="_blank"
   >
-    <span
-      class="box mm-text mm-button-base__content mm-text--body-md mm-text--color-inherit box--gap-2 box--flex-direction-row box--justify-content-center box--align-items-center box--display-flex"
-    >
-      Button Base
-    </span>
+    Button Base
   </a>
 </div>
 `;
 
 exports[`ButtonBase should render button element correctly and match snapshot 1`] = `
 <div>
   <button
-    class="box mm-button-base mm-button-base--size-md box--padding-right-4 box--padding-left-4 box--display-inline-flex box--flex-direction-row box--justify-content-center box--align-items-center box--color-text-default box--background-color-background-alternative box--rounded-pill"
+    class="box mm-text mm-button-base mm-button-base--size-md mm-text--body-md mm-text--color-text-default box--padding-right-4 box--padding-left-4 box--display-inline-flex box--flex-direction-row box--justify-content-center box--align-items-center box--background-color-background-alternative box--rounded-pill"
     data-testid="button-base"
   >
-    <span
-      class="box mm-text mm-button-base__content mm-text--body-md mm-text--color-inherit box--gap-2 box--flex-direction-row box--justify-content-center box--align-items-center box--display-flex"
-    >
-      Button base
-    </span>
+    Button base
   </button>
 </div>
 `;
diff --git a/ui/components/component-library/button-base/button-base.js b/ui/components/component-library/button-base/button-base.js
@@ -15,6 +15,7 @@ import {
   Size,
   BorderRadius,
   BackgroundColor,
+  IconColor,
 } from '../../../helpers/constants/design-system';
 import { BUTTON_BASE_SIZES } from './button-base.constants';
 
@@ -24,6 +25,7 @@ export const ButtonBase = ({
   children,
   className,
   href,
+  ellipsis = false,
   externalLink,
   size = BUTTON_BASE_SIZES.MD,
   startIconName,
@@ -34,6 +36,7 @@ export const ButtonBase = ({
   disabled,
   iconLoadingProps,
   textProps,
+  color = TextColor.textDefault,
   ...props
 }) => {
   const Tag = href ? 'a' : as;
@@ -42,13 +45,14 @@ export const ButtonBase = ({
     props.rel = 'noopener noreferrer';
   }
   return (
-    <Box
+    <Text
       as={Tag}
       backgroundColor={BackgroundColor.backgroundAlternative}
-      color={TextColor.textDefault}
+      color={loading ? TextColor.transparent : color}
       href={href}
       paddingLeft={4}
       paddingRight={4}
+      ellipsis={ellipsis}
       className={classnames(
         'mm-button-base',
         {
@@ -57,6 +61,7 @@ export const ButtonBase = ({
           'mm-button-base--loading': loading,
           'mm-button-base--disabled': disabled,
           'mm-button-base--block': block,
+          'mm-button-base--ellipsis': ellipsis,
         },
         className,
       )}
@@ -67,33 +72,52 @@ export const ButtonBase = ({
       borderRadius={BorderRadius.pill}
       {...props}
     >
-      <Text
-        as="span"
-        className="mm-button-base__content"
-        justifyContent={JustifyContent.center}
-        alignItems={AlignItems.center}
-        gap={2}
-        variant={TextVariant.bodyMd}
-        color={TextColor.inherit}
-        {...textProps}
-      >
-        {startIconName && (
-          <Icon name={startIconName} size={Size.SM} {...startIconProps} />
-        )}
-        {children}
-        {endIconName && (
-          <Icon name={endIconName} size={Size.SM} {...endIconProps} />
-        )}
-      </Text>
+      {startIconName && (
+        <Icon
+          name={startIconName}
+          size={Size.SM}
+          marginInlineEnd={1}
+          {...startIconProps}
+          color={loading ? IconColor.transparent : startIconProps?.color}
+        />
+      )}
+      {/*
+       * If children is a string and doesn't need truncation or loading
+       * prevent html bloat by rendering just the string
+       * otherwise render with wrapper to allow truncation or loading
+       */}
+      {typeof children === 'string' && !ellipsis && !loading ? (
+        children
+      ) : (
+        <Text
+          as="span"
+          ellipsis={ellipsis}
+          variant={TextVariant.inherit}
+          color={loading ? TextColor.transparent : color}
+          {...textProps}
+        >
+          {children}
+        </Text>
+      )}
+      {endIconName && (
+        <Icon
+          name={endIconName}
+          size={Size.SM}
+          marginInlineStart={1}
+          {...endIconProps}
+          color={loading ? IconColor.transparent : endIconProps?.color}
+        />
+      )}
       {loading && (
         <Icon
           className="mm-button-base__icon-loading"
           name={ICON_NAMES.LOADING}
+          color={color}
           size={Size.MD}
           {...iconLoadingProps}
         />
       )}
-    </Box>
+    </Text>
   );
 };
 
@@ -126,6 +150,10 @@ ButtonBase.propTypes = {
    * When an `href` prop is passed, ButtonBase will automatically change the root element to be an `a` (anchor) tag
    */
   href: PropTypes.string,
+  /**
+   * Used for long strings that can be cut off...
+   */
+  ellipsis: PropTypes.bool,
   /**
    * Boolean indicating if the link targets external content, it will cause the link to open in a new tab
    */

diff --git a/ui/components/component-library/button-base/button-base.scss b/ui/components/component-library/button-base/button-base.scss
@@ -6,18 +6,13 @@
   vertical-align: middle;
   user-select: none;
 
-  &:active,
-  &:hover {
-    color: var(--color-text-default);
-  }
-
   &--block {
     display: block;
     width: 100%;
   }
 
-  &__content {
-    height: 100%;
+  &--ellipsis {
+    max-width: 100%;
   }
 
   &--size-sm {
@@ -36,10 +31,6 @@
     cursor: not-allowed;
   }
 
-  &--loading &__content {
-    color: transparent;
-  }
-
   &--disabled,
   &:disabled {
     opacity: 0.3;

diff --git a/ui/components/component-library/button-base/button-base.stories.js b/ui/components/component-library/button-base/button-base.stories.js
@@ -1,6 +1,7 @@
 import React from 'react';
 import {
   AlignItems,
+  Color,
   DISPLAY,
   FLEX_DIRECTION,
   Size,
@@ -199,11 +200,18 @@ export const Rtl = (args) => (
       {...args}
       startIconName={ICON_NAMES.ADD_SQUARE}
       endIconName={ICON_NAMES.ARROW_2_RIGHT}
-      textProps={{
-        textDirection: TEXT_DIRECTIONS.RIGHT_TO_LEFT,
-      }}
+      textDirection={TEXT_DIRECTIONS.RIGHT_TO_LEFT}
     >
       Button Demo
     </ButtonBase>
   </Box>
 );
+
+export const Ellipsis = (args) => (
+  <Box backgroundColor={Color.iconMuted} style={{ width: 150 }}>
+    <ButtonBase {...args}>Example without ellipsis</ButtonBase>
+    <ButtonBase {...args} ellipsis>
+      Example with ellipsis
+    </ButtonBase>
+  </Box>
+);
diff --git a/ui/components/component-library/button-link/README.mdx b/ui/components/component-library/button-link/README.mdx
@@ -67,7 +67,7 @@ import { ButtonLink, Text, TextVariant } from '../../component-library';
 </Text>
 
 <Text variant={TextVariant.bodyXs}>
-  Inherits the font-size of the parent element and example with textProps override for a success color.
+  Inherits the font-size of the parent element and example with override for a success color.
    <ButtonLink size={Size.inherit}>Learn more</ButtonLink>
 </Text>
 ```
@@ -103,7 +103,7 @@ import { ButtonLink } from '../../component-library';
 
 ### External Link
 
-When an `externalLink` prop is passed it adds the `target="_blank"` and `rel="noopener noreferrer"` attributes. 
+When an `externalLink` prop is passed it adds the `target="_blank"` and `rel="noopener noreferrer"` attributes.
 
 `rel="noreferrer noopener"` is used in links to prevent security vulnerabilities that can be exploited by malicious websites. It disables the window.opener property and prevents the new page from sending the referrer information, providing an additional layer of security.
 

diff --git a/ui/components/component-library/button-link/__snapshots__/button-link.test.js.snap b/ui/components/component-library/button-link/__snapshots__/button-link.test.js.snap
@@ -3,14 +3,10 @@
 exports[`ButtonLink should render button element correctly 1`] = `
 <div>
   <button
-    class="box mm-button-base mm-button-link mm-button-link--size-auto box--display-inline-flex box--flex-direction-row box--justify-content-center box--align-items-center box--color-primary-default box--background-color-transparent"
+    class="box mm-text mm-button-base mm-button-link mm-button-link--size-auto mm-text--body-md mm-text--color-primary-default box--display-inline-flex box--flex-direction-row box--justify-content-center box--align-items-center box--background-color-transparent"
     data-testid="button-link"
   >
-    <span
-      class="box mm-text mm-button-base__content mm-text--body-md mm-text--color-inherit box--gap-2 box--flex-direction-row box--justify-content-center box--align-items-center box--display-flex"
-    >
-      Button Link
-    </span>
+    Button Link
   </button>
 </div>
 `;