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

'isDragging' is incorrectly read as true when querying via 'getState()' #54

Open
emmettdn opened this issue Aug 9, 2020 · 2 comments
Open

Comments

@emmettdn
Copy link

emmettdn commented Aug 9, 2020

Hi, first of all thanks for the great library.

I'm currently having an issue where isDragging state is set to true but shouldn't be. This is reproducible when completing a mouse drag in the scrollable area then querying the state without clicking in the scrollable area again.

It seems to be to do with how we determine isDragging state in getState().

isDragging: !!(this.dragOffset.x || this.dragOffset.y),

Instead of using this.isDragging which is set to false on mouseup it relys on dragOffset which doesn't seem to be reset until another pointer event occurs.

Couldn't find any other issues relating to this so I am wondering if I am missing something?

Thanks in advance :)

@emmettdn emmettdn changed the title 'isDragging' is incorrectly set to true when querying via 'getState()' 'isDragging' is incorrectly read as true when querying via 'getState()' Aug 9, 2020
@mlshv
Copy link

mlshv commented May 17, 2021

I have the same problem! This issue definitely needs a fix

Upd
Just looked into the code and it seems like I have a problem of another kind but it looks similar. I'm trying to use drag-and-drop handler together with scrollbooster. I have shouldScroll function defined like this:

new ScrollBooster({
    viewport: containerRef.current,
    content: contentRef.current,
    scrollMode: 'native',
    direction: 'horizontal',
    textSelection: true,
    shouldScroll: (state, event) => {
        const isDragHandleToched = event.path.some(checkIsDragHandle)

        return !isDragHandleToched
    },
})

But it disables click handlers on a draggable element after a first scroll move with scrollbooster. And state.isDragging is always true on single click because of this line in scrollboster:

this.events.pointerdown = (event) => {
     // ....
     this.isDragging = true
}

I'm going to investigate further

Upd2
I think I've found the problem:

this.events.click = (event) => {
    const state = this.getState()
    const dragOffsetX = this.props.direction !== 'vertical' ? state.dragOffset.x : 0
    const dragOffsetY = this.props.direction !== 'horizontal' ? state.dragOffset.y : 0
    if (Math.max(Math.abs(dragOffsetX), Math.abs(dragOffsetY)) > CLICK_EVENT_THRESHOLD_PX) {
        // this is always true if you have scrolled to any amount of pixels more than 5
        // ...even if this.isDragging equals false
        event.preventDefault()
        event.stopPropagation()
    }
    this.props.onClick(state, event, isTouch)
}

My solution is to add this.isDragging to the if statement:

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

I didn't dig in enough to say it should be patched on the main branch or if it going to work in your case, but it worked for me

@asmedberg
Copy link

Upd2 I think I've found the problem:

this.events.click = (event) => {
    const state = this.getState()
    const dragOffsetX = this.props.direction !== 'vertical' ? state.dragOffset.x : 0
    const dragOffsetY = this.props.direction !== 'horizontal' ? state.dragOffset.y : 0
    if (Math.max(Math.abs(dragOffsetX), Math.abs(dragOffsetY)) > CLICK_EVENT_THRESHOLD_PX) {
        // this is always true if you have scrolled to any amount of pixels more than 5
        // ...even if this.isDragging equals false
        event.preventDefault()
        event.stopPropagation()
    }
    this.props.onClick(state, event, isTouch)
}

My solution is to add this.isDragging to the if statement:

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

I didn't dig in enough to say it should be patched on the main branch or if it going to work in your case, but it worked for me

Not sure this project is getting updated but this solution worked for my project as well.

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

3 participants