From 8957638681791d93577ddb3fd5bf2f4877afdc8d Mon Sep 17 00:00:00 2001 From: Malcolm Laing Date: Mon, 5 Aug 2019 10:48:11 +0200 Subject: [PATCH] refactor: improve `useToggleState` and use it across our components (#984) * refactor(hooks): improve useToggleState * refactor(primary-action-dropdown): use useToggleState hook * refactor(password-field): use useToggleState hook * refactor(money-input): add more memoized functions, and use useToggleState * refactor(calendar-body): use useToggleState hook * chore: pr comments * chore: remove unused func --- .../primary-action-dropdown.js | 15 +-- .../fields/password-field/password-field.js | 7 +- .../inputs/money-input/money-input.js | 109 ++++++++++-------- .../internals/calendar-body/calendar-body.js | 19 +-- .../use-toggle-state/use-toggle-state.js | 16 ++- .../use-toggle-state/use-toggle-state.spec.js | 23 ++++ 6 files changed, 118 insertions(+), 71 deletions(-) diff --git a/src/components/dropdowns/primary-action-dropdown/primary-action-dropdown.js b/src/components/dropdowns/primary-action-dropdown/primary-action-dropdown.js index 3ab00e48d7..ef9703caa1 100644 --- a/src/components/dropdowns/primary-action-dropdown/primary-action-dropdown.js +++ b/src/components/dropdowns/primary-action-dropdown/primary-action-dropdown.js @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import invariant from 'tiny-invariant'; import { css } from '@emotion/core'; import styled from '@emotion/styled'; +import useToggleState from '../../../hooks/use-toggle-state'; import vars from '../../../../materials/custom-properties'; import Text from '../../typography/text'; import { CaretDownIcon, CaretUpIcon } from '../../icons'; @@ -210,7 +211,7 @@ Option.defaultProps = { */ const PrimaryActionDropdown = props => { const ref = React.useRef(); - const [isOpen, setIsOpen] = React.useState(false); + const [isOpen, toggle] = useToggleState(false); const handleGlobalClick = React.useCallback( event => { @@ -220,10 +221,10 @@ const PrimaryActionDropdown = props => { event.target !== dropdownButton && !dropdownButton.contains(event.target) ) { - setIsOpen(false); + toggle(false); } }, - [ref] + [ref, toggle] ); React.useEffect(() => { window.addEventListener('click', handleGlobalClick); @@ -241,16 +242,16 @@ const PrimaryActionDropdown = props => { const handleClickOnHead = React.useCallback( event => { if (isOpen) { - setIsOpen(false); + toggle(true); } else { onClick(event); } }, - [isOpen, onClick] + [isOpen, onClick, toggle] ); const handleClickOnChevron = React.useCallback(() => { - setIsOpen(!isOpen); - }, [isOpen]); + toggle(); + }, [toggle]); invariant( childrenAsArray.length > 1, diff --git a/src/components/fields/password-field/password-field.js b/src/components/fields/password-field/password-field.js index 82e5b6936e..d488bf5888 100644 --- a/src/components/fields/password-field/password-field.js +++ b/src/components/fields/password-field/password-field.js @@ -2,6 +2,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import { useIntl } from 'react-intl'; import requiredIf from 'react-required-if'; +import useToggleState from '../../../hooks/use-toggle-state'; import Constraints from '../../constraints'; import Spacings from '../../spacings'; import FieldLabel from '../../field-label'; @@ -20,14 +21,10 @@ const hasErrors = errors => errors && Object.values(errors).some(Boolean); const PasswordField = props => { const intl = useIntl(); - const [isPasswordVisible, setIsPasswordVisible] = React.useState(false); + const [isPasswordVisible, togglePasswordVisibility] = useToggleState(false); const id = getFieldId(props, {}, sequentialId); const hasError = props.touched && hasErrors(props.errors); - const togglePasswordVisibility = React.useCallback(() => { - setIsPasswordVisible(!isPasswordVisible); - }, [isPasswordVisible]); - return ( diff --git a/src/components/inputs/money-input/money-input.js b/src/components/inputs/money-input/money-input.js index 231ca01493..5943844947 100644 --- a/src/components/inputs/money-input/money-input.js +++ b/src/components/inputs/money-input/money-input.js @@ -7,6 +7,7 @@ import Select, { components } from 'react-select'; import { useIntl } from 'react-intl'; import { css } from '@emotion/core'; import styled from '@emotion/styled'; +import useToggleState from '../../../hooks/use-toggle-state'; import vars from '../../../../materials/custom-properties'; import DropdownIndicator from '../../internals/dropdown-indicator'; import isNumberish from '../../../utils/is-numberish'; @@ -311,16 +312,28 @@ const getCurrencyDropdownName = name => const MoneyInput = props => { const intl = useIntl(); - const [currencyHasFocus, setCurrencyHasFocus] = React.useState(false); - const [amountHasFocus, setAmountHasFocus] = React.useState(false); + const [currencyHasFocus, toggleCurrencyHasFocus] = useToggleState(false); + const [amountHasFocus, toggleAmountHasFocus] = useToggleState(false); const containerRef = React.useRef(); const amountInputRef = React.useRef(); + const { onFocus } = props; + const handleAmountFocus = React.useCallback(() => { + if (onFocus) + onFocus({ + target: { + id: MoneyInput.getAmountInputId(props.id), + name: getAmountInputName(props.name), + }, + }); + toggleAmountHasFocus(true); + }, [toggleAmountHasFocus, onFocus, props.id, props.name]); + const { onChange } = props; const handleAmountBlur = React.useCallback(() => { const amount = props.value.amount.trim(); - setAmountHasFocus(false); + toggleAmountHasFocus(false); // Skip formatting for empty value or when the input is used with an // unknown currency. if (amount.length > 0 && currencies[props.value.currencyCode]) { @@ -352,6 +365,7 @@ const MoneyInput = props => { props.name, props.value.amount, props.value.currencyCode, + toggleAmountHasFocus, ]); const handleAmountChange = React.useCallback( @@ -428,6 +442,22 @@ const MoneyInput = props => { ] ); + const handleCurrencyFocus = React.useCallback(() => { + if (onFocus) + onFocus({ + target: { + id: MoneyInput.getCurrencyDropdownId(props.id), + name: getCurrencyDropdownName(props.name), + }, + }); + + toggleCurrencyHasFocus(true); + }, [onFocus, toggleCurrencyHasFocus, props.name, props.id]); + + const handleCurrencyBlur = React.useCallback(() => { + toggleCurrencyHasFocus(false); + }, [toggleCurrencyHasFocus]); + const hasNoCurrencies = props.currencies.length === 0; const hasFocus = currencyHasFocus || amountHasFocus; @@ -458,12 +488,39 @@ const MoneyInput = props => { }; return null; })(); + const id = MoneyInput.getCurrencyDropdownId(props.id); const isHighPrecision = !MoneyInput.isEmpty(props.value) && MoneyInput.isHighPrecision(props.value, intl.locale); + const { onBlur } = props; + const handleContainerBlur = React.useCallback( + event => { + // ensures that both fields are marked as touched when one of them + // is blurred + if ( + typeof onBlur === 'function' && + !containerRef.current.contains(event.relatedTarget) + ) { + onBlur({ + target: { + id: MoneyInput.getCurrencyDropdownId(props.id), + name: getCurrencyDropdownName(props.name), + }, + }); + onBlur({ + target: { + id: MoneyInput.getAmountInputId(props.id), + name: getAmountInputName(props.name), + }, + }); + } + }, + [onBlur, props.id, props.name] + ); + return (
{ display: flex; `} data-testid="money-input-container" - onBlur={event => { - // ensures that both fields are marked as touched when one of them - // is blurred - if ( - typeof props.onBlur === 'function' && - !containerRef.current.contains(event.relatedTarget) - ) { - props.onBlur({ - target: { - id: MoneyInput.getCurrencyDropdownId(props.id), - name: getCurrencyDropdownName(props.name), - }, - }); - props.onBlur({ - target: { - id: MoneyInput.getAmountInputId(props.id), - name: getAmountInputName(props.name), - }, - }); - } - }} + onBlur={handleContainerBlur} > {hasNoCurrencies ? ( { options={options} placeholder="" styles={currencySelectStyles} - onFocus={() => { - if (props.onFocus) - props.onFocus({ - target: { - id: MoneyInput.getCurrencyDropdownId(props.id), - name: getCurrencyDropdownName(props.name), - }, - }); - setCurrencyHasFocus(true); - }} + onFocus={handleCurrencyFocus} menuPortalTarget={props.menuPortalTarget} menuShouldBlockScroll={props.menuShouldBlockScroll} - onBlur={() => setCurrencyHasFocus(false)} + onBlur={handleCurrencyBlur} onChange={handleCurrencyChange} data-testid="currency-dropdown" /> @@ -550,16 +578,7 @@ const MoneyInput = props => { autoComplete={props.autoComplete} name={getAmountInputName(props.name)} type="text" - onFocus={() => { - if (props.onFocus) - props.onFocus({ - target: { - id: MoneyInput.getAmountInputId(props.id), - name: getAmountInputName(props.name), - }, - }); - setAmountHasFocus(true); - }} + onFocus={handleAmountFocus} value={props.value.amount} css={[ getAmountInputStyles({ ...props, hasFocus }), diff --git a/src/components/internals/calendar-body/calendar-body.js b/src/components/internals/calendar-body/calendar-body.js index 950f2815ae..a101b50ce0 100644 --- a/src/components/internals/calendar-body/calendar-body.js +++ b/src/components/internals/calendar-body/calendar-body.js @@ -2,6 +2,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import { CalendarIcon, ClockIcon, CloseIcon } from '../../icons'; import Spacings from '../../spacings'; +import useToggleState from '../../../hooks/use-toggle-state'; import { getClearSectionStyles, getCalendarIconContainerStyles, @@ -31,46 +32,46 @@ ClearSection.propTypes = { }; export const CalendarBody = props => { - const [isFocused, setIsFocused] = React.useState(false); + const [isFocused, toggleIsFocused] = useToggleState(false); const { onFocus: onInputFocus } = props.inputProps; const handleInputFocus = React.useCallback( event => { - setIsFocused(true); + toggleIsFocused(true); if (onInputFocus) onInputFocus(event); }, - [onInputFocus] + [onInputFocus, toggleIsFocused] ); const { onBlur: onInputBlur } = props.inputProps; const handleInputBlur = React.useCallback( event => { - setIsFocused(false); + toggleIsFocused(false); if (onInputBlur) onInputBlur(event); }, - [onInputBlur] + [onInputBlur, toggleIsFocused] ); const { onFocus: onToggleFocus } = props.toggleButtonProps; const handleToggleFocus = React.useCallback( event => { - setIsFocused(true); + toggleIsFocused(true); if (onToggleFocus) onToggleFocus(event); }, - [onToggleFocus] + [onToggleFocus, toggleIsFocused] ); const { onBlur: onToggleBlur } = props.toggleButtonProps; const handleToggleBlur = React.useCallback( event => { - setIsFocused(false); + toggleIsFocused(false); if (onToggleBlur) onToggleBlur(event); }, - [onToggleBlur] + [onToggleBlur, toggleIsFocused] ); return ( diff --git a/src/hooks/use-toggle-state/use-toggle-state.js b/src/hooks/use-toggle-state/use-toggle-state.js index dc5b36cfa6..79db120e02 100644 --- a/src/hooks/use-toggle-state/use-toggle-state.js +++ b/src/hooks/use-toggle-state/use-toggle-state.js @@ -1,10 +1,16 @@ -import React from 'react'; +import { useCallback, useState } from 'react'; const useToggleState = (defaultValue = true) => { - const [isToggled, setIsToggled] = React.useState(defaultValue); - const toggle = React.useCallback(() => { - setIsToggled(!isToggled); - }, [isToggled]); + const [isToggled, setIsToggled] = useState(defaultValue); + + const toggle = useCallback( + forceIsToggled => { + setIsToggled( + typeof forceIsToggled === 'boolean' ? forceIsToggled : !isToggled + ); + }, + [isToggled] + ); return [isToggled, toggle]; }; diff --git a/src/hooks/use-toggle-state/use-toggle-state.spec.js b/src/hooks/use-toggle-state/use-toggle-state.spec.js index c20635f244..cafbeee8ea 100644 --- a/src/hooks/use-toggle-state/use-toggle-state.spec.js +++ b/src/hooks/use-toggle-state/use-toggle-state.spec.js @@ -5,12 +5,26 @@ import useToggleState from './use-toggle-state'; const TestComponent = props => { // eslint-disable-next-line react/prop-types const [isOpen, toggle] = useToggleState(props.isDefaultOpen); + const setOff = React.useCallback(() => { + toggle(false); + }, [toggle]); + + const setOn = React.useCallback(() => { + toggle(true); + }, [toggle]); + return (
{isOpen ? 'open' : 'closed'}
+ +
); }; @@ -27,6 +41,15 @@ it('should be possible to toggle the open state', () => { expect(getByTestId('openState')).toHaveTextContent('closed'); }); +it('should be possible to set the state on and off', () => { + const { getByTestId } = render(); + expect(getByTestId('openState')).toHaveTextContent('open'); + getByTestId('setOff').click(); + expect(getByTestId('openState')).toHaveTextContent('closed'); + getByTestId('setOn').click(); + expect(getByTestId('openState')).toHaveTextContent('open'); +}); + it('should respect the default closed state', () => { const { getByTestId } = render(); expect(getByTestId('openState')).toHaveTextContent('closed');