From 6764f3ecfc3be9bb8873f85c69aa156c46ee9b3e Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Fri, 23 Jul 2021 14:46:14 +0200 Subject: [PATCH 01/49] chore: remove .state() assertions in tests (#4232) * Ember: remove .state() usage in Enzyme * AccordionAccordion: remove .state() usage in Enzyme * Checkbox: remove .state() usage in Enzyme * TransitionablePortal: remove .state() usage in Enzyme * Transition: remove .state() usage in Enzyme * Dropdown: remove .state() usage in Enzyme --- src/modules/Embed/Embed.js | 4 +- src/modules/Transition/Transition.js | 12 +- .../TransitionablePortal-test.js | 118 ++++++--------- .../Accordion/AccordionAccordion-test.js | 121 ++++++++------- test/specs/modules/Checkbox/Checkbox-test.js | 28 ++-- test/specs/modules/Dropdown/Dropdown-test.js | 139 +++++++++--------- test/specs/modules/Embed/Embed-test.js | 32 ++-- .../modules/Transition/Transition-test.js | 137 ++++++++--------- 8 files changed, 283 insertions(+), 308 deletions(-) diff --git a/src/modules/Embed/Embed.js b/src/modules/Embed/Embed.js index 55bb2c0d82..a27597700a 100644 --- a/src/modules/Embed/Embed.js +++ b/src/modules/Embed/Embed.js @@ -1,4 +1,5 @@ import cx from 'clsx' +import _ from 'lodash' import PropTypes from 'prop-types' import React from 'react' @@ -57,10 +58,9 @@ export default class Embed extends Component { } handleClick = (e) => { - const { onClick } = this.props const { active } = this.state - if (onClick) onClick(e, { ...this.props, active: true }) + _.invoke(this.props, 'onClick', e, { ...this.props, active: true }) if (!active) this.setState({ active: true }) } diff --git a/src/modules/Transition/Transition.js b/src/modules/Transition/Transition.js index 0cd0fb8e09..0fdea6a222 100644 --- a/src/modules/Transition/Transition.js +++ b/src/modules/Transition/Transition.js @@ -87,7 +87,11 @@ export default class Transition extends Component { const durationType = TRANSITION_CALLBACK_TYPE[nextStatus] const durationValue = normalizeTransitionDuration(duration, durationType) - this.timeoutId = setTimeout(() => this.setState({ status: nextStatus }), durationValue) + if (durationValue === 0) { + this.setState({ status: nextStatus }) + } else { + this.timeoutId = setTimeout(() => this.setState({ status: nextStatus }), durationValue) + } } updateStatus = (prevState) => { @@ -161,7 +165,7 @@ export default class Transition extends Component { debug('render(): state', this.state) const { children } = this.props - const { status } = this.state + const { nextStatus, status } = this.state if (status === TRANSITION_STATUS_UNMOUNTED) { return null @@ -170,6 +174,10 @@ export default class Transition extends Component { return cloneElement(children, { className: this.computeClasses(), style: this.computeStyle(), + ...(process.env.NODE_ENV !== 'production' && { + 'data-test-status': status, + 'data-test-next-status': nextStatus, + }), }) } } diff --git a/test/specs/addons/TransitionablePortal/TransitionablePortal-test.js b/test/specs/addons/TransitionablePortal/TransitionablePortal-test.js index 79d9f23a63..8dd4e6d4de 100644 --- a/test/specs/addons/TransitionablePortal/TransitionablePortal-test.js +++ b/test/specs/addons/TransitionablePortal/TransitionablePortal-test.js @@ -4,68 +4,26 @@ import TransitionablePortal from 'src/addons/TransitionablePortal/Transitionable import * as common from 'test/specs/commonTests' import { domEvent, sandbox, assertWithTimeout } from 'test/utils' -// ---------------------------------------- -// Wrapper -// ---------------------------------------- -let wrapper - -// we need to unmount the modal after every test to remove it from the document -// wrap the render methods to update a global wrapper that is unmounted after each test -const wrapperMount = (...args) => (wrapper = mount(...args)) -const wrapperShallow = (...args) => (wrapper = shallow(...args)) - const quickTransition = { duration: 0 } const requiredProps = { - children:
, + children:
, } describe('TransitionablePortal', () => { - beforeEach(() => { - wrapper = undefined - document.body.innerHTML = '' - }) - - afterEach(() => { - if (wrapper && wrapper.unmount) wrapper.unmount() - }) - common.isConformant(TransitionablePortal, { requiredProps }) describe('children', () => { - it('renders a Portal', () => { - wrapperShallow().should.have.descendants('Portal') - }) - it('renders a Transition', () => { - wrapperShallow().should.have.descendants( - 'Transition', - ) - }) - }) - - describe('getDerivedStateFromProps', () => { - it('passes `open` prop to `portalOpen` when defined', () => { - wrapperMount() + const wrapper = mount() - wrapper.setProps({ open: true }) - wrapper.should.have.state('portalOpen', true) - wrapper.setProps({ open: false }) - wrapper.should.have.state('portalOpen', false) - }) - - it('does not pass `open` prop to `portalOpen` when not defined', () => { - wrapperMount() - - wrapper.setProps({ transition: {} }) - wrapper.should.have.not.state('portalOpen') + wrapper.should.have.descendants('.transition') }) }) describe('onClose', () => { - it('is called with (null, data) when Portal closes', (done) => { + it('is called with (null, data) on a click outside', (done) => { const onClose = sandbox.spy() - - wrapperMount( + const wrapper = mount( { assertWithTimeout(() => { onClose.should.have.been.calledOnce() onClose.should.have.been.calledWithMatch(null, { portalOpen: false }) + + wrapper.unmount() }, done) }) - it('changes `portalOpen` to false', () => { - wrapperMount( - } - />, - ) + it('hides contents on a click outside', () => { + const wrapper = mount(} />) wrapper.find('button').simulate('click') - domEvent.click(document.body) + wrapper.should.have.descendants('.in#children') - wrapper.should.have.state('portalOpen', false) + domEvent.click(document.body) + wrapper.update() + wrapper.should.have.descendants('.out#children') }) }) describe('onHide', () => { it('is called with (null, data) when exiting transition finished', (done) => { const onHide = sandbox.spy() - const trigger = - - {(labelPosition === 'right' || !labelPosition) && labelElement} - - ) - } - - const classes = cx('ui', baseClasses, wrapperClasses, labeledClasses, 'button', className) - const hasChildren = !childrenUtils.isNil(children) - const role = this.computeButtonAriaRole(ElementType) + if (!_.isNil(label)) { + const buttonClasses = cx('ui', baseClasses, 'button', className) + const containerClasses = cx('ui', labeledClasses, 'button', className, wrapperClasses) + const labelElement = Label.create(label, { + defaultProps: { + basic: true, + pointing: labelPosition === 'left' ? 'right' : 'left', + }, + autoGenerateKey: false, + }) return ( - - + {labelPosition === 'left' && labelElement} + + {(labelPosition === 'right' || !labelPosition) && labelElement} + ) } -} + const classes = cx('ui', baseClasses, wrapperClasses, labeledClasses, 'button', className) + const hasChildren = !childrenUtils.isNil(children) + const role = computeButtonAriaRole(ElementType, props.role) + + return ( + + {hasChildren && children} + {!hasChildren && Icon.create(icon, { autoGenerateKey: false })} + {!hasChildren && content} + + ) +}) + +Button.displayName = 'Button' Button.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, diff --git a/src/elements/Button/ButtonContent.js b/src/elements/Button/ButtonContent.js index 7622007fbe..af248b1022 100644 --- a/src/elements/Button/ButtonContent.js +++ b/src/elements/Button/ButtonContent.js @@ -13,8 +13,9 @@ import { /** * Used in some Button types, such as `animated`. */ -function ButtonContent(props) { +const ButtonContent = React.forwardRef(function (props, ref) { const { children, className, content, hidden, visible } = props + const classes = cx( useKeyOnly(visible, 'visible'), useKeyOnly(hidden, 'hidden'), @@ -25,12 +26,13 @@ function ButtonContent(props) { const ElementType = getElementType(ButtonContent, props) return ( - + {childrenUtils.isNil(children) ? content : children} ) -} +}) +ButtonContent.displayName = 'ButtonContent' ButtonContent.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, diff --git a/src/elements/Button/ButtonGroup.js b/src/elements/Button/ButtonGroup.js index 06dfc61de9..79a1db17c5 100644 --- a/src/elements/Button/ButtonGroup.js +++ b/src/elements/Button/ButtonGroup.js @@ -19,7 +19,7 @@ import Button from './Button' /** * Buttons can be grouped. */ -function ButtonGroup(props) { +const ButtonGroup = React.forwardRef(function (props, ref) { const { attached, basic, @@ -71,19 +71,20 @@ function ButtonGroup(props) { if (_.isNil(buttons)) { return ( - + {childrenUtils.isNil(children) ? content : children} ) } return ( - + {_.map(buttons, (button) => Button.create(button))} ) -} +}) +ButtonGroup.displayName = 'ButtonGroup' ButtonGroup.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, diff --git a/src/elements/Button/ButtonOr.js b/src/elements/Button/ButtonOr.js index 834d088549..89b05a80fa 100644 --- a/src/elements/Button/ButtonOr.js +++ b/src/elements/Button/ButtonOr.js @@ -7,15 +7,16 @@ import { getElementType, getUnhandledProps } from '../../lib' /** * Button groups can contain conditionals. */ -function ButtonOr(props) { +const ButtonOr = React.forwardRef(function (props, ref) { const { className, text } = props const classes = cx('or', className) const rest = getUnhandledProps(ButtonOr, props) const ElementType = getElementType(ButtonOr, props) - return -} + return +}) +ButtonOr.displayName = 'ButtonOr' ButtonOr.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, diff --git a/src/lib/hooks/useMergedRefs.js b/src/lib/hooks/useMergedRefs.js new file mode 100644 index 0000000000..ae4af02565 --- /dev/null +++ b/src/lib/hooks/useMergedRefs.js @@ -0,0 +1,38 @@ +import * as React from 'react' + +/** + * @param {React.Ref} ref + * @param {HTMLElement} value + */ +function setRef(ref, value) { + if (typeof ref === 'function') { + ref(value) + } else if (ref) { + // eslint-disable-next-line no-param-reassign + ref.current = value + } +} + +/** + * React hook to merge multiple React refs (either MutableRefObjects or ref callbacks) into a single ref callback that + * updates all provided refs. + * + * @param {React.Ref} refA + * @param {React.Ref} refB + * + * @return {React.Ref} A function with an attached "current" prop, so that it can be treated like a React.RefObject. + */ +export default function useMergedRefs(refA, refB) { + const mergedCallback = React.useCallback( + (value) => { + // Update the "current" prop hanging on the function. + mergedCallback.current = value + + setRef(refA, value) + setRef(refB, value) + }, + [refA, refB], + ) + + return mergedCallback +} diff --git a/src/lib/index.js b/src/lib/index.js index 2b855718f6..597ccae801 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -49,3 +49,4 @@ export { makeDebugger } export useAutoControlledValue from './hooks/useAutoControlledValue' export useClassNamesOnNode from './hooks/useClassNamesOnNode' export useEventCallback from './hooks/useEventCallback' +export useMergedRefs from './hooks/useMergedRefs' diff --git a/test/specs/elements/Button/Button-test.js b/test/specs/elements/Button/Button-test.js index b6386c195f..c516c793c4 100644 --- a/test/specs/elements/Button/Button-test.js +++ b/test/specs/elements/Button/Button-test.js @@ -1,3 +1,4 @@ +import faker from 'faker' import React from 'react' import Button from 'src/elements/Button/Button' @@ -12,6 +13,8 @@ const syntheticEvent = { preventDefault: () => undefined } describe('Button', () => { common.isConformant(Button) + common.forwardsRef(Button, { tagName: 'button' }) + common.forwardsRef(Button, { requiredProps: { label: faker.lorem.word() }, tagName: 'button' }) common.hasSubcomponents(Button, [ButtonContent, ButtonGroup, ButtonOr]) common.hasUIClassName(Button) common.rendersChildren(Button) @@ -97,22 +100,6 @@ describe('Button', () => { }) }) - describe('focus', () => { - it('can be set via a ref', () => { - const mountNode = document.createElement('div') - document.body.appendChild(mountNode) - - const wrapper = mount(