From df8c92a485769e8fac025837565356c57d159a1b Mon Sep 17 00:00:00 2001 From: Kilian Collender <37899503+Kilian-Collender@users.noreply.github.com> Date: Fri, 24 May 2024 17:25:26 +0100 Subject: [PATCH] fix(tag): use refs to handle component access (#16571) * fix(tag): use refs to handle component access When using the querySelector it is easily broken if the id has reserved characters. Also the isEllipsisActive helper had no protection for a non element. * fix(tag): cast BaseComponent type * fix(tag): improve deprecation notices * fix: added forwardRef to Tag to grab ref in variants * fix: removed console log * fix: fixed spelling and remove ref from old filter * fix: updated snapshots * fix: fixed onMouseEnter error on console * fix: fixed TS error --------- Co-authored-by: Taylor Jones Co-authored-by: guidari --- .../__snapshots__/PublicAPI-test.js.snap | 2 + .../src/components/Tag/DismissibleTag.tsx | 19 ++--- .../src/components/Tag/OperationalTag.tsx | 23 +++--- .../src/components/Tag/SelectableTag.tsx | 24 +++--- packages/react/src/components/Tag/Tag.tsx | 74 ++++++++++--------- .../src/components/Tag/isEllipsisActive.ts | 13 ++++ .../react/src/components/Tooltip/Tooltip.tsx | 2 +- 7 files changed, 87 insertions(+), 70 deletions(-) create mode 100644 packages/react/src/components/Tag/isEllipsisActive.ts diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index 8b31a14bb4dc..f9e65cf49374 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -8240,6 +8240,7 @@ Map { }, }, "Tag" => Object { + "$$typeof": Symbol(react.forward_ref), "propTypes": Object { "as": Object { "type": "elementType", @@ -8305,6 +8306,7 @@ Map { "type": "oneOf", }, }, + "render": [Function], }, "TagSkeleton" => Object { "propTypes": Object { diff --git a/packages/react/src/components/Tag/DismissibleTag.tsx b/packages/react/src/components/Tag/DismissibleTag.tsx index b9223c6bdb5c..541602ea35ce 100644 --- a/packages/react/src/components/Tag/DismissibleTag.tsx +++ b/packages/react/src/components/Tag/DismissibleTag.tsx @@ -6,7 +6,7 @@ */ import PropTypes from 'prop-types'; -import React, { useLayoutEffect, useState, ReactNode } from 'react'; +import React, { useLayoutEffect, useState, ReactNode, useRef } from 'react'; import classNames from 'classnames'; import setupGetInstanceId from '../../tools/setupGetInstanceId'; import { usePrefix } from '../../internal/usePrefix'; @@ -15,6 +15,7 @@ import Tag, { SIZES, TYPES } from './Tag'; import { Close } from '@carbon/icons-react'; import { Tooltip } from '../Tooltip'; import { Text } from '../Text'; +import { isEllipsisActive } from './isEllipsisActive'; const getInstanceId = setupGetInstanceId(); @@ -91,22 +92,17 @@ const DismissibleTag = ({ ...other }: DismissibleTagProps) => { const prefix = usePrefix(); + const tagLabelRef = useRef(); const tagId = id || `tag-${getInstanceId()}`; const tagClasses = classNames(`${prefix}--tag--filter`, className); const [isEllipsisApplied, setIsEllipsisApplied] = useState(false); - const isEllipsisActive = (element: any) => { - setIsEllipsisApplied(element.offsetWidth < element.scrollWidth); - return element.offsetWidth < element.scrollWidth; - }; - useLayoutEffect(() => { - const elementTagId = document.querySelector(`#${tagId}`); - const newElement = elementTagId?.getElementsByClassName( + const newElement = tagLabelRef.current?.getElementsByClassName( `${prefix}--tag__label` )[0]; - isEllipsisActive(newElement); - }, [prefix, tagId]); + setIsEllipsisApplied(isEllipsisActive(newElement)); + }, [prefix, tagLabelRef]); const handleClose = (event: React.MouseEvent) => { if (onClose) { event.stopPropagation(); @@ -134,7 +130,8 @@ const DismissibleTag = ({ const dismissLabel = `Dismiss "${text}"`; return ( - + ({ ...other }: OperationalTagProps) => { const prefix = usePrefix(); + const tagRef = useRef(); const tagId = id || `tag-${getInstanceId()}`; const tagClasses = classNames(`${prefix}--tag--operational`, className); const [isEllipsisApplied, setIsEllipsisApplied] = useState(false); - const isEllipsisActive = (element: any) => { - setIsEllipsisApplied(element.offsetWidth < element.scrollWidth); - return element.offsetWidth < element.scrollWidth; - }; - useLayoutEffect(() => { - const elementTagId = document.querySelector(`#${tagId}`); - const newElement = elementTagId?.getElementsByClassName( + const newElement = tagRef.current?.getElementsByClassName( `${prefix}--tag__label` )[0]; - isEllipsisActive(newElement); - }, [prefix, tagId]); + setIsEllipsisApplied(isEllipsisActive(newElement)); + }, [prefix, tagRef]); let normalizedSlug; if (slug && slug['type']?.displayName === 'Slug') { @@ -135,9 +132,10 @@ const OperationalTag = ({ align="bottom" className={tooltipClasses} leaveDelayMs={0} - onMouseEnter={false} + onMouseEnter={() => false} closeOnActivation> - + ({ } return ( - + ({ ...other }: SelectableTagProps) => { const prefix = usePrefix(); + const tagRef = useRef(); const tagId = id || `tag-${getInstanceId()}`; const [selectedTag, setSelectedTag] = useState(selected); const tagClasses = classNames(`${prefix}--tag--selectable`, className, { @@ -85,18 +87,12 @@ const SelectableTag = ({ }); const [isEllipsisApplied, setIsEllipsisApplied] = useState(false); - const isEllipsisActive = (element: any) => { - setIsEllipsisApplied(element.offsetWidth < element.scrollWidth); - return element.offsetWidth < element.scrollWidth; - }; - useLayoutEffect(() => { - const elementTagId = document.querySelector(`#${tagId}`); - const newElement = elementTagId?.getElementsByClassName( + const newElement = tagRef.current?.getElementsByClassName( `${prefix}--tag__label` )[0]; - isEllipsisActive(newElement); - }, [prefix, tagId]); + setIsEllipsisApplied(isEllipsisActive(newElement)); + }, [prefix, tagRef]); let normalizedSlug; if (slug && slug['type']?.displayName === 'Slug') { @@ -122,8 +118,9 @@ const SelectableTag = ({ align="bottom" className={tooltipClasses} leaveDelayMs={0} - onMouseEnter={false}> - + onMouseEnter={() => false}> + ({ } return ( - + ) => void; @@ -87,7 +95,7 @@ export interface TagBaseProps { slug?: ReactNode; /** - * @deprecated This property is deprecated and will be removed in the next major version. Use DismissibleTag instead. + * @deprecated The `title` prop has been deprecated and will be removed in the next major version. Use DismissibleTag instead. */ title?: string; @@ -102,37 +110,36 @@ export type TagProps = PolymorphicProps< TagBaseProps >; -const Tag = ({ - children, - className, - id, - type, - filter, // remove filter in next major release - V12 - renderIcon: CustomIconElement, - title = 'Clear filter', // remove title in next major release - V12 - disabled, - onClose, // remove onClose in next major release - V12 - size, - as: BaseComponent, - slug, - ...other -}: TagProps) => { +const Tag = React.forwardRef(function Tag( + { + children, + className, + id, + type, + filter, // remove filter in next major release - V12 + renderIcon: CustomIconElement, + title = 'Clear filter', // remove title in next major release - V12 + disabled, + onClose, // remove onClose in next major release - V12 + size, + as: BaseComponent, + slug, + ...other + }: TagProps, + forwardRef: ForwardedRef +) { const prefix = usePrefix(); + const tagRef = useRef(); + const ref = useMergeRefs([forwardRef, tagRef]); const tagId = id || `tag-${getInstanceId()}`; const [isEllipsisApplied, setIsEllipsisApplied] = useState(false); - const isEllipsisActive = (element: any) => { - setIsEllipsisApplied(element.offsetWidth < element.scrollWidth); - return element.offsetWidth < element.scrollWidth; - }; - useLayoutEffect(() => { - const elementTagId = document.querySelector(`#${tagId}`); - const newElement = elementTagId?.getElementsByClassName( + const newElement = tagRef.current?.getElementsByClassName( `${prefix}--tag__label` )[0]; - isEllipsisActive(newElement); - }, [prefix, tagId]); + setIsEllipsisApplied(isEllipsisActive(newElement)); + }, [prefix, tagRef]); const conditions = [ `${prefix}--tag--selectable`, @@ -172,7 +179,7 @@ const Tag = ({ } if (filter) { - const ComponentTag = BaseComponent ?? 'div'; + const ComponentTag = (BaseComponent as React.ElementType) ?? 'div'; return ( {CustomIconElement && size !== 'sm' ? ( @@ -215,6 +222,7 @@ const Tag = ({ return ( ({ {normalizedSlug} ); -}; +}); Tag.propTypes = { /** @@ -283,7 +291,7 @@ Tag.propTypes = { */ filter: deprecate( PropTypes.bool, - 'This property is deprecated and will be removed in the next major version. Use DismissibleTag instead.' + 'The `filter` prop has been deprecated and will be removed in the next major version. Use DismissibleTag instead.' ), /** @@ -296,7 +304,7 @@ Tag.propTypes = { */ onClose: deprecate( PropTypes.func, - 'This property is deprecated and will be removed in the next major version. Use DismissibleTag instead.' + 'The `onClose` prop has been deprecated and will be removed in the next major version. Use DismissibleTag instead.' ), /** @@ -321,7 +329,7 @@ Tag.propTypes = { */ title: deprecate( PropTypes.string, - 'This property is deprecated and will be removed in the next major version. Use DismissibleTag instead.' + 'The `title` prop has been deprecated and will be removed in the next major version. Use DismissibleTag instead.' ), /** diff --git a/packages/react/src/components/Tag/isEllipsisActive.ts b/packages/react/src/components/Tag/isEllipsisActive.ts new file mode 100644 index 000000000000..8ae25c51e133 --- /dev/null +++ b/packages/react/src/components/Tag/isEllipsisActive.ts @@ -0,0 +1,13 @@ +/** + * Copyright IBM Corp. 2024 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +export const isEllipsisActive = (element: any) => { + if (element) { + return element?.offsetWidth < element?.scrollWidth; + } + return false; +}; diff --git a/packages/react/src/components/Tooltip/Tooltip.tsx b/packages/react/src/components/Tooltip/Tooltip.tsx index fd10188cb656..ac3fba2dca29 100644 --- a/packages/react/src/components/Tooltip/Tooltip.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.tsx @@ -182,7 +182,7 @@ function Tooltip({ function onMouseEnter() { // Interactive Tags should not support onMouseEnter - if (!rest?.onMouseEnter?.()) { + if (!rest?.onMouseEnter) { setIsPointerIntersecting(true); setOpen(true, enterDelayMs); }