Closed danilofuchs closed 4 years ago
@Secretmapper, Really useful feature that adds much more versatility to the library!
@Secretmapper any chance we can merge this? Would be great to get it in the lib
actually i can see why you're not merging it now.
@danilofuchs this is great work but why did you change so much in the repo. Also why are you calling e.preventDefault()
in getCoordPercentage()
, it's stopping click events from working so it breaks activeAnnotation
selection.
I accidentally included build artifacts in this PR. They were only supposed to be included in my fork's master, so I could use it in my project. I will fix this ASAP.
@stan-sack e.preventDefault
is only called in valid touch events, to disable single finger scroll. Most browsers have optimizations in place to take over the touch event handling in order to provide smooth scrolling experience.
As we need to intercept the browser optimized handler, preventDefault is necessary.
Can you explain why does this break other functionality?
Ah that's a good point. My image is full screen so I'm not seeing it the scroll issue. I'm not totally sure why, but when I use your branch I'm unable to see the onClick
event being fired. This is breaking the active annotation selection - ie I can't click on the existing annotations to highlight them. Is this feature working for you?
As I'm using a custom selector (with drawing support), I currently do not face this issue. In which device / browser are you accessing. You can clone my branch and serve with npm. The is an example there and it's all seems to be working
I'm using Chrome dev tools to emulate an Android phone. I've pulled the functionality out of the main repo and have it working now.
it's because you have e.preventDefault()
inside getCoordPercentage(e)
. This is calling preventDefault()
for touchStart
and touchEnd
events as well as touchMove
. To stop scrolling you only need to preventDefault()
for touchMove
. By preventing default on touchStart
and touchEnd
you are stopping the click event from being propagated.
Makes total sense. I'm fixing this and build artifacts right now. You should be able to use my fork's master as a dependency after I'm done changing it
Awesome! Thanks for that
Also, I cleared build artifacts from the PR. @Secretmapper do you need anything to get this merged? I see this project is not well maintained. The last commit was 10 months ago. Do you need any help? It's a really simple yet great library. It would be a shame if it was deprecated.
This is awesome, thank you for the PR! ^_^
Added optional touch support. RectangularSelector and OvalSelector were refactored to handle touch events properly. Changed docs and site to reflect those changes.
When
allowTouch
set totrue
, no one-finger scrolling will occur, but two-finger scrolling and-or panning is still available.