From 0f8ac62a98bc45a1e9def92b09eeb9f10c186fc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Pet=C5=99=C3=ADk?= <77832970+Dominik-Petrik@users.noreply.github.com> Date: Thu, 21 Sep 2023 21:11:54 +0200 Subject: [PATCH] feat(Label) add option to make label clickable (#9284) * feat(Label) add option to make label clickable * add type to label button * chore(Label): Address misc PR feedback * chore(Label): Remove mention of onClick being incompatible with overflow * chore(Label): Remove prop from effect deps where it's no longer needed --------- Co-authored-by: Austin Sullivan --- .../react-core/src/components/Label/Label.tsx | 32 +- .../components/Label/__tests__/Label.test.tsx | 35 ++ .../__snapshots__/Label.test.tsx.snap | 1 + .../components/Label/examples/LabelFilled.tsx | 388 ++++++++------ .../Label/examples/LabelOutline.tsx | 506 ++++++++++-------- 5 files changed, 565 insertions(+), 397 deletions(-) diff --git a/packages/react-core/src/components/Label/Label.tsx b/packages/react-core/src/components/Label/Label.tsx index 7f15a1a56ac..6fa01a5b447 100644 --- a/packages/react-core/src/components/Label/Label.tsx +++ b/packages/react-core/src/components/Label/Label.tsx @@ -55,10 +55,12 @@ export interface LabelProps extends React.HTMLProps { 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, @@ -94,6 +96,7 @@ export const Label: React.FunctionComponent = ({ tooltipPosition, icon, onClose, + onClick: onLabelClick, onEditCancel, onEditComplete, closeBtn, @@ -118,6 +121,20 @@ export const Label: React.FunctionComponent = ({ }; }); + 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 && @@ -235,14 +252,22 @@ export const Label: React.FunctionComponent = ({ 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) => { @@ -289,6 +314,7 @@ export const Label: React.FunctionComponent = ({ isEditableActive && styles.modifiers.editableActive, className )} + onClick={isOverflowLabel ? onLabelClick : undefined} > {!isEditableActive && labelComponentChild} {!isEditableActive && onClose && button} diff --git a/packages/react-core/src/components/Label/__tests__/Label.test.tsx b/packages/react-core/src/components/Label/__tests__/Label.test.tsx index be3c3605986..519d99969cd 100644 --- a/packages/react-core/src/components/Label/__tests__/Label.test.tsx +++ b/packages/react-core/src/components/Label/__tests__/Label.test.tsx @@ -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(); + + expect(screen.queryByRole(`button`)).not.toBeInTheDocument(); + }); + + test('a button is rendered when onClick is passed', () => { + const fn = jest.fn(); + + render(); + + 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(); + + expect(mockCallback).not.toHaveBeenCalled(); + }); + + test('clickable label calls passed callback on click', async () => { + const mockCallback = jest.fn(); + const user = userEvent.setup(); + + render(); + + const label = screen.getByRole('button'); + + await user.click(label); + + expect(mockCallback).toBeCalledTimes(1); + }); }); diff --git a/packages/react-core/src/components/Label/__tests__/__snapshots__/Label.test.tsx.snap b/packages/react-core/src/components/Label/__tests__/__snapshots__/Label.test.tsx.snap index f876111ced5..c8c126cb8d6 100644 --- a/packages/react-core/src/components/Label/__tests__/__snapshots__/Label.test.tsx.snap +++ b/packages/react-core/src/components/Label/__tests__/__snapshots__/Label.test.tsx.snap @@ -7,6 +7,7 @@ exports[`Label editable label 1`] = ` >