vue-image-compare icon indicating copy to clipboard operation
vue-image-compare copied to clipboard

Fix out of bounds error #9

Open weihao opened this issue 3 years ago • 2 comments

Problem

In the original short-circuit expression

this.pageX = event.pageX || event.targetTouches[0].pageX || event.originalEvent.targetTouches[0].pageX;

event.pageX could be number 0 which is a valid x location, but it was evaluated as false in the short-circuit and it would try to evaluate event.targetTouches[0].pageX which is undefined and caused an error. https://github.com/marcincichocki/vue-image-compare/issues/9

Fix

this.pageX = +(x !== 0) && (x || event.targetTouches[0].pageX || event.originalEvent.targetTouches[0].pageX)

Case 1, x is 0

+(x !== 0) = +(false) = 0 && (x || event.targetTouches[0].pageX || event.originalEvent.targetTouches[0].pageX) = 0
this.pageX = 0

Case 2, x is non-zero value, like 2

+(x !== 0) = +(true) = 1 && (x || event.targetTouches[0].pageX || event.originalEvent.targetTouches[0].pageX) = x
this.pageX = 2

Case 3, x is null

+(x !== 0) = +(true) = 1 && (x || event.targetTouches[0].pageX || event.originalEvent.targetTouches[0].pageX) = event.targetTouches[0].pageX || event.originalEvent.targetTouches[0].pageX
this.pageX = event.targetTouches[0].pageX || event.originalEvent.targetTouches[0].pageX

weihao avatar Apr 14 '21 01:04 weihao

Thanks for contributing! PR looks good, I'll merge it soon.

Unfortunately I'm busy at the moment so finding lost in time and space npm credentials, and actually deploying it to npm might take a little while. While I'm at it I would like to get this repo back to date with some tests and CI, so for now either use your own fork, or patch package locally with https://github.com/ds300/patch-package

You can track progress being made in #11

marcincichocki avatar Apr 14 '21 09:04 marcincichocki

thanks for the pointer for patching node modules. that's very helpful!

weihao avatar Apr 14 '21 15:04 weihao