Skip to content

Commit

Permalink
fix: only allow position prop to override internal position if it has…
Browse files Browse the repository at this point in the history
… changed
  • Loading branch information
nerdyman committed Jan 14, 2020
1 parent 701ef06 commit 9be371a
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 19 deletions.
38 changes: 19 additions & 19 deletions src/react-compare-slider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { useEffect, useCallback, useRef, useState } from 'react';

import {
useEventListener,
usePrevious,
UseResizeObserverHandlerParams,
useResizeObserver,
} from './utils';
Expand All @@ -11,7 +12,9 @@ type ReactCompareSliderPropPosition = number;

/** Common props shared between child components */
interface ReactCompareSliderCommonProps {
/** Orientation */
portrait?: boolean;
/** Divider position in pixels */
position: ReactCompareSliderPropPosition;
}

Expand Down Expand Up @@ -100,8 +103,8 @@ export interface ReactCompareSliderProps {
/** Callback on position change */
onPositionChange?: (position: ReactCompareSliderPropPosition) => void;
/** Orientation */
portrait?: boolean;
/** Percentage position of divide */
portrait?: ReactCompareSliderCommonProps['portrait'];
/** Percentage position of divide (`0-100`) */
position?: ReactCompareSliderPropPosition;
}

Expand All @@ -117,19 +120,21 @@ export const ReactCompareSlider: React.FC<ReactCompareSliderProps &
style,
...props
}): React.ReactElement => {
const [containerBounds, setContainerBounds] = useState({
width: 0,
height: 0,
});
const containerRef = useRef<HTMLDivElement>(document.createElement('div'));
const prevPropsPosition = usePrevious(position);
const internalPositionPc = useRef(position);
const [internalPositionPx, setInternalPositionPx] = useState(0);
const [isDragging, setIsDragging] = useState(false);
const hasWindowBinding = useRef(false);

const updateInternalPosition = useCallback(
({ x, y }: { x: number; y: number }) => {
const { top, left } = containerRef.current.getBoundingClientRect();
const {
top,
left,
width,
height,
} = containerRef.current.getBoundingClientRect();

const positionPx = portrait
? y - top - window.pageYOffset
Expand All @@ -139,22 +144,20 @@ export const ReactCompareSlider: React.FC<ReactCompareSliderProps &

// Calculate percentage with bounds checking
internalPositionPc.current = Math.min(
Math.max(
(positionPx /
(portrait ? containerBounds.height : containerBounds.width)) *
100,
0
),
Math.max((positionPx / (portrait ? height : width)) * 100, 0),
100
);

if (onPositionChange) onPositionChange(internalPositionPc.current);
},
[containerBounds.height, containerBounds.width, onPositionChange, portrait]
[onPositionChange, portrait]
);

// Update internal position if `position` prop changes
useEffect(() => {
// Early out if position hasn't changed
if (prevPropsPosition === position) return;

const {
top,
left,
Expand All @@ -166,6 +169,8 @@ export const ReactCompareSlider: React.FC<ReactCompareSliderProps &
x: (width / 100) * position + left,
y: (height / 100) * position + top,
});
// `prevPropsPosition` is a ref value so it shouldn't been in deps
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [position, updateInternalPosition]);

const handlePointerDown = useCallback(
Expand Down Expand Up @@ -202,11 +207,6 @@ export const ReactCompareSlider: React.FC<ReactCompareSliderProps &

const handleResize = useCallback(
({ width, height }: UseResizeObserverHandlerParams) => {
setContainerBounds({
width,
height,
});

setInternalPositionPx(
((portrait ? height : width) / 100) * internalPositionPc.current
);
Expand Down
16 changes: 16 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,25 @@ export const styleFitContainer = ({
height: '100%',
maxWidth: '100%',
objectFit,
objectPosition,
...props,
});

/**
* Use previous value
* @see https://usehooks.com/usePrevious/
*/
export const usePrevious = <T>(value: T): T | undefined => {
const ref = useRef<T>();

useEffect(() => {
ref.current = value;
}, [value]);

// Return previous value (happens before update in useEffect above)
return ref.current;
};

/**
* Event listener binding hook
* @param eventName - Event to bind to
Expand Down

0 comments on commit 9be371a

Please sign in to comment.