-
Notifications
You must be signed in to change notification settings - Fork 243
Video support for mp4 format #260
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
Support uses the raw html section in order to pass a video tag which is compatible with html5. Video size and style should be handled by the "video" class in the css file.
…duplicate of the image css. Code for the video extra has been extracted to a function to clean up a bit the if/elif/elif that is starting to get pretty big. Maybe the other conditions could be extracted to their own method to clean up a bit. Once again, there seems to be a lot of code duplication with image, so we may want to refactor images, videos, and audio as a media type and have some kind of template method. Basic unit tests were added to make sure that the extra was correctly added to the report when the extra was requested.
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 really good, thanks for this!
Would you mind creating an issue re. creating a media type (instead of separate image and video)?
…st-dev#260. These changes aimed at generelizing the approach used to introduce media types to the project. Images and videos now use a method called _make_media_html_div which generates the media div in a streamlined manner. Both _append_image and _append_video use this method, to which they can pass their base string and class. Unit tests were left untouched, as the changes only affect private methods.
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.
Again, looks really good! Thanks!
I think I would be done for this feature. Feel free to merge whenever you are ready! I might grab another issue in the meantime such as #245 |
This is awesome! Any date on when the new release will be built? |
I'm aiming to do a release during the weekend @robbiewareham |
Released in v2.1.0 |
Hey @ExaltedBagel ! I discovered a bug with your PR. On this line, Just wanted you to know, and I might ask you to review the fix. 👍 |
Summary
This feature enables the use of extra videos in the reports in the same way as images are used.
Details