diff --git a/src/lib/hooks/useAutoControlledValue.js b/src/lib/hooks/useAutoControlledValue.js new file mode 100644 index 0000000000..f211badf1d --- /dev/null +++ b/src/lib/hooks/useAutoControlledValue.js @@ -0,0 +1,82 @@ +import * as React from 'react' + +/** + * Helper hook to handle previous comparison of controlled/uncontrolled. Prints an error when "isControlled" value + * switches between subsequent renders. + */ +function useIsControlled(controlledValue) { + const [isControlled] = React.useState(controlledValue !== undefined) + + if (process.env.NODE_ENV !== 'production') { + // We don't want these warnings in production even though it is against native behaviour + React.useEffect(() => { + if (isControlled !== (controlledValue !== undefined)) { + const error = new Error() + + const controlWarning = isControlled + ? 'a controlled value to be uncontrolled' + : 'an uncontrolled value to be controlled' + const undefinedWarning = isControlled ? 'defined to an undefined' : 'undefined to a defined' + + // eslint-disable-next-line no-console + console.error( + [ + // Default react error + `A component is changing ${controlWarning}'. This is likely caused by the value changing from `, + `${undefinedWarning} value, which should not happen. Decide between using a controlled or uncontrolled `, + 'input element for the lifetime of the component.', + 'More info: https://reactjs.org/link/controlled-components', + error.stack, + ].join(' '), + ) + } + }, [isControlled, controlledValue]) + } + + return isControlled +} + +/** + * A hook that allows optional user control, implements an interface similar to `React.useState()`. + * Useful for components which allow uncontrolled and controlled behaviours for users. + * + * - defaultState - default state or factory initializer + * - state - controllable state, undefined state means internal state will be used + * - initialState - Used to initialize state if all user provided states are undefined + * + * @param {{ defaultState?: any, state: any, initialState: any }} options + * + * @see https://reactjs.org/docs/uncontrolled-components.html + * @see https://reactjs.org/docs/hooks-state.html + */ +function useAutoControlledValue(options) { + const isControlled = useIsControlled(options.state) + const initialState = + typeof options.defaultState === 'undefined' ? options.initialState : options.defaultState + + const [internalState, setInternalState] = React.useState(initialState) + const state = isControlled ? options.state : internalState + const stateRef = React.useRef(state) + + React.useEffect(() => { + stateRef.current = state + }, [state]) + + // To match the behavior of the setter returned by React.useState, this callback's identity + // should never change. This means it MUST NOT directly reference variables that can change. + const setState = React.useCallback((newState) => { + // React dispatch can use a factory + // https://reactjs.org/docs/hooks-reference.html#functional-updates + if (typeof newState === 'function') { + stateRef.current = newState(stateRef.current) + } else { + stateRef.current = newState + } + + setInternalState(stateRef.current) + }, []) + + return [state, setState] +} + +export default useAutoControlledValue diff --git a/src/lib/index.js b/src/lib/index.js index 41a8d97444..2b855718f6 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -46,5 +46,6 @@ export { makeDebugger } // Hooks // +export useAutoControlledValue from './hooks/useAutoControlledValue' export useClassNamesOnNode from './hooks/useClassNamesOnNode' export useEventCallback from './hooks/useEventCallback' diff --git a/src/modules/Accordion/Accordion.js b/src/modules/Accordion/Accordion.js index 33058ab169..4d76ecc440 100644 --- a/src/modules/Accordion/Accordion.js +++ b/src/modules/Accordion/Accordion.js @@ -11,7 +11,7 @@ import AccordionTitle from './AccordionTitle' /** * An accordion allows users to toggle the display of sections of content. */ -function Accordion(props) { +const Accordion = React.forwardRef(function (props, ref) { const { className, fluid, inverted, styled } = props const classes = cx( @@ -23,9 +23,11 @@ function Accordion(props) { ) const rest = getUnhandledProps(Accordion, props) - return -} + // TODO: extract behavior into useAccordion() hook instead of "AccordionAccordion" component + return +}) +Accordion.displayName = 'Accordion' Accordion.propTypes = { /** Additional classes. */ className: PropTypes.string, diff --git a/src/modules/Accordion/AccordionAccordion.js b/src/modules/Accordion/AccordionAccordion.js index ca2dbfbb6e..8b6227d7f5 100644 --- a/src/modules/Accordion/AccordionAccordion.js +++ b/src/modules/Accordion/AccordionAccordion.js @@ -4,97 +4,99 @@ import PropTypes from 'prop-types' import React from 'react' import { - ModernAutoControlledComponent as Component, childrenUtils, createShorthandFactory, customPropTypes, getElementType, getUnhandledProps, + useAutoControlledValue, + useEventCallback, } from '../../lib' import AccordionPanel from './AccordionPanel' -const warnIfPropsAreInvalid = (props, state) => { - const { exclusive } = props - const { activeIndex } = state - - /* eslint-disable no-console */ - if (exclusive && typeof activeIndex !== 'number') { - console.error('`activeIndex` must be a number if `exclusive` is true') - } else if (!exclusive && !_.isArray(activeIndex)) { - console.error('`activeIndex` must be an array if `exclusive` is false') - } - /* eslint-enable no-console */ +/** + * @param {Boolean} exclusive + * @param {Number} activeIndex + * @param {Number} itemIndex + */ +function isIndexActive(exclusive, activeIndex, itemIndex) { + return exclusive ? activeIndex === itemIndex : _.includes(activeIndex, itemIndex) } /** - * An Accordion can contain sub-accordions. + * @param {Boolean} exclusive + * @param {Number} activeIndex + * @param {Number} itemIndex */ -export default class AccordionAccordion extends Component { - getInitialAutoControlledState({ exclusive }) { - return { activeIndex: exclusive ? -1 : [] } - } - - componentDidMount() { - if (process.env.NODE_ENV !== 'production') { - warnIfPropsAreInvalid(this.props, this.state) - } +function computeNewIndex(exclusive, activeIndex, itemIndex) { + if (exclusive) { + return itemIndex === activeIndex ? -1 : itemIndex } - componentDidUpdate() { - if (process.env.NODE_ENV !== 'production') { - warnIfPropsAreInvalid(this.props, this.state) - } + // check to see if index is in array, and remove it, if not then add it + if (_.includes(activeIndex, itemIndex)) { + return _.without(activeIndex, itemIndex) } - computeNewIndex = (index) => { - const { exclusive } = this.props - const { activeIndex } = this.state - - if (exclusive) return index === activeIndex ? -1 : index - - // check to see if index is in array, and remove it, if not then add it - return _.includes(activeIndex, index) ? _.without(activeIndex, index) : [...activeIndex, index] - } + return [...activeIndex, itemIndex] +} - handleTitleClick = (e, titleProps) => { +/** + * An Accordion can contain sub-accordions. + */ +const AccordionAccordion = React.forwardRef(function (props, ref) { + const { className, children, exclusive, panels } = props + const [activeIndex, setActiveIndex] = useAutoControlledValue({ + state: props.activeIndex, + defaultState: props.defaultActiveIndex, + initialState: () => (exclusive ? -1 : []), + }) + + const classes = cx('accordion', className) + const rest = getUnhandledProps(AccordionAccordion, props) + const ElementType = getElementType(AccordionAccordion, props) + + const handleTitleClick = useEventCallback((e, titleProps) => { const { index } = titleProps - this.setState({ activeIndex: this.computeNewIndex(index) }) - _.invoke(this.props, 'onTitleClick', e, titleProps) + setActiveIndex(computeNewIndex(exclusive, activeIndex, index)) + _.invoke(props, 'onTitleClick', e, titleProps) + }) + + if (process.env.NODE_ENV !== 'production') { + React.useEffect(() => { + /* eslint-disable no-console */ + if (exclusive && typeof activeIndex !== 'number') { + console.error('`activeIndex` must be a number if `exclusive` is true') + } else if (!exclusive && !_.isArray(activeIndex)) { + console.error('`activeIndex` must be an array if `exclusive` is false') + } + /* eslint-enable no-console */ + }, [exclusive, activeIndex]) } - isIndexActive = (index) => { - const { exclusive } = this.props - const { activeIndex } = this.state - - return exclusive ? activeIndex === index : _.includes(activeIndex, index) - } + return ( + + {childrenUtils.isNil(children) + ? _.map(panels, (panel, index) => + AccordionPanel.create(panel, { + defaultProps: { + active: isIndexActive(exclusive, activeIndex, index), + index, + onTitleClick: handleTitleClick, + }, + }), + ) + : children} + + ) +}) - render() { - const { className, children, panels } = this.props - - const classes = cx('accordion', className) - const rest = getUnhandledProps(AccordionAccordion, this.props) - const ElementType = getElementType(AccordionAccordion, this.props) - - return ( - - {childrenUtils.isNil(children) - ? _.map(panels, (panel, index) => - AccordionPanel.create(panel, { - defaultProps: { - active: this.isIndexActive(index), - index, - onTitleClick: this.handleTitleClick, - }, - }), - ) - : children} - - ) - } +AccordionAccordion.defaultProps = { + exclusive: true, } +AccordionAccordion.displayName = 'AccordionAccordion' AccordionAccordion.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, @@ -140,10 +142,6 @@ AccordionAccordion.propTypes = { ]), } -AccordionAccordion.defaultProps = { - exclusive: true, -} - -AccordionAccordion.autoControlledProps = ['activeIndex'] - AccordionAccordion.create = createShorthandFactory(AccordionAccordion, (content) => ({ content })) + +export default AccordionAccordion diff --git a/src/modules/Accordion/AccordionContent.js b/src/modules/Accordion/AccordionContent.js index eb9af2d63e..9fae433ee0 100644 --- a/src/modules/Accordion/AccordionContent.js +++ b/src/modules/Accordion/AccordionContent.js @@ -14,19 +14,21 @@ import { /** * A content sub-component for Accordion component. */ -function AccordionContent(props) { +const AccordionContent = React.forwardRef(function (props, ref) { const { active, children, className, content } = props + const classes = cx('content', useKeyOnly(active, 'active'), className) const rest = getUnhandledProps(AccordionContent, props) const ElementType = getElementType(AccordionContent, props) return ( - + {childrenUtils.isNil(children) ? content : children} ) -} +}) +AccordionContent.displayName = 'AccordionContent' AccordionContent.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, diff --git a/src/modules/Accordion/AccordionTitle.js b/src/modules/Accordion/AccordionTitle.js index 5372df666d..f9c7ef7fde 100644 --- a/src/modules/Accordion/AccordionTitle.js +++ b/src/modules/Accordion/AccordionTitle.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, @@ -10,40 +10,42 @@ import { getElementType, getUnhandledProps, useKeyOnly, + useEventCallback, } from '../../lib' import Icon from '../../elements/Icon' /** * A title sub-component for Accordion component. */ -export default class AccordionTitle extends Component { - handleClick = (e) => _.invoke(this.props, 'onClick', e, this.props) +const AccordionTitle = React.forwardRef(function (props, ref) { + const { active, children, className, content, icon } = props - render() { - const { active, children, className, content, icon } = this.props + const classes = cx(useKeyOnly(active, 'active'), 'title', className) + const rest = getUnhandledProps(AccordionTitle, props) + const ElementType = getElementType(AccordionTitle, props) + const iconValue = _.isNil(icon) ? 'dropdown' : icon - const classes = cx(useKeyOnly(active, 'active'), 'title', className) - const rest = getUnhandledProps(AccordionTitle, this.props) - const ElementType = getElementType(AccordionTitle, this.props) - const iconValue = _.isNil(icon) ? 'dropdown' : icon - - if (!childrenUtils.isNil(children)) { - return ( - - {children} - - ) - } + const handleClick = useEventCallback((e) => { + _.invoke(props, 'onClick', e, props) + }) + if (!childrenUtils.isNil(children)) { return ( - - {Icon.create(iconValue, { autoGenerateKey: false })} - {content} + + {children} ) } -} + return ( + + {Icon.create(iconValue, { autoGenerateKey: false })} + {content} + + ) +}) + +AccordionTitle.displayName = 'AccordionTitle' AccordionTitle.propTypes = { /** An element type to render as (string or function). */ as: PropTypes.elementType, @@ -75,3 +77,5 @@ AccordionTitle.propTypes = { onClick: PropTypes.func, } AccordionTitle.create = createShorthandFactory(AccordionTitle, (content) => ({ content })) + +export default AccordionTitle diff --git a/test/specs/modules/Accordion/Accordion-test.js b/test/specs/modules/Accordion/Accordion-test.js index 19062fba5a..dc506c359e 100644 --- a/test/specs/modules/Accordion/Accordion-test.js +++ b/test/specs/modules/Accordion/Accordion-test.js @@ -9,6 +9,7 @@ import * as common from 'test/specs/commonTests' describe('Accordion', () => { common.isConformant(Accordion) + common.forwardsRef(Accordion) common.hasSubcomponents(Accordion, [ AccordionAccordion, AccordionContent, diff --git a/test/specs/modules/Accordion/AccordionAccordion-test.js b/test/specs/modules/Accordion/AccordionAccordion-test.js index 0577e95a8e..a986062c52 100644 --- a/test/specs/modules/Accordion/AccordionAccordion-test.js +++ b/test/specs/modules/Accordion/AccordionAccordion-test.js @@ -8,6 +8,7 @@ import { consoleUtil, sandbox } from 'test/utils' describe('AccordionAccordion', () => { common.isConformant(AccordionAccordion) + common.forwardsRef(AccordionAccordion) common.rendersChildren(AccordionAccordion, { rendersContent: false, }) diff --git a/test/specs/modules/Accordion/AccordionContent-test.js b/test/specs/modules/Accordion/AccordionContent-test.js index 8b11c47412..aa5df5a655 100644 --- a/test/specs/modules/Accordion/AccordionContent-test.js +++ b/test/specs/modules/Accordion/AccordionContent-test.js @@ -3,6 +3,7 @@ import * as common from 'test/specs/commonTests' describe('AccordionContent', () => { common.isConformant(AccordionContent) + common.forwardsRef(AccordionContent) common.rendersChildren(AccordionContent) common.implementsCreateMethod(AccordionContent) diff --git a/test/specs/modules/Accordion/AccordionTitle-test.js b/test/specs/modules/Accordion/AccordionTitle-test.js index 5f80d41fa1..0ed8e59bbd 100644 --- a/test/specs/modules/Accordion/AccordionTitle-test.js +++ b/test/specs/modules/Accordion/AccordionTitle-test.js @@ -6,6 +6,7 @@ import { sandbox } from 'test/utils' describe('AccordionTitle', () => { common.isConformant(AccordionTitle) + common.forwardsRef(AccordionTitle) common.rendersChildren(AccordionTitle) common.implementsCreateMethod(AccordionTitle) @@ -18,14 +19,14 @@ describe('AccordionTitle', () => { describe('onClick', () => { it('is called with (e, { name, index }) when clicked', () => { - const spy = sandbox.spy() + const onClick = sandbox.spy() const event = { target: null } const props = { content: 'title', index: 0 } - shallow().simulate('click', event) + mount().simulate('click', event) - spy.should.have.been.calledOnce() - spy.should.have.been.calledWithMatch(event, props) + onClick.should.have.been.calledOnce() + onClick.should.have.been.calledWithMatch(event, props) }) }) })