-
Notifications
You must be signed in to change notification settings - Fork 5
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
Allow for Vimeo vids in VideoEmbed
#11412
Conversation
Size Change: -10.5 kB (-1.04%) Total Size: 996 kB
ℹ️ View Unchanged
|
@@ -10,6 +10,8 @@ import PrismicHtmlBlock from '@weco/common/views/components/PrismicHtmlBlock/Pri | |||
|
|||
export type Props = { | |||
embedUrl: string; | |||
videoProvider?: string; |
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.
Can you constrain this type further?
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.
Done!
hasFullSizePoster ? 'maxresdefault' : 'hqdefault' | ||
}.jpg` | ||
: isVimeo | ||
? videoThumbnail |
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.
Did you see 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.
I did! But I wasn't sure we wanted to use some third party for this..? There's also this that could be worth a look https://github.com/vimeo/vimeo-oembed-examples ..
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.
Looking good. My only question is around whether we can auto-fill the thumbnail image, but don't think it's a big deal
What does this change?
#11408
Our slices allowed for
Youtube
andVimeo
in that it would still treat it in the Body transformer, but once you got toVideoPlayer
, it was strictly Youtube. This is probably due to the fact that we haven't had a Vimeo video in the time I've been here (So around 2 and a half years at a minimum).With the problems we're currently experiencing with YT, we'd like to go back to supporting Vimeo.
So this ticket:
VideoEmbed
/VideoPlayer
more flexible; now passing invideoProvider
(for easier conditionals) &videoThumbnail
(for the Vimeo Thumbnail)hcontent/webapp/services/prismic/transformers/exhibition-highlight-tours.ts
content/webapp/services/prismic/transformers/exhibition-guides.ts
as well, which supports legacy EG.There are a few TODOs in the code now, they're up for conversation with whoever reviews this/might be done as a later task (if so I will tidy and create new tickets), to prioritise just rendering those videos.
How to test
Run locally with Prismic staging API
How can we measure success?
Renders YT and Vimeo successfully
Have we considered potential risks?
If doubled QA'd, should be low risk