refactor: refactoring after review

openedx · Oct 18, 2023 · bf80855 · bf80855
1 parent 2142773
commit bf80855
Show file tree

Hide file tree

Showing 25 changed files with 127 additions and 169 deletions.
diff --git a/src/Chip/Chip.test.jsx b/src/Chip/Chip.test.jsx
@@ -2,18 +2,15 @@ import React from 'react';
 import renderer from 'react-test-renderer';
 import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
-import { IntlProvider } from 'react-intl';
 
 import { Close } from '../../icons';
 import Chip from '.';
 
 function TestChip(props) {
   return (
-    <IntlProvider locale="en">
-      <Chip {...props}>
-        Test
-      </Chip>
-    </IntlProvider>
+    <Chip {...props}>
+      Test
+    </Chip>
   );
 }
 
@@ -98,5 +95,19 @@ describe('<Chip />', () => {
       await userEvent.type(iconBefore, '{enter}');
       expect(func).toHaveBeenCalled();
     });
+    it('checks the absence of the `selected` class in the chip', async () => {
+      render(
+        <TestChip data-testid="chip" />,
+      );
+      const iconBefore = screen.getByTestId('chip');
+      expect(iconBefore).not.toHaveClass('selected');
+    });
+    it('checks the presence of the `selected` class in the chip', async () => {
+      render(
+        <TestChip isSelected data-testid="chip" />,
+      );
+      const iconBefore = screen.getByTestId('chip');
+      expect(iconBefore).toHaveClass('selected');
+    });
   });
 });
diff --git a/src/Chip/README.md b/src/Chip/README.md
@@ -25,34 +25,38 @@ notes: |
 </Stack>
 ```
 
-## With Icon Before and After
-### Basic Usage
+## With isSelected prop
 
 ```jsx live
 <Stack
     gap={2}
     direction="horizontal"
 >
-    <Chip iconBefore={Person}>New</Chip>
+    <Chip isSelected>New</Chip>
     <Chip
+        isSelected
         iconAfter={Close}
         onIconAfterClick={() => console.log('onIconAfterClick')}
-    >
-        New 1
-    </Chip>
-    <Chip
-        iconBefore={Person}
-        onIconBeforeClick={() => console.log('Remove Chip')}
     >
         New
     </Chip>
+</Stack>
+```
+
+## With Icon Before and After
+### Basic Usage
+
+```jsx live
+<Stack
+    gap={2}
+    direction="horizontal"
+>
+    <Chip iconBefore={Person}>New</Chip>
     <Chip
-        iconBefore={Person}
         iconAfter={Close}
-        onIconBeforeClick={() => console.log('onIconBeforeClick')}
         onIconAfterClick={() => console.log('onIconAfterClick')}
     >
-        New
+        New 1
     </Chip>
     <Chip
         iconBefore={Person}
@@ -81,22 +85,6 @@ notes: |
     >
         New 1
     </Chip>
-    <Chip
-        variant="dark"
-        iconBefore={Person}
-        onIconBeforeClick={() => console.log('Remove Chip')}
-    >
-        New
-    </Chip>
-    <Chip
-        variant="dark"
-        iconBefore={Person}
-        iconAfter={Close}
-        onIconBeforeClick={() => console.log('onIconBeforeClick')}
-        onIconAfterClick={() => console.log('onIconAfterClick')}
-    >
-        New
-    </Chip>
     <Chip
         variant="dark"
         iconAfter={Close}

