[#1713, #1714] Added basic video model, player & cms-plugin #773
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #773 +/- ##
===========================================
+ Coverage 93.58% 93.59% +0.01%
===========================================
Files 697 716 +19
Lines 24595 24793 +198
===========================================
+ Hits 23016 23204 +188
- Misses 1579 1589 +10
... and 7 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
@Bartvaderkin Yes, cool, the video frame seems to have a set width to 640px + could use a top-margin; and I'll need to make it responsive somehow :) |
| from djchoices import ChoiceItem, DjangoChoices | ||
|
|
||
|
|
||
| class VideoPlayerChoices(DjangoChoices): |
There was a problem hiding this comment.
djchoices is deprecated as Django supports enum types natively. See Django docs and the note in the djchoices docs
There was a problem hiding this comment.
True but the whole application is using djchoices so let's stick with it for now.
There was a problem hiding this comment.
@pi-sigma I made Taiga 1769 issue to swap the old choices for modern native ones.
| @property | ||
| def external_url(self): | ||
| if self.player_type == VideoPlayerChoices.youtube: | ||
| url = "https://www.youtube.com/watch?v={link_id}&enablejsapi=1" |
There was a problem hiding this comment.
This code was from an older project so used old .format(), but I replaced it with f-strings as I should've done before.
| @@ -101,6 +101,15 @@ class Product(models.Model): | |||
| "Product content with build-in WYSIWYG editor. By adding '[CTABUTTON]' you can embed a cta-button for linking to the defined form or link" | |||
| ), | |||
| ) | |||
| video = models.ForeignKey( | |||
There was a problem hiding this comment.
Is this what we want? A ForeignKey from A to B defines a many-to-one relationship. I this case, many products can be associated with a single video (as in: one video covers different products), but each product can have only one video. My first thought is that, if anything, it should be the other way around (you could have several videos for a single product, highlighting different aspects perhaps), or perhaps even have a many-to-many relationship.
There was a problem hiding this comment.
We specifically went for this because it was the simplest solution, until ultimately we convert the pages to be CMS pages (where the editor can add and position videos freely).
…omepage [#1756] Make video iFrames responsive
…t link_id validation
Bartvaderkin
force-pushed
the
feature/1714-video-homepage
branch
from
981ce22
to
a9ffd61
Compare
What is in here?
For clarity you can view the separate commits.
Possibly @jiromaykin want's to do some CSS or template work so the player adjusts size to available space? If so either amend the PR now, or create a ticket for this sprint.