From f04a0b13e5dc1f530ef814ae062168e6c1250a02 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Tue, 2 Jun 2020 20:05:00 +0200 Subject: [PATCH] chore(Dropdown): remove deprecated lifecycle methods --- src/lib/AutoControlledComponent.js | 199 ------------------ src/lib/ModernAutoControlledComponent.js | 42 +++- src/lib/index.js | 1 - src/modules/Dropdown/Dropdown.js | 193 +++++++++-------- .../Dropdown/utils/getEnabledIndicies.js | 12 ++ src/modules/Dropdown/utils/getMenuOptions.js | 62 ++++++ .../Dropdown/utils/getSelectedIndex.js | 62 ++++++ 7 files changed, 284 insertions(+), 287 deletions(-) delete mode 100644 src/lib/AutoControlledComponent.js create mode 100644 src/modules/Dropdown/utils/getEnabledIndicies.js create mode 100644 src/modules/Dropdown/utils/getMenuOptions.js create mode 100644 src/modules/Dropdown/utils/getSelectedIndex.js diff --git a/src/lib/AutoControlledComponent.js b/src/lib/AutoControlledComponent.js deleted file mode 100644 index 1e995ab32a..0000000000 --- a/src/lib/AutoControlledComponent.js +++ /dev/null @@ -1,199 +0,0 @@ -/* eslint-disable no-console */ -/** - * Why choose inheritance over a HOC? Multiple advantages for this particular use case. - * In short, we need identical functionality to setState(), unless there is a prop defined - * for the state key. Also: - * - * 1. Single Renders - * Calling trySetState() in constructor(), componentWillMount(), or componentWillReceiveProps() - * does not cause two renders. Consumers and tests do not have to wait two renders to get state. - * See www.react.run/4kJFdKoxb/27 for an example of this issue. - * - * 2. Simple Testing - * Using a HOC means you must either test the undecorated component or test through the decorator. - * Testing the undecorated component means you must mock the decorator functionality. - * Testing through the HOC means you can not simply shallow render your component. - * - * 3. Statics - * HOC wrap instances, so statics are no longer accessible. They can be hoisted, but this is more - * looping over properties and storing references. We rely heavily on statics for testing and sub - * components. - * - * 4. Instance Methods - * Some instance methods may be exposed to users via refs. Again, these are lost with HOC unless - * hoisted and exposed by the HOC. - */ -import _ from 'lodash' -import { Component } from 'react' - -export const getDefaultPropName = (prop) => `default${prop[0].toUpperCase() + prop.slice(1)}` - -/** - * Return the auto controlled state value for a give prop. The initial value is chosen in this order: - * - regular props - * - then, default props - * - then, initial state - * - then, `checked` defaults to false - * - then, `value` defaults to '' or [] if props.multiple - * - else, undefined - * - * @param {string} propName A prop name - * @param {object} [props] A props object - * @param {object} [state] A state object - * @param {boolean} [includeDefaults=false] Whether or not to heed the default props or initial state - */ -export const getAutoControlledStateValue = (propName, props, state, includeDefaults = false) => { - // regular props - const propValue = props[propName] - if (propValue !== undefined) return propValue - - if (includeDefaults) { - // defaultProps - const defaultProp = props[getDefaultPropName(propName)] - if (defaultProp !== undefined) return defaultProp - - // initial state - state may be null or undefined - if (state) { - const initialState = state[propName] - if (initialState !== undefined) return initialState - } - } - - // React doesn't allow changing from uncontrolled to controlled components, - // default checked/value if they were not present. - if (propName === 'checked') return false - if (propName === 'value') return props.multiple ? [] : '' - - // otherwise, undefined -} - -export default class AutoControlledComponent extends Component { - constructor(...args) { - super(...args) - - const { autoControlledProps } = this.constructor - const state = _.invoke(this, 'getInitialAutoControlledState', this.props) || {} - - if (process.env.NODE_ENV !== 'production') { - const { defaultProps, name, propTypes } = this.constructor - // require static autoControlledProps - if (!autoControlledProps) { - console.error(`Auto controlled ${name} must specify a static autoControlledProps array.`) - } - - // require propTypes - _.each(autoControlledProps, (prop) => { - const defaultProp = getDefaultPropName(prop) - // regular prop - if (!_.has(propTypes, defaultProp)) { - console.error( - `${name} is missing "${defaultProp}" propTypes validation for auto controlled prop "${prop}".`, - ) - } - // its default prop - if (!_.has(propTypes, prop)) { - console.error( - `${name} is missing propTypes validation for auto controlled prop "${prop}".`, - ) - } - }) - - // prevent autoControlledProps in defaultProps - // - // When setting state, auto controlled props values always win (so the parent can manage them). - // It is not reasonable to decipher the difference between props from the parent and defaultProps. - // Allowing defaultProps results in trySetState always deferring to the defaultProp value. - // Auto controlled props also listed in defaultProps can never be updated. - // - // To set defaults for an AutoControlled prop, you can set the initial state in the - // constructor or by using an ES7 property initializer: - // https://babeljs.io/blog/2015/06/07/react-on-es6-plus#property-initializers - const illegalDefaults = _.intersection(autoControlledProps, _.keys(defaultProps)) - if (!_.isEmpty(illegalDefaults)) { - console.error( - [ - 'Do not set defaultProps for autoControlledProps. You can set defaults by', - 'setting state in the constructor or using an ES7 property initializer', - '(https://babeljs.io/blog/2015/06/07/react-on-es6-plus#property-initializers)', - `See ${name} props: "${illegalDefaults}".`, - ].join(' '), - ) - } - - // prevent listing defaultProps in autoControlledProps - // - // Default props are automatically handled. - // Listing defaults in autoControlledProps would result in allowing defaultDefaultValue props. - const illegalAutoControlled = _.filter(autoControlledProps, (prop) => - _.startsWith(prop, 'default'), - ) - if (!_.isEmpty(illegalAutoControlled)) { - console.error( - [ - 'Do not add default props to autoControlledProps.', - 'Default props are automatically handled.', - `See ${name} autoControlledProps: "${illegalAutoControlled}".`, - ].join(' '), - ) - } - } - - // Auto controlled props are copied to state. - // Set initial state by copying auto controlled props to state. - // Also look for the default prop for any auto controlled props (foo => defaultFoo) - // so we can set initial values from defaults. - const initialAutoControlledState = autoControlledProps.reduce((acc, prop) => { - acc[prop] = getAutoControlledStateValue(prop, this.props, state, true) - - if (process.env.NODE_ENV !== 'production') { - const defaultPropName = getDefaultPropName(prop) - const { name } = this.constructor - // prevent defaultFoo={} along side foo={} - if (!_.isUndefined(this.props[defaultPropName]) && !_.isUndefined(this.props[prop])) { - console.error( - `${name} prop "${prop}" is auto controlled. Specify either ${defaultPropName} or ${prop}, but not both.`, - ) - } - } - - return acc - }, {}) - - this.state = { ...state, ...initialAutoControlledState } - } - - // eslint-disable-next-line camelcase - UNSAFE_componentWillReceiveProps(nextProps) { - const { autoControlledProps } = this.constructor - - // Solve the next state for autoControlledProps - const newState = autoControlledProps.reduce((acc, prop) => { - const isNextDefined = !_.isUndefined(nextProps[prop]) - - // if next is defined then use its value - if (isNextDefined) acc[prop] = nextProps[prop] - - return acc - }, {}) - - if (Object.keys(newState).length > 0) this.setState(newState) - } - - /** - * Safely attempt to set state for props that might be controlled by the user. - * Second argument is a state object that is always passed to setState. - * @param {object} state State that corresponds to controlled props. - * @param {function} [callback] Callback which is called after setState applied. - */ - trySetState = (state, callback) => { - const newState = Object.keys(state).reduce((acc, prop) => { - // ignore props defined by the parent - if (this.props[prop] !== undefined) return acc - - acc[prop] = state[prop] - return acc - }, {}) - - if (Object.keys(newState).length > 0) this.setState(newState, callback) - } -} diff --git a/src/lib/ModernAutoControlledComponent.js b/src/lib/ModernAutoControlledComponent.js index bad6711695..5a71fdf84a 100644 --- a/src/lib/ModernAutoControlledComponent.js +++ b/src/lib/ModernAutoControlledComponent.js @@ -25,7 +25,47 @@ */ import _ from 'lodash' import { Component } from 'react' -import { getAutoControlledStateValue, getDefaultPropName } from './AutoControlledComponent' + +export const getDefaultPropName = (prop) => `default${prop[0].toUpperCase() + prop.slice(1)}` + +/** + * Return the auto controlled state value for a give prop. The initial value is chosen in this order: + * - regular props + * - then, default props + * - then, initial state + * - then, `checked` defaults to false + * - then, `value` defaults to '' or [] if props.multiple + * - else, undefined + * + * @param {string} propName A prop name + * @param {object} [props] A props object + * @param {object} [state] A state object + * @param {boolean} [includeDefaults=false] Whether or not to heed the default props or initial state + */ +export const getAutoControlledStateValue = (propName, props, state, includeDefaults = false) => { + // regular props + const propValue = props[propName] + if (propValue !== undefined) return propValue + + if (includeDefaults) { + // defaultProps + const defaultProp = props[getDefaultPropName(propName)] + if (defaultProp !== undefined) return defaultProp + + // initial state - state may be null or undefined + if (state) { + const initialState = state[propName] + if (initialState !== undefined) return initialState + } + } + + // React doesn't allow changing from uncontrolled to controlled components, + // default checked/value if they were not present. + if (propName === 'checked') return false + if (propName === 'value') return props.multiple ? [] : '' + + // otherwise, undefined +} export default class ModernAutoControlledComponent extends Component { constructor(...args) { diff --git a/src/lib/index.js b/src/lib/index.js index cfefc62a28..c319467058 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -1,6 +1,5 @@ import makeDebugger from './makeDebugger' -export AutoControlledComponent from './AutoControlledComponent' export ModernAutoControlledComponent from './ModernAutoControlledComponent' export { getChildMapping, mergeChildMappings } from './childMapping' export * as childrenUtils from './childrenUtils' diff --git a/src/modules/Dropdown/Dropdown.js b/src/modules/Dropdown/Dropdown.js index 5b460bb4eb..93757cb2c7 100644 --- a/src/modules/Dropdown/Dropdown.js +++ b/src/modules/Dropdown/Dropdown.js @@ -8,7 +8,7 @@ import React, { Children, cloneElement, createRef } from 'react' import shallowEqual from 'shallowequal' import { - AutoControlledComponent as Component, + ModernAutoControlledComponent as Component, childrenUtils, customPropTypes, doesNodeContainClick, @@ -26,6 +26,8 @@ import DropdownItem from './DropdownItem' import DropdownHeader from './DropdownHeader' import DropdownMenu from './DropdownMenu' import DropdownSearchInput from './DropdownSearchInput' +import getMenuOptions from './utils/getMenuOptions' +import getSelectedIndex from './utils/getSelectedIndex' const debug = makeDebugger('dropdown') @@ -391,16 +393,23 @@ export default class Dropdown extends Component { return { focus: false, searchQuery: '' } } - // eslint-disable-next-line camelcase - UNSAFE_componentWillMount() { - debug('componentWillMount()') - const { open, value } = this.state - - this.setValue(value) - this.setSelectedIndex(value) + static getAutoControlledStateFromProps(props, state) { + const selectedIndex = getSelectedIndex({ + additionLabel: props.additionLabel, + additionPosition: props.additionPosition, + allowAdditions: props.allowAdditions, + deburr: props.deburr, + multiple: props.multiple, + search: props.search, + + options: props.options, + searchQuery: state.searchQuery, + selectedIndex: state.selectedIndex, + value: state.value, + }) - if (open) { - this.open() + return { + selectedIndex, } } @@ -432,7 +441,7 @@ export default class Dropdown extends Component { if (!shallowEqual(nextProps.value, this.props.value)) { debug('value changed, setting', nextProps.value) - this.setValue(nextProps.value) + this.setState({ value: nextProps.value }) this.setSelectedIndex(nextProps.value) } @@ -482,6 +491,15 @@ export default class Dropdown extends Component { } else if (prevState.open && !this.state.open) { debug('dropdown closed') } + + // TODO: fix me to use + const { disabled, open } = this.props + debug('open()', { disabled, open, search }) + + if (disabled) return + + if (search) _.invoke(this.searchRef.current, 'focus') + this.scrollSelectedItemIntoView() } // ---------------------------------------- @@ -565,7 +583,7 @@ export default class Dropdown extends Component { if (valueHasChanged) { // notify the onChange prop that the user is trying to change value - this.setValue(newValue) + this.setState({ value: newValue }) this.setSelectedIndex(newValue) this.handleChange(e, newValue) @@ -589,7 +607,19 @@ export default class Dropdown extends Component { if (!shouldSelect) return e.preventDefault() - const optionSize = _.size(this.getMenuOptions()) + const optionSize = _.size( + getMenuOptions({ + additionLabel: this.props.additionLabel, + additionPosition: this.props.additionPosition, + allowAdditions: this.props.allowAdditions, + deburr: this.props.deburr, + multiple: this.props.multiple, + options: this.props.options, + search: this.props.search, + searchQuery: this.state.searchQuery, + value: this.state.value, + }), + ) if (search && optionSize === 0) return this.makeSelectedItemActive(e) @@ -611,7 +641,7 @@ export default class Dropdown extends Component { // remove most recent value const newValue = _.dropRight(value) - this.setValue(newValue) + this.setState({ value: newValue }) this.setSelectedIndex(newValue) this.handleChange(e, newValue) } @@ -706,7 +736,7 @@ export default class Dropdown extends Component { // notify the onChange prop that the user is trying to change value if (valueHasChanged) { - this.setValue(newValue) + this.setState({ value: newValue }) this.setSelectedIndex(value) this.handleChange(e, newValue) @@ -772,7 +802,7 @@ export default class Dropdown extends Component { const newQuery = value _.invoke(this.props, 'onSearchChange', e, { ...this.props, searchQuery: newQuery }) - this.trySetState({ searchQuery: newQuery, selectedIndex: 0 }) + this.setState({ searchQuery: newQuery, selectedIndex: 0 }) // open search dropdown on search query if (!open && newQuery.length >= minCharacters) { @@ -842,8 +872,12 @@ export default class Dropdown extends Component { className: 'addition', 'data-additional': true, } - if (additionPosition === 'top') filteredOptions.unshift(addItem) - else filteredOptions.push(addItem) + + if (additionPosition === 'top') { + filteredOptions.unshift(addItem) + } else { + filteredOptions.push(addItem) + } } return filteredOptions @@ -851,36 +885,27 @@ export default class Dropdown extends Component { getSelectedItem = () => { const { selectedIndex } = this.state - const options = this.getMenuOptions() + const options = getMenuOptions({ + additionLabel: this.props.additionLabel, + additionPosition: this.props.additionPosition, + allowAdditions: this.props.allowAdditions, + deburr: this.props.deburr, + multiple: this.props.multiple, + options: this.props.options, + search: this.props.search, + searchQuery: this.state.searchQuery, + value: this.state.value, + }) return _.get(options, `[${selectedIndex}]`) } - getEnabledIndices = (givenOptions) => { - const options = givenOptions || this.getMenuOptions() - - return _.reduce( - options, - (memo, item, index) => { - if (!item.disabled) memo.push(index) - return memo - }, - [], - ) - } - getItemByValue = (value) => { const { options } = this.props return _.find(options, { value }) } - getMenuItemIndexByValue = (value, givenOptions) => { - const options = givenOptions || this.getMenuOptions() - - return _.findIndex(options, ['value', value]) - } - getDropdownAriaOptions = () => { const { loading, disabled, search, multiple } = this.props const { open } = this.state @@ -917,54 +942,29 @@ export default class Dropdown extends Component { const { searchQuery } = this.state if (searchQuery === undefined || searchQuery === '') return - this.trySetState({ searchQuery: '' }) + this.setState({ searchQuery: '' }) this.setSelectedIndex(value, undefined, '') } - setValue = (value) => { - debug('setValue()', value) - this.trySetState({ value }) - } - setSelectedIndex = ( value = this.state.value, - optionsProps = this.props.options, + options = this.props.options, searchQuery = this.state.searchQuery, + selectedIndex = this.state.selectedIndex, ) => { - const { multiple } = this.props - const { selectedIndex } = this.state - const options = this.getMenuOptions(value, optionsProps, searchQuery) - const enabledIndicies = this.getEnabledIndices(options) - - let newSelectedIndex - - // update the selected index - if (!selectedIndex || selectedIndex < 0) { - const firstIndex = enabledIndicies[0] - - // Select the currently active item, if none, use the first item. - // Multiple selects remove active items from the list, - // their initial selected index should be 0. - newSelectedIndex = multiple - ? firstIndex - : this.getMenuItemIndexByValue(value, options) || enabledIndicies[0] - } else if (multiple) { - // multiple selects remove options from the menu as they are made active - // keep the selected index within range of the remaining items - if (selectedIndex >= options.length - 1) { - newSelectedIndex = enabledIndicies[enabledIndicies.length - 1] - } - } else { - const activeIndex = this.getMenuItemIndexByValue(value, options) + const newSelectedIndex = getSelectedIndex({ + additionLabel: this.props.additionLabel, + additionPosition: this.props.additionPosition, + allowAdditions: this.props.allowAdditions, + deburr: this.props.deburr, + multiple: this.props.multiple, + search: this.props.search, - // regular selects can only have one active item - // set the selected index to the currently active item - newSelectedIndex = _.includes(enabledIndicies, activeIndex) ? activeIndex : undefined - } - - if (!newSelectedIndex || newSelectedIndex < 0) { - newSelectedIndex = enabledIndicies[0] - } + options, + searchQuery, + selectedIndex, + value, + }) this.setState({ selectedIndex: newSelectedIndex }) } @@ -989,7 +989,7 @@ export default class Dropdown extends Component { debug('remove value:', labelProps.value) debug('new value:', newValue) - this.setValue(newValue) + this.setState({ value: newValue }) this.setSelectedIndex(newValue) this.handleChange(e, newValue) } @@ -998,7 +998,17 @@ export default class Dropdown extends Component { debug('moveSelectionBy()') debug(`offset: ${offset}`) - const options = this.getMenuOptions() + const options = getMenuOptions({ + additionLabel: this.props.additionLabel, + additionPosition: this.props.additionPosition, + allowAdditions: this.props.allowAdditions, + deburr: this.props.deburr, + multiple: this.props.multiple, + options: this.props.options, + search: this.props.search, + searchQuery: this.state.searchQuery, + value: this.state.value, + }) // Prevent infinite loop // TODO: remove left part of condition after children API will be removed @@ -1050,7 +1060,7 @@ export default class Dropdown extends Component { const { multiple } = this.props const newValue = multiple ? [] : '' - this.setValue(newValue) + this.setState({ value: newValue }) this.setSelectedIndex(newValue) this.handleChange(e, newValue) } @@ -1141,12 +1151,13 @@ export default class Dropdown extends Component { // set state only if there's a relevant difference if (!upward !== !this.state.upward) { - this.trySetState({ upward }) + this.setState({ upward }) } } open = (e) => { - const { disabled, open, search } = this.props + const { disabled, search } = this.props + const { open } = this.state debug('open()', { disabled, open, search }) if (disabled) return @@ -1154,7 +1165,7 @@ export default class Dropdown extends Component { _.invoke(this.props, 'onOpen', e, this.props) - this.trySetState({ open: true }) + this.setState({ open: true }) this.scrollSelectedItemIntoView() } @@ -1164,7 +1175,7 @@ export default class Dropdown extends Component { if (open) { _.invoke(this.props, 'onClose', e, this.props) - this.trySetState({ open: false }, callback) + this.setState({ open: false }, callback) } } @@ -1279,7 +1290,17 @@ export default class Dropdown extends Component { // lazy load, only render options when open if (lazyLoad && !open) return null - const options = this.getMenuOptions() + const options = getMenuOptions({ + additionLabel: this.props.additionLabel, + additionPosition: this.props.additionPosition, + allowAdditions: this.props.allowAdditions, + deburr: this.props.deburr, + multiple: this.props.multiple, + options: this.props.options, + search: this.props.search, + searchQuery: this.state.searchQuery, + value: this.state.value, + }) if (noResultsMessage !== null && search && _.isEmpty(options)) { return
{noResultsMessage}
diff --git a/src/modules/Dropdown/utils/getEnabledIndicies.js b/src/modules/Dropdown/utils/getEnabledIndicies.js new file mode 100644 index 0000000000..813f49f956 --- /dev/null +++ b/src/modules/Dropdown/utils/getEnabledIndicies.js @@ -0,0 +1,12 @@ +import _ from 'lodash' + +export default function getEnabledIndices(options) { + return _.reduce( + options, + (memo, item, index) => { + if (!item.disabled) memo.push(index) + return memo + }, + [], + ) +} diff --git a/src/modules/Dropdown/utils/getMenuOptions.js b/src/modules/Dropdown/utils/getMenuOptions.js new file mode 100644 index 0000000000..e719f44ca0 --- /dev/null +++ b/src/modules/Dropdown/utils/getMenuOptions.js @@ -0,0 +1,62 @@ +import _ from 'lodash' +import React from 'react' + +export default function getMenuOptions({ + additionLabel, + additionPosition, + allowAdditions, + deburr, + multiple, + options, + search, + searchQuery, + value, +}) { + let filteredOptions = options + + // filter out active options + if (multiple) { + filteredOptions = _.filter(filteredOptions, (opt) => !_.includes(value, opt.value)) + } + + // filter by search query + if (search && searchQuery) { + if (_.isFunction(search)) { + filteredOptions = search(filteredOptions, searchQuery) + } else { + // remove diacritics on search input and options, if deburr prop is set + const strippedQuery = deburr ? _.deburr(searchQuery) : searchQuery + + const re = new RegExp(_.escapeRegExp(strippedQuery), 'i') + + filteredOptions = _.filter(filteredOptions, (opt) => + re.test(deburr ? _.deburr(opt.text) : opt.text), + ) + } + } + + // insert the "add" item + if (allowAdditions && search && searchQuery && !_.some(filteredOptions, { text: searchQuery })) { + const additionLabelElement = React.isValidElement(additionLabel) + ? React.cloneElement(additionLabel, { key: 'addition-label' }) + : additionLabel || '' + + const addItem = { + key: 'addition', + // by using an array, we can pass multiple elements, but when doing so + // we must specify a `key` for React to know which one is which + text: [additionLabelElement, {searchQuery}], + value: searchQuery, + className: 'addition', + 'data-additional': true, + } + + if (additionPosition === 'top') { + filteredOptions.unshift(addItem) + } else { + filteredOptions.push(addItem) + } + } + + return filteredOptions +} diff --git a/src/modules/Dropdown/utils/getSelectedIndex.js b/src/modules/Dropdown/utils/getSelectedIndex.js new file mode 100644 index 0000000000..595dd0f9b2 --- /dev/null +++ b/src/modules/Dropdown/utils/getSelectedIndex.js @@ -0,0 +1,62 @@ +import _ from 'lodash' + +import getMenuOptions from './getMenuOptions' +import getEnabledIndices from './getEnabledIndicies' + +export default function setSelectedIndex({ + additionLabel, + additionPosition, + allowAdditions, + deburr, + multiple, + options, + search, + selectedIndex, + searchQuery, + value, +}) { + const menuOptions = getMenuOptions({ + additionLabel, + additionPosition, + allowAdditions, + deburr, + multiple, + options, + search, + searchQuery, + value, + }) + const enabledIndicies = getEnabledIndices(menuOptions) + + let newSelectedIndex + + // update the selected index + if (!selectedIndex || selectedIndex < 0) { + const firstIndex = enabledIndicies[0] + + // Select the currently active item, if none, use the first item. + // Multiple selects remove active items from the list, + // their initial selected index should be 0. + newSelectedIndex = multiple + ? firstIndex + : _.findIndex(menuOptions, ['value', value]) || enabledIndicies[0] + } else if (multiple) { + // multiple selects remove options from the menu as they are made active + // keep the selected index within range of the remaining items + if (selectedIndex >= menuOptions.length - 1) { + newSelectedIndex = enabledIndicies[enabledIndicies.length - 1] + } + } else { + const activeIndex = _.findIndex(menuOptions, ['value', value]) + + // regular selects can only have one active item + // set the selected index to the currently active item + newSelectedIndex = _.includes(enabledIndicies, activeIndex) ? activeIndex : undefined + } + + if (!newSelectedIndex || newSelectedIndex < 0) { + newSelectedIndex = enabledIndicies[0] + } + + return newSelectedIndex +}