Fix: if annotation click is outside the doc, ignore #192
Conversation
|
Hi @mxiao6, thanks for the pull request. Before we can merge it, we need you to sign our Contributor License Agreement. You can do so electronically here: http://opensource.box.com/cla Once you have signed, just add a comment to this pull request saying, "CLA signed". Thanks! |
|
CLA signed |
src/doc/DocAnnotator.js
Outdated
| @@ -169,6 +169,11 @@ class DocAnnotator extends Annotator { | |||
| ]; | |||
| let [x, y] = browserCoordinates; | |||
|
|
|||
| // If click is outside the page, ignore | |||
| if (x < 0 || x > pageWidth || y < 0 || y > pageHeight) { | |||
There was a problem hiding this comment.
Might be worth moving this into a reusable util method in docUtil.js and combine the check w/ the one below
src/doc/DocAnnotator.js
Outdated
| @@ -167,16 +167,16 @@ class DocAnnotator extends Annotator { | |||
| clientEvent.clientX - pageDimensions.left, | |||
| clientEvent.clientY - pageDimensions.top - PAGE_PADDING_TOP | |||
| ]; | |||
| let [x, y] = browserCoordinates; | |||
|
|
|||
| // Do not create annotation if event doesn't have coordinates | |||
There was a problem hiding this comment.
Combine comments
// Do not create annotation if event doesn't have coordinates or occurs outside the page
src/doc/DocAnnotator.js
Outdated
|
|
||
| // Do not create annotation if event doesn't have coordinates | ||
| if (Number.isNaN(x) || Number.isNaN(y)) { | ||
| // If click is outside the page, ignore | ||
| if (!docUtil.isCoordValid(browserCoordinates, pageWidth, pageHeight)) { |
There was a problem hiding this comment.
Separate the Number.isNaN check from the docUtil.isCoordValid() check so that only the isNaN check emits an error and the boundary check does not.
No description provided.