Skip to content

Commit 9c93cb4

Browse files
authored
Remove lightbox deduplication logic (#13580)
The lightbox code has some de-duplicating logic that is meant to prevent two or more of the same image appearing in the lightbox. This was designed to handle cases where galleries repeat the main media image in the body of the article. Currently the assumption is that two images are the same if they have the same image id, and can therefore be de-duped. However, some articles intentionally display the same image multiple times, but with different crops. As these will all have the same image id they will be de-duped, and all but the last image will be dropped from the lightbox. One solution is to use both the id and the crop of an image to determine whether it is unique in the article, so that different crops of the same image will not be considered identical. However, galleries often use different crops of the same image in the main media and the body. As a result these will no longer be considered duplicates, and will both appear in the lightbox anyway, defeating the purpose of the deduplication logic. The choice, therefore, is between: 1. Two similar images appearing in the lightbox in some galleries. 2. Articles that rely on different crops of the same image having multiple images missing from the lightbox. (2) is considered more of a problem than (1), so this change removes the lightbox deduplication logic.
1 parent 0b40b07 commit 9c93cb4

File tree

1 file changed

+2
-44
lines changed

1 file changed

+2
-44
lines changed

dotcom-rendering/src/model/buildLightboxImages.ts

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -28,34 +28,6 @@ const THRESHOLD = 620;
2828
const isLightboxable = (width: number, height: number): boolean =>
2929
Math.max(width, height) > THRESHOLD;
3030

31-
/**
32-
* Older legacy images use a different url format so we need to use a different
33-
* approach when extracting the id of the image
34-
*/
35-
const decideImageId = ({
36-
masterUrl,
37-
}: Pick<ImageForLightbox, 'masterUrl'>): string | undefined => {
38-
const url = new URL(masterUrl);
39-
switch (url.hostname) {
40-
case 'media.guim.co.uk': {
41-
// E.g.
42-
// https://media.guim.co.uk/be634f340e477a975c7352f289c4353105ba9e67/288_121_3702_2221/140.jpg
43-
// This bit ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
44-
return url.pathname.split('/').at(1);
45-
}
46-
case 'i.guim.co.uk':
47-
case 'uploads.guim.co.uk':
48-
case 'static-secure.guim.co.uk':
49-
case 'static.guim.co.uk':
50-
// E.g.
51-
// https://static.guim.co.uk/sys-images/Guardian/Pix/pictures/2015/5/26/1432666797165/59de49e2-553f-4b52-b7ac-09bda7f63e4b-220x132.jpeg
52-
// This bit ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
53-
return url.pathname.split('/').at(-1);
54-
default:
55-
return undefined;
56-
}
57-
};
58-
5931
const buildLightboxImage = (
6032
element: ImageBlockElement | CartoonBlockElement,
6133
): Omit<ImageForLightbox, 'position'> | undefined => {
@@ -144,8 +116,8 @@ export const buildLightboxImages = (
144116
format: FEFormat,
145117
blocks: Block[],
146118
mainMediaElements: FEElement[],
147-
): ImageForLightbox[] => {
148-
const lightboxImages = mainMediaElements
119+
): ImageForLightbox[] =>
120+
mainMediaElements
149121
.flatMap<Omit<ImageForLightbox, 'position'>>((element) => {
150122
if (!isImage(element) && !isCartoon(element)) return [];
151123
const lightboxImage = buildLightboxImage(element);
@@ -171,17 +143,3 @@ export const buildLightboxImages = (
171143
),
172144
)
173145
.map((image, index) => ({ ...image, position: index + 1 }));
174-
175-
// On gallery articles the main media is often repeated as an element in the article body so
176-
// we deduplicate the array here
177-
return [
178-
...new Map(
179-
lightboxImages.map<[string, ImageForLightbox]>((image, index) => [
180-
decideImageId(image) ?? `lightbox-image-id-${index}`,
181-
image,
182-
]),
183-
).values(),
184-
]
185-
.sort((a, b) => a.position - b.position)
186-
.map((image, index) => ({ ...image, position: index + 1 }));
187-
};

0 commit comments

Comments
 (0)