-
Notifications
You must be signed in to change notification settings - Fork 113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New: Added enabledTypes property. Backwards compat for disabledTypes #439
New: Added enabledTypes property. Backwards compat for disabledTypes #439
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, do you mind updating the README as well?
}, | ||
{ | ||
NAME: 'Image', | ||
CONSTRUCTOR: ImageAnnotator, | ||
VIEWER: ['Image', 'MultiImage'], | ||
TYPE: [TYPES.point] | ||
TYPE: [TYPES.point], | ||
DEFAULT_ENABLED: [TYPES.point] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with using DEFAULT_TYPES
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
return !viewerConfig.disabledTypes.includes(type); | ||
}); | ||
} | ||
const enabledTypes = viewerConfig.enabledTypes || [...modifiedAnnotator.DEFAULT_ENABLED]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need the spread operator and then an array? Isn't modifiedAnnotator.DEFAULT_ENABLED
already an array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to make a copy of the array, not modify the original
…ew into New-add-enabled-annotation-types
src/lib/annotations/README.md
Outdated
@@ -139,15 +139,16 @@ preview.show(..., { | |||
VIEWER_NAME: { | |||
annotations: { | |||
enabled: Boolean, // Enables/disables if set. Respects "showAnnotations" if empty | |||
disabledTypes: String[] // List of annotation types to disable | |||
enabledTypes: String[] | null // List of annotation types to enable for this viewer. If empty, will respect default types for that annotator. | |||
disabledTypes: String[] | null // DEPRECATED. List of annotation types to disable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's not add this line so people won't use disabledTypes from here on out
…ew into New-add-enabled-annotation-types
Adding
enabledTypes
to the annotation configuration, as well as a default enabled annotation types(DEFAULT_ENABLED
), if none provided, toANNOTATORS
. This allows us to specify all of the allowed kinds of annotators, and the ones that are enabled by default (if no config provided).IE)