From 87f4310064865907ba4cae159f71380c2224c22d Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Thu, 29 Jul 2021 17:50:01 +0200 Subject: [PATCH] chore(Input): use React.forwardRef() (#4267) --- src/elements/Input/Input.js | 226 ++++++++++++------------ src/lib/hooks/useMergedRefs.js | 4 +- src/lib/index.js | 2 +- test/specs/elements/Input/Input-test.js | 109 ++++++------ 4 files changed, 173 insertions(+), 168 deletions(-) diff --git a/src/elements/Input/Input.js b/src/elements/Input/Input.js index 59197d6fae..7f830a4a65 100644 --- a/src/elements/Input/Input.js +++ b/src/elements/Input/Input.js @@ -1,8 +1,7 @@ -import { handleRef } from '@fluentui/react-component-ref' import cx from 'clsx' import _ from 'lodash' import PropTypes from 'prop-types' -import React, { Children, cloneElement, Component, createRef } from 'react' +import React from 'react' import { childrenUtils, @@ -14,6 +13,7 @@ import { partitionHTMLProps, useKeyOnly, useValueAndKey, + setRef, } from '../../lib' import Button from '../Button' import Icon from '../Icon' @@ -26,146 +26,144 @@ import Label from '../Label' * @see Icon * @see Label */ -class Input extends Component { - inputRef = createRef() - - computeIcon = () => { - const { loading, icon } = this.props +const Input = React.forwardRef(function (props, ref) { + const { + action, + actionPosition, + children, + className, + disabled, + error, + fluid, + focus, + icon, + iconPosition, + input, + inverted, + label, + labelPosition, + loading, + size, + tabIndex, + transparent, + type, + } = props + + const computeIcon = () => { + if (!_.isNil(icon)) { + return icon + } - if (!_.isNil(icon)) return icon - if (loading) return 'spinner' + if (loading) { + return 'spinner' + } } - computeTabIndex = () => { - const { disabled, tabIndex } = this.props + const computeTabIndex = () => { + if (!_.isNil(tabIndex)) { + return tabIndex + } - if (!_.isNil(tabIndex)) return tabIndex - if (disabled) return -1 + if (disabled) { + return -1 + } } - focus = () => this.inputRef.current.focus() + const handleChange = (e) => { + const newValue = _.get(e, 'target.value') - select = () => this.inputRef.current.select() - - handleChange = (e) => { - const value = _.get(e, 'target.value') - - _.invoke(this.props, 'onChange', e, { ...this.props, value }) + _.invoke(props, 'onChange', e, { ...props, value: newValue }) } - handleChildOverrides = (child, defaultProps) => ({ - ...defaultProps, - ...child.props, - ref: (c) => { - handleRef(child.ref, c) - this.inputRef.current = c - }, - }) - - partitionProps = () => { - const { disabled, type } = this.props - - const tabIndex = this.computeTabIndex() - const unhandled = getUnhandledProps(Input, this.props) - const [htmlInputProps, rest] = partitionHTMLProps(unhandled) + const partitionProps = () => { + const unhandledProps = getUnhandledProps(Input, props) + const [htmlInputProps, rest] = partitionHTMLProps(unhandledProps) return [ { ...htmlInputProps, disabled, type, - tabIndex, - onChange: this.handleChange, - ref: this.inputRef, + tabIndex: computeTabIndex(), + onChange: handleChange, + ref, }, rest, ] } - render() { - const { - action, - actionPosition, - children, - className, - disabled, - error, - fluid, - focus, - icon, - iconPosition, - input, - inverted, - label, - labelPosition, - loading, - size, - transparent, - type, - } = this.props - - const classes = cx( - 'ui', - size, - useKeyOnly(disabled, 'disabled'), - useKeyOnly(error, 'error'), - useKeyOnly(fluid, 'fluid'), - useKeyOnly(focus, 'focus'), - useKeyOnly(inverted, 'inverted'), - useKeyOnly(loading, 'loading'), - useKeyOnly(transparent, 'transparent'), - useValueAndKey(actionPosition, 'action') || useKeyOnly(action, 'action'), - useValueAndKey(iconPosition, 'icon') || useKeyOnly(icon || loading, 'icon'), - useValueAndKey(labelPosition, 'labeled') || useKeyOnly(label, 'labeled'), - 'input', - className, - ) - const ElementType = getElementType(Input, this.props) - const [htmlInputProps, rest] = this.partitionProps() - - // Render with children - // ---------------------------------------- - if (!childrenUtils.isNil(children)) { - // add htmlInputProps to the `` child - const childElements = _.map(Children.toArray(children), (child) => { - if (child.type !== 'input') return child - return cloneElement(child, this.handleChildOverrides(child, htmlInputProps)) - }) - - return ( - - {childElements} - - ) - } - - // Render Shorthand - // ---------------------------------------- - const actionElement = Button.create(action, { autoGenerateKey: false }) - const labelElement = Label.create(label, { - defaultProps: { - className: cx( - 'label', - // add 'left|right corner' - _.includes(labelPosition, 'corner') && labelPosition, - ), - }, - autoGenerateKey: false, + const classes = cx( + 'ui', + size, + useKeyOnly(disabled, 'disabled'), + useKeyOnly(error, 'error'), + useKeyOnly(fluid, 'fluid'), + useKeyOnly(focus, 'focus'), + useKeyOnly(inverted, 'inverted'), + useKeyOnly(loading, 'loading'), + useKeyOnly(transparent, 'transparent'), + useValueAndKey(actionPosition, 'action') || useKeyOnly(action, 'action'), + useValueAndKey(iconPosition, 'icon') || useKeyOnly(icon || loading, 'icon'), + useValueAndKey(labelPosition, 'labeled') || useKeyOnly(label, 'labeled'), + 'input', + className, + ) + const ElementType = getElementType(Input, props) + const [htmlInputProps, rest] = partitionProps() + + // Render with children + // ---------------------------------------- + if (!childrenUtils.isNil(children)) { + // add htmlInputProps to the `` child + const childElements = _.map(React.Children.toArray(children), (child) => { + if (child.type === 'input') { + return React.cloneElement(child, { + ...htmlInputProps, + ...child.props, + ref: (c) => { + setRef(child.ref, c) + setRef(ref, c) + }, + }) + } + + return child }) return ( - {actionPosition === 'left' && actionElement} - {labelPosition !== 'right' && labelElement} - {createHTMLInput(input || type, { defaultProps: htmlInputProps, autoGenerateKey: false })} - {Icon.create(this.computeIcon(), { autoGenerateKey: false })} - {actionPosition !== 'left' && actionElement} - {labelPosition === 'right' && labelElement} + {childElements} ) } -} + // Render Shorthand + // ---------------------------------------- + const actionElement = Button.create(action, { autoGenerateKey: false }) + const labelElement = Label.create(label, { + defaultProps: { + className: cx( + 'label', + // add 'left|right corner' + _.includes(labelPosition, 'corner') && labelPosition, + ), + }, + autoGenerateKey: false, + }) + + return ( + + {actionPosition === 'left' && actionElement} + {labelPosition !== 'right' && labelElement} + {createHTMLInput(input || type, { defaultProps: htmlInputProps, autoGenerateKey: false })} + {Icon.create(computeIcon(), { autoGenerateKey: false })} + {actionPosition !== 'left' && actionElement} + {labelPosition === 'right' && labelElement} + + ) +}) + +Input.displayName = 'Input' Input.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 index ae4af02565..17b569cd77 100644 --- a/src/lib/hooks/useMergedRefs.js +++ b/src/lib/hooks/useMergedRefs.js @@ -1,10 +1,12 @@ import * as React from 'react' /** + * Assigns a value to a React ref. + * * @param {React.Ref} ref * @param {HTMLElement} value */ -function setRef(ref, value) { +export function setRef(ref, value) { if (typeof ref === 'function') { ref(value) } else if (ref) { diff --git a/src/lib/index.js b/src/lib/index.js index 227cd7a1a8..80fb2cadaa 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -50,5 +50,5 @@ export useAutoControlledValue from './hooks/useAutoControlledValue' export useClassNamesOnNode from './hooks/useClassNamesOnNode' export useEventCallback from './hooks/useEventCallback' export useIsomorphicLayoutEffect from './hooks/useIsomorphicLayoutEffect' -export useMergedRefs from './hooks/useMergedRefs' +export useMergedRefs, { setRef } from './hooks/useMergedRefs' export usePrevious from './hooks/usePrevious' diff --git a/test/specs/elements/Input/Input-test.js b/test/specs/elements/Input/Input-test.js index c005931db3..12265b9db3 100644 --- a/test/specs/elements/Input/Input-test.js +++ b/test/specs/elements/Input/Input-test.js @@ -51,6 +51,7 @@ describe('Input', () => { onTouchStart: 'input', }, }) + common.forwardsRef(Input, { tagName: 'input' }) common.hasUIClassName(Input) common.rendersChildren(Input, { rendersContent: false, @@ -129,9 +130,11 @@ describe('Input', () => { const wrapper = shallow() // account for overloading the onChange prop - const expectedValue = propName === 'onChange' ? wrapper.instance().handleChange : propValue - - wrapper.find('input').should.have.prop(propName, expectedValue) + if (propName === 'onChange') { + wrapper.find('input').should.have.prop(propName).to.be.a('function') + } else { + wrapper.find('input').should.have.prop(propName, propValue) + } }) it(`passes \`${propName}\` to the when using children`, () => { @@ -143,45 +146,15 @@ describe('Input', () => { ) // account for overloading the onChange prop - const expectedValue = propName === 'onChange' ? wrapper.instance().handleChange : propValue - - wrapper.find('input').should.have.prop(propName, expectedValue) + if (propName === 'onChange') { + wrapper.find('input').should.have.prop(propName).to.be.a('function') + } else { + wrapper.find('input').should.have.prop(propName, propValue) + } }) }) }) - describe('focus', () => { - it('can be set via a ref', () => { - const mountNode = document.createElement('div') - document.body.appendChild(mountNode) - - const wrapper = mount(, { attachTo: mountNode }) - wrapper.instance().focus() - - const input = document.querySelector('.ui.input input') - document.activeElement.should.equal(input) - - wrapper.detach() - document.body.removeChild(mountNode) - }) - }) - - describe('select', () => { - it('can be set via a ref', () => { - const mountNode = document.createElement('div') - document.body.appendChild(mountNode) - - const value = 'expect this text to be selected' - const wrapper = mount(, { attachTo: mountNode }) - wrapper.instance().select() - - window.getSelection().toString().should.equal(value) - - wrapper.detach() - document.body.removeChild(mountNode) - }) - }) - describe('loading', () => { it("don't add icon if it's defined", () => { shallow() @@ -198,22 +171,22 @@ describe('Input', () => { describe('onChange', () => { it('is called with (e, data) on change', () => { - const spy = sandbox.spy() + const onChange = sandbox.spy() const e = { target: { value: 'name' } } - const props = { 'data-foo': 'bar', onChange: spy } + const props = { 'data-foo': 'bar', onChange } const wrapper = shallow() wrapper.find('input').simulate('change', e) - spy.should.have.been.calledOnce() - spy.should.have.been.calledWithMatch(e, { ...props, value: e.target.value }) + onChange.should.have.been.calledOnce() + onChange.should.have.been.calledWithMatch(e, { ...props, value: e.target.value }) }) it('is called with (e, data) on change when using children', () => { - const spy = sandbox.spy() + const onChange = sandbox.spy() const e = { target: { value: 'name' } } - const props = { 'data-foo': 'bar', onChange: spy } + const props = { 'data-foo': 'bar', onChange } const wrapper = shallow( @@ -223,28 +196,60 @@ describe('Input', () => { wrapper.find('input').simulate('change', e) - spy.should.have.been.calledOnce() - spy.should.have.been.calledWithMatch(e, { ...props, value: e.target.value }) + onChange.should.have.been.calledOnce() + onChange.should.have.been.calledWithMatch(e, { ...props, value: e.target.value }) }) }) describe('ref', () => { + it('"focus" can be set via a ref', () => { + const inputRef = React.createRef() + const mountNode = document.createElement('div') + document.body.appendChild(mountNode) + + const wrapper = mount(, { attachTo: mountNode }) + inputRef.current.focus() + + const input = document.querySelector('.ui.input input') + document.activeElement.should.equal(input) + + wrapper.detach() + document.body.removeChild(mountNode) + }) + + it('"select" can be set via a ref', () => { + const inputRef = React.createRef() + const mountNode = document.createElement('div') + document.body.appendChild(mountNode) + + const value = 'expect this text to be selected' + const wrapper = mount(, { attachTo: mountNode }) + inputRef.current.select() + + window.getSelection().toString().should.equal(value) + + wrapper.detach() + document.body.removeChild(mountNode) + }) + it('maintains ref on child node', () => { - const ref = sandbox.spy() + const elementRef = sandbox.spy() + const inputRef = sandbox.spy() + const mountNode = document.createElement('div') document.body.appendChild(mountNode) const wrapper = mount( - - + + , { attachTo: mountNode }, ) const input = document.querySelector('.ui.input input') - ref.should.have.been.calledOnce() - ref.should.have.been.calledWithMatch(input) - wrapper.instance().inputRef.current.should.equal(input) + elementRef.should.have.been.calledOnce() + elementRef.should.have.been.calledWithMatch(input) + inputRef.should.have.been.calledWithMatch(input) wrapper.detach() document.body.removeChild(mountNode)