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

Replaces click #2

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Replaces click #2

wants to merge 5 commits into from

Conversation

diachedelic
Copy link
Contributor

Hey, first of all great lib - none of the other seem to handle both mouse and touch events correctly. I am trying to use this lib to trigger actions on items in a list (i.e. hold to delete), but I wanted to avoid actually selecting the item. I've added a param which suppresses the click event if a long click is triggered.

@kaechele
Copy link

kaechele commented Aug 7, 2019

I tried this and it didn't work for me on Chrome on Mac and iPhone. The click event still registers and the handler is fired.

While testing I also noticed that this doesn't prevent force touch events from occurring on phones that support it. In my case an image inside the button would pop out and offer itself for sharing or saving.
This can be prevented by setting the CSS property pointer-events: none; on the img element.

@diachedelic
Copy link
Contributor Author

@kaechele thats odd, I developed it on Chrome on a Mac, it sure works for me. Did you do a rebuild?

@kaechele
Copy link

kaechele commented Sep 7, 2019

I did some more debugging. Seems to be a problem when this is used in conjunction with BootstrapVue.
JSFiddle: https://jsfiddle.net/kaechele/pqjv0eu4/

@kaechele
Copy link

kaechele commented Sep 7, 2019

So BootstrapVue re-implements the click handler on the b-button component. That re-implementation doesn't honour stopPropagation from inner functions at all. So the @click handler is fired regardless.

@tmorehouse
Copy link

B-button just forwards the native click event up. So you probably need to check if evt.defaultPrevented is truthy. The click event's only argument is the native event

@kaechele
Copy link

kaechele commented Sep 8, 2019

I ended up getting lazy and am working around it like this: https://jsfiddle.net/kaechele/s46xgjdz/

@diachedelic
Copy link
Contributor Author

@tmorehouse e.preventDefault() is not called here, only e.stopPropagation(), so I don't think you'd be able to use the e.defaultPrevented property

@tmorehouse
Copy link

Ah.. you might be able to check if bubbling was prevented then...

@tmorehouse
Copy link

tmorehouse commented Sep 8, 2019

Since b-button is a functional component, its click event is the native button's event. maybe stopImmediatePropagation() (the current element's event loop listeners) might be better to use than stopPropagation() (which is only for listeners higher up in the DOM tree)?

@diachedelic
Copy link
Contributor Author

According to MDN:

If several listeners are attached to the same element for the same event type, they are called in order in which they have been added. If during one such call, event.stopImmediatePropagation() is called, no remaining listeners will be called.

Do we have any guarantee that v-long-click is bound before b-button's click event? I'll add it anyway, we can see if it works

@diachedelic
Copy link
Contributor Author

stopImmediatePropagation doesn't fix the BootstrapVue issue :(

https://jsfiddle.net/2qsxk39u/

@tmorehouse
Copy link

tmorehouse commented Sep 8, 2019

If your button isn't doing anything fancy, you could just use a button like this:

<button class="btn btn-primary" v-longclick="() => longClick()" @click="shortClick">
  Button
</button>

Since b-button is just a functional component, it adds it's click handler (really the user's click handler) as it is rendered... so it probably gets attached before the directive event handler does.

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.

4 participants