ngx-gallery
ngx-gallery copied to clipboard
MurhafSousli
Replace hammerJS with native touch events
Currently I implemented swiping only for touch events (not mouse) and only for gallery-swiper component (not thumbs). MurhafSousli Can you check please if my implementation is good enough and I can continue work in this way? For #449
@MurhafSousli strange about swiping. I just retested it in chrome. Looks like is working https://amdaris-my.sharepoint.com/:v:/p/vladimir_petrusevici/EShrbtcG6pFDng1Z2TeSJQ8BzEmBGQIiCwkzAaM7tEAXzg?e=UBLKuq
Also I see a problem with autoplay. Application will never have a stable state when autoPlay = true in the current implementation.
You can check it in this way
I think this observable with delay should be also moved outside ngZone.
Also I see a problem with autoplay. Application will never have a stable state when autoPlay = true in the current implementation. You can check it in this way
I think this observable with delay should be also moved outside ngZone.
Fixed with custom async pipe. Now the app is always stable.
@MurhafSousli should i change the docs or is it better to leave it to you?
@MurhafSousli Hi, I fixed the conflicts. Can you check this PR please?
Hi @MurhafSousli, did you have a chance to check it out? Сan't wait to finish this PR :)
Hi @MurhafSousli, did you have a chance to check it out? Сan't wait to finish this PR :)
When you add many unnecessary changes, you make it difficult for me to review! please keep the PR focused next time and don't refactor parts that are not related to the replacement of hammer.js
Hi @MurhafSousli, did you have a chance to check it out? Сan't wait to finish this PR :)
When you add many unnecessary changes, you make it difficult for me to review! please keep the PR focused next time and don't refactor parts that are not related to the replacement of hammer.js
Sorry, i can try to recreate the PR with only necessary changes, but don't think it will be much smaller
I think HammerJS is not bad, comparing the benefits we would get from implementing our own touch-events solution does not bring any value in terms of feature or functionality, we are just adding more code which we will have to maintenance, in favor of getting rid of one dependency.
I will close this for now, but will keep an eye on your solution if something changed.
Thanks for the PR anyway!