Repository navigation
ENH Convert react option class components to functional - #2207
emteknetnz wants to merge 1 commit into
Conversation
| return inputProps; | ||
| } | ||
| const OptionField = (_props) => { | ||
| const defaultProps = { |
There was a problem hiding this comment.
Moved the class OptionField.defaultProps into a local defaultProps object that is merged under _props, because a function component should not use defaultProps. The values are unchanged: extraClass: '', className: '', type: 'radio', leftTitle: null and rightTitle: null, and the comment about uncontrolled components is kept.
| */ | ||
| handleChange(event) { | ||
| if (this.props.readOnly || this.props.disabled) { | ||
| const handleChange = (event) => { |
There was a problem hiding this comment.
Moved above getInputProps, which references it, because the no-use-before-define lint rule rejects the class order once the methods are const arrow functions.
| import 'expose-loader?exposes=VersionedBadge!components/VersionedBadge/VersionedBadge'; | ||
| import 'expose-loader?exposes=CheckboxSetField!components/CheckboxSetField/CheckboxSetField'; | ||
| import 'expose-loader?exposes=LegacyCheckboxSetField!legacy/ReactComponents/LegacyCheckboxSetField'; | ||
| import 'expose-loader?exposes=LegacyOptionField!legacy/ReactComponents/LegacyOptionField'; |
There was a problem hiding this comment.
Added the expose line for the new LegacyOptionField class copy, so modules that need the class version can still get it. OptionField had no expose line of its own, so this sits after the LegacyCheckboxSetField line.
There was a problem hiding this comment.
Which modules need it? This wasn't exposed before so no other modules should be using it? Same with below.
| import castStringToElement from 'lib/castStringToElement'; | ||
| import PropTypes from 'prop-types'; | ||
|
|
||
| class LegacyOptionField extends Component { |
There was a problem hiding this comment.
Added a copy of the original OptionField class for dual support. It is identical to the pre-conversion file except for the Legacy rename of the class, its propTypes and defaultProps assignments, and the exports.
| this.handleChange = this.handleChange.bind(this); | ||
| } | ||
| const OptionsetField = (_props) => { | ||
| const defaultProps = { |
There was a problem hiding this comment.
Moved the static OptionsetField.defaultProps into the function and merged it under _props, because function components should not rely on defaultProps. The values are unchanged: extraClass: '', className: '' and itemClass: '', with the original comment kept above them.
| { searchValue: '' } | ||
| ); | ||
| } | ||
| const handleToggle = () => { |
There was a problem hiding this comment.
Moved handleToggle below doToggle, which it calls, for the same no-use-before-define reason.
| <div className={classNames(props.buttonContainerClassName)}> | ||
| {buttonsToRender.map((button) => ( | ||
| <ButtonComponent | ||
| <props.ButtonComponent |
There was a problem hiding this comment.
Rendered ButtonComponent and PopoverComponent as <props.ButtonComponent> and <props.PopoverComponent>, because props are not destructured in the function component. JSX treats a member expression as a component reference, so the rendered output is the same.
| import 'expose-loader?exposes=LegacyCheckboxSetField!legacy/ReactComponents/LegacyCheckboxSetField'; | ||
| import 'expose-loader?exposes=LegacyOptionField!legacy/ReactComponents/LegacyOptionField'; | ||
| import 'expose-loader?exposes=LegacyOptionsetField!legacy/ReactComponents/LegacyOptionsetField'; | ||
| import 'expose-loader?exposes=LegacyPopoverOptionSet!legacy/ReactComponents/LegacyPopoverOptionSet'; |
There was a problem hiding this comment.
Added the expose line for the new LegacyPopoverOptionSet copy after LegacyOptionsetField. PopoverOptionSet itself had no expose line before, so none was added for the function component.
| constructor(props) { | ||
| super(props); | ||
| const PopoverOptionSetToggle = (_props) => { | ||
| const defaultProps = { |
There was a problem hiding this comment.
Moved the default from PopoverOptionSetToggle.defaultProps into a local defaultProps object that is merged under _props, because function components should not use defaultProps. The value is unchanged: toggleText: i18n._t('PopoverOptionSetToggle.TOGGLE', 'Toggle').
| // eslint-disable-next-line react/no-access-state-in-setstate | ||
| window.setTimeout(() => this.setState({ isOpen: !this.state.isOpen }), 0); | ||
| } | ||
| window.setTimeout(() => setIsOpen((prevIsOpen) => !prevIsOpen), 0); |
There was a problem hiding this comment.
Changed this.setState({ isOpen: !this.state.isOpen }) to an updater function, because the timeout callback in a function component would otherwise read the isOpen value from the render that created handleToggle. Removed the react/no-access-state-in-setstate suppression and its comment, since the rule no longer applies. That comment said the updater form once caused Behat failures. The two forms differ only when two queued toggles run before React commits the first one, which the Jest tests do not cover, so the Behat scenarios that open this toggle should be run before merge.
369405c to
be82b9f
Compare
| import 'expose-loader?exposes=VersionedBadge!components/VersionedBadge/VersionedBadge'; | ||
| import 'expose-loader?exposes=CheckboxSetField!components/CheckboxSetField/CheckboxSetField'; | ||
| import 'expose-loader?exposes=LegacyCheckboxSetField!legacy/ReactComponents/LegacyCheckboxSetField'; | ||
| import 'expose-loader?exposes=LegacyOptionField!legacy/ReactComponents/LegacyOptionField'; |
There was a problem hiding this comment.
Which modules need it? This wasn't exposed before so no other modules should be using it? Same with below.
be82b9f to
88b214b
Compare
|
#2206 (comment) and #2206 (comment) are probably relevant to the question you asked. |
Issue #2200
Kitchen sink CI with PR - https://github.com/creative-commoners/recipe-kitchen-sink/actions/runs/37764955873