From 575776f3004c6ac655b128fbdb30bd4b35115ab7 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Mon, 11 Nov 2019 11:35:26 +0100 Subject: [PATCH] [Autocomplete] Add controllable input value API (#18285) --- docs/pages/api/autocomplete.md | 2 + .../src/Autocomplete/Autocomplete.js | 13 +++++ .../src/Autocomplete/Autocomplete.test.js | 30 ++++++++++ .../material-ui-lab/src/TreeView/TreeView.js | 4 +- .../src/useAutocomplete/useAutocomplete.d.ts | 9 ++- .../src/useAutocomplete/useAutocomplete.js | 23 ++++++-- .../material-ui/src/RadioGroup/RadioGroup.js | 55 +++++++++---------- packages/material-ui/src/Slider/Slider.js | 23 +++++++- packages/material-ui/src/Tooltip/Tooltip.js | 48 ++++++++-------- .../material-ui/src/internal/SwitchBase.js | 8 +-- .../material-ui/src/utils/useEventCallback.js | 2 +- 11 files changed, 151 insertions(+), 66 deletions(-) diff --git a/docs/pages/api/autocomplete.md b/docs/pages/api/autocomplete.md index 0315e3a12d303b..d626ef39652cef 100644 --- a/docs/pages/api/autocomplete.md +++ b/docs/pages/api/autocomplete.md @@ -46,6 +46,7 @@ You can learn more about the difference by [reading this guide](/guides/minimizi | groupBy | func | | If provided, the options will be grouped under the returned string. The groupBy value is also used as the text for group headings when `renderGroup` is not provided.

**Signature:**
`function(options: any) => string`
*options:* The option to group. | | id | string | | This prop is used to help implement the accessibility logic. If you don't provide this prop. It falls back to a randomly generated id. | | includeInputInList | bool | false | If `true`, the highlight can move to the input. | +| inputValue | string | | The input value. | | ListboxComponent | elementType | 'ul' | The component used to render the listbox. | | loading | bool | false | If `true`, the component is in a loading state. | | loadingText | node | 'Loading…' | Text to display when in a loading state. | @@ -53,6 +54,7 @@ You can learn more about the difference by [reading this guide](/guides/minimizi | noOptionsText | node | 'No options' | Text to display when there are no options. | | onChange | func | | Callback fired when the value changes.

**Signature:**
`function(event: object, value: any) => void`
*event:* The event source of the callback
*value:* null | | onClose | func | | Callback fired when the popup requests to be closed. Use in controlled mode (see open).

**Signature:**
`function(event: object) => void`
*event:* The event source of the callback. | +| onInputChange | func | | Callback fired when the input value changes.

**Signature:**
`function(event: object, value: string) => void`
*event:* The event source of the callback.
*value:* null | | onOpen | func | | Callback fired when the popup requests to be opened. Use in controlled mode (see open).

**Signature:**
`function(event: object) => void`
*event:* The event source of the callback. | | open | bool | | Control the popup` open state. | | options | array | [] | Array of options. | diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js index e5f85d86aa8411..634248c445b84b 100644 --- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js +++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js @@ -181,6 +181,7 @@ const Autocomplete = React.forwardRef(function Autocomplete(props, ref) { groupBy, id: idProp, includeInputInList = false, + inputValue: inputValueProp, ListboxComponent = 'ul', loading = false, loadingText = 'Loading…', @@ -188,6 +189,7 @@ const Autocomplete = React.forwardRef(function Autocomplete(props, ref) { noOptionsText = 'No options', onChange, onClose, + onInputChange, onOpen, open, options = [], @@ -487,6 +489,10 @@ Autocomplete.propTypes = { * If `true`, the highlight can move to the input. */ includeInputInList: PropTypes.bool, + /** + * The input value. + */ + inputValue: PropTypes.string, /** * The component used to render the listbox. */ @@ -521,6 +527,13 @@ Autocomplete.propTypes = { * @param {object} event The event source of the callback. */ onClose: PropTypes.func, + /** + * Callback fired when the input value changes. + * + * @param {object} event The event source of the callback. + * @param {string} value + */ + onInputChange: PropTypes.func, /** * Callback fired when the popup requests to be opened. * Use in controlled mode (see open). diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js index 8e3439ec00aa34..ee6f66b22893e5 100644 --- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js +++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js @@ -549,4 +549,34 @@ describe('', () => { expect(textbox.selectionEnd).to.equal(3); }); }); + + describe('controlled input', () => { + it('controls the input value', () => { + const handleChange = spy(); + function MyComponent() { + const [, setInputValue] = React.useState(''); + const handleInputChange = (event, value) => { + handleChange(value); + setInputValue(value); + }; + return ( + } + /> + ); + } + + const { getByRole } = render(); + + const textbox = getByRole('textbox'); + expect(handleChange.callCount).to.equal(1); + expect(handleChange.args[0][0]).to.equal(''); + fireEvent.change(textbox, { target: { value: 'a' } }); + expect(handleChange.callCount).to.equal(2); + expect(handleChange.args[1][0]).to.equal('a'); + expect(textbox.value).to.equal(''); + }); + }); }); diff --git a/packages/material-ui-lab/src/TreeView/TreeView.js b/packages/material-ui-lab/src/TreeView/TreeView.js index a4d4a4b797ec29..8e9fb32088aa38 100644 --- a/packages/material-ui-lab/src/TreeView/TreeView.js +++ b/packages/material-ui-lab/src/TreeView/TreeView.js @@ -39,7 +39,6 @@ const TreeView = React.forwardRef(function TreeView(props, ref) { onNodeToggle, ...other } = props; - const [expandedState, setExpandedState] = React.useState(defaultExpanded); const [tabable, setTabable] = React.useState(null); const [focused, setFocused] = React.useState(null); @@ -48,7 +47,8 @@ const TreeView = React.forwardRef(function TreeView(props, ref) { const firstCharMap = React.useRef({}); const { current: isControlled } = React.useRef(expandedProp !== undefined); - const expanded = (isControlled ? expandedProp : expandedState) || []; + const [expandedState, setExpandedState] = React.useState(defaultExpanded); + const expanded = (isControlled ? expandedProp : expandedState) || defaultExpandedDefault; if (process.env.NODE_ENV !== 'production') { // eslint-disable-next-line react-hooks/rules-of-hooks diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts index 8a52dda624ccf6..1bb54dd052f614 100644 --- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts +++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts @@ -107,6 +107,10 @@ export interface UseAutocompleteProps { * If `true`, the highlight can move to the input. */ includeInputInList?: boolean; + /** + * The input value. + */ + inputValue?: string; /** * If true, `value` must be an array and the menu will support multiple selections. */ @@ -127,8 +131,11 @@ export interface UseAutocompleteProps { onClose?: (event: React.ChangeEvent<{}>) => void; /** * Callback fired when the input value changes. + * + * @param {object} event The event source of the callback. + * @param {string} value */ - onInputChange?: React.ChangeEventHandler; + onInputChange?: (event: React.ChangeEvent<{}>, value: any) => void; /** * Callback fired when the popup requests to be opened. * Use in controlled mode (see open). diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js index 90567d039e2cec..5de91aaa427c7f 100644 --- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js +++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js @@ -68,10 +68,12 @@ export default function useAutocomplete(props) { groupBy, id: idProp, includeInputInList = false, + inputValue: inputValueProp, multiple = false, onChange, onClose, onOpen, + onInputChange, open: openProp, options = [], value: valueProp, @@ -165,10 +167,13 @@ export default function useAutocomplete(props) { }); const value = isControlled ? valueProp : valueState; - const [inputValue, setInputValue] = React.useState(''); + const { current: isInputValueControlled } = React.useRef(inputValueProp != null); + const [inputValueState, setInputValue] = React.useState(''); + const inputValue = isInputValueControlled ? inputValueProp : inputValueState; + const [focused, setFocused] = React.useState(false); - const resetInputValue = useEventCallback(newValue => { + const resetInputValue = useEventCallback((event, newValue) => { let newInputValue; if (multiple) { newInputValue = ''; @@ -195,10 +200,14 @@ export default function useAutocomplete(props) { } setInputValue(newInputValue); + + if (onInputChange) { + onInputChange(event, newInputValue); + } }); React.useEffect(() => { - resetInputValue(value); + resetInputValue(null, value); }, [value, resetInputValue]); const { current: isOpenControlled } = React.useRef(openProp != null); @@ -404,7 +413,7 @@ export default function useAutocomplete(props) { handleClose(event); } - resetInputValue(newValue); + resetInputValue(event, newValue); selectedIndexRef.current = -1; }; @@ -590,7 +599,7 @@ export default function useAutocomplete(props) { if (autoSelect && selectedIndexRef.current !== -1) { handleValue(event, filteredOptions[selectedIndexRef.current]); } else if (!freeSolo) { - resetInputValue(value); + resetInputValue(event, value); } handleClose(event); @@ -612,6 +621,10 @@ export default function useAutocomplete(props) { } setInputValue(newValue); + + if (onInputChange) { + onInputChange(event, newValue); + } }; const handleOptionMouseOver = event => { diff --git a/packages/material-ui/src/RadioGroup/RadioGroup.js b/packages/material-ui/src/RadioGroup/RadioGroup.js index 82c548c233e9a0..28ccb21b7306d9 100644 --- a/packages/material-ui/src/RadioGroup/RadioGroup.js +++ b/packages/material-ui/src/RadioGroup/RadioGroup.js @@ -7,28 +7,10 @@ import RadioGroupContext from './RadioGroupContext'; const RadioGroup = React.forwardRef(function RadioGroup(props, ref) { const { actions, children, name, value: valueProp, onChange, ...other } = props; const rootRef = React.useRef(null); - const { current: isControlled } = React.useRef(valueProp != null); - const [valueState, setValue] = React.useState(() => { - return !isControlled ? props.defaultValue : null; - }); - - React.useImperativeHandle( - actions, - () => ({ - focus: () => { - let input = rootRef.current.querySelector('input:not(:disabled):checked'); - - if (!input) { - input = rootRef.current.querySelector('input:not(:disabled)'); - } - if (input) { - input.focus(); - } - }, - }), - [], - ); + const { current: isControlled } = React.useRef(valueProp != null); + const [valueState, setValue] = React.useState(props.defaultValue); + const value = isControlled ? valueProp : valueState; if (process.env.NODE_ENV !== 'production') { // eslint-disable-next-line react-hooks/rules-of-hooks @@ -49,7 +31,25 @@ const RadioGroup = React.forwardRef(function RadioGroup(props, ref) { }, [valueProp, isControlled]); } - const value = isControlled ? valueProp : valueState; + React.useImperativeHandle( + actions, + () => ({ + focus: () => { + let input = rootRef.current.querySelector('input:not(:disabled):checked'); + + if (!input) { + input = rootRef.current.querySelector('input:not(:disabled)'); + } + + if (input) { + input.focus(); + } + }, + }), + [], + ); + + const handleRef = useForkRef(ref, rootRef); const handleChange = event => { if (!isControlled) { @@ -60,14 +60,13 @@ const RadioGroup = React.forwardRef(function RadioGroup(props, ref) { onChange(event, event.target.value); } }; - const context = { name, onChange: handleChange, value }; - - const handleRef = useForkRef(ref, rootRef); return ( - - {children} - + + + {children} + + ); }); diff --git a/packages/material-ui/src/Slider/Slider.js b/packages/material-ui/src/Slider/Slider.js index ed3487c0af741c..ad7f144e18055f 100644 --- a/packages/material-ui/src/Slider/Slider.js +++ b/packages/material-ui/src/Slider/Slider.js @@ -369,15 +369,36 @@ const Slider = React.forwardRef(function Slider(props, ref) { ...other } = props; const theme = useTheme(); - const { current: isControlled } = React.useRef(valueProp != null); const touchId = React.useRef(); // We can't use the :active browser pseudo-classes. // - The active state isn't triggered when clicking on the rail. // - The active state isn't transfered when inversing a range slider. const [active, setActive] = React.useState(-1); const [open, setOpen] = React.useState(-1); + + const { current: isControlled } = React.useRef(valueProp != null); const [valueState, setValueState] = React.useState(defaultValue); const valueDerived = isControlled ? valueProp : valueState; + + if (process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line react-hooks/rules-of-hooks + React.useEffect(() => { + if (isControlled !== (valueProp != null)) { + console.error( + [ + `Material-UI: A component is changing ${ + isControlled ? 'a ' : 'an un' + }controlled Slider to be ${isControlled ? 'un' : ''}controlled.`, + 'Elements should not switch from uncontrolled to controlled (or vice versa).', + 'Decide between using a controlled or uncontrolled Slider ' + + 'element for the lifetime of the component.', + 'More info: https://fb.me/react-controlled-components', + ].join('\n'), + ); + } + }, [valueProp, isControlled]); + } + const range = Array.isArray(valueDerived); const instanceRef = React.useRef(); let values = range ? [...valueDerived].sort(asc) : [valueDerived]; diff --git a/packages/material-ui/src/Tooltip/Tooltip.js b/packages/material-ui/src/Tooltip/Tooltip.js index 54ec52c68a07c7..15e8adc142ad9b 100644 --- a/packages/material-ui/src/Tooltip/Tooltip.js +++ b/packages/material-ui/src/Tooltip/Tooltip.js @@ -107,17 +107,38 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) { } = props; const theme = useTheme(); - const [openState, setOpenState] = React.useState(false); const [, forceUpdate] = React.useState(0); const [childNode, setChildNode] = React.useState(); const ignoreNonTouchEvents = React.useRef(false); - const { current: isControlled } = React.useRef(openProp != null); const defaultId = React.useRef(); const closeTimer = React.useRef(); const enterTimer = React.useRef(); const leaveTimer = React.useRef(); const touchTimer = React.useRef(); + const { current: isControlled } = React.useRef(openProp != null); + const [openState, setOpenState] = React.useState(false); + let open = isControlled ? openProp : openState; + + if (process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line react-hooks/rules-of-hooks + React.useEffect(() => { + if (isControlled !== (openProp != null)) { + console.error( + [ + `Material-UI: A component is changing ${ + isControlled ? 'a ' : 'an un' + }controlled Tooltip to be ${isControlled ? 'un' : ''}controlled.`, + 'Elements should not switch from uncontrolled to controlled (or vice versa).', + 'Decide between using a controlled or uncontrolled Tooltip ' + + 'element for the lifetime of the component.', + 'More info: https://fb.me/react-controlled-components', + ].join('\n'), + ); + } + }, [openProp, isControlled]); + } + if (process.env.NODE_ENV !== 'production') { // eslint-disable-next-line react-hooks/rules-of-hooks React.useEffect(() => { @@ -164,30 +185,11 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) { }; }, []); - if (process.env.NODE_ENV !== 'production') { - // eslint-disable-next-line react-hooks/rules-of-hooks - React.useEffect(() => { - if (isControlled !== (openProp != null)) { - console.error( - [ - `Material-UI: A component is changing ${ - isControlled ? 'a ' : 'an un' - }controlled Tooltip to be ${isControlled ? 'un' : ''}controlled.`, - 'Elements should not switch from uncontrolled to controlled (or vice versa).', - 'Decide between using a controlled or uncontrolled Tooltip ' + - 'element for the lifetime of the component.', - 'More info: https://fb.me/react-controlled-components', - ].join('\n'), - ); - } - }, [openProp, isControlled]); - } - const handleOpen = event => { // The mouseover event will trigger for every nested element in the tooltip. // We can skip rerendering when the tooltip is already open. // We are using the mouseover event instead of the mouseenter event to fix a hide/show issue. - if (!isControlled && !openState) { + if (!isControlled) { setOpenState(true); } @@ -333,8 +335,6 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) { ); const handleRef = useForkRef(children.ref, handleOwnRef); - let open = isControlled ? openProp : openState; - // There is no point in displaying an empty tooltip. if (title === '') { open = false; diff --git a/packages/material-ui/src/internal/SwitchBase.js b/packages/material-ui/src/internal/SwitchBase.js index 885f6cbae3d7e5..20457baf6b1dbb 100644 --- a/packages/material-ui/src/internal/SwitchBase.js +++ b/packages/material-ui/src/internal/SwitchBase.js @@ -55,6 +55,7 @@ const SwitchBase = React.forwardRef(function SwitchBase(props, ref) { } = props; const { current: isControlled } = React.useRef(checkedProp != null); const [checkedState, setCheckedState] = React.useState(Boolean(defaultChecked)); + const checked = isControlled ? checkedProp : checkedState; const muiFormControl = useFormControl(); @@ -79,14 +80,14 @@ const SwitchBase = React.forwardRef(function SwitchBase(props, ref) { }; const handleInputChange = event => { - const checked = event.target.checked; + const newChecked = event.target.checked; if (!isControlled) { - setCheckedState(checked); + setCheckedState(newChecked); } if (onChange) { - onChange(event, checked); + onChange(event, newChecked); } }; @@ -98,7 +99,6 @@ const SwitchBase = React.forwardRef(function SwitchBase(props, ref) { } } - const checked = isControlled ? checkedProp : checkedState; const hasLabelFor = type === 'checkbox' || type === 'radio'; return ( diff --git a/packages/material-ui/src/utils/useEventCallback.js b/packages/material-ui/src/utils/useEventCallback.js index 73f861ff0034f4..733cc3693badbb 100644 --- a/packages/material-ui/src/utils/useEventCallback.js +++ b/packages/material-ui/src/utils/useEventCallback.js @@ -12,5 +12,5 @@ export default function useEventCallback(fn) { useEnhancedEffect(() => { ref.current = fn; }); - return React.useCallback(event => (0, ref.current)(event), []); + return React.useCallback((...args) => (0, ref.current)(...args), []); }