diff --git a/src/Chip/index.scss b/src/Chip/index.scss
@@ -4,6 +4,7 @@
 .pgn__chip {
   border-radius: $chip-border-radius;
   display: inline-flex;
+  justify-content: space-between;
   margin: $chip-margin;
   border: 1px solid $chip-border-color;
   padding: $chip-padding-y $chip-padding-x;

diff --git a/src/Chip/index.tsx b/src/Chip/index.tsx
@@ -1,105 +1,105 @@
 import React, { ForwardedRef, KeyboardEventHandler, MouseEventHandler } from 'react';
-import { useIntl } from 'react-intl';
 import PropTypes from 'prop-types';
 import classNames from 'classnames';
 // @ts-ignore
 import Icon from '../Icon';
 // @ts-ignore
 import IconButton from '../IconButton';
-// @ts-ignore
-import messages from './messages';
 
-const STYLE_VARIANTS = [
+export const ICON_AFTER_ALT_TEXT = 'Chip icon after';
+export const ICON_BEFORE_ALT_TEXT = 'Chip icon before';
+
+export const STYLE_VARIANTS = [
   'light',
   'dark',
 ];
 
+export const CHIP_PGN_CLASS = 'pgn__chip';
+
 export interface IChip {
   children: React.ReactNode,
   className?: string,
   variant?: string,
   iconBefore?: React.ReactElement | Function,
+  iconBeforeAlt?: string,
   iconAfter?: React.ReactElement | Function,
+  iconAfterAlt?: string,
   onIconBeforeClick?: KeyboardEventHandler & MouseEventHandler,
   onIconAfterClick?: KeyboardEventHandler & MouseEventHandler,
   disabled?: boolean,
   isSelected?: boolean,
 }
 
-export const CHIP_PGN_CLASS = 'pgn__chip';
-
 const Chip = React.forwardRef(({
   children,
   className,
   variant,
   iconBefore,
+  iconBeforeAlt,
   iconAfter,
+  iconAfterAlt,
   onIconBeforeClick,
   onIconAfterClick,
   disabled,
   isSelected,
   ...props
-}: IChip, ref: ForwardedRef<HTMLDivElement>) => {
-  const intl = useIntl();
-
-  return (
+}: IChip, ref: ForwardedRef<HTMLDivElement>) => (
+  <div
+    tabIndex={0}
+    role="button"
+    className={classNames(
+      CHIP_PGN_CLASS,
+      `pgn__chip-${variant}`,
+      className,
+      { disabled, selected: isSelected },
+    )}
+    ref={ref}
+    {...props}
+  >
+    {iconBefore && (
+      <div className={classNames('pgn__chip__icon-before', { active: onIconBeforeClick })}>
+        {onIconBeforeClick ? (
+          <IconButton
+            src={iconBefore}
+            onClick={onIconBeforeClick}
+            onKeyPress={onIconBeforeClick}
+            iconAs={Icon}
+            alt={iconBeforeAlt}
+            invertColors
+            data-testid="icon-before"
+          />
+        ) : (
+          <Icon src={iconBefore} />
+        )}
+      </div>
+    )}
     <div
-      tabIndex={0}
-      role="button"
-      className={classNames(
-        CHIP_PGN_CLASS,
-        `pgn__chip-${variant}`,
-        className,
-        { disabled, selected: isSelected },
-      )}
-      ref={ref}
-      {...props}
+      className={classNames('pgn__chip__label', {
+        'p-before': iconBefore,
+        'p-after': iconAfter,
+      })}
     >
-      {iconBefore && (
-        <div className={classNames('pgn__chip__icon-before', { active: onIconBeforeClick })}>
-          {onIconBeforeClick ? (
-            <IconButton
-              src={iconBefore}
-              onClick={onIconBeforeClick}
-              onKeyPress={onIconBeforeClick}
-              iconAs={Icon}
-              alt={intl.formatMessage(messages.iconBeforeAltText)}
-              invertColors
-              data-testid="icon-before"
-            />
-          ) : (
-            <Icon src={iconBefore} />
-          )}
-        </div>
-      )}
-      <div
-        className={classNames('pgn__chip__label', {
-          'p-before': iconBefore,
-          'p-after': iconAfter,
-        })}
-      >
-        {children}
-      </div>
-      {iconAfter && (
-        <div className={classNames('pgn__chip__icon-after', { active: onIconAfterClick })}>
-          {onIconAfterClick ? (
-            <IconButton
-              onClick={onIconAfterClick}
-              onKeyPress={onIconAfterClick}
-              src={iconAfter}
-              iconAs={Icon}
-              alt={intl.formatMessage(messages.iconAfterAltText)}
-              invertColors
-              data-testid="icon-after"
-            />
-          ) : (
-            <Icon src={iconAfter} />
-          )}
-        </div>
-      )}
+      {children}
     </div>
-  );
-});
+    {iconAfter && (
+      <div className={classNames('pgn__chip__icon-after', { active: onIconAfterClick })}>
+        {onIconAfterClick ? (
+          <IconButton
+            onClick={onIconAfterClick}
+            onKeyPress={onIconAfterClick}
+            src={iconAfter}
+            iconAs={Icon}
+            alt={iconAfterAlt}
+            invertColors
+            data-testid="icon-after"
+          />
+        ) : (
+          <Icon src={iconAfter} />
+        )}
+      </div>
+    )}
+  </div>
+));
 
 Chip.propTypes = {
   /** Specifies the content of the `Chip`. */
@@ -117,6 +117,8 @@ Chip.propTypes = {
    * `import { Check } from '@edx/paragon/icons';`
    */
   iconBefore: PropTypes.oneOfType([PropTypes.element, PropTypes.func]),
+  /** Specifies icon alt text. */
+  iconBeforeAlt: PropTypes.string,
   /** A click handler for the `Chip` icon before. */
   onIconBeforeClick: PropTypes.func,
   /**
@@ -126,8 +128,11 @@ Chip.propTypes = {
    * `import { Check } from '@edx/paragon/icons';`
    */
   iconAfter: PropTypes.oneOfType([PropTypes.element, PropTypes.func]),
+  /** Specifies icon alt text. */
+  iconAfterAlt: PropTypes.string,
   /** A click handler for the `Chip` icon after. */
   onIconAfterClick: PropTypes.func,
+  /** Indicates if `Chip` has been selected. */
   isSelected: PropTypes.bool,
 };
 
@@ -140,6 +145,8 @@ Chip.defaultProps = {
   onIconBeforeClick: undefined,
   onIconAfterClick: undefined,
   isSelected: false,
+  iconAfterAlt: ICON_AFTER_ALT_TEXT,
+  iconBeforeAlt: ICON_BEFORE_ALT_TEXT,
 };
 
 export default Chip;
diff --git a/src/Chip/messages.js b/src/Chip/messages.js
diff --git a/src/ChipCarousel/_variables.scss b/src/ChipCarousel/_variables.scss
@@ -1 +1,3 @@
-$chip-carousel-controls-top-offset:   -3px !default;
+$chip-carousel-controls-top-offset:    .375rem !default;
+$chip-carousel-container-padding-x:    .625rem !default;
+$chip-carousel-container-padding-y:    .313rem !default;
diff --git a/src/ChipCarousel/index.scss b/src/ChipCarousel/index.scss
@@ -11,6 +11,7 @@
     &.pgn__chip-carousel-gap__#{$level} {
       .pgn__overflow-scroll-overflow-container {
         column-gap: $space;
+        padding: $chip-carousel-container-padding-x $chip-carousel-container-padding-y;
       }
     }
   }

