Merge pull request #14117 from guardian/doml/loop-progress-bar

Ensure Loop Video looks correct in DCR containers

Author: domlander

dotcom-rendering/src/components/Card/components/ImageWrapper.tsx

@@ -159,12 +159,14 @@ export const ImageWrapper = ({
 		imagePositionOnDesktop === 'left' || imagePositionOnDesktop === 'right';
 	const isHorizontalOnMobile =
 		imagePositionOnMobile === 'left' || imagePositionOnMobile === 'right';
+
 	return (
 		<div
 			css={[
 				(imageType === 'slideshow' ||
 					imageType === 'picture' ||
-					imageType === 'video') &&
+					imageType === 'video' ||
+					imageType === 'loop-video') &&
 					isHorizontalOnDesktop &&
 					flexBasisStyles({
 						imageSize,
@@ -182,7 +184,6 @@ export const ImageWrapper = ({
 						}
 					`,
 				isHorizontalOnMobile && fixImageWidth(imageFixedSizes),
-
 				isHorizontalOnDesktop &&
 					css`
 						${from.tablet} {

dotcom-rendering/src/components/FlexibleGeneral.tsx

@@ -105,8 +105,8 @@ const ImmersiveCardLayout = ({
 	const isLoopingVideo = card.mainMedia?.type === 'LoopVideo';
 
 	return (
-		<UL padBottom={true}>
-			<LI key={card.url} padSides={true}>
+		<UL padBottom={true} key={card.url}>
+			<LI padSides={true}>
 				<FeatureCard
 					collectionId={collectionId}
 					linkTo={card.url}
@@ -447,6 +447,7 @@ const FullWidthCardLayout = ({
 			showTopBar={!isFirstRow}
 			padBottom={!isLastRow}
 			hasLargeSpacing={!isLastRow}
+			key={card.url}
 		>
 			<LI
 				padSides={true}
@@ -501,6 +502,7 @@ const HalfWidthCardLayout = ({
 	isFirstRow,
 	isFirstStandardRow,
 	aspectRatio,
+	row,
 	isLastRow,
 	containerLevel,
 	isInHideTrailsAbTest,
@@ -513,6 +515,7 @@ const HalfWidthCardLayout = ({
 	showAge?: boolean;
 	absoluteServerTimes: boolean;
 	aspectRatio: AspectRatio;
+	row: number;
 	isLastRow: boolean;
 	containerLevel: DCRContainerLevel;
 	isInHideTrailsAbTest?: boolean;
@@ -527,6 +530,7 @@ const HalfWidthCardLayout = ({
 			showTopBar={!isFirstRow}
 			/** We use one full top bar for the first row and use a split one for subsequent rows */
 			splitTopBar={!isFirstStandardRow}
+			key={row}
 		>
 			{cards.map((card, cardIndex) => {
 				return (
@@ -642,6 +646,7 @@ export const FlexibleGeneral = ({
 								isFirstRow={!splash.length && i === 0}
 								isFirstStandardRow={i === 0}
 								aspectRatio={aspectRatio}
+								row={i + 1}
 								isLastRow={i === groupedCards.length - 1}
 								containerLevel={containerLevel}
 								isInHideTrailsAbTest={isInHideTrailsAbTest}

dotcom-rendering/src/components/LoopVideo.importable.tsx

@@ -26,8 +26,8 @@ type Props = {
 export const LoopVideo = ({
 	src,
 	videoId,
-	width = 600,
-	height = 360,
+	width,
+	height,
 	thumbnailImage,
 	fallbackImageComponent,
 	hasAudio = true,

dotcom-rendering/src/components/LoopVideo.stories.tsx

@@ -21,8 +21,8 @@ export const Default = {
 	args: {
 		src: 'https://uploads.guim.co.uk/2024/10/01/241001HeleneLoop_2.mp4',
 		videoId: 'test-video-1',
-		height: 337.5,
-		width: 600,
+		height: 1080,
+		width: 1920,
 		thumbnailImage:
 			'https://media.guim.co.uk/9bdb802e6da5d3fd249b5060f367b3a817965f0c/0_0_1800_1080/master/1800.jpg',
 		fallbackImageComponent: (
@@ -39,6 +39,8 @@ export const WithWebmFile = {
 	name: 'With Webm File',
 	args: {
 		...Default.args,
+		height: 496,
+		width: 620,
 		src: 'https://interactive.guim.co.uk/atoms/2023/01/2025-trump-100-days/assets/v/1746020259/videos/header-video.webm',
 	},
 } satisfies StoryObj<typeof LoopVideo>;

dotcom-rendering/src/components/LoopVideoPlayer.tsx

@@ -9,9 +9,9 @@ import { LoopVideoProgressBar } from './LoopVideoProgressBar';
 
 const videoStyles = (width: number, height: number) => css`
 	position: relative;
+	display: block;
+	height: auto;
 	width: 100%;
-	/* Find out why this is needed to align the video with its container. */
-	margin-bottom: -3px;
 	cursor: pointer;
 	/* Prevents CLS by letting the browser know the space the video will take up. */
 	aspect-ratio: ${width} / ${height};

dotcom-rendering/src/components/LoopVideoProgressBar.tsx

@@ -30,8 +30,8 @@ type Props = {
 /**
  * A progress bar for the loop video component.
  *
- * Why don't we use the <progress /> element?
- * It was not possible to properly style the native progress element in safari.
+ * Q. Why don't we use the <progress /> element?
+ * A. It was not possible to properly style the native progress element in safari.
  */
 export const LoopVideoProgressBar = ({
 	videoId,
@@ -44,17 +44,18 @@ export const LoopVideoProgressBar = ({
 	if (Number.isNaN(progressPercentage)) {
 		return null;
 	}
+	const roundedProgressPercentage = Number(progressPercentage.toFixed(2));
 
 	return (
 		<div
 			role="progressbar"
 			aria-labelledby={videoId}
-			aria-valuenow={progressPercentage}
+			aria-valuenow={roundedProgressPercentage}
 			aria-valuemin={0}
 			aria-valuemax={100}
 			css={styles}
 		>
-			<span css={foregroundStyles(progressPercentage)} />
+			<span css={foregroundStyles(roundedProgressPercentage)} />
 		</div>
 	);
 };