ilyashubin/scrollbooster

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

Opened this issue · 5 comments

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.

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

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

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)

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!

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