-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
DRY up VideoSource code #3353
DRY up VideoSource code #3353
Conversation
Forgot to mention that this PR also fixes |
@@ -107,92 +97,18 @@ VideoSource.prototype = util.inherit(Evented, /** @lends VideoSource.prototype * | |||
/** | |||
* Sets the video's coordinates and re-renders the map. | |||
* | |||
* @method setCoordinates | |||
* @param {Array<Array<number>>} coordinates Four geographical coordinates, | |||
* represented as arrays of longitude and latitude numbers, which define the corners of the video. | |||
* The coordinates start at the top left corner of the video and proceed in clockwise order. | |||
* They do not have to represent a rectangle. | |||
* @returns {VideoSource} this | |||
*/ |
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.
Could we eliminate the need for this inherited comment by rewording it? s/video/source
or s/video/data
or s/video/layer
?
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.
Maybe, but what to do with @returns {VideoSource}
? cc @jfirebaugh
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.
Oooh. Good 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.
Looks good! 84 lines lighter! 🎈
While looking into #3350, I noticed that most of the
VideoSource
code is a copy-paste fromImageSource
. This PR removes a lot of duplication by makingVideoSource
inherit fromImageSource
.We might later consider some kind of composition instead of inheritance, or inheriting both from a shared base class (e.g.
GeoreferencedSource
), but I'd leave that for later when we address #3186.👀 @lucaswoj @ansis