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(Chip): Add the Chip component to PF4 #1076

Closed
wants to merge 1 commit into from
Closed
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
20 changes: 20 additions & 0 deletions packages/patternfly-4/react-core/src/components/Chip/Chip.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { SFC, HTMLProps } from 'react';
import { Omit, OneOf } from '../../typeUtils';
import { TooltipPosition } from '../Tooltip';

export const ChipVariant = {
overflow: 'overflow',
closable: 'closable'
};

export interface ChipProps extends Omit<HTMLProps<HTMLDivElement>, 'children' > {
children: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

This file should be updated as well. It is missing id and onClick and isOverflowed is no longer a prop.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you get rid of the Omit 'children' and remove children from the interface? Also don't need id and onClick I believe since they are included in HTMLProps

isOverflowed?: boolean;
variant: OneOf<typeof ChipVariant, keyof typeof ChipVariant>;
position: OneOf<typeof TooltipPosition, keyof typeof TooltipPosition>;
}

declare const Chip: SFC<ChipProps>;

export default Chip;

15 changes: 15 additions & 0 deletions packages/patternfly-4/react-core/src/components/Chip/Chip.docs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { Chip } from '@patternfly/react-core';
import SimpleExample from './examples/SimpleChip';

export default {
title: 'Chip',
components: {
Chip,
},
examples: [
{
component: SimpleExample,
title: 'Simple Example'
}
]
};
69 changes: 69 additions & 0 deletions packages/patternfly-4/react-core/src/components/Chip/Chip.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import React from 'react';
import { css } from '@patternfly/react-styles';
import PropTypes from 'prop-types';
import ChipButton from './ChipButton';
import { Tooltip, TooltipPosition } from '../Tooltip';
import { TimesCircleIcon } from '@patternfly/react-icons';
import styles from '@patternfly/patternfly-next/components/Chip/chip.css';
import { getUniqueId } from '../../internal/util';

export const ChipVariant = {
overflow: 'overflow',
closable: 'closable'
};

const Chip = ({ variant, onClick, children, id, position, className, ...props }) => {
const idChip = id || getUniqueId()
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not work if there are multiple chips on the page. They will all have the same id. You can wrap it with the GenerateId component to get a unique id.

switch (variant) {
case ChipVariant.overflow:
return (
<div className={css(styles.chip, styles.modifiers.overflow, className)} {...props}>
<ChipButton onClick={onClick} aria-label="Expand chip">
Copy link
Contributor

Choose a reason for hiding this comment

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

aria-label should be removed here, to match core. It is not needed since the button has text.

<span className={css(styles.chipText)}>{children}</span>
</ChipButton>
</div>
);
default:
const ChipComponent = (
<div className={css(styles.chip, className)} {...props}>
<span className={css(styles.chipText)} id={idChip}>
{children}
</span>
<ChipButton onClick={onClick} aria-label="Remove" id={`remove_${idChip}`} aria-labelledby={`remove_${idChip} ${idChip}`}>
Copy link
Contributor

Choose a reason for hiding this comment

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

For localization purposes, all user visible/audible text should be customizable. Maybe we can add an optional prop for the button's aria Label.

<TimesCircleIcon aria-hidden="true" />
</ChipButton>
</div>
);
return children.length < 16 ? (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Curious why 16? Should this be customizable?

ChipComponent
) : (
<Tooltip position={position} content={children}>
{ChipComponent}
</Tooltip>
);
}
};

Chip.propTypes = {
/** Content rendered inside the chip text */
children: PropTypes.string.isRequired,
/** ID of the chip */
id: PropTypes.string,
/** Additional classes added to the chip item */
className: PropTypes.string,
/** Builds the chip structure according to variant */
variant: PropTypes.oneOf(Object.values(ChipVariant)),
/** Position of the tooltip which is displayed if text is longer */
position: PropTypes.oneOf(Object.values(TooltipPosition)),
/** Function that is called when clicking on the chip button */
onClick: PropTypes.func
};

Chip.defaultProps = {
id: undefined,
className: '',
position: 'top',
variant: ChipVariant.closable
};

export default Chip;
54 changes: 54 additions & 0 deletions packages/patternfly-4/react-core/src/components/Chip/Chip.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import React from 'react';
import { shallow } from 'enzyme';
import ChipButton from './ChipButton';
import Chip from './Chip';

test('ChipButton', () => {
const view = shallow(
<ChipButton id="my-chip-button" className="chip-bttn-cls">
<b>Close</b>
</ChipButton>
);
expect(view).toMatchSnapshot();
});

