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

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

Merged
merged 4 commits into from
Jan 11, 2024

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Jan 10, 2024

Summary

closes #4931

This PR updates EuiResizableContainer to use window event listeners for mouseup/mousemove, which is also how EuiFlyoutResizable works (#7439). Event listeners are created on mousedown and removed on mouseup.

The net result is that users can now drag outside resizable containers and not have their drag event abruptly ended. IMO, this provides a user experience with less friction and matches many other drag/resize experiences around the web.

Before After
resizable_container_drag_before Production UX resizable_container_drag New UX

QA

General checklist

  • Browser QA
    - [ ] Checked in both light and dark modes
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
    • N/A, no existing documentation around this behavior
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.
      - [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist - N/A

…ener

- which allows the user to drag their mouse wherever they need to on the window, instead of stopping outside the container only
+ update util for touch vs mouse events to check for touch instead of mouse, and to use `targetTouches` as opposed to just `touches`
- avoids TS casting (yay!) and also uses `clientX` over `pageX` (which on further googling is probably better)
@cee-chen cee-chen force-pushed the resizable-container-4931 branch from 940d9d4 to 1e20797 Compare January 10, 2024 21:43
@cee-chen cee-chen marked this pull request as ready for review January 10, 2024 21:47
@cee-chen cee-chen requested a review from a team as a code owner January 10, 2024 21:47
@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

): 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.

Copy link
Member

@JasonStoltz JasonStoltz left a comment

Choose a reason for hiding this comment

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

Works as advertised, and code LGTM.

@@ -155,13 +156,3 @@ export const EuiFlyoutResizable = forwardRef(
}
);
EuiFlyoutResizable.displayName = 'EuiFlyoutResizable';

const getMouseOrTouchX = (
Copy link
Member

Choose a reason for hiding this comment

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

FWIW, our docs still have a copy of this method in use. Is getPosition exported from our package in a way that we can update our docs to use this approach as well?

Copy link
Member

Choose a reason for hiding this comment

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

Screenshot 2024-01-11 at 2 16 23 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooo, thanks for catching this! It looks like we don't export any resizable utils, but I'm now wondering if we should. What do you think? Should we add a new public export for this, probably under src/services? Maybe src/services/drag, or src/services/touch_or_mouse? 🤷

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 not sure if its worth it or not. I had a suspicion people were copy/pasting this example verbatim and it might clean that up -- but I don't see any examples in Kibana where it looks like someone would have done so. I don't think we need to solve this now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair! Will leave this uncollapsed for now in case we decide to come back to this later on.

@cee-chen cee-chen merged commit 154014e into elastic:main Jan 11, 2024
8 checks passed
@cee-chen cee-chen deleted the resizable-container-4931 branch January 11, 2024 22:17
cee-chen added a commit to elastic/kibana that referenced this pull request Jan 24, 2024
`v92.0.0-backport.0`⏩ `v92.1.1`

---

## [`v92.1.1`](https://github.com/elastic/eui/releases/v92.1.1)

**Bug fixes**

- Minor `EuiDataGrid` cell performance fixes
([#7465](elastic/eui#7465))

## [`v92.1.0`](https://github.com/elastic/eui/releases/v92.1.0)

- Updated `EuiResizableButton` to allow customizing the `indicator`
style with either `handle` (default) or `border`
([#7455](elastic/eui#7455))
- Enhanced `EuiResizableContainer` to preserve the drag/resize event
when the user's mouse leaves the parent container and re-enters
([#7456](elastic/eui#7456))

**Bug fixes**

- Fixed an `EuiTreeView` JSX Typescript error
([#7452](elastic/eui#7452))
- Fixed a color console warning being generated by disabled `EuiStep`s
([#7454](elastic/eui#7454))

**Accessibility**

- `EuiDataGrid`'s keyboard/screenreader experience has been tweaked to
be more consistent for varying complex data:
([#7448](elastic/eui#7448))
- Headers are now always navigable by arrow key, regardless of whether
the header cells contain interactive content
- Non-expandable cells containing any amount of interactive content now
must be entered via Enter or F2 keypress
  - Expandable cells continue to be toggled via Enter or F2 keypress
- `EuiDataGrid` now provides a direct screen reader hint for Enter key
behavior for expandable & interactive cells
([#7448](elastic/eui#7448))
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
`v92.0.0-backport.0`⏩ `v92.1.1`

---

## [`v92.1.1`](https://github.com/elastic/eui/releases/v92.1.1)

**Bug fixes**

- Minor `EuiDataGrid` cell performance fixes
([elastic#7465](elastic/eui#7465))

## [`v92.1.0`](https://github.com/elastic/eui/releases/v92.1.0)

- Updated `EuiResizableButton` to allow customizing the `indicator`
style with either `handle` (default) or `border`
([elastic#7455](elastic/eui#7455))
- Enhanced `EuiResizableContainer` to preserve the drag/resize event
when the user's mouse leaves the parent container and re-enters
([elastic#7456](elastic/eui#7456))

**Bug fixes**

- Fixed an `EuiTreeView` JSX Typescript error
([elastic#7452](elastic/eui#7452))
- Fixed a color console warning being generated by disabled `EuiStep`s
([elastic#7454](elastic/eui#7454))

**Accessibility**

- `EuiDataGrid`'s keyboard/screenreader experience has been tweaked to
be more consistent for varying complex data:
([elastic#7448](elastic/eui#7448))
- Headers are now always navigable by arrow key, regardless of whether
the header cells contain interactive content
- Non-expandable cells containing any amount of interactive content now
must be entered via Enter or F2 keypress
  - Expandable cells continue to be toggled via Enter or F2 keypress
- `EuiDataGrid` now provides a direct screen reader hint for Enter key
behavior for expandable & interactive cells
([elastic#7448](elastic/eui#7448))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EuiResizableContainer] Resizing mouse loses dragging if move out of panels
4 participants