feat(HelperText): consumed Penta updates (#10029)

* feat(HelperText): consumed Penta updates

* Updated examples and tests

* Fixed example file names

* Updated tests per Austin feedback

* Updated test to avoid false positive

packages/react-core/src/components/HelperText/HelperTextItem.tsx

@@ -6,41 +6,42 @@ import ExclamationTriangleIcon from '@patternfly/react-icons/dist/esm/icons/excl
 import CheckCircleIcon from '@patternfly/react-icons/dist/esm/icons/check-circle-icon';
 import ExclamationCircleIcon from '@patternfly/react-icons/dist/esm/icons/exclamation-circle-icon';
 
+export enum HelperTextItemVariant {
+  default = 'default',
+  warning = 'warning',
+  error = 'error',
+  success = 'success'
+}
+
 export interface HelperTextItemProps extends React.HTMLProps<HTMLDivElement | HTMLLIElement> {
   /** Content rendered inside the helper text item. */
   children?: React.ReactNode;
   /** Additional classes applied to the helper text item. */
   className?: string;
   /** Sets the component type of the helper text item. */
   component?: 'div' | 'li';
-  /** Variant styling of the helper text item. */
+  /** Variant styling of the helper text item. Will also render a default icon, which can be overridden
+   * with the icon prop.
+   */
   variant?: 'default' | 'indeterminate' | 'warning' | 'success' | 'error';
-  /** Custom icon prefixing the helper text. This property will override the default icon paired with each helper text variant. */
+  /** Custom icon prefixing the helper text. This property will override the default icon when the variant property is passed in. */
   icon?: React.ReactNode;
-  /** Flag indicating the helper text item is dynamic. This prop should be used when the
-   * text content of the helper text item will never change, but the icon and styling will
-   * be dynamically updated via the `variant` prop.
-   */
-  isDynamic?: boolean;
-  /** Flag indicating the helper text should have an icon. Dynamic helper texts include icons by default while static helper texts do not. */
-  hasIcon?: boolean;
   /** ID for the helper text item. The value of this prop can be passed into a form component's
    * aria-describedby prop when you intend for only specific helper text items to be announced to
    * assistive technologies.
    */
   id?: string;
-  /** Text that is only accessible to screen readers in order to announce the status of a helper text item.
-   * This prop can only be used when the isDynamic prop is also passed in.
+  /** Text that is only accessible to screen readers in order to announce the variant of a helper text item.
+   * This prop can only be used when the variant prop has a value other than "default".
    */
   screenReaderText?: string;
 }
 
-const variantStyle = {
-  default: '',
-  indeterminate: styles.modifiers.indeterminate,
-  warning: styles.modifiers.warning,
-  success: styles.modifiers.success,
-  error: styles.modifiers.error
+const defaultVariantIcons = {
+  indeterminate: <MinusIcon />,
+  warning: <ExclamationTriangleIcon />,
+  success: <CheckCircleIcon />,
+  error: <ExclamationCircleIcon />
 };
 
 export const HelperTextItem: React.FunctionComponent<HelperTextItemProps> = ({
@@ -49,36 +50,28 @@ export const HelperTextItem: React.FunctionComponent<HelperTextItemProps> = ({
   component = 'div',
   variant = 'default',
   icon,
-  isDynamic = false,
-  hasIcon = isDynamic,
   id,
   screenReaderText = `${variant} status`,
   ...props
 }: HelperTextItemProps) => {
   const Component = component as any;
+  const isNotDefaultVariant = variant !== 'default';
+  const defaultIcon = isNotDefaultVariant && defaultVariantIcons[variant];
   return (
     <Component
-      className={css(styles.helperTextItem, variantStyle[variant], isDynamic && styles.modifiers.dynamic, className)}
+      className={css(styles.helperTextItem, isNotDefaultVariant && styles.modifiers[variant], className)}
       id={id}
       {...props}
     >
-      {icon && (
-        <span className={css(styles.helperTextItemIcon)} aria-hidden>
-          {icon}
-        </span>
-      )}
-      {hasIcon && !icon && (
+      {(defaultIcon || icon) && (
         <span className={css(styles.helperTextItemIcon)} aria-hidden>
-          {(variant === 'default' || variant === 'indeterminate') && <MinusIcon />}
-          {variant === 'warning' && <ExclamationTriangleIcon />}
-          {variant === 'success' && <CheckCircleIcon />}
-          {variant === 'error' && <ExclamationCircleIcon />}
+          {icon || defaultIcon}
         </span>
       )}
 
       <span className={css(styles.helperTextItemText)}>
         {children}
-        {isDynamic && <span className="pf-v5-screen-reader">: {screenReaderText};</span>}
+        {isNotDefaultVariant && <span className="pf-v5-screen-reader">: {screenReaderText};</span>}
       </span>
     </Component>
   );

packages/react-core/src/components/HelperText/__tests__/HelperTextItem.test.tsx

@@ -30,6 +30,12 @@ test('Renders custom className', () => {
   expect(screen.getByText('help test text 1').parentElement).toHaveClass('custom');
 });
 
+test('Does not render screen reader text by default', () => {
+  render(<HelperTextItem>help test text 1</HelperTextItem>);
+
+  expect(screen.queryByText('status', { exact: false })).not.toBeInTheDocument();
+});
+
 Object.values(['indeterminate', 'warning', 'success', 'error']).forEach((variant) => {
   test(`Renders with class ${styles.modifiers[variant]} when variant = ${variant}`, () => {
     render(
@@ -39,6 +45,26 @@ Object.values(['indeterminate', 'warning', 'success', 'error']).forEach((variant
     );
     expect(screen.getByText('text').parentElement).toHaveClass(styles.modifiers[variant]);
   });
+
+  test(`Renders default screenreader text when variant = ${variant}`, () => {
+    render(
+      <HelperTextItem variant={variant as 'default' | 'indeterminate' | 'warning' | 'success' | 'error'}>
+        text
+      </HelperTextItem>
+    );
+    expect(screen.getByText(`: ${variant} status;`)).toBeInTheDocument();
+  });
+});
+
+test('Renders custom screen reader text', () => {
+  render(
+    <HelperTextItem variant="error" screenReaderText="danger">
+      help test text 1
+    </HelperTextItem>
+  );
+
+  expect(screen.queryByText(': error status;')).not.toBeInTheDocument();
+  expect(screen.getByText(': danger;')).toBeInTheDocument();
 });
 
 test('Renders id when id is passed', () => {
@@ -56,38 +82,26 @@ test('Renders with element passed to component prop', () => {
   expect(screen.getByText('help test text 1').parentElement?.tagName).toBe('LI');
 });
 
-test('Renders custom icon', () => {
-  render(<HelperTextItem icon={<div>test</div>}>help test text</HelperTextItem>);
-  expect(screen.getByText('test').parentElement).toHaveClass(styles.helperTextItemIcon);
-});
-
-test('Renders default icon when hasIcon = true and icon is not passed', () => {
-  render(<HelperTextItem hasIcon>help test text</HelperTextItem>);
-  expect(screen.getByText('help test text').parentElement?.querySelector('span')).toHaveClass(
-    styles.helperTextItemIcon
-  );
+test('Does not render an icon by default', () => {
+  render(<HelperTextItem>help test text</HelperTextItem>);
+  expect(screen.queryByText('help test text')?.previousSibling).not.toBeInTheDocument();
 });
 
-test('Renders custom icon when icon is passed and hasIcon = true', () => {
-  render(
-    <HelperTextItem hasIcon icon={<div>test</div>}>
-      help test text
-    </HelperTextItem>
-  );
-  expect(screen.getByText('test').parentElement).toHaveClass(styles.helperTextItemIcon);
+test('Renders a default icon when variant is passed and icon is not passed', () => {
+  render(<HelperTextItem variant="success">help test text</HelperTextItem>);
+  expect(screen.getByText('help test text').previousSibling).toHaveClass(styles.helperTextItemIcon);
 });
 
-test('Renders dynamic helper text', () => {
-  render(<HelperTextItem isDynamic>help test text</HelperTextItem>);
-  expect(screen.getByText('help test text').parentElement).toHaveClass(styles.modifiers.dynamic);
-  expect(screen.getByText('help test text').querySelector('span')).toHaveClass('pf-v5-screen-reader');
+test('Renders custom icon when icon prop is passed', () => {
+  render(<HelperTextItem icon={<div>icon content</div>}>help test text</HelperTextItem>);
+  expect(screen.getByText('icon content').parentElement).toHaveClass(styles.helperTextItemIcon);
 });
 
-test('Renders custom screenreader text when isDynamic = true and screenReaderText is passed', () => {
+test('Renders custom icon instead of variant icon when icon and variant are passed', () => {
   render(
-    <HelperTextItem isDynamic screenReaderText="sr test">
+    <HelperTextItem icon={<div>icon content</div>} variant="success">
       help test text
     </HelperTextItem>
   );
-  expect(screen.getByText('help test text').querySelector('span')).toHaveTextContent('sr test');
+  expect(screen.getByText('icon content').parentElement).toHaveClass(styles.helperTextItemIcon);
 });

packages/react-core/src/components/HelperText/examples/HelperText.md

@@ -13,32 +13,22 @@ import ExclamationIcon from '@patternfly/react-icons/dist/esm/icons/exclamation-
 
 ## Examples
 
-### Static
+### Basic
 
-```ts file="HelperTextStatic.tsx"
-```
-
-### Static with default icons
+```ts file="HelperTextBasic.tsx"
 
-```ts file="HelperTextStaticWithDefaultIcon.tsx"
 ```
 
-### Static with custom icons
+### With custom icons
 
-```ts file="HelperTextStaticWithCustomIcon.tsx"
-```
+```ts file="HelperTextWithCustomIcon.tsx"
 
-### Multiple static
-
-```ts file="HelperTextMultipleStatic.tsx"
 ```
 
-### Dynamic
+### Multiple items
 
-```ts file="HelperTextDynamic.tsx"
-```
+You can pass multiple `<HelperTextItem>` components inside a single `<Helpertext>` container.
 
-### Dynamic list
+```ts file="HelperTextMultipleItems.tsx"
 
-```ts file="HelperTextDynamicList.tsx"
 ```

packages/react-core/src/components/HelperText/examples/HelperTextStatic.tsx → packages/react-core/src/components/HelperText/examples/HelperTextBasic.tsx

@@ -1,7 +1,7 @@
 import React from 'react';
 import { HelperText, HelperTextItem } from '@patternfly/react-core';
 
-export const HelperTextStatic: React.FunctionComponent = () => (
+export const HelperTextBasic: React.FunctionComponent = () => (
   <React.Fragment>
     <HelperText>
       <HelperTextItem>This is default helper text</HelperTextItem>

packages/react-core/src/components/HelperText/examples/HelperTextMultipleItems.tsx

@@ -0,0 +1,10 @@
+import React from 'react';
+import { HelperText, HelperTextItem } from '@patternfly/react-core';
+
+export const HelperTextMultipleItems: React.FunctionComponent = () => (
+  <HelperText component="ul">
+    <HelperTextItem component="li">This is default helper text</HelperTextItem>
+    <HelperTextItem component="li">This is another default helper text in the same HelperText block</HelperTextItem>
+    <HelperTextItem component="li">And this is more default text in the same HelperText block</HelperTextItem>
+  </HelperText>
+);

packages/react-core/src/components/HelperText/examples/HelperTextStaticWithCustomIcon.tsx → packages/react-core/src/components/HelperText/examples/HelperTextWithCustomIcon.tsx

@@ -6,7 +6,7 @@ import ExclamationIcon from '@patternfly/react-icons/dist/esm/icons/exclamation-
 import CheckIcon from '@patternfly/react-icons/dist/esm/icons/check-icon';
 import TimesIcon from '@patternfly/react-icons/dist/esm/icons/times-icon';
 
-export const HelperTextStaticWithCustomIcon: React.FunctionComponent = () => (
+export const HelperTextWithCustomIcon: React.FunctionComponent = () => (
   <React.Fragment>
     <HelperText>
       <HelperTextItem icon={<InfoIcon />}>This is default helper text</HelperTextItem>

packages/react-core/src/components/MultipleFileUpload/examples/MultipleFileUploadBasic.tsx

@@ -63,7 +63,7 @@ export const MultipleFileUploadBasic: React.FunctionComponent = () => {
     if (fileUploadShouldFail) {
       const corruptedFiles = files.map((file) => ({ ...file, lastModified: 'foo' as unknown as number }));
       // eslint-disable-next-line
-      setCurrentFiles((prevFiles) => [...prevFiles, ...corruptedFiles as any]);
+      setCurrentFiles((prevFiles) => [...prevFiles, ...(corruptedFiles as any)]);
     } else {
       setCurrentFiles((prevFiles) => [...prevFiles, ...files]);
     }
@@ -101,7 +101,7 @@ export const MultipleFileUploadBasic: React.FunctionComponent = () => {
     if (fileResult?.loadError) {
       return (
         <HelperText isLiveRegion>
-          <HelperTextItem variant={'error'}>{fileResult.loadError.toString()}</HelperTextItem>
+          <HelperTextItem variant="error">{fileResult.loadError.toString()}</HelperTextItem>
         </HelperText>
       );
     }