Skip to content

Commit

Permalink
Components: refactor RangeControl to pass exhaustive-deps (#44271)
Browse files Browse the repository at this point in the history
* RangeControl: add missing deps to tooltip useCallback

* RangeControl: add missing dep to useControlledRangeValue useCallback

* RangeControl: add missing deps to useDebouncedHoverInteraction useCallbacks

* Components: update CHANGELOG

* Remove unused show/hide tooltip logic and the whole `useDebouncedHoverInteraction` hook

* Use Storybook actions for all `on.*` props on RangeControl

* Cleanup unused imports

Co-authored-by: Marco Ciampini <[email protected]>
  • Loading branch information
chad1008 and ciampo authored Sep 21, 2022
1 parent c1f792d commit 148c816
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 100 deletions.
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,
};
}

0 comments on commit 148c816

Please sign in to comment.