Expect frontend to supply audio ad urls#15740
Conversation
|
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
davidfurey
force-pushed
the
df/podcast-ads-refactor
branch
from
7c6cc48 to
f40d1a4
Compare
Rather than adding flex.acast.com prefix, frontend should provide ad urls for audio elements and audio atoms
davidfurey
force-pushed
the
df/podcast-ads-refactor
branch
from
f40d1a4 to
c34298a
Compare
| urlWithAds: string, | ||
| shouldUseAcast?: boolean, | ||
| ) => { | ||
| return shouldUseAcast ? urlWithAds : basicUrl; |
There was a problem hiding this comment.
Might we want to move this logic up to the Wrapper, and have that determine the correct URL to use? I'm not sure if the audio component needs to know if it's running an Acast feed or not, it kust needs to know which URL it should use.
I appreciate this logic predates this change, so maybe we want to deal with it separately.
There was a problem hiding this comment.
Thanks for the feedback, I've pushed a commit that moves the logic up to the wrapper.
SiAdcock
left a comment
SiAdcock
left a comment
There was a problem hiding this comment.
One minor and probably unrelated comment, but looks good 💎
Responding to PR feedback
github-actions
Bot
removed
the
run_chromatic
3 participants
What does this change?
Rather than adding the https://flex.acast.com prefix to audio files in DCR, frontend will provide ad and ad-free urls for audio elements and audio atoms.
Do not merge until guardian/frontend#28747 is merged
Why?
We are pushing the generation of the URLs of audio assets containing ads to Concierge. This is because the logic is about to get more complicated. We'd like to centralise it before we make changes.