Dynamic data attribute for self hosted video, remove wrapper by frederickobrien · Pull Request #16322 · guardian/dotcom-rendering

frederickobrien · 2026-07-03T10:40:46Z

A bit of tidying around self hosted videos, this adjusts the figure's gu data element to be dynamic like the class name is and also removes an unnecessary wrapper <div> element by moving some bottom padding to the figure element inside the island, where it probably belongs anyway.

github-actions · 2026-07-03T10:41:23Z

Hello 👋! When you're ready to run Chromatic, please apply the run_chromatic label to this PR.

You will need to reapply the label each time you want to run Chromatic.

Click here to see the Chromatic project.

github-actions · 2026-07-03T10:45:15Z

Deploy build 30500 of `dotcom:rendering-all` to CODE

All deployment options

From guardian/actions-riff-raff.

domlander · 2026-07-03T10:57:12Z

 	return (
 		<figure
+			css={css`
+				margin-bottom: ${space[3]}px;


Just a note that this will affect self hosted video rendered on fronts, as well as articles, which I don't think is intended.

Ah good catch, sorry. Have made it apply only in articles in 15fb6de

github-actions · 2026-07-07T16:07:23Z

Deploy build 2839 of `dotcom:ab-testing` to CODE

All deployment options

From guardian/actions-riff-raff.

domlander · 2026-07-07T16:35:59Z

+			css={
+				isInArticle
+					? css`
+							margin-bottom: ${space[3]}px;


I'm not a huge fan of adding margins to the child component, instead of the parent controlling the spacing. I think the code can end up a bit messy with this approach.

I see from following the code upwards that it is not easy to add this margin to a parent container and creating a new div seems quite unnecessary. I think my preference is for the latter (and current) of these three options but happy to be overruled.

domlander · 2026-07-07T16:36:02Z

 				role === 'immersive' ? 'element-video-immersive' : ''
 			}`}
-			data-component="gu-video-loop"
+			data-component={`gu-video-${videoStyleFormat}`}


Nice spot. I would run this change by the Fronts & Curation team. I think their analytics might be relying on this name

I think it would be OK to update this name provided the change is run past Ophan before its merged

domlander · 2026-07-07T16:45:27Z

 		isExternalLink: boolean;
 	};
 	isInLoopClickTestVariant?: boolean;
+	isInArticle?: boolean;


Could this be renamed to something that describes the behaviour of this component instead of the features of the parent? It can be easier to understand how to use props and predict what they do that follow this pattern

I agree with @domlander, perhaps hasBottomMargin or similar?

akash1810 · 2026-07-09T13:34:58Z

Please rebase this branch against main before deploying to CODE. #16321 made some changes to CI and infrastructure. Deploying this branch without these changes present will either:

Fail when using Riff-Raff's default update strategy
OR delete the new infrastructure if using Riff-Raff's "dangerous" mode

Please rebase this branch against main before deploying to CODE.

Co-Authored-By: Pip <47357469+pippinpen@users.noreply.github.com> Co-Authored-By: Alessia Amitrano <alessia.amitranobo@gmail.com>

github-actions · 2026-08-27T11:16:15Z

🚀 Image pushed to AWS ECR

Image digest: sha256:db4e0ad114b613882760266df727cabb9e04b1f4ec248c69eff3b49ad1d12219

🐛 Run the image locally

The following can be used to run the image locally:

# Refer to image using the immutable digest. Find alternatives below.
IMAGE_IDENTIFIER="@sha256:db4e0ad114b613882760266df727cabb9e04b1f4ec248c69eff3b49ad1d12219"

# Refer to image using branch tag
# IMAGE_IDENTIFIER=":branch-class-name-for-long-non-yt-vids"

# Refer to image using build tag
# IMAGE_IDENTIFIER=":build-30500"

# Refer to image via the GitHub commit SHA tag
# IMAGE_IDENTIFIER=":sha-e972417beb9417c1f6a8a9a9a4d72d37182acff2"

# Set environment variables for the AWS CLI
AWS_PROFILE="<A_PROFILE_FROM_JANUS>"
AWS_DEFAULT_REGION="eu-west-1"

IMAGE_ACCOUNT_ID=$(aws ssm get-parameter --name /organisation/accounts/deployTools --query "Parameter.Value" --output text)
REGISTRY="${IMAGE_ACCOUNT_ID}.dkr.ecr.${AWS_DEFAULT_REGION}.amazonaws.com"
IMAGE="${REGISTRY}/guardian/dotcom-rendering${IMAGE_IDENTIFIER}"

# Login to AWS ECR https://docs.aws.amazon.com/AmazonECR/latest/userguide/registry_auth.html
aws ecr get-login-password | docker login --username AWS --password-stdin $REGISTRY

# Pull the image
docker pull $IMAGE

# Run the image. You'll likely need to set additional flags. See https://docs.docker.com/reference/cli/docker/container/run.
docker run $IMAGE

From guardian/actions-publish-image.

abeddow91 · 2026-09-02T14:40:00Z

 		isExternalLink: boolean;
 	};
 	isInLoopClickTestVariant?: boolean;
+	isInArticle?: boolean;


I agree with @domlander, perhaps hasBottomMargin or similar?

abeddow91 · 2026-09-02T14:43:48Z

+				isInArticle
+					? css`
+							margin-bottom: ${space[3]}px;
+						`
+					: undefined


nit: could this style be lifted out into a const? This would be more consistet with the rest of the file and would allow for slightly more readable jsx eg
css={isInArticle && bottomMarginStyles}

abeddow91 · 2026-09-02T14:44:52Z

 	}

 	return (
-		<div css={containerStyles} data-spacefinder-role="inline">


Are we OK to drop the data-spacefinder-role attribute?

abeddow91 · 2026-09-02T14:47:55Z

 				role === 'immersive' ? 'element-video-immersive' : ''
 			}`}
