From 6f216f7a6403ff2027ef50159a650b19f81edc3b Mon Sep 17 00:00:00 2001 From: Jakub Trzebiatowski Date: Thu, 13 Jul 2023 11:10:41 +0200 Subject: [PATCH 1/4] Migrate BasePicker to function component --- src/components/Picker/BasePicker.js | 325 +++++++++++++--------------- 1 file changed, 155 insertions(+), 170 deletions(-) diff --git a/src/components/Picker/BasePicker.js b/src/components/Picker/BasePicker.js index d7fbb2ed8d6b..f86b8a34811b 100644 --- a/src/components/Picker/BasePicker.js +++ b/src/components/Picker/BasePicker.js @@ -1,5 +1,5 @@ import _ from 'underscore'; -import React, {PureComponent} from 'react'; +import React, {useContext, useEffect, useImperativeHandle, useRef, useState} from 'react'; import {View} from 'react-native'; import PropTypes from 'prop-types'; import RNPickerSelect from 'react-native-picker-select'; @@ -12,6 +12,9 @@ import themeColors from '../../styles/themes/default'; import {ScrollContext} from '../ScrollViewWithContext'; const propTypes = { + /** A forwarded ref */ + forwardedRef: PropTypes.oneOfType([PropTypes.func, PropTypes.shape({current: PropTypes.instanceOf(React.Component)})]).isRequired, + /** BasePicker label */ label: PropTypes.string, @@ -104,204 +107,186 @@ const defaultProps = { additionalPickerEvents: () => {}, }; -/** - * @property {View} root - a reference to the root View - * @property {Object} picker - a reference to @react-native-picker/picker - */ -class BasePicker extends PureComponent { - constructor(props) { - super(props); - this.state = { - isHighlighted: false, - }; - - this.onInputChange = this.onInputChange.bind(this); - this.enableHighlight = this.enableHighlight.bind(this); - this.disableHighlight = this.disableHighlight.bind(this); - this.focus = this.focus.bind(this); - this.measureLayout = this.measureLayout.bind(this); - - // Windows will reuse the text color of the select for each one of the options - // so we might need to color accordingly so it doesn't blend with the background. - this.placeholder = _.isEmpty(this.props.placeholder) - ? {} - : { - ...this.props.placeholder, - color: themeColors.pickerOptionsTextColor, - }; - } +function BasePicker(props) { + const [isHighlighted, setIsHighlighted] = useState(false); - componentDidMount() { - this.setDefaultValue(); - } + // reference to the root View + const root = useRef(null); - componentDidUpdate(prevProps) { - if (prevProps.items === this.props.items) { - return; - } - this.setDefaultValue(); - } + // reference to @react-native-picker/picker + const picker = useRef(null); - /** - * Forms use inputID to set values. But BasePicker passes an index as the second parameter to onInputChange - * We are overriding this behavior to make BasePicker work with Form - * @param {String} value - * @param {Number} index - */ - onInputChange(value, index) { - if (this.props.inputID) { - this.props.onInputChange(value); + // Windows will reuse the text color of the select for each one of the options + // so we might need to color accordingly so it doesn't blend with the background. + const placeholder = _.isEmpty(props.placeholder) + ? {} + : { + ...props.placeholder, + color: themeColors.pickerOptionsTextColor, + }; + + useEffect(() => { + if (props.value || !props.items || props.items.length !== 1 || !props.onInputChange) { return; } - this.props.onInputChange(value, index); - } - - setDefaultValue() { // When there is only 1 element in the selector, we do the user a favor and automatically select it for them // so they don't have to spend extra time selecting the only possible value. - if (this.props.value || !this.props.items || this.props.items.length !== 1 || !this.props.onInputChange) { - return; - } - this.props.onInputChange(this.props.items[0].value, 0); - } + props.onInputChange(props.items[0].value, 0); - enableHighlight() { - this.setState({ - isHighlighted: true, - }); - } - - disableHighlight() { - this.setState({ - isHighlighted: false, - }); - } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [props.items]); - /** - * Focuses the picker (if configured to do so) - * - * This method is used by Form - */ - focus() { - if (!this.props.shouldFocusPicker) { - return; - } - - // Defer the focusing to work around a bug on Mobile Safari, where focusing the `select` element in the same - // task when we scrolled to it left that element in a glitched state, where the dropdown list can't be opened - // until the element gets re-focused - _.defer(() => { - this.picker.focus(); - }); - } + const context = useContext(ScrollContext); /** - * Like measure(), but measures the view relative to an ancestor - * - * This method is used by Form when scrolling to the input - * - * @param {Object} relativeToNativeComponentRef - reference to an ancestor - * @param {function(x: number, y: number, width: number, height: number): void} onSuccess - callback called on success - * @param {function(): void} onFail - callback called on failure + * Forms use inputID to set values. But BasePicker passes an index as the second parameter to onInputChange + * We are overriding this behavior to make BasePicker work with Form + * @param {String} value + * @param {Number} index */ - measureLayout(relativeToNativeComponentRef, onSuccess, onFail) { - if (!this.root) { + const onInputChange = (value, index) => { + if (props.inputID) { + props.onInputChange(value); return; } - this.root.measureLayout(relativeToNativeComponentRef, onSuccess, onFail); - } - - render() { - const hasError = !_.isEmpty(this.props.errorText); - - if (this.props.isDisabled) { - return ( - - {Boolean(this.props.label) && ( - - {this.props.label} - - )} - {this.props.value} - {Boolean(this.props.hintText) && {this.props.hintText}} - - ); - } - + props.onInputChange(value, index); + }; + + const enableHighlight = () => { + setIsHighlighted(true); + }; + + const disableHighlight = () => { + setIsHighlighted(false); + }; + + useImperativeHandle(props.forwardedRef, () => ({ + /** + * Focuses the picker (if configured to do so) + * + * This method is used by Form + */ + focus() { + if (!props.shouldFocusPicker) { + return; + } + + // Defer the focusing to work around a bug on Mobile Safari, where focusing the `select` element in the + // same task when we scrolled to it left that element in a glitched state, where the dropdown list can't + // be opened until the element gets re-focused + _.defer(() => { + picker.current.focus(); + }); + }, + + /** + * Like measure(), but measures the view relative to an ancestor + * + * This method is used by Form when scrolling to the input + * + * @param {Object} relativeToNativeComponentRef - reference to an ancestor + * @param {function(x: number, y: number, width: number, height: number): void} onSuccess - callback called on success + * @param {function(): void} onFail - callback called on failure + */ + measureLayout(relativeToNativeComponentRef, onSuccess, onFail) { + if (!root.current) { + return; + } + + root.current.measureLayout(relativeToNativeComponentRef, onSuccess, onFail); + }, + })); + + const hasError = !_.isEmpty(props.errorText); + + if (props.isDisabled) { return ( - <> - (this.root = el)} - style={[ - styles.pickerContainer, - this.props.isDisabled && styles.inputDisabled, - ...this.props.containerStyles, - this.state.isHighlighted && styles.borderColorFocus, - hasError && styles.borderColorDanger, - ]} - > - {this.props.label && ( - - {this.props.label} - - )} - ({...item, color: themeColors.pickerOptionsTextColor}))} - style={this.props.size === 'normal' ? styles.picker(this.props.isDisabled, this.props.backgroundColor) : styles.pickerSmall(this.props.backgroundColor)} - useNativeAndroidPickerStyle={false} - placeholder={this.placeholder} - value={this.props.value} - Icon={() => this.props.icon(this.props.size)} - disabled={this.props.isDisabled} - fixAndroidTouchableBug - onOpen={this.enableHighlight} - onClose={this.disableHighlight} - textInputProps={{ - allowFontScaling: false, - }} - pickerProps={{ - ref: (el) => (this.picker = el), - onFocus: this.enableHighlight, - onBlur: () => { - this.disableHighlight(); - this.props.onBlur(); - }, - ...this.props.additionalPickerEvents(this.enableHighlight, (value, index) => { - this.onInputChange(value, index); - this.disableHighlight(); - }), - }} - scrollViewRef={this.context && this.context.scrollViewRef} - scrollViewContentOffsetY={this.context && this.context.contentOffsetY} - /> - - - {Boolean(this.props.hintText) && {this.props.hintText}} - + + {Boolean(props.label) && ( + + {props.label} + + )} + {props.value} + {Boolean(props.hintText) && {props.hintText}} + ); } + + return ( + <> + + {props.label && ( + + {props.label} + + )} + ({...item, color: themeColors.pickerOptionsTextColor}))} + style={props.size === 'normal' ? styles.picker(props.isDisabled, props.backgroundColor) : styles.pickerSmall(props.backgroundColor)} + useNativeAndroidPickerStyle={false} + placeholder={placeholder} + value={props.value} + Icon={() => props.icon(props.size)} + disabled={props.isDisabled} + fixAndroidTouchableBug + onOpen={enableHighlight} + onClose={disableHighlight} + textInputProps={{ + allowFontScaling: false, + }} + pickerProps={{ + ref: picker, + onFocus: enableHighlight, + onBlur: () => { + disableHighlight(); + props.onBlur(); + }, + ...props.additionalPickerEvents(enableHighlight, (value, index) => { + onInputChange(value, index); + disableHighlight(); + }), + }} + scrollViewRef={context && context.scrollViewRef} + scrollViewContentOffsetY={context && context.contentOffsetY} + /> + + + {Boolean(props.hintText) && {props.hintText}} + + ); } BasePicker.propTypes = propTypes; BasePicker.defaultProps = defaultProps; -BasePicker.contextType = ScrollContext; +BasePicker.displayName = 'BasePicker'; export default React.forwardRef((props, ref) => ( )); From a43e3be5dd030d89b8673cbcff5902ff4f40a984 Mon Sep 17 00:00:00 2001 From: Jakub Trzebiatowski Date: Mon, 24 Jul 2023 12:33:15 +0200 Subject: [PATCH 2/4] BasePicker: Mark forwardedRef property as not required --- src/components/Picker/BasePicker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Picker/BasePicker.js b/src/components/Picker/BasePicker.js index f86b8a34811b..d936a3fadbc9 100644 --- a/src/components/Picker/BasePicker.js +++ b/src/components/Picker/BasePicker.js @@ -13,7 +13,7 @@ import {ScrollContext} from '../ScrollViewWithContext'; const propTypes = { /** A forwarded ref */ - forwardedRef: PropTypes.oneOfType([PropTypes.func, PropTypes.shape({current: PropTypes.instanceOf(React.Component)})]).isRequired, + forwardedRef: PropTypes.oneOfType([PropTypes.func, PropTypes.shape({current: PropTypes.instanceOf(React.Component)})]), /** BasePicker label */ label: PropTypes.string, From 2f53176e95b9a0e7e0bd3c747cfd22814a4d6158 Mon Sep 17 00:00:00 2001 From: Jakub Trzebiatowski Date: Mon, 24 Jul 2023 12:45:54 +0200 Subject: [PATCH 3/4] BasePicker: Add a default forwardedRef value --- src/components/Picker/BasePicker.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/Picker/BasePicker.js b/src/components/Picker/BasePicker.js index d936a3fadbc9..62c8832a128c 100644 --- a/src/components/Picker/BasePicker.js +++ b/src/components/Picker/BasePicker.js @@ -84,6 +84,7 @@ const propTypes = { }; const defaultProps = { + forwardedRef: null, label: '', isDisabled: false, errorText: '', From 3b685823d42ff8dee806d2cd5e45813844624487 Mon Sep 17 00:00:00 2001 From: Jakub Trzebiatowski Date: Mon, 24 Jul 2023 13:07:57 +0200 Subject: [PATCH 4/4] BasePicker: Change forwardedRef default value to undefined --- src/components/Picker/BasePicker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Picker/BasePicker.js b/src/components/Picker/BasePicker.js index 62c8832a128c..173b863edfcc 100644 --- a/src/components/Picker/BasePicker.js +++ b/src/components/Picker/BasePicker.js @@ -84,7 +84,7 @@ const propTypes = { }; const defaultProps = { - forwardedRef: null, + forwardedRef: undefined, label: '', isDisabled: false, errorText: '',