Video annotation: Extending the Point Tool #2099
Conversation
per the Zooniverse style guide
From the SingleVideoViewerContainer to DrawingToolMarks so we can determine when to show an annotation. Added a ’t’ property to the Point model to represent the timeStamp of a video. # Conflicts: # packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/InteractionLayer.js # packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/SingleVideoViewer/SingleVideoViewerContainer.js
to easily reuse in other components.
And only show the annotation if current video timeStamp is greater or equal to the annotation timeStamp.
per the Zooniverse style guide
…o video-annotation-timestamp # Conflicts: # packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/InteractionLayer.js # packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/DrawingToolMarks/DrawingToolMarks.js # packages/lib-classifier/src/plugins/drawingTools/components/Mark/Mark.js # packages/lib-classifier/src/plugins/drawingTools/models/marks/Mark/Mark.js
| TemporalPoint.propTypes = { | ||
| active: PropTypes.bool, | ||
| mark: PropTypes.object, | ||
| scale: PropTypes.number | ||
| } |
There was a problem hiding this comment.
Suggestion to add the other prop onFinish to propTypes here.
There was a problem hiding this comment.
Good catch! But based on other comments and some refactoring, this file is being removed.
| import PointTool from '../PointTool' | ||
| import { TemporalPoint } from '../../marks' | ||
|
|
||
| const TemporalPointTool = types |
There was a problem hiding this comment.
You probably already have this planned, but a note to add a TemporalPointTool.spec test file 😃
There was a problem hiding this comment.
I should have mentioned this in the PR - apologies. All unit tests will be in a future PR.
|
@ErikOstlund we may need to give @CKrawczyk notice about this new drawing tool type for aggregation purposes. It'd be good to have a write up about the new tool types intended to be used with video, so perhaps a follow up ADR for this so it can be documented? Thanks! |
|
|
||
| const TemporalPointModel = types | ||
| .model('TemporalPointModel', { | ||
| t: types.optional(types.number, 0) |
There was a problem hiding this comment.
I prefer object properties and variable names to be spelled out, so it's absolutely clear as possible on intent. How about time here?
There was a problem hiding this comment.
time is good for this iteration. 👍
Though I suspect in the future this will evolve into startTime and endTime in a later enhancement. (Time ranges appears to be the direction the research project's requirements are going, IIRC?)
There was a problem hiding this comment.
Just a question: The TemporalPoint component looks to be identical to the Point component. Is there a plan for TemporalPoint to diverge so that having a separate point component be necessary? Totally ok if this is setup for some future planned work. If it's not, perhaps the TemporalPoint model could return the existing Point mark component?
This was identical to Point componet. So, in the TemporalPoint model, the Point componet is now returned instead of the TemporalPoint component. Updated the property name from ’t’ to ‘displayTime’ for the TemporalPoint model to better explain what this proerty is.
…ont-end-monorepo into video-annotation-timestamp
@srallen, good catch! Initially, we weren't sure if the |
Time is now being captured correctly.
| @@ -40,11 +40,12 @@ const icons = { | |||
| polygon: Polygon, | |||
| rectangle: Rectangle, | |||
| rotateRectangle: RotateRectangle, | |||
| temporalPoint: Point, | |||
There was a problem hiding this comment.
We might consider a different icon, but something that can be followed up later.
Package:
lib-classifierProject:
Video AnnotatorProject URL
Closes #2098
Changes:
The main chunk of this PR is to extend the following:
to the following new files:
This was done by simulating inheritance by using type composition (reference).
The Point tool has the following properties:
After extending the Points tool to TemporalTool, a
time(t) property was added:When the TemporalPoint tool is used, the annotation will only display from the video timestamp it was created to the end of the video.
Misc clean-up was done including ESlint/Prettier fixes per Zooniverse Style Guide following: JavaScript Standard Style
Testing
In the front-end-monorepo folder, run
yarn bootstrapto build the packages. Then cd packages/lib-classifier and runyarn dev.Project URL: https://localhost:8080/?env=production&project=13789&workflow=16843