-			data-component="gu-video-loop"
+			data-component={`gu-video-${videoStyleFormat}`}


I think it would be OK to update this name provided the change is run past Ophan before its merged

frederickobrien self-assigned this Jul 3, 2026

frederickobrien added the maintenance Departmental tracking: maintenance work, not a fix or a feature label Jul 3, 2026

frederickobrien added this to the Interactives milestone Jul 3, 2026

frederickobrien added the run_chromatic Runs chromatic when label is applied label Jul 3, 2026

github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Jul 3, 2026

domlander reviewed Jul 3, 2026

View reviewed changes

frederickobrien added the run_chromatic Runs chromatic when label is applied label Jul 3, 2026

github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Jul 3, 2026

frederickobrien force-pushed the class-name-for-long-non-yt-vids branch from 15fb6de to a0bfbc1 Compare July 7, 2026 16:05

frederickobrien requested a review from domlander July 7, 2026 16:05

domlander reviewed Jul 7, 2026

View reviewed changes

frederickobrien force-pushed the class-name-for-long-non-yt-vids branch from a0bfbc1 to e6e6ed8 Compare July 9, 2026 13:25

frederickobrien force-pushed the class-name-for-long-non-yt-vids branch from e6e6ed8 to 3967b3f Compare July 15, 2026 09:43

frederickobrien force-pushed the class-name-for-long-non-yt-vids branch from 3967b3f to 0d37add Compare July 23, 2026 15:57

frederickobrien and others added 2 commits July 31, 2026 13:42

Dynamic data attribute for self hosted video, remove wrapper

4e73678

Co-Authored-By: Pip <47357469+pippinpen@users.noreply.github.com> Co-Authored-By: Alessia Amitrano <alessia.amitranobo@gmail.com>

Only apply bottom padding in articles

dd4ab5e

frederickobrien force-pushed the class-name-for-long-non-yt-vids branch from 0d37add to dd4ab5e Compare July 31, 2026 12:42

Merge branch 'main' into class-name-for-long-non-yt-vids

e972417

abeddow91 reviewed Sep 2, 2026

View reviewed changes

Uh oh!

Conversation

frederickobrien commented Jul 3, 2026

Uh oh!

github-actions Bot commented Jul 3, 2026

Uh oh!

github-actions Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akash1810 commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 27, 2026

🚀 Image pushed to AWS ECR

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Jul 3, 2026 •

edited

Loading

github-actions Bot commented Jul 7, 2026 •

edited

Loading

akash1810 commented Jul 9, 2026 •

edited

Loading