Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Icon): add new icon sizes for penta #10030

Merged
merged 2 commits into from
Jan 31, 2024
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
4 changes: 3 additions & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ module.exports = {
'^.+\\.svg$': 'jest-transform-stub'
},
setupFilesAfterEnv: ['<rootDir>/packages/testSetup.ts'],
transformIgnorePatterns: ['node_modules/(?!@patternfly|@novnc|@popperjs|lodash|monaco-editor|react-monaco-editor)'],
transformIgnorePatterns: [
'node_modules/(?!@patternfly|@novnc|@popperjs|lodash|monaco-editor|react-monaco-editor|case-anything)'
],
testPathIgnorePatterns: ['<rootDir>/packages/react-integration/'],
coveragePathIgnorePatterns: ['/dist/'],
moduleNameMapper: {
Expand Down
3 changes: 2 additions & 1 deletion packages/react-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@
"rollup-plugin-scss": "^4.0.0",
"rollup-plugin-svg": "2.0.0",
"rollup-plugin-terser": "^7.0.2",
"typescript": "^4.7.4"
"typescript": "^4.7.4",
"case-anything": "^2.1.13"
},
"peerDependencies": {
"react": "^17 || ^18",
Expand Down
23 changes: 20 additions & 3 deletions packages/react-core/src/components/Icon/Icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,23 @@ import styles from '@patternfly/react-styles/css/components/Icon/icon';
import { css } from '@patternfly/react-styles';
import { Spinner } from '../Spinner';

export type IconSize =
| 'sm'
| 'md'
| 'lg'
| 'xl'
| '2xl'
| '3xl'
| 'headingSm'
| 'headingMd'
| 'headingLg'
| 'headingXl'
| 'heading_2xl'
| 'heading_3xl'
| 'bodySm'
| 'bodyDefault'
| 'bodyLg';

export interface IconComponentProps extends Omit<React.HTMLProps<HTMLSpanElement>, 'size'> {
/** Icon content */
children?: React.ReactNode;
Expand All @@ -11,11 +28,11 @@ export interface IconComponentProps extends Omit<React.HTMLProps<HTMLSpanElement
/** Additional classes applied to the icon container */
className?: string;
/** Size of the icon component container and icon. */
size?: 'sm' | 'md' | 'lg' | 'xl';
size?: IconSize;
gitdallas marked this conversation as resolved.
Show resolved Hide resolved
/** Size of icon. Overrides the icon size set by the size property. */
iconSize?: 'sm' | 'md' | 'lg' | 'xl';
iconSize?: IconSize;
/** Size of progress icon. Overrides the icon size set by the size property. */
progressIconSize?: 'sm' | 'md' | 'lg' | 'xl';
progressIconSize?: IconSize;
/** Status color of the icon */
status?: 'custom' | 'info' | 'success' | 'warning' | 'danger';
/** Indicates the icon is inline and should inherit the text font size and color. Overriden by size and iconSize properties. */
Expand Down
79 changes: 49 additions & 30 deletions packages/react-core/src/components/Icon/__tests__/Icon.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react';
import { render, screen } from '@testing-library/react';
import { Icon } from '../Icon';
import { kebabCase } from 'case-anything';
import { Icon, IconSize } from '../Icon';
import CheckIcon from '@patternfly/react-icons/dist/esm/icons/check-icon';
import styles from '@patternfly/react-styles/css/components/Icon/icon';

Expand Down Expand Up @@ -41,16 +42,60 @@ test('sets additional custom class successfully', () => {
expect(iconContainer).toHaveClass('test');
});

Object.values(['sm', 'md', 'lg', 'xl']).forEach((size) => {
Object.values([
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: could we instead combine the 3 instances of this forEach into a single one, then just run all the tests that need to in here

'sm',
'md',
'lg',
'xl',
'2xl',
'3xl',
'headingSm',
'headingMd',
'headingLg',
'headingXl',
'heading_2xl',
'heading_3xl',
'bodySm',
'bodyDefault',
'bodyLg'
]).forEach((size) => {
test(`sets icon size modifier successfully - ${size}`, () => {
render(
<Icon iconSize={size as 'sm' | 'md' | 'lg' | 'xl'} title={`content-${size}-icon`}>
<Icon iconSize={size as IconSize} title={`content-${size}-icon`}>
<CheckIcon />
</Icon>
);
const iconContainer = screen.getByTitle(`content-${size}-icon`).querySelector(`.${styles.iconContent}`);

expect(iconContainer).toHaveClass(`pf-m-${size}`);
const formattedSize = kebabCase(size).replace(/(\d)(-)/, '$1');

expect(iconContainer).toHaveClass(`pf-m-${formattedSize}`);
});

test(`sets progress icon size modifier successfully - ${size}`, () => {
render(
<Icon isInProgress progressIconSize={size as IconSize} title={`progress-content-${size}-icon`}>
<CheckIcon />
</Icon>
);
const iconContainer = screen.getByTitle(`progress-content-${size}-icon`).querySelector(`.${styles.iconProgress}`);

const formattedSize = kebabCase(size).replace(/(\d)(-)/, '$1');

expect(iconContainer).toHaveClass(`pf-m-${formattedSize}`);
});

test(`sets size modifier successfully - ${size}`, () => {
render(
<Icon size={size as IconSize} title={`${size}-icon`}>
<CheckIcon />
</Icon>
);
const iconContainer = screen.getByTitle(`${size}-icon`);

const formattedSize = kebabCase(size).replace(/(\d)(-)/, '$1');

expect(iconContainer).toHaveClass(`pf-m-${formattedSize}`);
});
});

Expand All @@ -64,19 +109,6 @@ test('check icon without iconSize', () => {
expect(Array.from(iconContainer?.classList || []).some((c) => /pf-m-*/.test(c))); // Check no modifier classes have been added
});

Object.values(['sm', 'md', 'lg', 'xl']).forEach((size) => {
test(`sets progress icon size modifier successfully - ${size}`, () => {
render(
<Icon isInProgress progressIconSize={size as 'sm' | 'md' | 'lg' | 'xl'} title={`progress-content-${size}-icon`}>
<CheckIcon />
</Icon>
);
const iconContainer = screen.getByTitle(`progress-content-${size}-icon`).querySelector(`.${styles.iconProgress}`);

expect(iconContainer).toHaveClass(`pf-m-${size}`);
});
});

test('check icon without progress icon size', () => {
render(
<Icon title="no-progress-icon-size">
Expand All @@ -87,19 +119,6 @@ test('check icon without progress icon size', () => {
expect(Array.from(iconContainer?.classList || []).some((c) => /pf-m-*/.test(c))); // Check no modifier classes have been added
});

Object.values(['sm', 'md', 'lg', 'xl']).forEach((size) => {
test(`sets size modifier successfully - ${size}`, () => {
render(
<Icon size={size as 'sm' | 'md' | 'lg' | 'xl'} title={`${size}-icon`}>
<CheckIcon />
</Icon>
);
const iconContainer = screen.getByTitle(`${size}-icon`);

expect(iconContainer).toHaveClass(`pf-m-${size}`);
});
});

test('check icon without size', () => {
render(
<Icon title="no-size">
Expand Down
17 changes: 17 additions & 0 deletions packages/react-core/src/components/Icon/examples/BodyIconSizes.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import React from 'react';
import { Icon } from '@patternfly/react-core';
import PlusCircleIcon from '@patternfly/react-icons/dist/esm/icons/plus-circle-icon';

export const BodyIconSizes: React.FunctionComponent = () => (
<React.Fragment>
<Icon size="bodySm">
<PlusCircleIcon />
</Icon>
<Icon size="bodyDefault">
<PlusCircleIcon />
</Icon>
<Icon size="bodyLg">
<PlusCircleIcon />
</Icon>
</React.Fragment>
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import React from 'react';
import { Icon } from '@patternfly/react-core';
import PlusCircleIcon from '@patternfly/react-icons/dist/esm/icons/plus-circle-icon';

export const HeadingIconSizes: React.FunctionComponent = () => (
<React.Fragment>
<Icon size="headingSm">
<PlusCircleIcon />
</Icon>
<Icon size="headingMd">
<PlusCircleIcon />
</Icon>
<Icon size="headingLg">
<PlusCircleIcon />
</Icon>
<Icon size="headingXl">
<PlusCircleIcon />
</Icon>
<Icon size="heading_2xl">
<PlusCircleIcon />
</Icon>
<Icon size="heading_3xl">
<PlusCircleIcon />
</Icon>
</React.Fragment>
);
20 changes: 18 additions & 2 deletions packages/react-core/src/components/Icon/examples/Icon.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,25 @@ import BellIcon from '@patternfly/react-icons/dist/esm/icons/bell-icon';
```ts file="IconBasic.tsx"
```

### Sizes
### Standalone icon sizes

```ts file="IconSizes.tsx"
These are the standard options for sizing icons.

```ts file="StandaloneIconSizes.tsx"
```

### Body sizes

These size options are meant to make icons match the size of body text.

```ts file="BodyIconSizes.tsx"
```

### Heading sizes

These size options are meant to make icons match the size of heading text.

```ts file="HeadingIconSizes.tsx"
```

### Status colors
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ import PlusCircleIcon from '@patternfly/react-icons/dist/esm/icons/plus-circle-i

export const IconContentSizes: React.FunctionComponent = () => (
<React.Fragment>
<Icon size="xl" iconSize="sm">
<Icon size="3xl" iconSize="lg">
<PlusCircleIcon />
</Icon>
<Icon size="xl" iconSize="md">
<Icon size="3xl" iconSize="xl">
<PlusCircleIcon />
</Icon>
<Icon size="xl" iconSize="lg">
<Icon size="3xl" iconSize="2xl">
<PlusCircleIcon />
</Icon>
<Icon size="xl">
<Icon size="3xl">
<PlusCircleIcon />
</Icon>
</React.Fragment>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import { Icon } from '@patternfly/react-core';
import PlusCircleIcon from '@patternfly/react-icons/dist/esm/icons/plus-circle-icon';

export const IconSizes: React.FunctionComponent = () => (
export const StandaloneIconSizes: React.FunctionComponent = () => (
<React.Fragment>
<Icon size="sm">
<PlusCircleIcon />
Expand All @@ -16,5 +16,11 @@ export const IconSizes: React.FunctionComponent = () => (
<Icon size="xl">
<PlusCircleIcon />
</Icon>
<Icon size="2xl">
<PlusCircleIcon />
</Icon>
<Icon size="3xl">
<PlusCircleIcon />
</Icon>
</React.Fragment>
);
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6851,6 +6851,11 @@ capture-stack-trace@^1.0.0:
version "1.0.1"
resolved "https://registry.npmjs.org/capture-stack-trace/-/capture-stack-trace-1.0.1.tgz"

case-anything@^2.1.13:
version "2.1.13"
resolved "https://registry.yarnpkg.com/case-anything/-/case-anything-2.1.13.tgz#0cdc16278cb29a7fcdeb072400da3f342ba329e9"
integrity sha512-zlOQ80VrQ2Ue+ymH5OuM/DlDq64mEm+B9UTdHULv5osUMD6HalNTblf2b1u/m6QecjsnOkBpqVZ+XPwIVsy7Ng==

caseless@~0.12.0:
version "0.12.0"
resolved "https://registry.npmjs.org/caseless/-/caseless-0.12.0.tgz"
Expand Down
Loading