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.

Sign up for GitHub

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

[EuiResizableContainer] Mouse drags outside of the container no longer lose dragging state #7456

Merged
merged 4 commits into from
Jan 11, 2024
Merged

[EuiResizableContainer] Mouse drags outside of the container no longer lose dragging state #7456

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3f1346b
[EuiResizableContainer] Switch from React events to window event list…
cee-chen Jan 10, 2024
a291648
Update resizable helper to work with native touch/mouse events
cee-chen Jan 10, 2024
10bf4ee
Update EuiFlyoutResizable to use EuiResizableContainer helper
cee-chen Jan 10, 2024
1e20797
changelog
cee-chen Jan 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 10 additions & 14 deletions src/components/resizable_container/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,10 @@ interface Params {
onPanelWidthChange?: ({}: { [key: string]: number }) => any;
}

function isMouseEvent(
event: ReactMouseEvent | ReactTouchEvent
): event is ReactMouseEvent {
return typeof event === 'object' && 'pageX' in event && 'pageY' in event;
}
export const isTouchEvent = (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't previously exported. Did you mean to add this as an export?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured I might as well, although I'm happy to revert if needed! I originally grabbed this util for EuiFlyoutResizable to replace some similar/duplicated code, before realizing I could actually use the parent exported getPosition() util entirely.

That being said, I could definitely see myself using this util again in other places in EUI in the future, if needed 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine either way, I just wanted to double check.

event: MouseEvent | ReactMouseEvent | TouchEvent | ReactTouchEvent
): event is TouchEvent | ReactTouchEvent =>
typeof event === 'object' && 'targetTouches' in event;

