From 49768d963e797886558a1933de196e86b873a494 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Mon, 26 Jul 2021 17:37:55 +0200 Subject: [PATCH] chore(Label|List): use React.forwardRef() (#4252) --- src/elements/Label/Label.js | 174 ++++++++-------- src/elements/Label/LabelDetail.js | 8 +- src/elements/Label/LabelGroup.js | 7 +- src/elements/List/List.js | 137 ++++++------- src/elements/List/ListContent.js | 9 +- src/elements/List/ListDescription.js | 7 +- src/elements/List/ListHeader.js | 8 +- src/elements/List/ListIcon.js | 7 +- src/elements/List/ListItem.js | 187 +++++++++--------- src/elements/List/ListList.js | 7 +- test/specs/elements/Label/Label-test.js | 15 +- test/specs/elements/Label/LabelDetail-test.js | 1 + test/specs/elements/Label/LabelGroup-test.js | 1 + test/specs/elements/List/List-test.js | 12 +- test/specs/elements/List/ListContent-test.js | 3 +- .../elements/List/ListDescription-test.js | 1 + test/specs/elements/List/ListHeader-test.js | 1 + test/specs/elements/List/ListItem-test.js | 10 +- test/specs/elements/List/ListList-test.js | 1 + 19 files changed, 306 insertions(+), 290 deletions(-) diff --git a/src/elements/Label/Label.js b/src/elements/Label/Label.js index 1dedfffa50..30abb13cde 100644 --- a/src/elements/Label/Label.js +++ b/src/elements/Label/Label.js @@ -1,7 +1,7 @@ import cx from 'clsx' import _ from 'lodash' import PropTypes from 'prop-types' -import React, { Component } from 'react' +import React from 'react' import { childrenUtils, @@ -13,6 +13,7 @@ import { useKeyOnly, useKeyOrValueAndKey, useValueAndKey, + useEventCallback, } from '../../lib' import Icon from '../Icon/Icon' import Image from '../Image/Image' @@ -22,100 +23,95 @@ import LabelGroup from './LabelGroup' /** * A label displays content classification. */ -export default class Label extends Component { - handleClick = (e) => { - const { onClick } = this.props - - if (onClick) onClick(e, this.props) - } - - handleIconOverrides = (predefinedProps) => ({ - onClick: (e) => { - _.invoke(predefinedProps, 'onClick', e) - _.invoke(this.props, 'onRemove', e, this.props) - }, +const Label = React.forwardRef(function (props, ref) { + const { + active, + attached, + basic, + children, + circular, + className, + color, + content, + corner, + detail, + empty, + floating, + horizontal, + icon, + image, + onRemove, + pointing, + prompt, + removeIcon, + ribbon, + size, + tag, + } = props + + const pointingClass = + (pointing === true && 'pointing') || + ((pointing === 'left' || pointing === 'right') && `${pointing} pointing`) || + ((pointing === 'above' || pointing === 'below') && `pointing ${pointing}`) + + const classes = cx( + 'ui', + color, + pointingClass, + size, + useKeyOnly(active, 'active'), + useKeyOnly(basic, 'basic'), + useKeyOnly(circular, 'circular'), + useKeyOnly(empty, 'empty'), + useKeyOnly(floating, 'floating'), + useKeyOnly(horizontal, 'horizontal'), + useKeyOnly(image === true, 'image'), + useKeyOnly(prompt, 'prompt'), + useKeyOnly(tag, 'tag'), + useKeyOrValueAndKey(corner, 'corner'), + useKeyOrValueAndKey(ribbon, 'ribbon'), + useValueAndKey(attached, 'attached'), + 'label', + className, + ) + const rest = getUnhandledProps(Label, props) + const ElementType = getElementType(Label, props) + + const handleClick = useEventCallback((e) => { + _.invoke(props, 'onClick', e, props) }) - render() { - const { - active, - attached, - basic, - children, - circular, - className, - color, - content, - corner, - detail, - empty, - floating, - horizontal, - icon, - image, - onRemove, - pointing, - prompt, - removeIcon, - ribbon, - size, - tag, - } = this.props - - const pointingClass = - (pointing === true && 'pointing') || - ((pointing === 'left' || pointing === 'right') && `${pointing} pointing`) || - ((pointing === 'above' || pointing === 'below') && `pointing ${pointing}`) - - const classes = cx( - 'ui', - color, - pointingClass, - size, - useKeyOnly(active, 'active'), - useKeyOnly(basic, 'basic'), - useKeyOnly(circular, 'circular'), - useKeyOnly(empty, 'empty'), - useKeyOnly(floating, 'floating'), - useKeyOnly(horizontal, 'horizontal'), - useKeyOnly(image === true, 'image'), - useKeyOnly(prompt, 'prompt'), - useKeyOnly(tag, 'tag'), - useKeyOrValueAndKey(corner, 'corner'), - useKeyOrValueAndKey(ribbon, 'ribbon'), - useValueAndKey(attached, 'attached'), - 'label', - className, - ) - const rest = getUnhandledProps(Label, this.props) - const ElementType = getElementType(Label, this.props) - - if (!childrenUtils.isNil(children)) { - return ( - - {children} - - ) - } - - const removeIconShorthand = _.isUndefined(removeIcon) ? 'delete' : removeIcon - + if (!childrenUtils.isNil(children)) { return ( - - {Icon.create(icon, { autoGenerateKey: false })} - {typeof image !== 'boolean' && Image.create(image, { autoGenerateKey: false })} - {content} - {LabelDetail.create(detail, { autoGenerateKey: false })} - {onRemove && - Icon.create(removeIconShorthand, { - autoGenerateKey: false, - overrideProps: this.handleIconOverrides, - })} + + {children} ) } -} + const removeIconShorthand = _.isUndefined(removeIcon) ? 'delete' : removeIcon + + return ( + + {Icon.create(icon, { autoGenerateKey: false })} + {typeof image !== 'boolean' && Image.create(image, { autoGenerateKey: false })} + {content} + {LabelDetail.create(detail, { autoGenerateKey: false })} + {onRemove && + Icon.create(removeIconShorthand, { + autoGenerateKey: false, + overrideProps: (predefinedProps) => ({ + onClick: (e) => { + _.invoke(predefinedProps, 'onClick', e) + _.invoke(props, 'onRemove', e, props) + }, + }), + })} + + ) +}) + +Label.displayName = 'Label' Label.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, @@ -214,3 +210,5 @@ Label.Detail = LabelDetail Label.Group = LabelGroup Label.create = createShorthandFactory(Label, (value) => ({ content: value })) + +export default Label diff --git a/src/elements/Label/LabelDetail.js b/src/elements/Label/LabelDetail.js index 67c10fb714..6da4de001a 100644 --- a/src/elements/Label/LabelDetail.js +++ b/src/elements/Label/LabelDetail.js @@ -10,19 +10,21 @@ import { getUnhandledProps, } from '../../lib' -function LabelDetail(props) { +const LabelDetail = React.forwardRef(function (props, ref) { const { children, className, content } = props + const classes = cx('detail', className) const rest = getUnhandledProps(LabelDetail, props) const ElementType = getElementType(LabelDetail, props) return ( - + {childrenUtils.isNil(children) ? content : children} ) -} +}) +LabelDetail.displayName = 'LabelDetail' LabelDetail.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, diff --git a/src/elements/Label/LabelGroup.js b/src/elements/Label/LabelGroup.js index 795707aee0..01cf51dc5d 100644 --- a/src/elements/Label/LabelGroup.js +++ b/src/elements/Label/LabelGroup.js @@ -14,7 +14,7 @@ import { /** * A label can be grouped. */ -function LabelGroup(props) { +const LabelGroup = React.forwardRef(function (props, ref) { const { children, circular, className, color, content, size, tag } = props const classes = cx( @@ -30,12 +30,13 @@ function LabelGroup(props) { const ElementType = getElementType(LabelGroup, props) return ( - + {childrenUtils.isNil(children) ? content : children} ) -} +}) +LabelGroup.displayName = 'LabelGroup' LabelGroup.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, diff --git a/src/elements/List/List.js b/src/elements/List/List.js index 19fec4094d..f805838b3c 100644 --- a/src/elements/List/List.js +++ b/src/elements/List/List.js @@ -1,7 +1,7 @@ import cx from 'clsx' import _ from 'lodash' import PropTypes from 'prop-types' -import React, { Component } from 'react' +import React from 'react' import { childrenUtils, @@ -24,80 +24,81 @@ import ListList from './ListList' /** * A list groups related content. */ -class List extends Component { - handleItemOverrides = (predefinedProps) => ({ - onClick: (e, itemProps) => { - _.invoke(predefinedProps, 'onClick', e, itemProps) - _.invoke(this.props, 'onItemClick', e, itemProps) - }, - }) - - render() { - const { - animated, - bulleted, - celled, - children, - className, - content, - divided, - floated, - horizontal, - inverted, - items, - link, - ordered, - relaxed, - selection, - size, - verticalAlign, - } = this.props - - const classes = cx( - 'ui', - size, - useKeyOnly(animated, 'animated'), - useKeyOnly(bulleted, 'bulleted'), - useKeyOnly(celled, 'celled'), - useKeyOnly(divided, 'divided'), - useKeyOnly(horizontal, 'horizontal'), - useKeyOnly(inverted, 'inverted'), - useKeyOnly(link, 'link'), - useKeyOnly(ordered, 'ordered'), - useKeyOnly(selection, 'selection'), - useKeyOrValueAndKey(relaxed, 'relaxed'), - useValueAndKey(floated, 'floated'), - useVerticalAlignProp(verticalAlign), - 'list', - className, +const List = React.forwardRef(function (props, ref) { + const { + animated, + bulleted, + celled, + children, + className, + content, + divided, + floated, + horizontal, + inverted, + items, + link, + ordered, + relaxed, + selection, + size, + verticalAlign, + } = props + + const classes = cx( + 'ui', + size, + useKeyOnly(animated, 'animated'), + useKeyOnly(bulleted, 'bulleted'), + useKeyOnly(celled, 'celled'), + useKeyOnly(divided, 'divided'), + useKeyOnly(horizontal, 'horizontal'), + useKeyOnly(inverted, 'inverted'), + useKeyOnly(link, 'link'), + useKeyOnly(ordered, 'ordered'), + useKeyOnly(selection, 'selection'), + useKeyOrValueAndKey(relaxed, 'relaxed'), + useValueAndKey(floated, 'floated'), + useVerticalAlignProp(verticalAlign), + 'list', + className, + ) + const rest = getUnhandledProps(List, props) + const ElementType = getElementType(List, props) + + if (!childrenUtils.isNil(children)) { + return ( + + {children} + ) - const rest = getUnhandledProps(List, this.props) - const ElementType = getElementType(List, this.props) - - if (!childrenUtils.isNil(children)) { - return ( - - {children} - - ) - } - - if (!childrenUtils.isNil(content)) { - return ( - - {content} - - ) - } + } + if (!childrenUtils.isNil(content)) { return ( - - {_.map(items, (item) => ListItem.create(item, { overrideProps: this.handleItemOverrides }))} + + {content} ) } -} + return ( + + {_.map(items, (item) => + ListItem.create(item, { + overrideProps: (predefinedProps) => ({ + onClick: (e, itemProps) => { + _.invoke(predefinedProps, 'onClick', e, itemProps) + _.invoke(props, 'onItemClick', e, itemProps) + }, + }), + }), + )} + + ) +}) + +List.displayName = 'List' List.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, diff --git a/src/elements/List/ListContent.js b/src/elements/List/ListContent.js index 467da05b57..667e6bf0f6 100644 --- a/src/elements/List/ListContent.js +++ b/src/elements/List/ListContent.js @@ -18,7 +18,7 @@ import ListHeader from './ListHeader' /** * A list item can contain a content. */ -function ListContent(props) { +const ListContent = React.forwardRef(function (props, ref) { const { children, className, content, description, floated, header, verticalAlign } = props const classes = cx( @@ -32,21 +32,22 @@ function ListContent(props) { if (!childrenUtils.isNil(children)) { return ( - + {children} ) } return ( - + {ListHeader.create(header)} {ListDescription.create(description)} {content} ) -} +}) +ListContent.displayName = 'ListContent' ListContent.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, diff --git a/src/elements/List/ListDescription.js b/src/elements/List/ListDescription.js index 95b143af61..b2672442e3 100644 --- a/src/elements/List/ListDescription.js +++ b/src/elements/List/ListDescription.js @@ -13,19 +13,20 @@ import { /** * A list item can contain a description. */ -function ListDescription(props) { +const ListDescription = React.forwardRef(function (props, ref) { const { children, className, content } = props const classes = cx(className, 'description') const rest = getUnhandledProps(ListDescription, props) const ElementType = getElementType(ListDescription, props) return ( - + {childrenUtils.isNil(children) ? content : children} ) -} +}) +ListDescription.displayName = 'ListDescription' ListDescription.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, diff --git a/src/elements/List/ListHeader.js b/src/elements/List/ListHeader.js index e4f7af0248..c26850c199 100644 --- a/src/elements/List/ListHeader.js +++ b/src/elements/List/ListHeader.js @@ -13,19 +13,21 @@ import { /** * A list item can contain a header. */ -function ListHeader(props) { +const ListHeader = React.forwardRef(function (props, ref) { const { children, className, content } = props + const classes = cx('header', className) const rest = getUnhandledProps(ListHeader, props) const ElementType = getElementType(ListHeader, props) return ( - + {childrenUtils.isNil(children) ? content : children} ) -} +}) +ListHeader.displayName = 'ListHeader' ListHeader.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, diff --git a/src/elements/List/ListIcon.js b/src/elements/List/ListIcon.js index 0cb223af06..64d91edaac 100644 --- a/src/elements/List/ListIcon.js +++ b/src/elements/List/ListIcon.js @@ -8,14 +8,15 @@ import Icon from '../Icon/Icon' /** * A list item can contain an icon. */ -function ListIcon(props) { +const ListIcon = React.forwardRef(function (props, ref) { const { className, verticalAlign } = props const classes = cx(useVerticalAlignProp(verticalAlign), className) const rest = getUnhandledProps(ListIcon, props) - return -} + return +}) +ListIcon.displayName = 'ListIcon' ListIcon.propTypes = { /** Additional classes. */ className: PropTypes.string, diff --git a/src/elements/List/ListItem.js b/src/elements/List/ListItem.js index c5dc711489..c0fba5fb72 100644 --- a/src/elements/List/ListItem.js +++ b/src/elements/List/ListItem.js @@ -1,7 +1,7 @@ import cx from 'clsx' import _ from 'lodash' import PropTypes from 'prop-types' -import React, { Component, isValidElement } from 'react' +import React, { isValidElement } from 'react' import { childrenUtils, @@ -10,6 +10,7 @@ import { getElementType, getUnhandledProps, useKeyOnly, + useEventCallback, } from '../../lib' import Image from '../Image' import ListContent from './ListContent' @@ -20,112 +21,116 @@ import ListIcon from './ListIcon' /** * A list item can contain a set of items. */ -class ListItem extends Component { - handleClick = (e) => { - const { disabled } = this.props +const ListItem = React.forwardRef(function (props, ref) { + const { + active, + children, + className, + content, + description, + disabled, + header, + icon, + image, + value, + } = props + + const ElementType = getElementType(ListItem, props) + const classes = cx( + useKeyOnly(active, 'active'), + useKeyOnly(disabled, 'disabled'), + useKeyOnly(ElementType !== 'li', 'item'), + className, + ) + const rest = getUnhandledProps(ListItem, props) + + const handleClick = useEventCallback((e) => { + if (!disabled) { + _.invoke(props, 'onClick', e, props) + } + }) + const valueProp = ElementType === 'li' ? { value } : { 'data-value': value } - if (!disabled) _.invoke(this.props, 'onClick', e, this.props) + if (!childrenUtils.isNil(children)) { + return ( + + {children} + + ) } - render() { - const { - active, - children, - className, - content, - description, - disabled, - header, - icon, - image, - value, - } = this.props - - const ElementType = getElementType(ListItem, this.props) - const classes = cx( - useKeyOnly(active, 'active'), - useKeyOnly(disabled, 'disabled'), - useKeyOnly(ElementType !== 'li', 'item'), - className, - ) - const rest = getUnhandledProps(ListItem, this.props) - const valueProp = ElementType === 'li' ? { value } : { 'data-value': value } - - if (!childrenUtils.isNil(children)) { - return ( - - {children} - - ) - } + const iconElement = ListIcon.create(icon, { autoGenerateKey: false }) + const imageElement = Image.create(image, { autoGenerateKey: false }) - const iconElement = ListIcon.create(icon, { autoGenerateKey: false }) - const imageElement = Image.create(image, { autoGenerateKey: false }) - - // See description of `content` prop for explanation about why this is necessary. - if (!isValidElement(content) && _.isPlainObject(content)) { - return ( - - {iconElement || imageElement} - {ListContent.create(content, { - autoGenerateKey: false, - defaultProps: { header, description }, - })} - - ) - } + // See description of `content` prop for explanation about why this is necessary. + if (!isValidElement(content) && _.isPlainObject(content)) { + return ( + + {iconElement || imageElement} + {ListContent.create(content, { + autoGenerateKey: false, + defaultProps: { header, description }, + })} + + ) + } - const headerElement = ListHeader.create(header, { autoGenerateKey: false }) - const descriptionElement = ListDescription.create(description, { autoGenerateKey: false }) - if (iconElement || imageElement) { - return ( - - {iconElement || imageElement} - {(content || headerElement || descriptionElement) && ( - - {headerElement} - {descriptionElement} - {content} - - )} - - ) - } + const headerElement = ListHeader.create(header, { autoGenerateKey: false }) + const descriptionElement = ListDescription.create(description, { autoGenerateKey: false }) + if (iconElement || imageElement) { return ( - {headerElement} - {descriptionElement} - {content} + {iconElement || imageElement} + {(content || headerElement || descriptionElement) && ( + + {headerElement} + {descriptionElement} + {content} + + )} ) } -} + return ( + + {headerElement} + {descriptionElement} + {content} + + ) +}) + +ListItem.displayName = 'ListItem' ListItem.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, diff --git a/src/elements/List/ListList.js b/src/elements/List/ListList.js index fac22a8491..403d243b79 100644 --- a/src/elements/List/ListList.js +++ b/src/elements/List/ListList.js @@ -13,7 +13,7 @@ import { /** * A list can contain a sub list. */ -function ListList(props) { +const ListList = React.forwardRef(function (props, ref) { const { children, className, content } = props const rest = getUnhandledProps(ListList, props) @@ -21,12 +21,13 @@ function ListList(props) { const classes = cx(useKeyOnly(ElementType !== 'ul' && ElementType !== 'ol', 'list'), className) return ( - + {childrenUtils.isNil(children) ? content : children} ) -} +}) +ListList.displayName = 'ListList' ListList.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, diff --git a/test/specs/elements/Label/Label-test.js b/test/specs/elements/Label/Label-test.js index 0c21df6b9d..ee6492778c 100644 --- a/test/specs/elements/Label/Label-test.js +++ b/test/specs/elements/Label/Label-test.js @@ -10,6 +10,8 @@ import { sandbox } from 'test/utils' describe('Label', () => { common.isConformant(Label) + common.forwardsRef(Label) + common.forwardsRef(Label, { requiredProps: { children: } }) common.hasSubcomponents(Label, [LabelDetail, LabelGroup]) common.hasUIClassName(Label) common.rendersChildren(Label) @@ -103,19 +105,14 @@ describe('Label', () => { }) describe('onClick', () => { - it('omitted when not defined', () => { - const click = () => shallow(