describe('Chip', () => {
test('overflow', () => {
const view = shallow(
<Chip className="my-chp-cls" variant="overflow">
4 more
</Chip>
);
expect(view).toMatchSnapshot();
});

test('closable', () => {
const view = shallow(
<Chip className="my-chp-cls" variant="closable" id="chip_one">
Chip
</Chip>
);
expect(view).toMatchSnapshot();
});


test('closable with tooltip', () => {
const view = shallow(
<Chip className="my-chp-cls" variant="closable" id="chip_one">
12345678901234567891
</Chip>
);
expect(view).toMatchSnapshot();
});

test('onClick', () => {
const clickFnc = jest.fn();
const view = shallow(
<Chip onClick={() => clickFnc('chip_one')} id="chip_one">
Chip
</Chip>
);
view.find(ChipButton).simulate('click');
expect(clickFnc).toBeCalledWith('chip_one');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { SFC, HTMLProps } from 'react';

export interface ChipButtonProps extends HTMLProps<HTMLButtonElement> {}

declare const ChipButton: SFC<ChipButtonProps>;

export default ChipButton;
30 changes: 30 additions & 0 deletions packages/patternfly-4/react-core/src/components/Chip/ChipButton.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import React from 'react';
import { css } from '@patternfly/react-styles';
import PropTypes from 'prop-types';
import styles from '@patternfly/patternfly-next/components/Chip/chip.css';
import { Button } from '../Button';

const ChipButton = ({ children, className, onClick, ...props }) => {
return (
<Button variant="plain" onClick={onClick} className={className} {...props}>
{children}
</Button>
);
};

ChipButton.propTypes = {
/** Content rendered inside the chip item */
children: PropTypes.node,
/** Additional classes added to the chip item */
className: PropTypes.string,
/** Function that is called when clicking on the chip button */
onClick: PropTypes.func,
};

ChipButton.defaultProps = {
children: null,
className: '',
onClick: () => {}
};

export default ChipButton;
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Chip closable 1`] = `
.pf-c-chip__text {
display: block;
max-width: 7.5rem;
overflow: hidden;
font-size: 0.75rem;
color: #282d33;
text-overflow: ellipsis;
white-space: nowrap;
}
.pf-c-chip.my-chp-cls {
display: inline-flex;
position: relative;
align-items: center;
padding-left: 0.5rem;
background-color: #ffffff;
border-radius: 3px;
}

<div
className="pf-c-chip my-chp-cls"
>
<span
className="pf-c-chip__text"
id="chip_one"
>
Chip
</span>
<ChipButton
aria-label="Remove"
aria-labelledby="remove_chip_one chip_one"
className=""
id="remove_chip_one"
onClick={[Function]}
>
<TimesCircleIcon
aria-hidden="true"
color="currentColor"
size="sm"
title={null}
/>
</ChipButton>
</div>
`;

exports[`Chip closable with tooltip 1`] = `
.pf-c-chip__text {
display: block;
max-width: 7.5rem;
overflow: hidden;
font-size: 0.75rem;
color: #282d33;
text-overflow: ellipsis;
white-space: nowrap;
}
.pf-c-chip.my-chp-cls {
display: inline-flex;
position: relative;
align-items: center;
padding-left: 0.5rem;
background-color: #ffffff;
border-radius: 3px;
}

<Tooltip
appendTo={[Function]}
className={null}
content="12345678901234567891"
enableFlip={true}
position="top"
size="small"
zIndex={9999}
>
<div
className="pf-c-chip my-chp-cls"
>
<span
className="pf-c-chip__text"
id="chip_one"
>
12345678901234567891
</span>
<ChipButton
aria-label="Remove"
aria-labelledby="remove_chip_one chip_one"
className=""
id="remove_chip_one"
onClick={[Function]}
>
<TimesCircleIcon
aria-hidden="true"
color="currentColor"
size="sm"
title={null}
/>
</ChipButton>
</div>
</Tooltip>
`;

exports[`Chip overflow 1`] = `
.pf-c-chip__text {
display: block;
max-width: 7.5rem;
overflow: hidden;
font-size: 0.75rem;
color: #282d33;
text-overflow: ellipsis;
white-space: nowrap;
}
.pf-c-chip.pf-m-overflow.my-chp-cls {
display: inline-flex;
position: relative;
align-items: center;
padding-left: 0.5rem;
background-color: #ffffff;
border-radius: 3px;
}

<div
className="pf-c-chip pf-m-overflow my-chp-cls"
>
<ChipButton
aria-label="Expand chip"
className=""
onClick={[Function]}
>
<span
className="pf-c-chip__text"
>
4 more
</span>
</ChipButton>
</div>
`;

exports[`ChipButton 1`] = `
<Button
aria-label={null}
className="chip-bttn-cls"
component="button"
id="my-chip-button"
isActive={false}
isBlock={false}
isDisabled={false}
isFocus={false}
isHover={false}
onClick={[Function]}
type="button"
variant="plain"
>
<b>
Close
</b>
</Button>
`;
Loading