Add placeholderSrc if AMP #4049
Conversation
| @@ -11,6 +11,9 @@ import { RequestContext } from '#contexts/RequestContext'; | |||
| import { ServiceContext } from '#contexts/ServiceContext'; | |||
| import embedUrl from '../../../MediaPlayer/helpers/embedUrl'; | |||
|
|
|||
| const liveRadioPlaceholderImageSrc = | |||
| 'https://news.files.bbci.co.uk/include/articles/public/images/audio-player-placeholder.png'; | |||
There was a problem hiding this comment.
Potentially a stupid question but should we be putting this image through iChef to ensure the most optimal size is returned?
There was a problem hiding this comment.
For it to go through ichef it needs to be published on one of the stacks ichef knows about. Given we want this fixed quickly just using the single static image and allowing the browser to stretch it should be fine.
We can look to improve this when UX provide us an asset (this image is currently one I've generated temporarily), and then we can evaluate what this placeholder for AMP should look like on mobile widths.
…rgh into liveradio-AMP-placeholder-image
|
MP4 example of the AMP placeholder being loaded before the SMP iframe is loading into the page. Audio placeholder for AMP.mp4.zip |
pjlee11
requested review from
jamesbhobbs,
dr3,
jamesdonoh,
j-pendlebury,
jroebu14 and
sareh
as code owners
…rgh into liveradio-AMP-placeholder-image
part of 4044
Requires the image in #4048 to be publicly available
Overall change: If the request is AMP add
placeholderSrcif it's canonical addshowPlaceholder=falseTesting ensure a placeholder loads on
.ampand the validation warning has been removed.