search

knownasilya / ember-toggle

Checkbox based Toggle Switches for Ember
http://knownasilya.github.io/ember-toggle/
MIT License
112 stars 52 forks source link

Replace hammerjs with ember-gesture-modifiers, fixes #138 #139

Closed lolmaus closed 3 years ago

lolmaus commented 3 years ago

I've tried to tackle #138 and ember-gesture-modifiers don't work for me: the action does not fire.

I tried debugging the modifier a bit and discovered that the addEventListeners hook does fire but actions below do not.

@nickschot, can you please have a look and see what I'm missing? 🙏

nickschot commented 3 years ago

Just checked locally what is happening and everything seems to be working as intended except that it is mirrored (drag left, toggle right).

One not of importance here is that currently ember-gesture-modifiers only supports touch events, not pointer events, so it will only work on touch screens right now (or Chrome's responsive mode). I have started working on pointer event support in ember-gesture-modifiers, but did not finish it yet.

If we want to keep ember-toggle exactly like it is we'll need the pointer event support first before merging it.

lolmaus commented 3 years ago

Ah! This makes sense. 😬

I'm not currently using ember-toggle in any of my active projects, but if I did, I would prefer the mouse drag to keep working. That would definitely be a breaking change otherwise. Breaking the user rather than CI. 😅

@nickschot Ping in in the PR when you have it for more 👀.

nickschot commented 3 years ago

I've made a branch with PointerEvents support which should make this branch work. https://github.com/nickschot/ember-gesture-modifiers/pull/16 Would be great if we could try the branch before I do a release.

You'll need to pass a pointerTypes=['touch', 'mouse'] to enable cursor support.

knownasilya commented 3 years ago

Tests are running. Thanks for working on this @nickschot

knownasilya commented 3 years ago

@nickschot do the way we test a pan need to be updated?

nickschot commented 3 years ago

Oh yes! You probably used something like my pan helper? You'll need to let it create a PointerEvent now instead of a TouchEvent. The PR has updated helpers.

edit: I don't know how those jquery (?) triggered pan events in the test work? Seems to be a hammerjs thing?

knownasilya commented 3 years ago

Yeah those events were hammerjs specific I believe.

knownasilya commented 3 years ago

@nickschot could you export the pan helper as part of addon-test-support?

nickschot commented 3 years ago

@nickschot could you export the pan helper as part of addon-test-support?

Done! 

import { pan } from 'ember-gesture-modifiers/test-support';

...

await pan('.css-selector', 'right');
knownasilya commented 3 years ago

@nickschot waiting on tests, but I think you can merge that pointer events PR.

nickschot commented 3 years ago

@knownasilya great! I'll prepare a release then.

nickschot commented 3 years ago

Done: https://github.com/nickschot/ember-gesture-modifiers/releases/tag/v1.0.0

knownasilya commented 3 years ago

What do you guys think about semver for this change. It seems like a patch, but people could be using hammerjs from this addon. Thoughts?