Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 24 additions & 31 deletions packages/react-core/src/components/HelperText/HelperTextItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
Comment on lines +9 to +14
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we use this same set of options in a good few places, I wonder if it would make sense for us to extract this kind of "status" enum/type into the constants file. Not a blocking thing, just a thought.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though how each component uses these variants can be slightly different. Here it's these 4 variants, Alert has danger | success | warning | info | custom, Progress only has dange | success | warning, etc. So it could also be a reason to make components follow a more universal variant naming when we can. At the very least, re-evaluating whether "error" should be used instead of danger in some places or vice versa, and whether ___Variant or ___Status makes more sense as an enum/prop name.


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> = ({
Expand All @@ -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>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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', () => {
Expand All @@ -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);
});
Original file line number Diff line number Diff line change
Expand Up @@ -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"
```
Original file line number Diff line number Diff line change
@@ -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>
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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>
);

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand Down Expand Up @@ -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>
);
}
Expand Down
Loading