-
Notifications
You must be signed in to change notification settings - Fork 344
use pointer events instead of mouse + touch #960
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
use pointer events instead of mouse + touch #960
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR replaces mouse and touch events with pointer events across the codebase for more consistent handling on mobile devices.
- Replaced
mouse*
andtouch*
event invocations in tests withpointer*
- Updated
DataGrid
andClickOutsideContainer
to register pointer event listeners and normalize pointer input - Added a JSDOM polyfill for
PointerEvent
instandardBeforeEach
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
packages/core/test/test-utils.tsx | Switched sendClick /sendTouchClick helpers to pointer events |
packages/core/test/data-grid.test.tsx | Replaced test mouse* with pointer* events and added setup call |
packages/core/test/data-editor.test.tsx | Replaced test mouse* with pointer* events |
packages/core/test/data-editor-resize.test.tsx | Replaced test mouse* with pointer* events |
packages/core/test/data-editor-input.test.tsx | Imported and invoked standardBeforeEach in tests |
packages/core/src/internal/data-grid/data-grid.tsx | Reworked event listeners and handlers to use pointer events |
packages/core/src/internal/click-outside-container/click-outside-container.tsx | Swapped touch/mouse listeners for pointerdown |
Comments suppressed due to low confidence (4)
packages/core/test/test-utils.tsx:36
- [nitpick] Variable name
mouseOptions
is misleading since it’s used for pointer events; consider renaming it topointerOptions
oreventOptions
.
const mouseOptions = {
packages/core/src/internal/data-grid/data-grid.tsx:1263
- The
onPointerMove
callback is typed asMouseEvent
but should acceptPointerEvent
to align with thepointermove
listener and access properties likepointerType
.
const onPointerMove = React.useCallback((ev: MouseEvent) => {
packages/core/test/data-editor.test.tsx:12
- Tests now use
pointer*
events butstandardBeforeEach
(which polyfillsPointerEvent
) isn’t imported or called; addimport { standardBeforeEach } from "./test-utils.js";
and invoke it inbeforeEach
.
import { vi, expect, describe, test, beforeEach, afterEach } from "vitest";
packages/core/test/data-editor-resize.test.tsx:1
- This test file uses
pointer*
events but does not set upPointerEvent
polyfill; import and callstandardBeforeEach
fromtest-utils.tsx
in thebeforeEach
setup.
import { render, screen, fireEvent, act } from "@testing-library/react";
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Merging it. @BrianHung there is also the use-kinetic-scroll.ts
that uses touchstart
/ touchend
, but there is probably no need to update this hook since its optional and should work fine with the current events, right?
Yeah, that hook is intentionally for iOS mobile so I kept touch events there. |
This reverts commit 1e493de.
Fixes #806
Not entirely sure why pointer events are more consistent on mobile than touch events.