export const pxToPercent = (proportion: number, whole: number) => {
if (whole < 1 || proportion < 0) return 0;
Expand Down Expand Up @@ -81,16 +80,13 @@ export const getPanelMinSize = (
};

export const getPosition = (
event: ReactMouseEvent | ReactTouchEvent,
event: ReactMouseEvent | MouseEvent | ReactTouchEvent | TouchEvent,
isHorizontal: boolean
) => {
const clientX = isMouseEvent(event)
? event.clientX
: event.touches[0].clientX;
const clientY = isMouseEvent(event)
? event.clientY
: event.touches[0].clientY;
return isHorizontal ? clientX : clientY;
): number => {
const direction = isHorizontal ? 'clientX' : 'clientY';
return isTouchEvent(event)
? event.targetTouches[0][direction]
: event[direction];
};

const getSiblingPanel = (
Expand Down
24 changes: 16 additions & 8 deletions src/components/resizable_container/resizable_container.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
*/

import React from 'react';
import { act } from 'react-dom/test-utils';
import { mount } from 'enzyme';

import { findTestSubject, requiredProps } from '../../test';
import { shouldRenderCustomStyles } from '../../test/internal';
import { render } from '../../test/rtl';
Expand Down Expand Up @@ -217,8 +219,7 @@ describe('EuiResizableContainer', () => {
};

test('onResizeStart and onResizeEnd are called for pointer events', () => {
const { container, button, onResizeStart, onResizeEnd } =
mountWithCallbacks();
const { button, onResizeStart, onResizeEnd } = mountWithCallbacks();
button.simulate('mousedown', {
pageX: 0,
pageY: 0,
Expand All @@ -227,7 +228,9 @@ describe('EuiResizableContainer', () => {
});
expect(onResizeStart).toHaveBeenCalledTimes(1);
expect(onResizeStart).toHaveBeenLastCalledWith('pointer');
container.simulate('mouseup');
act(() => {
window.dispatchEvent(new Event('mouseup'));
});
expect(onResizeEnd).toHaveBeenCalledTimes(1);
button.simulate('mousedown', {
pageX: 0,
Expand All @@ -237,7 +240,9 @@ describe('EuiResizableContainer', () => {
});
expect(onResizeStart).toHaveBeenCalledTimes(2);
expect(onResizeStart).toHaveBeenLastCalledWith('pointer');
container.simulate('mouseleave');
act(() => {
window.dispatchEvent(new Event('mouseup'));
});
expect(onResizeEnd).toHaveBeenCalledTimes(2);
button.simulate('touchstart', {
touches: [
Expand All @@ -249,7 +254,9 @@ describe('EuiResizableContainer', () => {
});
expect(onResizeStart).toHaveBeenCalledTimes(3);
expect(onResizeStart).toHaveBeenLastCalledWith('pointer');
container.simulate('touchend');
act(() => {
window.dispatchEvent(new Event('touchend'));
});
expect(onResizeEnd).toHaveBeenCalledTimes(3);
});

Expand Down Expand Up @@ -312,8 +319,7 @@ describe('EuiResizableContainer', () => {
});

test('onResizeEnd is called before starting a new resize if a keyboard resize is triggered while a pointer resize is in progress', () => {
const { container, button, onResizeStart, onResizeEnd } =
mountWithCallbacks();
const { button, onResizeStart, onResizeEnd } = mountWithCallbacks();
button.simulate('mousedown', {
pageX: 0,
pageY: 0,
Expand All @@ -326,7 +332,9 @@ describe('EuiResizableContainer', () => {
expect(onResizeEnd).toHaveBeenCalledTimes(1);
expect(onResizeStart).toHaveBeenCalledTimes(2);
expect(onResizeStart).toHaveBeenLastCalledWith('key');
container.simulate('mouseup');
act(() => {
window.dispatchEvent(new Event('mouseup'));
});
expect(onResizeEnd).toHaveBeenCalledTimes(1);
button.simulate('keyup', { key: keys.ARROW_RIGHT });
expect(onResizeEnd).toHaveBeenCalledTimes(2);
Expand Down
67 changes: 25 additions & 42 deletions src/components/resizable_container/resizable_container.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,32 +165,32 @@ export const EuiResizableContainer: FunctionComponent<
const position = getPosition(event, isHorizontal);
resizeStart('pointer');
actions.dragStart({ position, prevPanelId, nextPanelId });
},
[actions, isHorizontal, resizeStart]
);

const onMouseMove = useCallback(
(event: React.MouseEvent | React.TouchEvent) => {
if (
!reducerState.prevPanelId ||
!reducerState.nextPanelId ||
!reducerState.isDragging
)
return;
const position = getPosition(event, isHorizontal);
actions.dragMove({
position,
prevPanelId: reducerState.prevPanelId,
nextPanelId: reducerState.nextPanelId,
});
// Window event listeners instead of React events are used to continue
// detecting movement even if the user's mouse leaves the container

const onMouseMove = (event: MouseEvent | TouchEvent) => {
const position = getPosition(event, isHorizontal);
actions.dragMove({ position, prevPanelId, nextPanelId });
};

const onMouseUp = () => {
if (resizeContext.current.trigger === 'pointer') {
resizeEnd();
}
actions.reset();

window.removeEventListener('mousemove', onMouseMove);
window.removeEventListener('mouseup', onMouseUp);
window.removeEventListener('touchmove', onMouseMove);
window.removeEventListener('touchend', onMouseUp);
};
window.addEventListener('mousemove', onMouseMove);
window.addEventListener('mouseup', onMouseUp);
window.addEventListener('touchmove', onMouseMove);
window.addEventListener('touchend', onMouseUp);
},
[
actions,
isHorizontal,
reducerState.prevPanelId,
reducerState.nextPanelId,
reducerState.isDragging,
]
[actions, isHorizontal, resizeStart, resizeEnd]
);

const getKeyMoveDirection = useCallback(
Expand Down Expand Up @@ -246,13 +246,6 @@ export const EuiResizableContainer: FunctionComponent<
[getKeyMoveDirection, resizeEnd]
);

const onMouseUp = useCallback(() => {
if (resizeContext.current.trigger === 'pointer') {
resizeEnd();
}
actions.reset();
}, [actions, resizeEnd]);

const onBlur = useCallback(() => {
if (resizeContext.current.trigger === 'key') {
resizeEnd();
Expand Down Expand Up @@ -322,17 +315,7 @@ export const EuiResizableContainer: FunctionComponent<
resizers: reducerState.resizers,
}}
>
<div
css={cssStyles}
className={classes}
ref={containerRef}
onMouseMove={reducerState.isDragging ? onMouseMove : undefined}
onMouseUp={onMouseUp}
onMouseLeave={onMouseUp}
onTouchMove={onMouseMove}
onTouchEnd={onMouseUp}
{...rest}
>
<div css={cssStyles} className={classes} ref={containerRef} {...rest}>
{render()}
</div>
</EuiResizableContainerContextProvider>
Expand Down