Skip to content

fix: manual click event trigger as the card was detached from DOM during dragging#133

Merged
gajus merged 1 commit intogajus:masterfrom
adelura:fix/no_click_triggered
Sep 3, 2018
Merged

fix: manual click event trigger as the card was detached from DOM during dragging#133
gajus merged 1 commit intogajus:masterfrom
adelura:fix/no_click_triggered

Conversation

@adelura
Copy link
Copy Markdown
Contributor

@adelura adelura commented Sep 3, 2018

Problem

I noticed that click event is not triggered when user interact with the card for the very first time. The issues occurs only on the desktop browsers.

Findings

Card is detached from the DOM on mousedown so it can be moved to front of the stack.

Solution

Tracking whether card was detached from the DOM when card was pressed (mousedown), if so then clicked event is triggered manually.

@gajus gajus merged commit 9a19ea2 into gajus:master Sep 3, 2018
@adelura
Copy link
Copy Markdown
Contributor Author

adelura commented Sep 3, 2018

I don't know how to start... I think that this PR shouldn't be merged. I didn't expected your responce so quick 😢 (I know it's weird to complain about that...) I was writing a message here when I realised that you merged my PR. I am sorry to say that - it's my fault.

Although it's a standard case that the event is not triggered when the element is detached from the DOM tree, I noticed that it works on firefox... As it works on Firefox, it means that the event is triggered twice once, user click. I was trying to figure out what to do on FF, but it's quite tricky.

Here is the simple example:
http://jsfiddle.net/adelura/63L9uohj/9/

I was thinking about adding isChrome and trigger click event only on chrome (I got code already).

adelura@32f97e1 but this requires some more testing, so I'm not sure which browsers can be affected. Please tell me what do you think about that.

But from the other side maybe it's better when event is triggered twice than never.

@gajus
Copy link
Copy Markdown
Owner

gajus commented Sep 4, 2018

Feel free to improve it and raise a PR again.

@adelura
Copy link
Copy Markdown
Contributor Author

adelura commented Sep 4, 2018

Definitely will do as it's critical feature for my app. Will look at this next Friday or so, as I'm heading off for holiday now.

@adelura
Copy link
Copy Markdown
Contributor Author

adelura commented Sep 16, 2018

I tested this functionality against the following desktop browsers (as this feature is implemented on non-touch devices).

  • Latest Firefox
  • Latest Chrome
  • Latest Opera
  • Microsoft Edge

It turns out that only firefox trigger the click event. It means that after applying my fix, the "click" event is triggered twice.

I propose to exclude firefox from my fix (triggering an event manually). I would prefer to solve this through feature detection, but I have no idea how to figure out whether the click event was triggered manually.

Please tell me if you are happy with that. If so, then I will open PR with FF check.


Update

Okay, I figured out how to determine if the click was triggered netively. It's a little bit hacky as I had to us setTimeout. I will push the code tomorrow.

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

Successfully merging this pull request may close these issues.

2 participants