Hookify Avatar component

Use react-testing in some Avatar tests as Enzyme doesn't like updating
after hooky components after a setState

UNRELEASED.md

@@ -42,6 +42,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
 ### Code quality
 
 - Removed mocks in various tests suites that are now redundant ([#1978](https://github.com/Shopify/polaris-react/pull/1978))
+- Migrated `Avatar` to use hooks instead of withAppProvider ([#2067](https://github.com/Shopify/polaris-react/pull/2067))
 
 ### Deprecations
 

src/components/Avatar/Avatar.tsx

@@ -1,18 +1,21 @@
-import React from 'react';
+import React, {useState, useCallback, useEffect} from 'react';
 
 import {classNames, variationName} from '../../utilities/css';
+import {useI18n} from '../../utilities/i18n';
 import {isServer} from '../../utilities/target';
-import {
-  withAppProvider,
-  WithAppProviderProps,
-} from '../../utilities/with-app-provider';
 import {Image} from '../Image';
 
 import styles from './Avatar.scss';
 import * as avatars from './images';
 
 export type Size = 'small' | 'medium' | 'large';
 
+enum Status {
+  Pending,
+  Loaded,
+  Errored,
+}
+
 const STYLE_CLASSES = ['one', 'two', 'three', 'four', 'five', 'six'];
 const AVATAR_IMAGES = Object.keys(avatars).map(
   // import/namespace does not allow computed values by default
@@ -38,127 +41,106 @@ export interface AvatarProps {
   accessibilityLabel?: string;
 }
 
-interface State {
-  hasError: boolean;
-  hasLoaded: boolean;
-  prevSource?: string;
-}
-
-type CombinedProps = AvatarProps & WithAppProviderProps;
-
-class Avatar extends React.PureComponent<CombinedProps, State> {
-  static getDerivedStateFromProps(props: AvatarProps, state: State) {
-    if (props.source !== state.prevSource) {
-      return {
-        prevSource: props.source,
-        hasError: false,
-        hasLoaded: false,
-      };
-    }
-
-    return null;
+export function Avatar({
+  name,
+  source,
+  initials,
+  customer,
+  size = 'medium',
+  accessibilityLabel,
+}: AvatarProps) {
+  const i18n = useI18n();
+
+  const [status, setStatus] = useState<Status>(Status.Pending);
+
+  // If the source changes, set the status back to pending
+  useEffect(
+    () => {
+      setStatus(Status.Pending);
+    },
+    [source],
+  );
+
+  const handleError = useCallback(() => {
+    setStatus(Status.Errored);
+  }, []);
+  const handleLoad = useCallback(() => {
+    setStatus(Status.Loaded);
+  }, []);
+
+  const hasImage = (source || customer) && status !== Status.Errored;
+
+  const nameString = name || initials;
+
+  let finalSource: string | undefined;
+  let label: string | undefined;
+
+  if (accessibilityLabel) {
+    label = accessibilityLabel;
+  } else if (name) {
+    label = name;
+  } else if (initials) {
+    const splitInitials = initials.split('').join(' ');
+    label = i18n.translate('Polaris.Avatar.labelWithInitials', {
+      initials: splitInitials,
+    });
+  } else {
+    label = i18n.translate('Polaris.Avatar.label');
   }
 
-  state: State = {
-    hasError: false,
-    hasLoaded: false,
-  };
-
-  render() {
-    const {
-      name,
-      source,
-      initials,
-      customer,
-      size = 'medium',
-      accessibilityLabel,
-      polaris: {intl},
-    } = this.props;
-
-    const {hasError, hasLoaded} = this.state;
-
-    const hasImage = (source || customer) && !hasError;
-
-    const nameString = name || initials;
-
-    let finalSource: string | undefined;
-    let label: string | undefined;
-
-    if (accessibilityLabel) {
-      label = accessibilityLabel;
-    } else if (name) {
-      label = name;
-    } else if (initials) {
-      const splitInitials = initials.split('').join(' ');
-      label = intl.translate('Polaris.Avatar.labelWithInitials', {
-        initials: splitInitials,
-      });
-    } else {
-      label = intl.translate('Polaris.Avatar.label');
-    }
-
-    if (source) {
-      finalSource = source;
-    } else if (customer) {
-      finalSource = customerPlaceholder(nameString);
-    }
-
-    const className = classNames(
-      styles.Avatar,
-      styles[variationName('style', styleClass(nameString))],
-      size && styles[variationName('size', size)],
-      hasImage && !hasLoaded && styles.hidden,
-      hasImage && styles.hasImage,
-    );
-
-    const imageMarkUp =
-      finalSource && !isServer && !hasError ? (
-        <Image
-          className={styles.Image}
-          source={finalSource}
-          alt=""
-          role="presentation"
-          onLoad={this.handleLoad}
-          onError={this.handleError}
-        />
-      ) : null;
-
-    // Use `dominant-baseline: central` instead of `dy` when Edge supports it.
-    const verticalOffset = '0.35em';
-
-    const initialsMarkup =
-      initials && !hasImage ? (
-        <span className={styles.Initials}>
-          <svg className={styles.Svg} viewBox="0 0 48 48">
-            <text
-              x="50%"
-              y="50%"
-              dy={verticalOffset}
-              fill="currentColor"
-              fontSize="26"
-              textAnchor="middle"
-            >
-              {initials}
-            </text>
-          </svg>
-        </span>
-      ) : null;
-
-    return (
-      <span aria-label={label} role="img" className={className}>
-        {initialsMarkup}
-        {imageMarkUp}
-      </span>
-    );
+  if (source) {
+    finalSource = source;
+  } else if (customer) {
+    finalSource = customerPlaceholder(nameString);
   }
 
-  handleError = () => {
-    this.setState({hasError: true, hasLoaded: false});
-  };
-
-  handleLoad = () => {
-    this.setState({hasLoaded: true, hasError: false});
-  };
+  const className = classNames(
+    styles.Avatar,
+    styles[variationName('style', styleClass(nameString))],
+    size && styles[variationName('size', size)],
+    hasImage && status !== Status.Loaded && styles.hidden,
+    hasImage && styles.hasImage,
+  );
+
+  const imageMarkUp =
+    finalSource && !isServer && status !== Status.Errored ? (
+      <Image
+        className={styles.Image}
+        source={finalSource}
+        alt=""
+        role="presentation"
+        onLoad={handleLoad}
+        onError={handleError}
+      />
+    ) : null;
+
+  // Use `dominant-baseline: central` instead of `dy` when Edge supports it.
+  const verticalOffset = '0.35em';
+
+  const initialsMarkup =
+    initials && !hasImage ? (
+      <span className={styles.Initials}>
+        <svg className={styles.Svg} viewBox="0 0 48 48">
+          <text
+            x="50%"
+            y="50%"
+            dy={verticalOffset}
+            fill="currentColor"
+            fontSize="26"
+            textAnchor="middle"
+          >
+            {initials}
+          </text>
+        </svg>
+      </span>
+    ) : null;
+
+  return (
+    <span aria-label={label} role="img" className={className}>
+      {initialsMarkup}
+      {imageMarkUp}
+    </span>
+  );
 }
 
 function styleClass(name?: string) {
@@ -172,7 +154,3 @@ function customerPlaceholder(name?: string) {
     ? AVATAR_IMAGES[name.charCodeAt(0) % AVATAR_IMAGES.length]
     : AVATAR_IMAGES[0];
 }
-
-// Use named export once withAppProvider is refactored away
-// eslint-disable-next-line import/no-default-export
-export default withAppProvider<AvatarProps>()(Avatar);

src/components/Avatar/index.ts

@@ -1,3 +1 @@
-import Avatar, {AvatarProps} from './Avatar';
-
-export {Avatar, AvatarProps};
+export {Avatar, AvatarProps} from './Avatar';

src/components/Avatar/tests/Avatar-ssr.test.tsx

@@ -1,7 +1,6 @@
 import React from 'react';
 import {mountWithAppProvider} from 'test-utilities/legacy';
-import {Image} from 'components';
-import Avatar from '../Avatar';
+import {Avatar, Image} from 'components';
 
 jest.mock('../../../utilities/target', () => ({
   get isServer() {

src/components/Avatar/tests/Avatar.test.tsx

@@ -1,7 +1,7 @@
 import React from 'react';
-import {mountWithAppProvider, trigger} from 'test-utilities/legacy';
-import {Image} from 'components';
-import Avatar from '../Avatar';
+import {mountWithAppProvider} from 'test-utilities/legacy';
+import {mountWithApp} from 'test-utilities';
+import {Avatar, Image} from 'components';
 
 describe('<Avatar />', () => {
   describe('intials', () => {
@@ -45,36 +45,44 @@ describe('<Avatar />', () => {
 
   describe('on Error with Initials', () => {
     it('renders initials if the Image onError prop is triggered and the Intials are provided', () => {
-      const src = 'image/path/';
-      const avatar = mountWithAppProvider(
-        <Avatar size="large" initials="DL" source={src} />,
+      const avatar = mountWithApp(
+        <Avatar size="large" initials="DL" source="image/path/" />,
       );
-      expect(avatar.find('span[role="img"] span svg')).toHaveLength(0);
-      trigger(avatar.find(Image), 'onError');
-      expect(avatar.find('span[role="img"] span svg')).toHaveLength(1);
+
+      expect(avatar).toContainReactComponent(Image);
+      expect(avatar).not.toContainReactComponent('span', {
+        className: 'Initials',
+      });
+
+      avatar.find(Image)!.trigger('onError');
+
+      expect(avatar).not.toContainReactComponent(Image);
+      expect(avatar).toContainReactComponent('span', {
+        className: 'Initials',
+      });
     });
   });
 
   describe('on Error with changed props', () => {
     it('re-renders the image if a the source prop is changed after an error', () => {
-      const src = 'image/path/';
       const workingSrc = 'image/goodPath/';
-      const avatar = mountWithAppProvider(
-        <Avatar size="large" initials="DL" source={src} />,
+      const avatar = mountWithApp(
+        <Avatar size="large" initials="DL" source="image/path/" />,
       );
-      trigger(avatar.find(Image), 'onError');
-      expect(avatar.find(Image)).toHaveLength(0);
+      avatar.find(Image)!.trigger('onError');
+
+      expect(avatar).not.toContainReactComponent(Image);
+
       avatar.setProps({source: workingSrc});
-      const image = avatar.find(Image);
-      expect(image).toHaveLength(1);
+      expect(avatar).toContainReactComponent(Image);
     });
   });
 
   describe('on Load', () => {
     it('safely triggers onLoad', () => {
-      const avatar = mountWithAppProvider(<Avatar source="image/path/" />);
+      const avatar = mountWithApp(<Avatar source="image/path/" />);
       expect(() => {
-        trigger(avatar.find(Image), 'onLoad');
+        avatar.find(Image)!.trigger('onLoad');
       }).not.toThrow();
     });
   });

tests/build.test.js

@@ -134,7 +134,7 @@ describe('build', () => {
         'esnext/components/Avatar/index.js',
         'utf8',
       );
-      expect(contents).toMatch("import Avatar from './Avatar'");
+      expect(contents).toMatch("export { Avatar } from './Avatar'");
     });
 
     it('preserves ES scss imports', () => {