Skip to content

Commit 85f28cf

Browse files
authored
Merge pull request #14888 from guardian/doml/carousel-clicks
Enable clicks on carousel images to link through to the article
2 parents 3da1e6c + bf38fac commit 85f28cf

File tree

6 files changed

+98
-74
lines changed

6 files changed

+98
-74
lines changed

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

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -917,31 +917,26 @@ export const Card = ({
917917
mediaType={media.type}
918918
mediaPositionOnDesktop={mediaPositionOnDesktop}
919919
mediaPositionOnMobile={mediaPositionOnMobile}
920-
hideMediaOverlay={media.type === 'slideshow'}
921920
padMedia={isMediaCardOrNewsletter && isBetaContainer}
922921
isBetaContainer={isBetaContainer}
923922
isSmallCard={isSmallCard}
924923
>
925924
{media.type === 'slideshow' && (
926-
<div
927-
css={css`
928-
position: relative;
929-
z-index: ${getZIndex('card-nested-link')};
930-
`}
925+
<Island
926+
priority="feature"
927+
defer={{ until: 'visible' }}
931928
>
932-
<Island
933-
priority="feature"
934-
defer={{ until: 'visible' }}
935-
>
936-
<SlideshowCarousel
937-
images={media.slideshowImages}
938-
imageSize={mediaSize}
939-
hasNavigationBackgroundColour={
940-
!!hasSublinks
941-
}
942-
/>
943-
</Island>
944-
</div>
929+
<SlideshowCarousel
930+
images={media.slideshowImages}
931+
imageSize={mediaSize}
932+
hasNavigationBackgroundColour={
933+
!!hasSublinks
934+
}
935+
linkTo={linkTo}
936+
linkAriaLabel={headlineText}
937+
dataLinkName={resolvedDataLinkName}
938+
/>
939+
</Island>
945940
)}
946941
{media.type === 'avatar' && (
947942
<AvatarContainer

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ const hoverStyles = css`
5959
:has(
6060
ul.sublinks:hover,
6161
.video-container.loop:hover,
62-
.slideshow-carousel:hover,
62+
.slideshow-carousel-footer:hover,
6363
.branding-logo:hover
6464
) {
6565
.card-headline .show-underline {

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

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,6 @@ type Props = {
3232
mediaType?: CardMediaType;
3333
mediaPositionOnDesktop: MediaPositionType;
3434
mediaPositionOnMobile: MediaPositionType;
35-
/**
36-
* Forces hiding the image overlay added to pictures & slideshows on hover.
37-
* This is to allow hiding the overlay on slideshow carousels where we don't
38-
* want it to be shown whilst retaining it for existing slideshows.
39-
*/
40-
hideMediaOverlay?: boolean;
4135
isBetaContainer: boolean;
4236
isSmallCard: boolean;
4337
padMedia?: boolean;
@@ -207,7 +201,6 @@ export const MediaWrapper = ({
207201
mediaType,
208202
mediaPositionOnDesktop,
209203
mediaPositionOnMobile,
210-
hideMediaOverlay,
211204
isBetaContainer,
212205
isSmallCard,
213206
padMedia,
@@ -274,25 +267,22 @@ export const MediaWrapper = ({
274267
<>
275268
{children}
276269
{/* This overlay is styled when the CardLink is hovered */}
277-
{(mediaType === 'picture' ||
278-
mediaType === 'slideshow' ||
279-
mediaType === 'cinemagraph') &&
280-
!hideMediaOverlay && (
281-
<div
282-
css={[
283-
mediaOverlayContainerStyles,
284-
padMedia &&
285-
mediaPaddingStyles(
286-
mediaPositionOnDesktop,
287-
mediaPositionOnMobile,
288-
),
289-
]}
290-
>
291-
{/* This child div is needed as the hover background colour covers the padded
270+
{(mediaType === 'picture' || mediaType === 'cinemagraph') && (
271+
<div
272+
css={[
273+
mediaOverlayContainerStyles,
274+
padMedia &&
275+
mediaPaddingStyles(
276+
mediaPositionOnDesktop,
277+
mediaPositionOnMobile,
278+
),
279+
]}
280+
>
281+
{/* This child div is needed as the hover background colour covers the padded
292282
area around the image when the hover styles are applied to the top-level div */}
293-
<div className="media-overlay" />
294-
</div>
295-
)}
283+
<div className="media-overlay" />
284+
</div>
285+
)}
296286
</>
297287
</div>
298288
);

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

Lines changed: 64 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
SvgChevronRightSingle,
1414
} from '@guardian/source/react-components';
1515
import { useEffect, useRef, useState } from 'react';
16+
import { getZIndex } from '../lib/getZIndex';
1617
import { takeFirst } from '../lib/tuple';
1718
import { palette } from '../palette';
1819
import type { DCRSlideshowImage } from '../types/front';
@@ -55,6 +56,11 @@ const carouselItemStyles = css`
5556
scroll-snap-align: start;
5657
`;
5758

59+
const carouselLinkStyles = css`
60+
position: relative;
61+
z-index: ${getZIndex('card-nested-link')};
62+
`;
63+
5864
const captionStyles = css`
5965
position: absolute;
6066
bottom: 0;
@@ -74,6 +80,8 @@ const navigationStyles = (hasBackgroundColour: boolean) => css`
7480
display: flex;
7581
align-items: center;
7682
padding-top: ${space[2]}px;
83+
position: relative;
84+
z-index: ${getZIndex('card-nested-link')};
7785
7886
${until.tablet} {
7987
background-color: ${hasBackgroundColour
@@ -104,16 +112,30 @@ const scrollingDotStyles = css`
104112
}
105113
`;
106114

115+
const mediaOverlayStyles = css`
116+
position: absolute;
117+
top: 0;
118+
left: 0;
119+
height: 100%;
120+
width: 100%;
121+
`;
122+
107123
type Props = {
108124
images: readonly DCRSlideshowImage[];
109125
imageSize: MediaSizeType;
110126
hasNavigationBackgroundColour: boolean;
127+
linkTo: string;
128+
linkAriaLabel: string;
129+
dataLinkName?: string;
111130
};
112131

113132
export const SlideshowCarousel = ({
114133
images,
115134
imageSize,
116135
hasNavigationBackgroundColour,
136+
linkTo,
137+
linkAriaLabel,
138+
dataLinkName,
117139
}: Props) => {
118140
const carouselRef = useRef<HTMLUListElement | null>(null);
119141
const [previousButtonEnabled, setPreviousButtonEnabled] = useState(false);
@@ -192,37 +214,51 @@ export const SlideshowCarousel = ({
192214
const slideshowImageCount = slideshowImages.length;
193215

194216
return (
195-
<div className="slideshow-carousel">
196-
<ul
197-
ref={carouselRef}
198-
css={carouselStyles}
199-
data-heatphan-type="carousel"
217+
<>
218+
<a
219+
href={linkTo}
220+
css={carouselLinkStyles}
221+
aria-label={linkAriaLabel}
222+
data-link-name={dataLinkName}
200223
>
201-
{slideshowImages.map((image, index) => {
202-
const loading = index > 0 ? 'lazy' : 'eager';
203-
return (
204-
<li css={carouselItemStyles} key={image.imageSrc}>
205-
<figure>
206-
<CardPicture
207-
mainImage={image.imageSrc}
208-
imageSize={imageSize}
209-
aspectRatio="5:4"
210-
alt={image.imageCaption}
211-
loading={loading}
224+
<ul
225+
ref={carouselRef}
226+
css={carouselStyles}
227+
data-heatphan-type="carousel"
228+
>
229+
{slideshowImages.map((image, index) => {
230+
const loading = index > 0 ? 'lazy' : 'eager';
231+
return (
232+
<li css={carouselItemStyles} key={image.imageSrc}>
233+
<figure>
234+
<CardPicture
235+
mainImage={image.imageSrc}
236+
imageSize={imageSize}
237+
aspectRatio="5:4"
238+
alt={image.imageCaption}
239+
loading={loading}
240+
/>
241+
{!!image.imageCaption && (
242+
<figcaption css={captionStyles}>
243+
{image.imageCaption}
244+
</figcaption>
245+
)}
246+
</figure>
247+
<div
248+
css={mediaOverlayStyles}
249+
className="media-overlay"
212250
/>
213-
{!!image.imageCaption && (
214-
<figcaption css={captionStyles}>
215-
{image.imageCaption}
216-
</figcaption>
217-
)}
218-
</figure>
219-
</li>
220-
);
221-
})}
222-
</ul>
251+
</li>
252+
);
253+
})}
254+
</ul>
255+
</a>
223256

224257
{slideshowImageCount > 1 && (
225-
<div css={navigationStyles(hasNavigationBackgroundColour)}>
258+
<div
259+
className="slideshow-carousel-footer"
260+
css={navigationStyles(hasNavigationBackgroundColour)}
261+
>
226262
<div css={scrollingDotStyles}>
227263
<SlideshowCarouselScrollingDots
228264
total={slideshowImageCount}
@@ -266,6 +302,6 @@ export const SlideshowCarousel = ({
266302
</div>
267303
</div>
268304
)}
269-
</div>
305+
</>
270306
);
271307
};

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ const meta = {
9191
images,
9292
imageSize: 'medium',
9393
hasNavigationBackgroundColour: false,
94+
linkTo: 'https://www.theguardian.com',
95+
linkAriaLabel: 'news | group-3 | card-@1 | media-slideshow',
96+
dataLinkName: 'slideshow-carousel',
9497
},
9598
render: (args) => (
9699
<Wrapper>

dotcom-rendering/src/components/YoutubeAtom/YoutubeAtomFeatureCardOverlay.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ const hoverStyles = css`
4242
:hover .media-overlay {
4343
position: absolute;
4444
top: 0;
45+
left: 0;
4546
width: 100%;
4647
height: 100%;
47-
left: 0;
4848
background-color: ${palette('--card-background-hover')};
4949
}
5050

0 commit comments

Comments
 (0)