Skip to content

Commit 6396cf2

Browse files
committed
chore(Search): use React.forwardRef() (#4330)
* chore(Search): use React.forwardRef() * add test for ref forwarding * fix tests, add comments
1 parent 5f8b882 commit 6396cf2

File tree

6 files changed

+101
-67
lines changed

6 files changed

+101
-67
lines changed

gulp/plugins/util/getComponentInfo.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const getComponentInfo = (filepath) => {
1818
const filename = path.basename(absPath)
1919
const filenameWithoutExt = path.basename(absPath, path.extname(absPath))
2020

21+
const componentName = path.parse(filename).name
2122
// singular form of the component's ../../ directory
2223
// "element" for "src/elements/Button/Button.js"
2324
const componentType = path.basename(path.dirname(dir)).replace(/s$/, '')
@@ -27,18 +28,21 @@ const getComponentInfo = (filepath) => {
2728
...defaultHandlers,
2829
parserCustomHandler,
2930
])
31+
3032
if (!components.length) {
3133
throw new Error(`Could not find a component definition in "${filepath}".`)
3234
}
33-
if (components.length > 1) {
35+
36+
const info = components.find((component) => component.displayName === componentName)
37+
38+
if (!info) {
3439
throw new Error(
3540
[
36-
`Found more than one component definition in "${filepath}".`,
37-
'This is currently not supported, please ensure your module only defines a single React component.',
41+
`Failed to find a component definition for "${componentName}" in "${filepath}".`,
42+
'Please ensure your module defines matching React component.',
3843
].join(' '),
3944
)
4045
}
41-
const info = components[0]
4246

4347
// remove keys we don't use
4448
delete info.methods

src/lib/getUnhandledProps.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ const getUnhandledProps = (Component, props) => {
1010
const { handledProps = [] } = Component
1111

1212
return Object.keys(props).reduce((acc, prop) => {
13-
if (prop === 'childKey') return acc
13+
// "childKey" and "innerRef" are internal props of Semantic UI React
14+
// "innerRef" can be removed when "Search" & "Dropdown components will be removed to be functional
15+
if (prop === 'childKey' || prop === 'innerRef') return acc
1416
if (handledProps.indexOf(prop) === -1) acc[prop] = props[prop]
1517
return acc
1618
}, {})

src/modules/Search/Search.js

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,11 @@ const overrideSearchInputProps = (predefinedProps) => {
4444
/**
4545
* A search module allows a user to query for results from a selection of data
4646
*/
47-
export default class Search extends Component {
47+
const Search = React.forwardRef((props, ref) => {
48+
return <SearchInner {...props} innerRef={ref} />
49+
})
50+
51+
class SearchInner extends Component {
4852
static getAutoControlledStateFromProps(props, state) {
4953
debug('getAutoControlledStateFromProps()')
5054

@@ -476,9 +480,9 @@ export default class Search extends Component {
476480
debug('render()')
477481
debug('props', this.props)
478482
debug('state', this.state)
479-
const { searchClasses, focus, open } = this.state
480483

481-
const { aligned, category, className, fluid, loading, size } = this.props
484+
const { searchClasses, focus, open } = this.state
485+
const { aligned, category, className, innerRef, fluid, loading, size } = this.props
482486

483487
// Classes
484488
const classes = cx(
@@ -507,6 +511,7 @@ export default class Search extends Component {
507511
onBlur={this.handleBlur}
508512
onFocus={this.handleFocus}
509513
onMouseDown={this.handleMouseDown}
514+
ref={innerRef}
510515
>
511516
{this.renderSearchInput(htmlInputProps)}
512517
{this.renderResultsMenu()}
@@ -515,6 +520,7 @@ export default class Search extends Component {
515520
}
516521
}
517522

523+
Search.displayName = 'Search'
518524
Search.propTypes = {
519525
/** An element type to render as (string or function). */
520526
as: PropTypes.elementType,
@@ -681,9 +687,16 @@ Search.defaultProps = {
681687
showNoResults: true,
682688
}
683689

684-
Search.autoControlledProps = ['open', 'value']
690+
SearchInner.autoControlledProps = ['open', 'value']
691+
692+
if (process.env.NODE_ENV !== 'production') {
693+
SearchInner.defaultProps = Search.defaultProps
694+
SearchInner.propTypes = Search.propTypes
695+
}
685696

686697
Search.Category = SearchCategory
687698
Search.CategoryLayout = SearchCategoryLayout
688699
Search.Result = SearchResult
689700
Search.Results = SearchResults
701+
702+
export default Search

test/specs/commonTests/hasUIClassName.js

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,16 @@ import helpers from './commonHelpers'
55
* Assert a component adds the Semantic UI "ui" className.
66
* @param {React.Component|Function} Component The Component.
77
* @param {Object} [options={}]
8-
* @param {Number} [options.nestingLevel=0] The nesting level of the component.
98
* @param {Object} [options.requiredProps={}] Props required to render the component.
109
*/
1110
export default (Component, options = {}) => {
12-
const { nestingLevel = 0, requiredProps = {} } = options
11+
const { requiredProps = {} } = options
1312
const { assertRequired } = helpers('hasUIClassName', Component)
1413

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

18-
shallow(<Component {...requiredProps} />, {
19-
autoNesting: true,
20-
nestingLevel,
21-
}).should.have.className('ui')
18+
wrapper.should.have.className('ui')
2219
})
2320
}

test/specs/commonTests/implementsClassNameProps.js

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { createElement } from 'react'
1+
import React from 'react'
22
import _ from 'lodash'
33

44
import { consoleUtil } from 'test/utils'
@@ -37,12 +37,11 @@ export const propKeyAndValueToClassName = (Component, propKey, propValues, optio
3737
* @param {String} propKey A props key.
3838
* @param {Object} [options={}]
3939
* @param {Object} [options.className=propKey] The className to assert exists.
40-
* @param {Number} [options.nestingLevel=0] The nesting level of the component.
4140
* @param {Object} [options.requiredProps={}] Props required to render the component.
4241
* @param {Object} [options.className=propKey] The className to assert exists.
4342
*/
4443
export const propKeyOnlyToClassName = (Component, propKey, options = {}) => {
45-
const { className = propKey, nestingLevel = 0, requiredProps = {} } = options
44+
const { className = propKey, requiredProps = {} } = options
4645
const { assertRequired } = helpers('propKeyOnlyToClassName', Component)
4746

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

5453
it('adds prop name to className', () => {
5554
consoleUtil.disableOnce()
56-
shallow(createElement(Component, { ...requiredProps, [propKey]: true }), {
57-
autoNesting: true,
58-
nestingLevel,
59-
}).should.have.className(className)
55+
56+
const element = React.createElement(Component, { ...requiredProps, [propKey]: true })
57+
const wrapper = mount(element)
58+
59+
// ".should.have.className" with "mount" renderer does not handle properly cases when "className" contains
60+
// multiple classes. That's why ".split()" is required.
61+
className.split(' ').forEach((classNamePart) => {
62+
wrapper.childAt(0).should.have.className(classNamePart)
63+
})
6064
})
6165

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

6569
const value = 'foo-bar-baz'
66-
shallow(createElement(Component, { ...requiredProps, [propKey]: value }), {
67-
autoNesting: true,
68-
nestingLevel,
69-
}).should.not.have.className(value)
70+
const element = React.createElement(Component, { ...requiredProps, [propKey]: value })
71+
const wrapper = mount(element)
72+
73+
wrapper.childAt(0).should.not.have.className(value)
7074
})
7175
})
7276
}
@@ -96,13 +100,15 @@ export const propKeyOrValueAndKeyToClassName = (Component, propKey, propValues,
96100
})
97101

98102
it('adds only the name to className when true', () => {
99-
shallow(createElement(Component, { ...requiredProps, [propKey]: true }), {
103+
shallow(React.createElement(Component, { ...requiredProps, [propKey]: true }), {
100104
autoNesting: true,
101105
}).should.have.className(className)
102106
})
103107

104108
it('adds no className when false', () => {
105-
const wrapper = shallow(createElement(Component, { ...requiredProps, [propKey]: false }))
109+
const wrapper = shallow(
110+
React.createElement(Component, { ...requiredProps, [propKey]: false }),
111+
)
106112

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

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

151157
propValues.forEach((propValue) => {
152-
shallow(createElement(Component, { ...requiredProps, [propKey]: propValue }), {
158+
shallow(React.createElement(Component, { ...requiredProps, [propKey]: propValue }), {
153159
autoNesting: true,
154160
nestingLevel,
155161
}).should.not.have.className(propKey)

0 commit comments

Comments
 (0)