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(Label) add option to make label clickable #9284

Merged
merged 5 commits into from
Sep 21, 2023
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
32 changes: 29 additions & 3 deletions packages/react-core/src/components/Label/Label.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +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. */
isOverflowLabel?: boolean;
/** Callback for when the label is clicked. This should not be passed in if the href or isEditable 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 @@ -94,6 +96,7 @@ export const Label: React.FunctionComponent<LabelProps> = ({
tooltipPosition,
icon,
onClose,
onClick: onLabelClick,
onEditCancel,
onEditComplete,
closeBtn,
Expand All @@ -118,6 +121,20 @@ export const Label: React.FunctionComponent<LabelProps> = ({
};
});

React.useEffect(() => {
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, href, isEditable]);

const onDocMouseDown = (event: MouseEvent) => {
if (
isEditableActive &&
Expand Down Expand Up @@ -235,14 +252,22 @@ export const Label: React.FunctionComponent<LabelProps> = ({
let LabelComponentChildElement = 'span';
if (href) {
LabelComponentChildElement = 'a';
} else if (isEditable) {
} 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 Down Expand Up @@ -289,6 +314,7 @@ export const Label: React.FunctionComponent<LabelProps> = ({
isEditableActive && styles.modifiers.editableActive,
className
)}
onClick={isOverflowLabel ? onLabelClick : undefined}
>
{!isEditableActive && labelComponentChild}
{!isEditableActive && onClose && button}
Expand Down
35 changes: 35 additions & 0 deletions packages/react-core/src/components/Label/__tests__/Label.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,4 +109,39 @@ describe('Label', () => {
expect(screen.queryByRole('button', { name: 'Something' })).toBeNull();
expect(asFragment()).toMatchSnapshot();
});

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 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 onClick={mockCallback}>Click me</Label>);

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

await user.click(label);

expect(mockCallback).toBeCalledTimes(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ exports[`Label editable label 1`] = `
>
<button
class="pf-v5-c-label__content"
type="button"
>
<span
class="pf-v5-c-label__text"
Expand Down
Loading