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

event.preventDefault() in onClick does not work in this use case #39

Open
WilliamVenner opened this issue Mar 12, 2020 · 5 comments
Open

Comments

@WilliamVenner
Copy link

Use case: http://i.venner.io/2020-03-12_20-03-12.webm

I fixed it by changing:

this.props.viewport.addEventListener('click', this.events.click);

to

this.props.viewport.addEventListener('click', this.events.click, true);

However, I am not sure of the implications this would have on other users.

@WilliamVenner
Copy link
Author

I tried using event.preventDefault(), event.stopPropagation() and event.stopImmediatePropagation() in onClick to try and fix this originally, but none of these worked.

@ilyashubin
Copy link
Owner

Hello. Could you please provide your initialization code? I cannot reproduce it yet

@mdings
Copy link

mdings commented Jul 23, 2020

I have the same issue. In my case this happens when the elements of the scollable container have an onClick property.

<div class="scroll-element" onclick={DoSomething}></div>

They seem to trigger first before the onClick method from scrollBooster is being triggered and therefore don't seem to be preventable. Adding true to the eventListener as suggested as a 3d param also fixes this in my case

As a workaround without having to alter the source code this works as well:

const viewport = document.querySelector('.viewport')
const sb = new ScrollBooster({
  //options
  viewport: viewport,
  onClick: (state, event) => {
    // your onclick logic
  }
})

// add this to prevent any underlying clicks from happening
viewport.addEventListener('click', sb.events.click, true)

@exil0867
Copy link

I have the same issue. In my case this happens when the elements of the scollable container have an onClick property.

<div class="scroll-element" onclick={DoSomething}></div>

They seem to trigger first before the onClick method from scrollBooster is being triggered and therefore don't seem to be preventable. Adding true to the eventListener as suggested as a 3d param also fixes this in my case

As a workaround without having to alter the source code this works as well:

const viewport = document.querySelector('.viewport')
const sb = new ScrollBooster({
  //options
  viewport: viewport,
  onClick: (state, event) => {
    // your onclick logic
  }
})

// add this to prevent any underlying clicks from happening
viewport.addEventListener('click', sb.events.click, true)

Thank you, works!

@Techn1x
Copy link

Techn1x commented Mar 30, 2022

Came here to make the same suggestion, that the viewport should probably be capturing events and handling them first with capture: true
this.props.viewport.addEventListener('click', this.events.click, true)

The click handler correctly detects whether a scroll was initiated or not, and if dragging stops the event

scrollbooster/src/index.js

Lines 595 to 598 in 31ee2d1

if (Math.max(Math.abs(dragOffsetX), Math.abs(dragOffsetY)) > CLICK_EVENT_THRESHOLD_PX) {
event.preventDefault();
event.stopPropagation();
}

The problem is that when addEventListener doesn't have capture set, the click event is dispatched to listeners on elements lower down in the DOM tree inside the viewport first (eg the link you clicked on, or something). But we need the scrollbooster click handler called first to check if it's a drag or not.
https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#syntax

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

No branches or pull requests

5 participants