diff --git a/src/i18n/messages/ar.json b/src/i18n/messages/ar.json
@@ -27,7 +27,5 @@
   "pgn.Dropzone.UploadProgress.uploadLabel": "رفع {filename} جارٍ.",
   "pgn.FormAutosuggest.iconButtonClosed": "إغلاق قائمة الخيارات",
   "pgn.FormAutosuggest.iconButtonOpened": "فتح قائمة الخيارات",
-  "pgn.Toast.closeLabel": "إغلاق ",
-  "pgn.Chip.iconBeforeAltText": "Chip icon before",
-  "pgn.Chip.iconAfterAltText": "Chip icon after"
+  "pgn.Toast.closeLabel": "إغلاق "
 }
diff --git a/src/i18n/messages/ca.json b/src/i18n/messages/ca.json
@@ -27,7 +27,5 @@
   "pgn.Dropzone.UploadProgress.uploadLabel": "Uploading {filename}.",
   "pgn.FormAutosuggest.iconButtonClosed": "Close the options menu",
   "pgn.FormAutosuggest.iconButtonOpened": "Open the options menu",
-  "pgn.Toast.closeLabel": "Close",
-  "pgn.Chip.iconBeforeAltText": "Chip icon before",
-  "pgn.Chip.iconAfterAltText": "Chip icon after"
+  "pgn.Toast.closeLabel": "Close"
 }
diff --git a/src/i18n/messages/es_419.json b/src/i18n/messages/es_419.json
@@ -27,7 +27,5 @@
   "pgn.Dropzone.UploadProgress.uploadLabel": "Subiendo {filename}.",
   "pgn.FormAutosuggest.iconButtonClosed": "Cerrar el menú de opciones",
   "pgn.FormAutosuggest.iconButtonOpened": "Abre el menú de opciones",
-  "pgn.Toast.closeLabel": "Cerrar",
-  "pgn.Chip.iconBeforeAltText": "Chip icon before",
-  "pgn.Chip.iconAfterAltText": "Chip icon after"
+  "pgn.Toast.closeLabel": "Cerrar"
 }
diff --git a/src/i18n/messages/es_AR.json b/src/i18n/messages/es_AR.json
@@ -27,7 +27,5 @@
   "pgn.Dropzone.UploadProgress.uploadLabel": "Subiendo {filename}.",
   "pgn.FormAutosuggest.iconButtonClosed": "Cerrar el menú de opciones",
   "pgn.FormAutosuggest.iconButtonOpened": "Abre el menú de opciones",
-  "pgn.Toast.closeLabel": "Cerrar",
-  "pgn.Chip.iconBeforeAltText": "Chip icon before",
-  "pgn.Chip.iconAfterAltText": "Chip icon after"
+  "pgn.Toast.closeLabel": "Cerrar"
 }
diff --git a/src/i18n/messages/es_ES.json b/src/i18n/messages/es_ES.json
@@ -27,7 +27,5 @@
   "pgn.Dropzone.UploadProgress.uploadLabel": "Subiendo {filename}.",
   "pgn.FormAutosuggest.iconButtonClosed": "Cerrar el menú de opciones",
   "pgn.FormAutosuggest.iconButtonOpened": "Abre el menú de opciones",
-  "pgn.Toast.closeLabel": "Cerrar",
-  "pgn.Chip.iconBeforeAltText": "Chip icon before",
-  "pgn.Chip.iconAfterAltText": "Chip icon after"
+  "pgn.Toast.closeLabel": "Cerrar"
 }