Skip to content

Commit

Permalink
chore(Label): Address misc PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
wise-king-sullyman committed Sep 21, 2023
1 parent 6e01015 commit bdc4589
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 58 deletions.
48 changes: 35 additions & 13 deletions packages/react-core/src/components/Label/Label.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ export interface LabelProps extends React.HTMLProps<HTMLSpanElement> {
closeBtnAriaLabel?: string;
/** Additional properties for the default close button. */
closeBtnProps?: any;
/** Href for a label that is a link. If present, the label will change to an anchor element. */
/** Href for a label that is a link. If present, the label will change to an anchor element. This should not be passed in if the onClick prop is also passed in. */
href?: string;
/** Flag indicating if the label is an overflow label */
/** Flag indicating if the label is an overflow label. This should not be passed in if the onClick prop is also passed in. */
isOverflowLabel?: boolean;
/** On click callback. If present, label will be clickable. */
onLabelClick?: (event: React.MouseEvent) => void;
/** Callback for when the label is clicked. This should not be passed in if the href or isOverflowLabel props are also passed in. */
onClick?: (event: React.MouseEvent) => void;
/** Forwards the label content and className to rendered function. Use this prop for react router support.*/
render?: ({
className,
Expand Down Expand Up @@ -96,7 +96,7 @@ export const Label: React.FunctionComponent<LabelProps> = ({
tooltipPosition,
icon,
onClose,
onLabelClick,
onClick: onLabelClick,
onEditCancel,
onEditComplete,
closeBtn,
Expand All @@ -121,6 +121,25 @@ export const Label: React.FunctionComponent<LabelProps> = ({
};
});

React.useEffect(() => {
if (onLabelClick && isOverflowLabel) {
// eslint-disable-next-line no-console
console.warn(
'Overflow labels cannot have onClick passed, this results in invalid HTML. Please remove either the isOverflowLabel or onClick prop.'
);
} else if (onLabelClick && href) {
// eslint-disable-next-line no-console
console.warn(
'Link labels cannot have onClick passed, this results in invalid HTML. Please remove either the href or onClick prop.'
);
} else if (onLabelClick && isEditable) {
// eslint-disable-next-line no-console
console.warn(
'Editable labels cannot have onClick passed, clicking starts the label edit process. Please remove either the isEditable or onClick prop.'
);
}
}, [onLabelClick, isOverflowLabel, href, isEditable]);

const onDocMouseDown = (event: MouseEvent) => {
if (
isEditableActive &&
Expand Down Expand Up @@ -238,14 +257,22 @@ export const Label: React.FunctionComponent<LabelProps> = ({
let LabelComponentChildElement = 'span';
if (href) {
LabelComponentChildElement = 'a';
} else if (isEditable || onLabelClick) {
} else if (isEditable || (onLabelClick && !isOverflowLabel)) {
LabelComponentChildElement = 'button';
}

const clickableLabelProps = {
type: 'button',
onClick: onLabelClick
};

const isButton = LabelComponentChildElement === 'button';

const labelComponentChildProps = {
className: css(styles.labelContent),
...(isTooltipVisible && { tabIndex: 0 }),
...(href && { href }),
...(isButton && clickableLabelProps),
...(isEditable && {
ref: editableButtonRef,
onClick: (e: React.MouseEvent) => {
Expand All @@ -257,13 +284,7 @@ export const Label: React.FunctionComponent<LabelProps> = ({
};

let labelComponentChild = (
<LabelComponentChildElement
type={LabelComponentChildElement === 'button' ? 'button' : undefined}
onClick={onLabelClick}
{...labelComponentChildProps}
>
{content}
</LabelComponentChildElement>
<LabelComponentChildElement {...labelComponentChildProps}>{content}</LabelComponentChildElement>
);

if (render) {
Expand Down Expand Up @@ -298,6 +319,7 @@ export const Label: React.FunctionComponent<LabelProps> = ({
isEditableActive && styles.modifiers.editableActive,
className
)}
onClick={isOverflowLabel ? onLabelClick : undefined}
>
{!isEditableActive && labelComponentChild}
{!isEditableActive && onClose && button}
Expand Down
20 changes: 17 additions & 3 deletions packages/react-core/src/components/Label/__tests__/Label.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,19 +110,33 @@ describe('Label', () => {
expect(asFragment()).toMatchSnapshot();
});

test('clickable label', () => {
test('a button is not rendered when onClick is not passed', () => {
render(<Label>Click me</Label>);

expect(screen.queryByRole(`button`)).not.toBeInTheDocument();
});

test('a button is rendered when onClick is passed', () => {
const fn = jest.fn();

render(<Label onLabelClick={fn}>Click me</Label>);
render(<Label onClick={fn}>Click me</Label>);

expect(screen.getByRole(`button`)).toBeVisible();
});

test('clickable label does not call the passed callback when it is not clicked', async () => {
const mockCallback = jest.fn();

render(<Label onClick={mockCallback}>Click me</Label>);

expect(mockCallback).not.toHaveBeenCalled();
});

test('clickable label calls passed callback on click', async () => {
const mockCallback = jest.fn();
const user = userEvent.setup();

render(<Label onLabelClick={mockCallback}>Click me</Label>);
render(<Label onClick={mockCallback}>Click me</Label>);

const label = screen.getByRole('button');

Expand Down
32 changes: 16 additions & 16 deletions packages/react-core/src/components/Label/examples/LabelFilled.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ export const LabelFilled: React.FunctionComponent = () => {
<Label href="#filled" onClose={() => Function.prototype}>
Grey link removable
</Label>
<Label onLabelClick={() => logColor('grey')}>Grey clickable</Label>{' '}
<Label onLabelClick={() => logColor('grey')} onClose={() => Function.prototype}>
<Label onClick={() => logColor('grey')}>Grey clickable</Label>{' '}
<Label onClick={() => logColor('grey')} onClose={() => Function.prototype}>
Grey clickable removable
</Label>
<Label icon={<InfoCircleIcon />} onClose={() => Function.prototype} textMaxWidth="16ch">
Expand All @@ -43,10 +43,10 @@ export const LabelFilled: React.FunctionComponent = () => {
<Label color="blue" href="#filled" onClose={() => Function.prototype}>
Blue link removable
</Label>
<Label color="blue" onLabelClick={() => logColor('blue')}>
<Label color="blue" onClick={() => logColor('blue')}>
Blue clickable
</Label>{' '}
<Label color="blue" onLabelClick={() => logColor('blue')} onClose={() => Function.prototype}>
<Label color="blue" onClick={() => logColor('blue')} onClose={() => Function.prototype}>
Blue clickable removable
</Label>
<Label color="blue" icon={<InfoCircleIcon />} onClose={() => Function.prototype} textMaxWidth="16ch">
Expand All @@ -70,10 +70,10 @@ export const LabelFilled: React.FunctionComponent = () => {
<Label color="green" href="#filled" onClose={() => Function.prototype}>
Green link removable
</Label>
<Label color="green" onLabelClick={() => logColor('green')}>
<Label color="green" onClick={() => logColor('green')}>
Green clickable
</Label>{' '}
<Label color="green" onLabelClick={() => logColor('green')} onClose={() => Function.prototype}>
<Label color="green" onClick={() => logColor('green')} onClose={() => Function.prototype}>
Green clickable removable
</Label>
<Label color="green" icon={<InfoCircleIcon />} onClose={() => Function.prototype} textMaxWidth="16ch">
Expand All @@ -97,10 +97,10 @@ export const LabelFilled: React.FunctionComponent = () => {
<Label color="orange" href="#filled" onClose={() => Function.prototype}>
Orange link removable
</Label>
<Label color="orange" onLabelClick={() => logColor('orange')}>
<Label color="orange" onClick={() => logColor('orange')}>
Orange clickable
</Label>{' '}
<Label color="orange" onLabelClick={() => logColor('orange')} onClose={() => Function.prototype}>
<Label color="orange" onClick={() => logColor('orange')} onClose={() => Function.prototype}>
Orange clickable removable
</Label>
<Label color="orange" icon={<InfoCircleIcon />} onClose={() => Function.prototype} textMaxWidth="16ch">
Expand All @@ -124,10 +124,10 @@ export const LabelFilled: React.FunctionComponent = () => {
<Label color="red" href="#filled" onClose={() => Function.prototype}>
Red link removable
</Label>
<Label color="red" onLabelClick={() => logColor('red')}>
<Label color="red" onClick={() => logColor('red')}>
Red clickable
</Label>{' '}
<Label color="red" onLabelClick={() => logColor('red')} onClose={() => Function.prototype}>
<Label color="red" onClick={() => logColor('red')} onClose={() => Function.prototype}>
Red clickable removable
</Label>
<Label color="red" icon={<InfoCircleIcon />} onClose={() => Function.prototype} textMaxWidth="16ch">
Expand All @@ -151,10 +151,10 @@ export const LabelFilled: React.FunctionComponent = () => {
<Label color="purple" href="#filled" onClose={() => Function.prototype}>
Purple link removable
</Label>
<Label color="purple" onLabelClick={() => logColor('purple')}>
<Label color="purple" onClick={() => logColor('purple')}>
Purple clickable
</Label>{' '}
<Label color="purple" onLabelClick={() => logColor('purple')} onClose={() => Function.prototype}>
<Label color="purple" onClick={() => logColor('purple')} onClose={() => Function.prototype}>
Purple clickable removable
</Label>
<Label color="purple" icon={<InfoCircleIcon />} onClose={() => Function.prototype} textMaxWidth="16ch">
Expand All @@ -178,10 +178,10 @@ export const LabelFilled: React.FunctionComponent = () => {
<Label color="cyan" href="#filled" onClose={() => Function.prototype}>
Cyan link removable
</Label>
<Label color="cyan" onLabelClick={() => logColor('cyan')}>
<Label color="cyan" onClick={() => logColor('cyan')}>
Cyan clickable
</Label>{' '}
<Label color="cyan" onLabelClick={() => logColor('cyan')} onClose={() => Function.prototype}>
<Label color="cyan" onClick={() => logColor('cyan')} onClose={() => Function.prototype}>
Cyan clickable removable
</Label>
<Label color="cyan" icon={<InfoCircleIcon />} onClose={() => Function.prototype} textMaxWidth="16ch">
Expand All @@ -205,10 +205,10 @@ export const LabelFilled: React.FunctionComponent = () => {
<Label color="gold" href="#filled" onClose={() => Function.prototype}>
Gold link removable
</Label>
<Label color="gold" onLabelClick={() => logColor('gold')}>
<Label color="gold" onClick={() => logColor('gold')}>
Gold clickable
</Label>{' '}
<Label color="gold" onLabelClick={() => logColor('gold')} onClose={() => Function.prototype}>
<Label color="gold" onClick={() => logColor('gold')} onClose={() => Function.prototype}>
Gold clickable removable
</Label>
<Label color="gold" icon={<InfoCircleIcon />} onClose={() => Function.prototype} textMaxWidth="16ch">
Expand Down
42 changes: 16 additions & 26 deletions packages/react-core/src/components/Label/examples/LabelOutline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ export const LabelOutline: React.FunctionComponent = () => {
<Label variant="outline" href="#outline" onClose={() => Function.prototype}>
Grey link removable
</Label>
<Label variant="outline" onLabelClick={() => logColor('grey')}>
<Label variant="outline" onClick={() => logColor('grey')}>
Grey clickable
</Label>{' '}
<Label variant="outline" onLabelClick={() => logColor('grey')} onClose={() => Function.prototype}>
<Label variant="outline" onClick={() => logColor('grey')} onClose={() => Function.prototype}>
Grey clickable removable
</Label>
<Label variant="outline" icon={<InfoCircleIcon />} onClose={() => Function.prototype} textMaxWidth="16ch">
Expand All @@ -54,10 +54,10 @@ export const LabelOutline: React.FunctionComponent = () => {
<Label variant="outline" color="blue" href="#outline" onClose={() => Function.prototype}>
Blue link removable
</Label>
<Label variant="outline" color="blue" onLabelClick={() => logColor('blue')}>
<Label variant="outline" color="blue" onClick={() => logColor('blue')}>
Blue clickable
</Label>{' '}
<Label variant="outline" color="blue" onLabelClick={() => logColor('blue')} onClose={() => Function.prototype}>
<Label variant="outline" color="blue" onClick={() => logColor('blue')} onClose={() => Function.prototype}>
Blue clickable removable
</Label>
<Label
Expand Down Expand Up @@ -89,10 +89,10 @@ export const LabelOutline: React.FunctionComponent = () => {
<Label variant="outline" color="green" href="#outline" onClose={() => Function.prototype}>
Green link removable
</Label>
<Label variant="outline" color="green" onLabelClick={() => logColor('green')}>
<Label variant="outline" color="green" onClick={() => logColor('green')}>
Green clickable
</Label>{' '}
<Label variant="outline" color="green" onLabelClick={() => logColor('green')} onClose={() => Function.prototype}>
<Label variant="outline" color="green" onClick={() => logColor('green')} onClose={() => Function.prototype}>
Green clickable removable
</Label>
<Label
Expand Down Expand Up @@ -124,15 +124,10 @@ export const LabelOutline: React.FunctionComponent = () => {
<Label variant="outline" color="orange" href="#outline" onClose={() => Function.prototype}>
Orange link removable
</Label>
<Label variant="outline" color="orange" onLabelClick={() => logColor('orange')}>
<Label variant="outline" color="orange" onClick={() => logColor('orange')}>
Orange clickable
</Label>{' '}
<Label
variant="outline"
color="orange"
onLabelClick={() => logColor('orange')}
onClose={() => Function.prototype}
>
<Label variant="outline" color="orange" onClick={() => logColor('orange')} onClose={() => Function.prototype}>
Orange clickable removable
</Label>
<Label
Expand Down Expand Up @@ -164,10 +159,10 @@ export const LabelOutline: React.FunctionComponent = () => {
<Label variant="outline" color="red" href="#outline" onClose={() => Function.prototype}>
Red link removable
</Label>
<Label variant="outline" color="red" onLabelClick={() => logColor('red')}>
<Label variant="outline" color="red" onClick={() => logColor('red')}>
Red clickable
</Label>{' '}
<Label variant="outline" color="red" onLabelClick={() => logColor('red')} onClose={() => Function.prototype}>
<Label variant="outline" color="red" onClick={() => logColor('red')} onClose={() => Function.prototype}>
Red clickable removable
</Label>
<Label
Expand Down Expand Up @@ -199,15 +194,10 @@ export const LabelOutline: React.FunctionComponent = () => {
<Label variant="outline" color="purple" href="#outline" onClose={() => Function.prototype}>
Purple link removable
</Label>
<Label variant="outline" color="purple" onLabelClick={() => logColor('purple')}>
<Label variant="outline" color="purple" onClick={() => logColor('purple')}>
Purple clickable
</Label>{' '}
<Label
variant="outline"
color="purple"
onLabelClick={() => logColor('purple')}
onClose={() => Function.prototype}
>
<Label variant="outline" color="purple" onClick={() => logColor('purple')} onClose={() => Function.prototype}>
Purple clickable removable
</Label>
<Label
Expand Down Expand Up @@ -239,10 +229,10 @@ export const LabelOutline: React.FunctionComponent = () => {
<Label variant="outline" color="cyan" href="#outline" onClose={() => Function.prototype}>
Cyan link removable
</Label>
<Label variant="outline" color="cyan" onLabelClick={() => logColor('cyan')}>
<Label variant="outline" color="cyan" onClick={() => logColor('cyan')}>
Cyan clickable
</Label>{' '}
<Label variant="outline" color="cyan" onLabelClick={() => logColor('cyan')} onClose={() => Function.prototype}>
<Label variant="outline" color="cyan" onClick={() => logColor('cyan')} onClose={() => Function.prototype}>
Cyan clickable removable
</Label>
<Label
Expand Down Expand Up @@ -274,10 +264,10 @@ export const LabelOutline: React.FunctionComponent = () => {
<Label variant="outline" color="gold" href="#outline" onClose={() => Function.prototype}>
Gold link removable
</Label>
<Label variant="outline" color="gold" onLabelClick={() => logColor('gold')}>
<Label variant="outline" color="gold" onClick={() => logColor('gold')}>
Gold clickable
</Label>{' '}
<Label variant="outline" color="gold" onLabelClick={() => logColor('gold')} onClose={() => Function.prototype}>
<Label variant="outline" color="gold" onClick={() => logColor('gold')} onClose={() => Function.prototype}>
Gold clickable removable
</Label>
<Label
Expand Down

0 comments on commit bdc4589

Please sign in to comment.