From db716dedf374e00ea8edad38a06e69a678b6dd54 Mon Sep 17 00:00:00 2001 From: kiliancol Date: Fri, 24 May 2024 14:17:38 +0100 Subject: [PATCH 1/9] 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. --- .../src/components/Tag/DismissibleTag.tsx | 17 +++++++---------- .../src/components/Tag/OperationalTag.tsx | 17 ++++++++--------- .../src/components/Tag/SelectableTag.tsx | 18 ++++++++---------- packages/react/src/components/Tag/Tag.tsx | 19 ++++++++----------- .../src/components/Tag/isEllipsisActive.ts | 13 +++++++++++++ 5 files changed, 44 insertions(+), 40 deletions(-) create mode 100644 packages/react/src/components/Tag/isEllipsisActive.ts diff --git a/packages/react/src/components/Tag/DismissibleTag.tsx b/packages/react/src/components/Tag/DismissibleTag.tsx index b9223c6bdb5c..1768617e351c 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(); @@ -135,6 +131,7 @@ const DismissibleTag = ({ return ( + ref={tagLabelRef} type={type} size={size} renderIcon={renderIcon} diff --git a/packages/react/src/components/Tag/OperationalTag.tsx b/packages/react/src/components/Tag/OperationalTag.tsx index 4b250b016473..3e1c36988b5e 100644 --- a/packages/react/src/components/Tag/OperationalTag.tsx +++ b/packages/react/src/components/Tag/OperationalTag.tsx @@ -11,6 +11,7 @@ import React, { useLayoutEffect, useState, ReactNode, + useRef, } from 'react'; import classNames from 'classnames'; import setupGetInstanceId from '../../tools/setupGetInstanceId'; @@ -19,6 +20,7 @@ import { PolymorphicProps } from '../../types/common'; import Tag, { SIZES } from './Tag'; import { Tooltip } from '../Tooltip'; import { Text } from '../Text'; +import { isEllipsisActive } from './isEllipsisActive'; const getInstanceId = setupGetInstanceId(); @@ -97,23 +99,18 @@ const OperationalTag = ({ ...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') { @@ -138,6 +135,7 @@ const OperationalTag = ({ onMouseEnter={false} closeOnActivation> + ref={tagRef} type={type} size={size} renderIcon={renderIcon} @@ -156,6 +154,7 @@ const OperationalTag = ({ return ( + ref={tagRef} type={type} size={size} renderIcon={renderIcon} diff --git a/packages/react/src/components/Tag/SelectableTag.tsx b/packages/react/src/components/Tag/SelectableTag.tsx index 129dbb5b45ac..792c1dfb854f 100644 --- a/packages/react/src/components/Tag/SelectableTag.tsx +++ b/packages/react/src/components/Tag/SelectableTag.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'; @@ -14,6 +14,7 @@ import { PolymorphicProps } from '../../types/common'; import Tag, { SIZES } from './Tag'; import { Tooltip } from '../Tooltip'; import { Text } from '../Text'; +import { isEllipsisActive } from './isEllipsisActive'; const getInstanceId = setupGetInstanceId(); @@ -78,6 +79,7 @@ const SelectableTag = ({ ...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') { @@ -124,6 +120,7 @@ const SelectableTag = ({ leaveDelayMs={0} onMouseEnter={false}> + ref={tagRef} slug={slug} size={size} renderIcon={renderIcon} @@ -143,6 +140,7 @@ const SelectableTag = ({ return ( + ref={tagRef} slug={slug} size={size} renderIcon={renderIcon} diff --git a/packages/react/src/components/Tag/Tag.tsx b/packages/react/src/components/Tag/Tag.tsx index 3ef10aef9486..e1739e3bc2ca 100644 --- a/packages/react/src/components/Tag/Tag.tsx +++ b/packages/react/src/components/Tag/Tag.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 { Close } from '@carbon/icons-react'; import setupGetInstanceId from '../../tools/setupGetInstanceId'; @@ -15,6 +15,7 @@ import { PolymorphicProps } from '../../types/common'; import { Text } from '../Text'; import deprecate from '../../prop-types/deprecate'; import { DefinitionTooltip } from '../Tooltip'; +import { isEllipsisActive } from './isEllipsisActive'; const getInstanceId = setupGetInstanceId(); export const TYPES = { @@ -118,21 +119,16 @@ const Tag = ({ ...other }: TagProps) => { const prefix = usePrefix(); + const tagRef = useRef(); 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`, @@ -174,7 +170,7 @@ const Tag = ({ if (filter) { const ComponentTag = BaseComponent ?? 'div'; return ( - + {CustomIconElement && size !== 'sm' ? (
@@ -215,6 +211,7 @@ const Tag = ({ return ( { + if (element) { + return element?.offsetWidth < element?.scrollWidth; + } + return false; +}; From fe7a3abb48d46b109efdde38060e787527ceb559 Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Fri, 24 May 2024 08:46:54 -0500 Subject: [PATCH 2/9] fix(tag): cast BaseComponent type --- packages/react/src/components/Tag/Tag.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/components/Tag/Tag.tsx b/packages/react/src/components/Tag/Tag.tsx index e1739e3bc2ca..d09bc52e7980 100644 --- a/packages/react/src/components/Tag/Tag.tsx +++ b/packages/react/src/components/Tag/Tag.tsx @@ -168,7 +168,7 @@ const Tag = ({ } if (filter) { - const ComponentTag = BaseComponent ?? 'div'; + const ComponentTag = (BaseComponent as React.ElementType) ?? 'div'; return ( {CustomIconElement && size !== 'sm' ? ( From 303aee938b30c335e9696c7d893bf2d28b3d3e4e Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Fri, 24 May 2024 09:02:08 -0500 Subject: [PATCH 3/9] fix(tag): improve deprecation notices --- packages/react/src/components/Tag/Tag.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/react/src/components/Tag/Tag.tsx b/packages/react/src/components/Tag/Tag.tsx index d09bc52e7980..fe612735505b 100644 --- a/packages/react/src/components/Tag/Tag.tsx +++ b/packages/react/src/components/Tag/Tag.tsx @@ -56,7 +56,7 @@ export interface TagBaseProps { disabled?: boolean; /** - * @deprecated This property is deprecated and will be removed in the next major version. Use DismissibleTag instead. + * @deprecated The `filter` prop has been deprecated and will be removed in the next major version. Use DismissibleTag instead. */ filter?: boolean; @@ -66,7 +66,7 @@ export interface TagBaseProps { id?: string; /** - * @deprecated This property is deprecated and will be removed in the next major version. Use DismissibleTag instead. + * @deprecated The `onClose` prop has been deprecated and will be removed in the next major version. Use DismissibleTag instead. */ onClose?: (event: React.MouseEvent) => void; @@ -88,7 +88,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; @@ -280,7 +280,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.' ), /** @@ -293,7 +293,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.' ), /** @@ -318,7 +318,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.' ), /** From 8687d3c3ef22d2f34718beb26a943f4d5e43cbfa Mon Sep 17 00:00:00 2001 From: guidari Date: Fri, 24 May 2024 11:11:38 -0300 Subject: [PATCH 4/9] fix: added forwardRef to Tag to grab ref in variants --- .../src/components/Tag/DismissibleTag.tsx | 3 +- .../src/components/Tag/OperationalTag.tsx | 6 ++- .../src/components/Tag/SelectableTag.tsx | 7 ++- packages/react/src/components/Tag/Tag.tsx | 47 ++++++++++++------- .../src/components/Tag/isEllipsisActive.ts | 1 + 5 files changed, 41 insertions(+), 23 deletions(-) diff --git a/packages/react/src/components/Tag/DismissibleTag.tsx b/packages/react/src/components/Tag/DismissibleTag.tsx index 1768617e351c..7d07e339637a 100644 --- a/packages/react/src/components/Tag/DismissibleTag.tsx +++ b/packages/react/src/components/Tag/DismissibleTag.tsx @@ -130,7 +130,8 @@ const DismissibleTag = ({ const dismissLabel = `Dismiss "${text}"`; return ( - + // @ts-ignore-error Popover throws a TS error everytime is imported + ({ leaveDelayMs={0} onMouseEnter={false} closeOnActivation> - + {/* @ts-ignore-error Popover throws a TS error everytime is imported */} + ({ } return ( - + // @ts-ignore-error Popover throws a TS error everytime is imported + ({ className={tooltipClasses} leaveDelayMs={0} onMouseEnter={false}> - + {/* @ts-ignore-error Popover throws a TS error everytime is imported */} + + ({ } return ( - + // @ts-ignore-error Popover throws a TS error everytime is imported + = 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); @@ -211,7 +222,7 @@ const Tag = ({ return ( ({ {normalizedSlug} ); -}; +}); Tag.propTypes = { /** diff --git a/packages/react/src/components/Tag/isEllipsisActive.ts b/packages/react/src/components/Tag/isEllipsisActive.ts index 8ae25c51e133..b92bd630bec1 100644 --- a/packages/react/src/components/Tag/isEllipsisActive.ts +++ b/packages/react/src/components/Tag/isEllipsisActive.ts @@ -7,6 +7,7 @@ export const isEllipsisActive = (element: any) => { if (element) { + console.log('element offste', element?.offsetWidth < element?.scrollWidth); return element?.offsetWidth < element?.scrollWidth; } return false; From b7dcfdd62cf79738a9d8b745f9dc1469ed3ca32a Mon Sep 17 00:00:00 2001 From: guidari Date: Fri, 24 May 2024 11:16:02 -0300 Subject: [PATCH 5/9] fix: removed console log --- packages/react/src/components/Tag/isEllipsisActive.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react/src/components/Tag/isEllipsisActive.ts b/packages/react/src/components/Tag/isEllipsisActive.ts index b92bd630bec1..8ae25c51e133 100644 --- a/packages/react/src/components/Tag/isEllipsisActive.ts +++ b/packages/react/src/components/Tag/isEllipsisActive.ts @@ -7,7 +7,6 @@ export const isEllipsisActive = (element: any) => { if (element) { - console.log('element offste', element?.offsetWidth < element?.scrollWidth); return element?.offsetWidth < element?.scrollWidth; } return false; From 0d529ed08a8fb8e090627904d65e95c622edcd70 Mon Sep 17 00:00:00 2001 From: guidari Date: Fri, 24 May 2024 11:22:31 -0300 Subject: [PATCH 6/9] fix: fixed spelling and remove ref from old filter --- packages/react/src/components/Tag/DismissibleTag.tsx | 2 +- packages/react/src/components/Tag/OperationalTag.tsx | 4 ++-- packages/react/src/components/Tag/SelectableTag.tsx | 4 ++-- packages/react/src/components/Tag/Tag.tsx | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/react/src/components/Tag/DismissibleTag.tsx b/packages/react/src/components/Tag/DismissibleTag.tsx index 7d07e339637a..7c3822d805e2 100644 --- a/packages/react/src/components/Tag/DismissibleTag.tsx +++ b/packages/react/src/components/Tag/DismissibleTag.tsx @@ -130,7 +130,7 @@ const DismissibleTag = ({ const dismissLabel = `Dismiss "${text}"`; return ( - // @ts-ignore-error Popover throws a TS error everytime is imported + // @ts-ignore-error Tag throws a TS error everytime is imported ({ leaveDelayMs={0} onMouseEnter={false} closeOnActivation> - {/* @ts-ignore-error Popover throws a TS error everytime is imported */} + {/* @ts-ignore-error Tag throws a TS error everytime is imported */} ({ } return ( - // @ts-ignore-error Popover throws a TS error everytime is imported + // @ts-ignore-error Tag throws a TS error everytime is imported ({ className={tooltipClasses} leaveDelayMs={0} onMouseEnter={false}> - {/* @ts-ignore-error Popover throws a TS error everytime is imported */} + {/* @ts-ignore-error Tag throws a TS error everytime is imported */} ({ } return ( - // @ts-ignore-error Popover throws a TS error everytime is imported + // @ts-ignore-error Tag throws a TS error everytime is imported ( if (filter) { const ComponentTag = (BaseComponent as React.ElementType) ?? 'div'; return ( - + {CustomIconElement && size !== 'sm' ? (
From 6f9852b5acbd0dc6663c5926676928aa4b770937 Mon Sep 17 00:00:00 2001 From: guidari Date: Fri, 24 May 2024 11:41:11 -0300 Subject: [PATCH 7/9] fix: updated snapshots --- packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap | 2 ++ 1 file changed, 2 insertions(+) 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 { From 4e997f60093a70a6e2acaf5147fdd7369ec0cded Mon Sep 17 00:00:00 2001 From: guidari Date: Fri, 24 May 2024 12:07:44 -0300 Subject: [PATCH 8/9] fix: fixed onMouseEnter error on console --- packages/react/src/components/Tag/OperationalTag.tsx | 2 +- packages/react/src/components/Tag/SelectableTag.tsx | 3 +-- packages/react/src/components/Tooltip/Tooltip.tsx | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/react/src/components/Tag/OperationalTag.tsx b/packages/react/src/components/Tag/OperationalTag.tsx index dd6b00db87c7..b497c3170892 100644 --- a/packages/react/src/components/Tag/OperationalTag.tsx +++ b/packages/react/src/components/Tag/OperationalTag.tsx @@ -132,7 +132,7 @@ const OperationalTag = ({ align="bottom" className={tooltipClasses} leaveDelayMs={0} - onMouseEnter={false} + onMouseEnter={() => false} closeOnActivation> {/* @ts-ignore-error Tag throws a TS error everytime is imported */} ({ align="bottom" className={tooltipClasses} leaveDelayMs={0} - onMouseEnter={false}> + onMouseEnter={() => false}> {/* @ts-ignore-error Tag throws a TS error everytime is imported */} - ({ function onMouseEnter() { // Interactive Tags should not support onMouseEnter - if (!rest?.onMouseEnter?.()) { + if (!rest?.onMouseEnter) { setIsPointerIntersecting(true); setOpen(true, enterDelayMs); } From 1f52e433f0db3b6a4b157c2a97c68e698fe5d15b Mon Sep 17 00:00:00 2001 From: guidari Date: Fri, 24 May 2024 12:36:57 -0300 Subject: [PATCH 9/9] fix: fixed TS error --- packages/react/src/components/Tag/DismissibleTag.tsx | 1 - packages/react/src/components/Tag/OperationalTag.tsx | 2 -- packages/react/src/components/Tag/SelectableTag.tsx | 2 -- packages/react/src/components/Tag/Tag.tsx | 2 +- 4 files changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/react/src/components/Tag/DismissibleTag.tsx b/packages/react/src/components/Tag/DismissibleTag.tsx index 7c3822d805e2..541602ea35ce 100644 --- a/packages/react/src/components/Tag/DismissibleTag.tsx +++ b/packages/react/src/components/Tag/DismissibleTag.tsx @@ -130,7 +130,6 @@ const DismissibleTag = ({ const dismissLabel = `Dismiss "${text}"`; return ( - // @ts-ignore-error Tag throws a TS error everytime is imported ({ leaveDelayMs={0} onMouseEnter={() => false} closeOnActivation> - {/* @ts-ignore-error Tag throws a TS error everytime is imported */} ({ } return ( - // @ts-ignore-error Tag throws a TS error everytime is imported ({ className={tooltipClasses} leaveDelayMs={0} onMouseEnter={() => false}> - {/* @ts-ignore-error Tag throws a TS error everytime is imported */} ({ } return ( - // @ts-ignore-error Tag throws a TS error everytime is imported ( slug, ...other }: TagProps, - forwardRef: ForwardedRef + forwardRef: ForwardedRef ) { const prefix = usePrefix(); const tagRef = useRef();