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

Ripple: ripple start point is not init in mobile env #3526

Closed
Lanace opened this issue Oct 31, 2022 · 4 comments · Fixed by #3530
Closed

Ripple: ripple start point is not init in mobile env #3526

Lanace opened this issue Oct 31, 2022 · 4 comments · Fixed by #3530
Assignees
Labels
Type: Bug Issue contains a defect related to a specific component.
Milestone

Comments

@Lanace
Copy link

Lanace commented Oct 31, 2022

Describe the bug

in mobile, ripple position is always top: 0px, left: 0px.
because onMouseDown params is not MouseEvent in mobile
it's TouchDown event.

so use targetTouches pageX and pageY

var offset = DomHandler.getOffset(targetRef.current);

var x =
  event.targetTouches[0].pageX -
  offset.left +
  document.body.scrollTop -
  DomHandler.getWidth(inkRef.current) / 2;
var y =
  event.targetTouches[0].pageY -
  offset.top +
  document.body.scrollLeft -
  DomHandler.getHeight(inkRef.current) / 2;

and i have one question...
why event prevent in onTouchStart?
i want scroll event, but touch start event prevented.

var onTouchStart = function onTouchStart(event) {
  onMouseDown(event);
  // event.preventDefault();
};

Reproducer

No response

PrimeReact version

8.7.1-SNAPSHOT

React version

18.x

Language

TypeScript

Build / Runtime

Vite

Browser(s)

Chrome 106.0.5249.119

Steps to reproduce the behavior

  1. in mobile device, click button
  2. ripple effect start incorrect position

Expected behavior

ripple effect start on touched point

@Lanace Lanace added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Oct 31, 2022
@Lanace
Copy link
Author

Lanace commented Oct 31, 2022

cc @kkimdev

ref: #3499

@melloware
Copy link
Member

@Lanace the reason I use event.preventDefault is because of this: https://web.dev/mobile-touchandmouse/

The order of mouse events is this:

image

So it would call onMouseDown after calling onTouchStart and I don't think you want that happening if TouchStart is fired you don't want to then fire again on MouseDown.

@melloware melloware self-assigned this Oct 31, 2022
@melloware melloware added Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Oct 31, 2022
@melloware melloware added this to the 8.7.1 milestone Oct 31, 2022
@melloware
Copy link
Member

Can you review my PR.

@melloware
Copy link
Member

OK I was able to handle event.preventDefault a different way.

@melloware melloware added Type: Bug Issue contains a defect related to a specific component. and removed Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add labels Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a defect related to a specific component.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants