Skip to content

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

Merged
merged 3 commits into from
Jun 25, 2025

Conversation

BrianHung
Copy link
Collaborator

@BrianHung BrianHung commented Jun 1, 2024

Fixes #806

Not entirely sure why pointer events are more consistent on mobile than touch events.

@lukasmasuch lukasmasuch requested a review from Copilot June 16, 2025 23:43
Copy link
Contributor

@Copilot Copilot AI left a 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* and touch* event invocations in tests with pointer*
  • Updated DataGrid and ClickOutsideContainer to register pointer event listeners and normalize pointer input
  • Added a JSDOM polyfill for PointerEvent in standardBeforeEach

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 to pointerOptions or eventOptions.
const mouseOptions = {

packages/core/src/internal/data-grid/data-grid.tsx:1263

  • The onPointerMove callback is typed as MouseEvent but should accept PointerEvent to align with the pointermove listener and access properties like pointerType.
const onPointerMove = React.useCallback((ev: MouseEvent) => {

packages/core/test/data-editor.test.tsx:12

  • Tests now use pointer* events but standardBeforeEach (which polyfills PointerEvent) isn’t imported or called; add import { standardBeforeEach } from "./test-utils.js"; and invoke it in beforeEach.
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 up PointerEvent polyfill; import and call standardBeforeEach from test-utils.tsx in the beforeEach setup.
import { render, screen, fireEvent, act } from "@testing-library/react";

Copy link
Collaborator

@lukasmasuch lukasmasuch left a 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?

@lukasmasuch lukasmasuch merged commit 1e493de into glideapps:main Jun 25, 2025
4 checks passed
@BrianHung
Copy link
Collaborator Author

Yeah, that hook is intentionally for iOS mobile so I kept touch events there.

lukasmasuch added a commit that referenced this pull request Jun 26, 2025
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.

Touch doesn't execute header menu on mobile
2 participants