Skip to content

Commit

Permalink
chore(Search): use React.forwardRef() (#4330)
Browse files Browse the repository at this point in the history
* chore(Search): use React.forwardRef()

* add test for ref forwarding

* fix tests, add comments
  • Loading branch information
layershifter authored Feb 8, 2022
1 parent 3cecf2f commit abd59d0
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 67 deletions.
12 changes: 8 additions & 4 deletions gulp/plugins/util/getComponentInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const getComponentInfo = (filepath) => {
const filename = path.basename(absPath)
const filenameWithoutExt = path.basename(absPath, path.extname(absPath))

const componentName = path.parse(filename).name
// singular form of the component's ../../ directory
// "element" for "src/elements/Button/Button.js"
const componentType = path.basename(path.dirname(dir)).replace(/s$/, '')
Expand All @@ -27,18 +28,21 @@ const getComponentInfo = (filepath) => {
...defaultHandlers,
parserCustomHandler,
])

if (!components.length) {
throw new Error(`Could not find a component definition in "${filepath}".`)
}
if (components.length > 1) {

const info = components.find((component) => component.displayName === componentName)

if (!info) {
throw new Error(
[
`Found more than one component definition in "${filepath}".`,
'This is currently not supported, please ensure your module only defines a single React component.',
`Failed to find a component definition for "${componentName}" in "${filepath}".`,
'Please ensure your module defines matching React component.',
].join(' '),
)
}
const info = components[0]

// remove keys we don't use
delete info.methods
Expand Down
4 changes: 3 additions & 1 deletion src/lib/getUnhandledProps.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ const getUnhandledProps = (Component, props) => {
const { handledProps = [] } = Component

return Object.keys(props).reduce((acc, prop) => {
if (prop === 'childKey') return acc
// "childKey" and "innerRef" are internal props of Semantic UI React
// "innerRef" can be removed when "Search" & "Dropdown components will be removed to be functional
if (prop === 'childKey' || prop === 'innerRef') return acc
if (handledProps.indexOf(prop) === -1) acc[prop] = props[prop]
return acc
}, {})
Expand Down
21 changes: 17 additions & 4 deletions src/modules/Search/Search.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ const overrideSearchInputProps = (predefinedProps) => {
/**
* A search module allows a user to query for results from a selection of data
*/
export default class Search extends Component {
const Search = React.forwardRef((props, ref) => {
return <SearchInner {...props} innerRef={ref} />
})

class SearchInner extends Component {
static getAutoControlledStateFromProps(props, state) {
debug('getAutoControlledStateFromProps()')

Expand Down Expand Up @@ -476,9 +480,9 @@ export default class Search extends Component {
debug('render()')
debug('props', this.props)
debug('state', this.state)
const { searchClasses, focus, open } = this.state

const { aligned, category, className, fluid, loading, size } = this.props
const { searchClasses, focus, open } = this.state
const { aligned, category, className, innerRef, fluid, loading, size } = this.props

// Classes
const classes = cx(
Expand Down Expand Up @@ -507,6 +511,7 @@ export default class Search extends Component {
onBlur={this.handleBlur}
onFocus={this.handleFocus}
onMouseDown={this.handleMouseDown}
ref={innerRef}
>
{this.renderSearchInput(htmlInputProps)}
{this.renderResultsMenu()}
Expand All @@ -515,6 +520,7 @@ export default class Search extends Component {
}
}

Search.displayName = 'Search'
Search.propTypes = {
/** An element type to render as (string or function). */
as: PropTypes.elementType,
Expand Down Expand Up @@ -681,9 +687,16 @@ Search.defaultProps = {
showNoResults: true,
}

Search.autoControlledProps = ['open', 'value']
SearchInner.autoControlledProps = ['open', 'value']

if (process.env.NODE_ENV !== 'production') {
SearchInner.defaultProps = Search.defaultProps
SearchInner.propTypes = Search.propTypes
}

Search.Category = SearchCategory
Search.CategoryLayout = SearchCategoryLayout
Search.Result = SearchResult
Search.Results = SearchResults

export default Search
9 changes: 3 additions & 6 deletions test/specs/commonTests/hasUIClassName.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,16 @@ import helpers from './commonHelpers'
* Assert a component adds the Semantic UI "ui" className.
* @param {React.Component|Function} Component The Component.
* @param {Object} [options={}]
* @param {Number} [options.nestingLevel=0] The nesting level of the component.
* @param {Object} [options.requiredProps={}] Props required to render the component.
*/
export default (Component, options = {}) => {
const { nestingLevel = 0, requiredProps = {} } = options
const { requiredProps = {} } = options
const { assertRequired } = helpers('hasUIClassName', Component)

it('has the "ui" className', () => {
assertRequired(Component, 'a `Component`')
const wrapper = mount(<Component {...requiredProps} />)

shallow(<Component {...requiredProps} />, {
autoNesting: true,
nestingLevel,
}).should.have.className('ui')
wrapper.should.have.className('ui')
})
}
36 changes: 21 additions & 15 deletions test/specs/commonTests/implementsClassNameProps.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createElement } from 'react'
import React from 'react'
import _ from 'lodash'

import { consoleUtil } from 'test/utils'
Expand Down Expand Up @@ -37,12 +37,11 @@ export const propKeyAndValueToClassName = (Component, propKey, propValues, optio
* @param {String} propKey A props key.
* @param {Object} [options={}]
* @param {Object} [options.className=propKey] The className to assert exists.
* @param {Number} [options.nestingLevel=0] The nesting level of the component.
* @param {Object} [options.requiredProps={}] Props required to render the component.
* @param {Object} [options.className=propKey] The className to assert exists.
*/
export const propKeyOnlyToClassName = (Component, propKey, options = {}) => {
const { className = propKey, nestingLevel = 0, requiredProps = {} } = options
const { className = propKey, requiredProps = {} } = options
const { assertRequired } = helpers('propKeyOnlyToClassName', Component)

describe(`${propKey} (common)`, () => {
Expand All @@ -53,20 +52,25 @@ export const propKeyOnlyToClassName = (Component, propKey, options = {}) => {

it('adds prop name to className', () => {
consoleUtil.disableOnce()
shallow(createElement(Component, { ...requiredProps, [propKey]: true }), {
autoNesting: true,
nestingLevel,
}).should.have.className(className)

const element = React.createElement(Component, { ...requiredProps, [propKey]: true })
const wrapper = mount(element)

// ".should.have.className" with "mount" renderer does not handle properly cases when "className" contains
// multiple classes. That's why ".split()" is required.
className.split(' ').forEach((classNamePart) => {
wrapper.childAt(0).should.have.className(classNamePart)
})
})

it('does not add prop value to className', () => {
consoleUtil.disableOnce()

const value = 'foo-bar-baz'
shallow(createElement(Component, { ...requiredProps, [propKey]: value }), {
autoNesting: true,
nestingLevel,
}).should.not.have.className(value)
const element = React.createElement(Component, { ...requiredProps, [propKey]: value })
const wrapper = mount(element)

wrapper.childAt(0).should.not.have.className(value)
})
})
}
Expand Down Expand Up @@ -96,13 +100,15 @@ export const propKeyOrValueAndKeyToClassName = (Component, propKey, propValues,
})

it('adds only the name to className when true', () => {
shallow(createElement(Component, { ...requiredProps, [propKey]: true }), {
shallow(React.createElement(Component, { ...requiredProps, [propKey]: true }), {
autoNesting: true,
}).should.have.className(className)
})

it('adds no className when false', () => {
const wrapper = shallow(createElement(Component, { ...requiredProps, [propKey]: false }))
const wrapper = shallow(
React.createElement(Component, { ...requiredProps, [propKey]: false }),
)

wrapper.should.not.have.className(className)
wrapper.should.not.have.className('true')
Expand Down Expand Up @@ -138,7 +144,7 @@ export const propValueOnlyToClassName = (Component, propKey, propValues, options

it('adds prop value to className', () => {
propValues.forEach((propValue) => {
shallow(createElement(Component, { ...requiredProps, [propKey]: propValue }), {
shallow(React.createElement(Component, { ...requiredProps, [propKey]: propValue }), {
autoNesting: true,
nestingLevel,
}).should.have.className(propValue)
Expand All @@ -149,7 +155,7 @@ export const propValueOnlyToClassName = (Component, propKey, propValues, options
consoleUtil.disableOnce()

propValues.forEach((propValue) => {
shallow(createElement(Component, { ...requiredProps, [propKey]: propValue }), {
shallow(React.createElement(Component, { ...requiredProps, [propKey]: propValue }), {
autoNesting: true,
nestingLevel,
}).should.not.have.className(propKey)
Expand Down
Loading

0 comments on commit abd59d0

Please sign in to comment.