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.

Sign up for GitHub

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

fix(tag): use refs to handle component access #16571

Merged
merged 10 commits into from
May 24, 2024
Merged

fix(tag): use refs to handle component access #16571

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
db716de
fix(tag): use refs to handle component access
Kilian-Collender May 24, 2024
fe7a3ab
fix(tag): cast BaseComponent type
tay1orjones May 24, 2024
303aee9
fix(tag): improve deprecation notices
tay1orjones May 24, 2024
8687d3c
fix: added forwardRef to Tag to grab ref in variants
guidari May 24, 2024
b4740c5
Merge branch 'kc-tag-bug-16570' of https://github.com/Kilian-Collende…
guidari May 24, 2024
b7dcfdd
fix: removed console log
guidari May 24, 2024
0d529ed
fix: fixed spelling and remove ref from old filter
guidari May 24, 2024
6f9852b
fix: updated snapshots
guidari May 24, 2024
4e997f6
fix: fixed onMouseEnter error on console
guidari May 24, 2024
1f52e43
fix: fixed TS error
guidari May 24, 2024
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
20 changes: 9 additions & 11 deletions packages/react/src/components/Tag/DismissibleTag.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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();

Expand Down Expand Up @@ -91,22 +92,17 @@ const DismissibleTag = <T extends React.ElementType>({
...other
}: DismissibleTagProps<T>) => {
const prefix = usePrefix();
const tagLabelRef = useRef<HTMLElement>();
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<HTMLButtonElement>) => {
if (onClose) {
event.stopPropagation();
Expand Down Expand Up @@ -134,7 +130,9 @@ const DismissibleTag = <T extends React.ElementType>({
const dismissLabel = `Dismiss "${text}"`;

return (
<Tag<any>
// @ts-ignore-error Popover throws a TS error everytime is imported
<Tag
ref={tagLabelRef}
type={type}
size={size}
renderIcon={renderIcon}
Expand Down
23 changes: 12 additions & 11 deletions packages/react/src/components/Tag/OperationalTag.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import React, {
useLayoutEffect,
useState,
ReactNode,
useRef,
} from 'react';
import classNames from 'classnames';
import setupGetInstanceId from '../../tools/setupGetInstanceId';
Expand All @@ -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();

Expand Down Expand Up @@ -97,23 +99,18 @@ const OperationalTag = <T extends React.ElementType>({
...other
}: OperationalTagProps<T>) => {
const prefix = usePrefix();
const tagRef = useRef<HTMLElement>();
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') {
Expand All @@ -137,7 +134,9 @@ const OperationalTag = <T extends React.ElementType>({
leaveDelayMs={0}
onMouseEnter={false}
closeOnActivation>
<Tag<any>
{/* @ts-ignore-error Popover throws a TS error everytime is imported */}
<Tag
ref={tagRef}
type={type}
size={size}
renderIcon={renderIcon}
Expand All @@ -155,7 +154,9 @@ const OperationalTag = <T extends React.ElementType>({
}

return (
<Tag<any>
// @ts-ignore-error Popover throws a TS error everytime is imported
<Tag
ref={tagRef}
type={type}
size={size}
renderIcon={renderIcon}
Expand Down
25 changes: 13 additions & 12 deletions packages/react/src/components/Tag/SelectableTag.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
*/

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';
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();

Expand Down Expand Up @@ -78,25 +79,20 @@ const SelectableTag = <T extends React.ElementType>({
...other
}: SelectableTagProps<T>) => {
const prefix = usePrefix();
const tagRef = useRef<HTMLElement>();
const tagId = id || `tag-${getInstanceId()}`;
const [selectedTag, setSelectedTag] = useState(selected);
const tagClasses = classNames(`${prefix}--tag--selectable`, className, {
[`${prefix}--tag--selectable-selected`]: selectedTag,
});
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') {
Expand All @@ -123,7 +119,10 @@ const SelectableTag = <T extends React.ElementType>({
className={tooltipClasses}
leaveDelayMs={0}
onMouseEnter={false}>
<Tag<any>
{/* @ts-ignore-error Popover throws a TS error everytime is imported */}

<Tag
ref={tagRef}
slug={slug}
size={size}
renderIcon={renderIcon}
Expand All @@ -142,7 +141,9 @@ const SelectableTag = <T extends React.ElementType>({
}

return (
<Tag<any>
// @ts-ignore-error Popover throws a TS error everytime is imported
<Tag
ref={tagRef}
slug={slug}
size={size}
renderIcon={renderIcon}
Expand Down
76 changes: 42 additions & 34 deletions packages/react/src/components/Tag/Tag.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@
*/

import PropTypes from 'prop-types';
import React, { useLayoutEffect, useState, ReactNode } from 'react';
import React, {
useLayoutEffect,
useState,
ReactNode,
useRef,
ForwardedRef,
} from 'react';
import classNames from 'classnames';
import { Close } from '@carbon/icons-react';
import setupGetInstanceId from '../../tools/setupGetInstanceId';
Expand All @@ -15,6 +21,8 @@ import { PolymorphicProps } from '../../types/common';
import { Text } from '../Text';
import deprecate from '../../prop-types/deprecate';
import { DefinitionTooltip } from '../Tooltip';
import { isEllipsisActive } from './isEllipsisActive';
import { useMergeRefs } from '@floating-ui/react';

const getInstanceId = setupGetInstanceId();
export const TYPES = {
Expand Down Expand Up @@ -55,7 +63,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;

Expand All @@ -65,7 +73,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<HTMLButtonElement>) => void;

Expand All @@ -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;

Expand All @@ -102,37 +110,36 @@ export type TagProps<T extends React.ElementType> = PolymorphicProps<
TagBaseProps
>;

const Tag = <T extends React.ElementType>({
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<T>) => {
const Tag = React.forwardRef(function Tag<T extends React.ElementType>(
{
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<T>,
forwardRef: ForwardedRef<Element>
) {
const prefix = usePrefix();
const tagRef = useRef<HTMLElement>();
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`,
Expand Down Expand Up @@ -172,9 +179,9 @@ const Tag = <T extends React.ElementType>({
}

if (filter) {
const ComponentTag = BaseComponent ?? 'div';
const ComponentTag = (BaseComponent as React.ElementType) ?? 'div';
return (
<ComponentTag className={tagClasses} id={tagId} {...other}>
<ComponentTag ref={tagRef} className={tagClasses} id={tagId} {...other}>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this not be ref too now?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we can remove the ref from this one. The filter prop should be replace with the new variant Dissmisible Tag so we don't have to add the new functionality to the old filter tag.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but isn't the Dismissible Tag experimental? so you still need it there until it is fully deprecated

Copy link
Contributor

@guidari guidari May 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point. It is something I will take to the team to talk about it. Since we have implemented in the Dissmisible Tag it would be quick to implement on the old filter prop.
But for now the spec we have is only in the new Intereactive Tag.

{CustomIconElement && size !== 'sm' ? (
<div className={`${prefix}--tag__custom-icon`}>
<CustomIconElement />
Expand Down Expand Up @@ -215,6 +222,7 @@ const Tag = <T extends React.ElementType>({

return (
<ComponentTag
ref={ref}
disabled={disabled}
className={tagClasses}
id={tagId}
Expand Down Expand Up @@ -254,7 +262,7 @@ const Tag = <T extends React.ElementType>({
{normalizedSlug}
</ComponentTag>
);
};
});

Tag.propTypes = {
/**
Expand Down Expand Up @@ -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.'
),

/**
Expand All @@ -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.'
),

/**
Expand All @@ -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.'
),

/**
Expand Down
14 changes: 14 additions & 0 deletions packages/react/src/components/Tag/isEllipsisActive.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* 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) {
console.log('element offste', element?.offsetWidth < element?.scrollWidth);
guidari marked this conversation as resolved.
Show resolved Hide resolved
return element?.offsetWidth < element?.scrollWidth;
}
return false;
};
Loading