From ab808b670a7553e8e8d5ab23530f5398f3418320 Mon Sep 17 00:00:00 2001 From: George Marshall Date: Wed, 23 Nov 2022 09:58:43 -0800 Subject: [PATCH] Icon house keeping updates (#16621) --- .../avatar-favicon/avatar-favicon.test.js | 2 +- .../__snapshots__/button-icon.test.js.snap | 4 +- .../button-link/button-link.test.js | 2 +- .../button-primary/button-primary.test.js | 2 +- .../button-secondary/button-secondary.test.js | 2 +- .../component-library/icon/README.mdx | 17 +- .../icon/__snapshots__/icon.test.js.snap | 11 + .../component-library/icon/icon.constants.js | 24 +- ui/components/component-library/icon/icon.js | 14 +- .../component-library/icon/icon.scss | 2 +- .../component-library/icon/icon.stories.js | 222 +++++++++++------- .../component-library/icon/icon.test.js | 53 ++++- ui/components/component-library/icon/index.js | 2 +- ui/components/component-library/index.js | 3 +- .../__snapshots__/picker-network.test.js.snap | 4 +- .../__snapshots__/tag-url.test.js.snap | 4 +- .../text-field-base/text-field-base.js | 3 +- .../text-field-base/text-field-base.scss | 1 + 18 files changed, 239 insertions(+), 133 deletions(-) create mode 100644 ui/components/component-library/icon/__snapshots__/icon.test.js.snap diff --git a/ui/components/component-library/avatar-favicon/avatar-favicon.test.js b/ui/components/component-library/avatar-favicon/avatar-favicon.test.js index f23bef224..3b508b411 100644 --- a/ui/components/component-library/avatar-favicon/avatar-favicon.test.js +++ b/ui/components/component-library/avatar-favicon/avatar-favicon.test.js @@ -27,7 +27,7 @@ describe('AvatarFavicon', () => { const { container } = render( , ); - expect(container.getElementsByClassName('icon')).toHaveLength(1); + expect(container.getElementsByClassName('mm-icon')).toHaveLength(1); }); it('should render fallback image with custom fallbackIconProps if no ImageSource is provided', () => { diff --git a/ui/components/component-library/button-icon/__snapshots__/button-icon.test.js.snap b/ui/components/component-library/button-icon/__snapshots__/button-icon.test.js.snap index ce55c094a..6100807d7 100644 --- a/ui/components/component-library/button-icon/__snapshots__/button-icon.test.js.snap +++ b/ui/components/component-library/button-icon/__snapshots__/button-icon.test.js.snap @@ -8,8 +8,8 @@ exports[`ButtonIcon should render button element correctly 1`] = ` data-testid="button-icon" >
diff --git a/ui/components/component-library/button-link/button-link.test.js b/ui/components/component-library/button-link/button-link.test.js index be6deb2b4..01d4abb47 100644 --- a/ui/components/component-library/button-link/button-link.test.js +++ b/ui/components/component-library/button-link/button-link.test.js @@ -79,7 +79,7 @@ describe('ButtonLink', () => { , ); - const icons = container.getElementsByClassName('icon').length; + const icons = container.getElementsByClassName('mm-icon').length; expect(icons).toBe(1); }); }); diff --git a/ui/components/component-library/button-primary/button-primary.test.js b/ui/components/component-library/button-primary/button-primary.test.js index 83892c345..cb9125c8e 100644 --- a/ui/components/component-library/button-primary/button-primary.test.js +++ b/ui/components/component-library/button-primary/button-primary.test.js @@ -92,7 +92,7 @@ describe('ButtonPrimary', () => { , ); - const icons = container.getElementsByClassName('icon').length; + const icons = container.getElementsByClassName('mm-icon').length; expect(icons).toBe(1); }); }); diff --git a/ui/components/component-library/button-secondary/button-secondary.test.js b/ui/components/component-library/button-secondary/button-secondary.test.js index 8b2b14896..7552ebabb 100644 --- a/ui/components/component-library/button-secondary/button-secondary.test.js +++ b/ui/components/component-library/button-secondary/button-secondary.test.js @@ -96,7 +96,7 @@ describe('ButtonSecondary', () => { , ); - const icons = container.getElementsByClassName('icon').length; + const icons = container.getElementsByClassName('mm-icon').length; expect(icons).toBe(1); }); }); diff --git a/ui/components/component-library/icon/README.mdx b/ui/components/component-library/icon/README.mdx index d18913802..55a7e3f93 100644 --- a/ui/components/component-library/icon/README.mdx +++ b/ui/components/component-library/icon/README.mdx @@ -20,21 +20,22 @@ The `Icon` accepts all props below as well as all [Box](/docs/ui-components-ui-b Use the `name` prop and the `ICON_NAMES` object to change the icon. -Use the [IconSearch](/ui-components-component-library-icon-icon-stories-js--name) story to find the icon you want to use. +Use the [IconSearch](/story/ui-components-component-library-icon-icon-stories-js--default-story) story to find the icon you want to use. + + + + ```jsx -import { Icon, ICON_NAMES } from '../../ui/components/component-library'; +import { Icon, ICON_NAMES } from '../../components/component-library'; +// etc... ``` - - - - ### Size Use the `size` prop and the `SIZES` object from `./ui/helpers/constants/design-system.js` to change the size of `Icon`. Defaults to `SIZES.SM` @@ -55,7 +56,7 @@ Possible sizes include: ```jsx import { SIZES } from '../../../helpers/constants/design-system'; -import { Icon, ICON_NAMES } from '../../ui/components/component-library'; +import { Icon, ICON_NAMES } from '../../components/component-library'; @@ -79,7 +80,7 @@ Use the `color` prop and the `COLORS` object from `./ui/helpers/constants/design ```jsx import { COLORS } from '../../../helpers/constants/design-system'; -import { Icon, ICON_NAMES } from '../../ui/components/component-library'; +import { Icon, ICON_NAMES } from '../../components/component-library'; diff --git a/ui/components/component-library/icon/__snapshots__/icon.test.js.snap b/ui/components/component-library/icon/__snapshots__/icon.test.js.snap new file mode 100644 index 000000000..fa37d3955 --- /dev/null +++ b/ui/components/component-library/icon/__snapshots__/icon.test.js.snap @@ -0,0 +1,11 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Icon should render correctly 1`] = ` +
+
+
+`; diff --git a/ui/components/component-library/icon/icon.constants.js b/ui/components/component-library/icon/icon.constants.js index ca47099bc..7cd4da2c1 100644 --- a/ui/components/component-library/icon/icon.constants.js +++ b/ui/components/component-library/icon/icon.constants.js @@ -1,11 +1,25 @@ +import { SIZES } from '../../../helpers/constants/design-system'; + /** * The ICON_NAMES object contains all the possible icon names. - * It is generated using the generateIconNames script in development/generate-icon-names.js - * and stored in the environment variable ICON_NAMES - * To add a new icon, add the icon svg file to app/images/icons - * Ensure the svg has been optimized, is kebab case and starts with "icon-" - * See "Adding a new icon" in ./README.md for more details + * + * Search for an icon: https://metamask.github.io/metamask-storybook/?path=/story/ui-components-component-library-icon-icon-stories-js--default-story + * + * Add an icon: https://metamask.github.io/metamask-storybook/?path=/docs/ui-components-component-library-icon-icon-stories-js--default-story#adding-a-new-icon + * + * ICON_NAMES is generated using svgs in app/images/icons and + * the generateIconNames script in development/generate-icon-names.js + * then stored as an environment variable */ /* eslint-disable prefer-destructuring*/ // process.env is not a standard JavaScript object, so we are not able to use object destructuring export const ICON_NAMES = JSON.parse(process.env.ICON_NAMES); +export const ICON_SIZES = { + XXS: SIZES.XXS, + XS: SIZES.XS, + SM: SIZES.SM, + MD: SIZES.MD, + LG: SIZES.LG, + XL: SIZES.XL, + AUTO: SIZES.AUTO, +}; diff --git a/ui/components/component-library/icon/icon.js b/ui/components/component-library/icon/icon.js index 8b18b183a..37c2c83a0 100644 --- a/ui/components/component-library/icon/icon.js +++ b/ui/components/component-library/icon/icon.js @@ -10,6 +10,8 @@ import { ICON_COLORS, } from '../../../helpers/constants/design-system'; +import { ICON_SIZES } from './icon.constants'; + export const Icon = ({ name, size = SIZES.MD, @@ -21,15 +23,15 @@ export const Icon = ({ return ( ; - -DefaultStory.storyName = 'Default'; - -export const Name = (args) => { +export const DefaultStory = (args) => { const [search, setSearch] = useState(''); const iconList = Object.keys(ICON_NAMES) .filter( @@ -106,98 +113,134 @@ export const Name = (args) => { setSearch(e.target.value); }; + const handleOnClear = () => { + setSearch(''); + }; + return ( <> Icon search - - - - - {iconList.length > 0 ? ( - <> - {iconList.map((item) => { - return ( - - - - - { - const tempEl = document.createElement('textarea'); - tempEl.value = item; - document.body.appendChild(tempEl); - tempEl.select(); - document.execCommand('copy'); - document.body.removeChild(tempEl); - }} - > - {item} - - - ); - })} - - ) : ( - - No matches. Please try again or ask in the{' '} - - #metamask-design-system - {' '} - channel on slack. - - )} + + {/* TODO replace with FormTextField */} + + + + {iconList.length > 0 ? ( + + {iconList.map((item) => { + return ( + + + { + const tempEl = document.createElement('textarea'); + tempEl.value = item; + document.body.appendChild(tempEl); + tempEl.select(); + document.execCommand('copy'); + document.body.removeChild(tempEl); + }} + /> + } + /> + + ); + })} + + ) : ( + + No matches. Please try again or ask in the{' '} + + #metamask-design-system + {' '} + channel on slack. + + )} ); }; +DefaultStory.storyName = 'Default'; + +export const Name = (args) => ( + <> + + {Object.keys(ICON_NAMES).map((item) => { + console.log('item:', item); + return ( + + + + ); + })} + + +); export const Size = (args) => ( <> @@ -220,6 +263,7 @@ export const Size = (args) => ( ); + export const Color = (args) => ( diff --git a/ui/components/component-library/icon/icon.test.js b/ui/components/component-library/icon/icon.test.js index 510a1c87c..d7922b89a 100644 --- a/ui/components/component-library/icon/icon.test.js +++ b/ui/components/component-library/icon/icon.test.js @@ -12,6 +12,37 @@ describe('Icon', () => { ); expect(getByTestId('icon')).toBeDefined(); expect(container.querySelector('svg')).toBeDefined(); + expect(container).toMatchSnapshot(); + }); + it('should render with a custom class', () => { + const { getByTestId } = render( + , + ); + expect(getByTestId('icon')).toHaveClass('test-class'); + }); + it('should render with an aria-label attribute', () => { + /** + * We aren't specifically adding an ariaLabel prop because in most cases + * the icon should be decorative or be accompanied by text. Also if the icon + * is to be used as a button in most cases ButtonIcon should be used. However + * we should test if it's possible to pass an aria-label attribute to the + * root html element. + */ + const { getByTestId } = render( + , + ); + expect(getByTestId('icon')).toHaveAttribute( + 'aria-label', + 'test aria label', + ); }); it('should render with different icons using mask-image and image urls', () => { const { getByTestId } = render( @@ -33,16 +64,16 @@ describe('Icon', () => { ); expect( window.getComputedStyle(getByTestId('icon-add-square-filled')).maskImage, - ).toBe(`url('./images/icons/icon-add-square-filled.svg`); + ).toBe(`url('./images/icons/icon-add-square-filled.svg')`); expect( window.getComputedStyle(getByTestId('icon-bank-filled')).maskImage, - ).toBe(`url('./images/icons/icon-bank-filled.svg`); + ).toBe(`url('./images/icons/icon-bank-filled.svg')`); expect( window.getComputedStyle(getByTestId('icon-bookmark-filled')).maskImage, - ).toBe(`url('./images/icons/icon-bookmark-filled.svg`); + ).toBe(`url('./images/icons/icon-bookmark-filled.svg')`); expect( window.getComputedStyle(getByTestId('icon-calculator-filled')).maskImage, - ).toBe(`url('./images/icons/icon-calculator-filled.svg`); + ).toBe(`url('./images/icons/icon-calculator-filled.svg')`); }); it('should render with different size classes', () => { const { getByTestId } = render( @@ -84,13 +115,13 @@ describe('Icon', () => { /> , ); - expect(getByTestId('icon-xxs')).toHaveClass('icon--size-xxs'); - expect(getByTestId('icon-xs')).toHaveClass('icon--size-xs'); - expect(getByTestId('icon-sm')).toHaveClass('icon--size-sm'); - expect(getByTestId('icon-md')).toHaveClass('icon--size-md'); - expect(getByTestId('icon-lg')).toHaveClass('icon--size-lg'); - expect(getByTestId('icon-xl')).toHaveClass('icon--size-xl'); - expect(getByTestId('icon-auto')).toHaveClass('icon--size-auto'); + expect(getByTestId('icon-xxs')).toHaveClass('mm-icon--size-xxs'); + expect(getByTestId('icon-xs')).toHaveClass('mm-icon--size-xs'); + expect(getByTestId('icon-sm')).toHaveClass('mm-icon--size-sm'); + expect(getByTestId('icon-md')).toHaveClass('mm-icon--size-md'); + expect(getByTestId('icon-lg')).toHaveClass('mm-icon--size-lg'); + expect(getByTestId('icon-xl')).toHaveClass('mm-icon--size-xl'); + expect(getByTestId('icon-auto')).toHaveClass('mm-icon--size-auto'); }); it('should render with icon colors', () => { const { getByTestId } = render( diff --git a/ui/components/component-library/icon/index.js b/ui/components/component-library/icon/index.js index 97ac040b5..270538ceb 100644 --- a/ui/components/component-library/icon/index.js +++ b/ui/components/component-library/icon/index.js @@ -1,2 +1,2 @@ export { Icon } from './icon'; -export { ICON_NAMES } from './icon.constants'; +export { ICON_NAMES, ICON_SIZES } from './icon.constants'; diff --git a/ui/components/component-library/index.js b/ui/components/component-library/index.js index 2ace6c6d4..d728e7423 100644 --- a/ui/components/component-library/index.js +++ b/ui/components/component-library/index.js @@ -12,7 +12,7 @@ export { ButtonPrimary } from './button-primary'; export { ButtonSecondary } from './button-secondary'; export { FormTextField } from './form-text-field'; export { HelpText } from './help-text'; -export { Icon, ICON_NAMES } from './icon'; +export { Icon, ICON_NAMES, ICON_SIZES } from './icon'; export { Label } from './label'; export { PickerNetwork } from './picker-network'; export { Tag } from './tag'; @@ -24,3 +24,4 @@ export { TEXT_FIELD_BASE_SIZES, TEXT_FIELD_BASE_TYPES, } from './text-field-base'; +export { TextFieldSearch } from './text-field-search'; diff --git a/ui/components/component-library/picker-network/__snapshots__/picker-network.test.js.snap b/ui/components/component-library/picker-network/__snapshots__/picker-network.test.js.snap index 1b4f6dadd..8abe2cbd5 100644 --- a/ui/components/component-library/picker-network/__snapshots__/picker-network.test.js.snap +++ b/ui/components/component-library/picker-network/__snapshots__/picker-network.test.js.snap @@ -17,8 +17,8 @@ exports[`PickerNetwork should render the label inside the PickerNetwork 1`] = ` Imported

diff --git a/ui/components/component-library/tag-url/__snapshots__/tag-url.test.js.snap b/ui/components/component-library/tag-url/__snapshots__/tag-url.test.js.snap index 771eda22c..cb9ba9753 100644 --- a/ui/components/component-library/tag-url/__snapshots__/tag-url.test.js.snap +++ b/ui/components/component-library/tag-url/__snapshots__/tag-url.test.js.snap @@ -11,8 +11,8 @@ exports[`TagUrl should render the label inside the TagUrl 1`] = ` >