Skip to content

Fix: Invalidate draw/highlight annotations without properly structured locations#9

Merged
pramodsum merged 6 commits intobox:masterfrom
pramodsum:draw
Oct 30, 2017
Merged

Fix: Invalidate draw/highlight annotations without properly structured locations#9
pramodsum merged 6 commits intobox:masterfrom
pramodsum:draw

Conversation

@pramodsum
Copy link
Copy Markdown
Contributor

No description provided.

@pramodsum pramodsum changed the title Fix: Invalidate draw annotations without min/max locations Fix: Invalidate draw/highlight annotations without properly structured locations Oct 27, 2017
Copy link
Copy Markdown

@jeremypress jeremypress left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests?

Comment thread src/annotatorUtil.js
*
* @param {Object} location Highlight annotation thread location object
* @return {boolean} Whether or not the highlight annotation has the correct location information
*/
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isValidHighlightLocation and isValidDrawLocation?

Sumedha Pramod added 2 commits October 27, 2017 16:01
- A temp draw thread is created without a valid location in order to set
  up the thread handlers in DrawingModeController. This changes the
  method to directly create a new DocDrawingThread without validating
  location.
- Cleaning up validation methods
Comment thread src/annotatorUtil.js Outdated
@@ -488,6 +508,12 @@ export function isPending(threadState) {
*/
export function validateThreadParams(thread) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above. areValidThreadParams()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@pramodsum pramodsum merged commit 39a9493 into box:master Oct 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants