Skip to content
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

feat(YoutubeAtom): player requires ad targeting #675

Merged
merged 1 commit into from
Jun 30, 2023

Conversation

mxdvl
Copy link
Contributor

@mxdvl mxdvl commented Jun 30, 2023

What are you changing?

  • The YoutubeAtomPlayer can only be initialised once, and we require the ad targeting to be present before initialisation.

Why?

@mxdvl mxdvl added the enhancement New feature or request label Jun 30, 2023
@mxdvl mxdvl requested review from a team as code owners June 30, 2023 13:02
@changeset-bot
Copy link

changeset-bot bot commented Jun 30, 2023

🦋 Changeset detected

Latest commit: 61657a6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@guardian/atoms-rendering Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the 📦 npm Affects a @guardian package on NPM label Jun 30, 2023
Copy link
Member

@arelra arelra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

The YoutubeAtomPlayer can only be initialised once, and we require
the ad targeting to be present before initialisation.

Co-authored-by: Ravi <7014230+arelra@users.noreply.github.com>
@mxdvl mxdvl force-pushed the mxdvl/ensure-ad-targeting-is-set branch from 3241f4b to 61657a6 Compare June 30, 2023 13:12
@mxdvl mxdvl added the run_chromatic Runs chromatic when label is applied label Jun 30, 2023
@mxdvl mxdvl enabled auto-merge June 30, 2023 13:26
@mxdvl mxdvl merged commit 5336eda into main Jun 30, 2023
@mxdvl mxdvl deleted the mxdvl/ensure-ad-targeting-is-set branch June 30, 2023 13:55
arelra added a commit that referenced this pull request Jul 10, 2023
## What are you changing?

Pass valid ad targeting to `YoutubeAtom` stories so the player is able
to load.

## Background

Since guardian/dotcom-rendering#8095 ad
targeting will be set async on the client so it could be undefined
initially and then defined in subsequent render passes.

However the player requires ad targeting before it initialises and makes
the request to YouTube so we block the player until ad targeting is
defined as implemented in #675.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 📦 npm Affects a @guardian package on NPM run_chromatic Runs chromatic when label is applied
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants