-
Notifications
You must be signed in to change notification settings - Fork 0
adds first version of timeline snapping, with a button to enable it #171
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
Conversation
…and snapping added for steps, VA/NVA, and walking/standing, with a 10px tolerance
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.
Found a strange bug where being to the right of a snap point would highlight poses as if it were to the left. Also looks like a visual bug where the walk vs stand is sometimes yellow for the first pose
weird.timeline.behavior.mov
src/motionStudy/timeline.js
Outdated
} | ||
|
||
// Returns the start/end times of colored regions in the poses - with similar logic as in drawSpaghettiPoses | ||
getPoseBreakTimestamps(poses) { |
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 think instead of duplicating the logic it would be better to store an array of lastBreakTimestamps in drawSpaghettiPoses
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.
Good idea! And thanks for catching that bug. I'll fix those
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 think you missed editing this one, the above getWalkStandSnappableTimes looks good though!
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.
Oops, forgot to delete that after swapping away from using it
…d end of entire timeline
Does this fix it for you? I fixed a couple of small bugs but I'm not sure if it covers the same one you were experiencing. |
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.
Tested locally and LGTM! Nice work!
Snapping added for steps, VA/NVA, and walking/standing, with a 10px tolerance
Still need to add snapping for ergonomics peaks, but that can probably be done in a follow up