Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Components: refactor RangeControl to pass exhaustive-deps #44271

Merged
merged 8 commits into from
Sep 21, 2022
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
### Internal

- `NavigationMenu` updated to ignore `react/exhaustive-deps` eslint rule ([#44090](https://github.com/WordPress/gutenberg/pull/44090)).
- `RangeControl`: updated to pass `react/exhaustive-deps` eslint rule ([#44271](https://github.com/WordPress/gutenberg/pull/44271)).
- `UnitControl` updated to pass the `react/exhaustive-deps` eslint rule ([#44161](https://github.com/WordPress/gutenberg/pull/44161)).

## 21.0.0 (2022-09-13)
Expand Down
22 changes: 1 addition & 21 deletions packages/components/src/range-control/input-range.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,39 +7,19 @@ import { forwardRef } from '@wordpress/element';
* Internal dependencies
*/
import { InputRange as BaseInputRange } from './styles/range-control-styles';
import { useDebouncedHoverInteraction } from './utils';

import type { InputRangeProps } from './types';
import type { WordPressComponentProps } from '../ui/context';

const noop = () => {};

function InputRange(
props: WordPressComponentProps< InputRangeProps, 'input' >,
ref: React.ForwardedRef< HTMLInputElement >
) {
const {
describedBy,
label,
onHideTooltip = noop,
onMouseLeave = noop,
onMouseMove = noop,
onShowTooltip = noop,
value,
...otherProps
} = props;

const hoverInteractions = useDebouncedHoverInteraction( {
onHide: onHideTooltip,
onMouseLeave,
onMouseMove,
onShow: onShowTooltip,
} );
const { describedBy, label, value, ...otherProps } = props;

return (
<BaseInputRange
{ ...otherProps }
{ ...hoverInteractions }
aria-describedby={ describedBy }
aria-label={ label }
aria-hidden={ false }
Expand Down
3 changes: 2 additions & 1 deletion packages/components/src/range-control/stories/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const meta: ComponentMeta< typeof RangeControl > = {
icon: { control: { type: null } },
marks: { control: { type: 'object' } },
onBlur: { control: { type: null } },
onChange: { action: 'onChange' },
onChange: { control: { type: null } },
onFocus: { control: { type: null } },
onMouseLeave: { control: { type: null } },
onMouseMove: { control: { type: null } },
Expand All @@ -46,6 +46,7 @@ const meta: ComponentMeta< typeof RangeControl > = {
value: { control: { type: null } },
},
parameters: {
actions: { argTypesRegex: '^on.*' },
controls: { expanded: true },
docs: { source: { state: 'open' } },
},
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/range-control/tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ function useTooltipPosition( { inputRef, tooltipPosition }: TooltipProps ) {
if ( inputRef && inputRef.current ) {
setPosition( tooltipPosition );
}
}, [ tooltipPosition ] );
}, [ tooltipPosition, inputRef ] );

useEffect( () => {
setTooltipPosition();
Expand Down
2 changes: 0 additions & 2 deletions packages/components/src/range-control/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,8 @@ export type RailProps = MarksProps & {
export type InputRangeProps = {
describedBy?: string;
label?: string;
onHideTooltip?: () => void;
onMouseLeave?: MouseEventHandler< HTMLInputElement >;
onMouseMove?: MouseEventHandler< HTMLInputElement >;
onShowTooltip?: () => void;
value?: number | '';
};

Expand Down
78 changes: 3 additions & 75 deletions packages/components/src/range-control/utils.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,15 @@
/**
* External dependencies
*/
import type { MouseEventHandler } from 'react';

/**
* WordPress dependencies
*/
import { useCallback, useRef, useEffect, useState } from '@wordpress/element';
import { useCallback } from '@wordpress/element';

/**
* Internal dependencies
*/
import { useControlledState } from '../utils/hooks';
import { clamp } from '../utils/math';

import type {
UseControlledRangeValueArgs,
UseDebouncedHoverInteractionArgs,
} from './types';

const noop = () => {};
import type { UseControlledRangeValueArgs } from './types';

/**
* A float supported clamp function for a specific value.
Expand Down Expand Up @@ -64,72 +54,10 @@ export function useControlledRangeValue(
setInternalState( floatClamp( nextValue, min, max ) );
}
},
[ min, max ]
[ min, max, setInternalState ]
);

// `state` can't be an empty string because we specified a fallback value of
// `null` in `useControlledState`
return [ state as Exclude< typeof state, '' >, setState ] as const;
}

/**
* Hook to encapsulate the debouncing "hover" to better handle the showing
* and hiding of the Tooltip.
*
* @param settings
* @return Bound properties for use on a React.Node.
*/
export function useDebouncedHoverInteraction(
settings: UseDebouncedHoverInteractionArgs
) {
const {
onHide = noop,
onMouseLeave = noop as MouseEventHandler,
onMouseMove = noop as MouseEventHandler,
onShow = noop,
timeout = 300,
} = settings;

const [ show, setShow ] = useState( false );
const timeoutRef = useRef< number | undefined >();

const setDebouncedTimeout = useCallback(
( callback ) => {
window.clearTimeout( timeoutRef.current );

timeoutRef.current = window.setTimeout( callback, timeout );
},
[ timeout ]
);

const handleOnMouseMove = useCallback( ( event ) => {
onMouseMove( event );

setDebouncedTimeout( () => {
if ( ! show ) {
setShow( true );
onShow();
}
} );
}, [] );

const handleOnMouseLeave = useCallback( ( event ) => {
onMouseLeave( event );

setDebouncedTimeout( () => {
setShow( false );
onHide();
} );
}, [] );

useEffect( () => {
return () => {
window.clearTimeout( timeoutRef.current );
};
} );

return {
onMouseMove: handleOnMouseMove,
onMouseLeave: handleOnMouseLeave,
};
}