Skip to content

Commit

Permalink
[EuiResizableContainer] Fix issue where onResizeEnd becomes stale whe…
Browse files Browse the repository at this point in the history
…n renders occur between resize start and end
  • Loading branch information
davismcphee committed Jan 18, 2024
1 parent 434c4d1 commit 60c32b7
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState } from 'react';
import React, { useCallback, useState } from 'react';
import {
EuiText,
EuiResizableContainer,
Expand Down Expand Up @@ -27,6 +27,16 @@ export default () => {
[secondPanelId]: 50,
});

const onResizeStart = useCallback((trigger: 'pointer' | 'key') => {
setResizeTrigger(trigger);
}, []);

const onResizeEnd = useCallback(() => {
if (resizeTrigger) {
setResizeTrigger(undefined);
}
}, [resizeTrigger]);

return (
<EuiFlexGroup direction="column">
<EuiFlexItem>
Expand Down Expand Up @@ -69,8 +79,8 @@ export default () => {
onPanelWidthChange={(newSizes) => {
setSizes((prevSizes) => ({ ...prevSizes, ...newSizes }));
}}
onResizeStart={(trigger) => setResizeTrigger(trigger)}
onResizeEnd={() => setResizeTrigger(undefined)}
onResizeStart={onResizeStart}
onResizeEnd={onResizeEnd}
>
{(EuiResizablePanel, EuiResizableButton) => (
<>
Expand Down
19 changes: 19 additions & 0 deletions src/components/resizable_container/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {
useReducer,
MouseEvent as ReactMouseEvent,
TouchEvent as ReactTouchEvent,
useRef,
useState,
} from 'react';

import { assertNever } from '../common';
Expand Down Expand Up @@ -606,3 +608,20 @@ export const useContainerCallbacks = ({

return [actions, reducerState];
};

/**
* Gives the provided callback a stable reference, allowing it to be
* be used outside the React lifecycle without causing stale closures.
*/
export const useStableCallback = <T extends (...args: any[]) => any>(
callback: T
): ((...args: Parameters<T>) => ReturnType<T>) => {
const callbackRef = useRef(callback);
const [stableCallback] = useState(() => (...args: Parameters<T>) => {
return callbackRef.current(...args);
});

callbackRef.current = callback;

return stableCallback;
};
22 changes: 21 additions & 1 deletion src/components/resizable_container/resizable_container.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ describe('EuiResizableContainer', () => {
);
const container = findTestSubject(component, 'euiResizableContainer');
const button = findTestSubject(component, 'euiResizableButton');
return { container, button, onResizeStart, onResizeEnd };
return { component, container, button, onResizeStart, onResizeEnd };
};

test('onResizeStart and onResizeEnd are called for pointer events', () => {
Expand Down Expand Up @@ -348,5 +348,25 @@ describe('EuiResizableContainer', () => {
button.simulate('blur');
expect(onResizeEnd).toHaveBeenCalledTimes(1);
});

test('onResizeEnd does not go stale when renders occur between resize start and end', async () => {
const { component, button, onResizeStart, onResizeEnd } =
mountWithCallbacks();
button.simulate('mousedown', {
pageX: 0,
pageY: 0,
clientX: 0,
clientY: 0,
});
expect(onResizeStart).toHaveBeenCalledTimes(1);
expect(onResizeStart).toHaveBeenLastCalledWith('pointer');
const newResizeEnd = jest.fn();
component.setProps({ onResizeEnd: newResizeEnd });
act(() => {
window.dispatchEvent(new Event('mouseup'));
});
expect(onResizeEnd).toHaveBeenCalledTimes(0);
expect(newResizeEnd).toHaveBeenCalledTimes(1);
});
});
});
10 changes: 7 additions & 3 deletions src/components/resizable_container/resizable_container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ import {
getModeType,
ToggleCollapseCallback,
} from './resizable_panel';
import { useContainerCallbacks, getPosition } from './helpers';
import {
useContainerCallbacks,
getPosition,
useStableCallback,
} from './helpers';
import {
EuiResizableButtonMouseEvent,
EuiResizableButtonKeyEvent,
Expand Down Expand Up @@ -134,10 +138,10 @@ export const EuiResizableContainer: FunctionComponent<
keyMoveDirection?: KeyMoveDirection;
}>({});

const resizeEnd = useCallback(() => {
const resizeEnd = useStableCallback(() => {
onResizeEnd?.();
resizeContext.current = {};
}, [onResizeEnd]);
});

const resizeStart = useCallback(
(trigger: ResizeTrigger, keyMoveDirection?: KeyMoveDirection) => {
Expand Down

0 comments on commit 60c32b7

Please sign in to comment.