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

Containing element receives mouse click after dragging #273

Closed
lukebarnard1 opened this issue Jan 16, 2018 · 12 comments
Closed

Containing element receives mouse click after dragging #273

lukebarnard1 opened this issue Jan 16, 2018 · 12 comments

Comments

@lukebarnard1
Copy link

lukebarnard1 commented Jan 16, 2018

Bug or feature request?

Bug

Expected behavior

A mouse click event is not emitted on the parent element of the draggables after dragging and dropping.

Actual behavior

A mouse click event is emitted on the parent element of the draggables after dragging and dropping.

Browser version

Chrome Version 63.0.3239.84 (Official Build) (64-bit)

Demo

See logs of the following demo after dragging and dropping:
https://www.webpackbin.com/bins/-L2yYhx5eZi1MJnlU-79

lukebarnard1 added a commit to matrix-org/matrix-react-sdk that referenced this issue Jan 16, 2018
For some reason, after dragging an item
the parent draggable receives a mouse click. The workaround is
to use onMouseDown for deselecting tags
lukebarnard1 added a commit to matrix-org/matrix-react-sdk that referenced this issue Jan 16, 2018
@alexreardon
Copy link
Collaborator

This is very strange. I can see the issue in the basic example, but i cannot reproduce it in our examples. It looks like the draggable wrapper div is the target of the event which seems strange. I'll keep looking

@alexreardon
Copy link
Collaborator

Webpack bin seems to be down so i'll take a look at this later

@alexreardon
Copy link
Collaborator

Okay, I can see what the issue is:

When we are dragging we add pointer-events: none. To the drag handles. This means that when the user lifts there mouse a click event may occur but its event.target will be the div above the drag handle.

This is not the case for anchor tags which still get the click event 🙃

Some suggestions:

  • remove: pointer-events: none off the drag handle. (You could make this patch yourself) However, doing this will prevent the standard scroll actions while a drag is occurring.
  • Add a onClick handler to the parent of the drag handle and call event.stopPropagation() to prevent the click from bubbling up.

Sadly I do not think we can remove pointer-events: none at a library level as it is the only way to scroll while dragging until #27 lands. So I think the second option is your best option for now

@alexreardon
Copy link
Collaborator

Given that there is no further action to be taken at this time i'll close this. If people think that this problem should be solve more thoroughly we could revisit it. It will be a slightly different conversation after #27 lands.

@alexreardon
Copy link
Collaborator

This makes me sad. I am going to see if we can do better

@alexreardon alexreardon reopened this Jan 17, 2018
@alexreardon
Copy link
Collaborator

The trouble is even when if we remove pointer-events: none from the drag handles directly after a mouse up it does not change the click behaviour. It needs to not be there when the click actually occurred. Given that we currently need pointer-events: none to enable scrolling we cannot change this. Also, pointer-events: none adds some awesome performance. We would need to investigate the perf impact of removing it.

For now here is the best workaround:

+<div onClick={provided.dragHandleProps.onClick}>
  <div
    ref={provided.innerRef}
    {...provided.draggableProps}
    {...provided.dragHandleProps}
    style={getItemStyle(
      snapshot.isDragging,
      provided.draggableProps.style
    )}
  >
    {item.content}
  </div>
  {provided.placeholder}
</div>

By binding the drag handle onClick function to the parent you get the correct click blocking behaviour. This will allow actual clicks to go through, while blocking clicks after a drag (which is what already occurs on an anchor)

@lukebarnard1
Copy link
Author

@alexreardon thanks for looking into this - your response was very speedy! 😄

The workaround seems fine to me.

@alexreardon
Copy link
Collaborator

The best fix would require us to add an event handler to the parent. This could done without the consumers needing to bind it...
🤔

lukebarnard1 added a commit to matrix-org/matrix-react-sdk that referenced this issue Feb 28, 2018
Fix two bugs in Riot due to a single bug in react-b-dnd

Fixes element-hq/element-web#6253
Fixes element-hq/element-web#6156
lukebarnard1 added a commit to matrix-org/matrix-react-sdk that referenced this issue Mar 1, 2018
Fix two bugs in Riot due to a single bug in react-b-dnd

Fixes element-hq/element-web#6253
Fixes element-hq/element-web#6156
dbkr added a commit to matrix-org/matrix-react-sdk that referenced this issue Mar 1, 2018
@jeduardo824
Copy link

jeduardo824 commented May 12, 2020

Hey @alexreardon, is this working? I'm with a similar error on IE 11, and I tried with what you proposed but no success.

<Draggable draggableId={`${this.props.id}`} index={this.props.index}>
    {
        (provided) => (
            <div onMouseDown={provided.dragHandleProps.onMouseDown}>
                <div
                  className={`container ${containerClass}`}
                  ref={provided.innerRef}
                  {...provided.draggableProps}
                  {...provided.dragHandleProps}
                >
                  <MyComponent
                      id={id}
                      headline={headline}
                      thumbnail={thumbnail}
                  />
                </div>
            </div>
        )
    }
</Draggable>

@davidhan527
Copy link

Hi @alexreardon , I see that future versions of the project removed the provided.dragHandleProps.onClick function and so your workaround in #273 (comment) no longer works. I'm on the most recent version of the library. Any ideas on a different workaround now that provided.dragHandleProps.onClick was removed?

@mrmowji
Copy link

mrmowji commented Mar 27, 2021

Have you tried event.defaultPrevented on the parent's onClick?

@glenarama
Copy link

A little helper while awaiting anything 'official' the onClick event of the parent has property defaultPrevented set to true after a drag event on its child. So in your parent onClick event handler you can do something like:

if (!e.defaultPrevented) doSomething...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants