Acknowledge existing srcset for suggested image widths

ausi · Jan 30, 2022 · 9d1c6d8 · 9d1c6d8
1 parent 2905e4a
commit 9d1c6d8
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 16 deletions.
diff --git a/src/linter/images/missingFittingSrc.js b/src/linter/images/missingFittingSrc.js
@@ -140,7 +140,12 @@ export default function(image) {
 
 		viewportRanges = viewportRanges.filter(Boolean);
 
-		const imageSrc = item.srcset[0] ? item.srcset[0].src : item.src;
+		const srcPaths = [...(item.srcset || [])];
+		if (item.src && !srcPaths.includes(item.src)) {
+			srcPaths.push(item.src);
+		}
+
+		const srcSizes = srcPaths.filter(src => !!image.images[src]).map(src => image.images[src].size);
 
 		error(__filename, item, {
 			viewport: firstItem.viewport.replace(/x/g, '×'),
@@ -149,7 +154,7 @@ export default function(image) {
 			distance: firstItem.distance,
 			megapixelDistance: firstItem.megapixelDistance,
 			viewportRanges: viewportRanges.map(range => range[0] === range[1] ? range[0] : range.join('–')).join(', ').replace(/x/g, '×'),
-			recommendation: '<br>' + buildRecommendation(dimensionsBySource[itemIndex], image.images[imageSrc] ? image.images[imageSrc].size : {}, Object.keys(viewportWidths).length),
+			recommendation: '<br>' + buildRecommendation(dimensionsBySource[itemIndex], srcSizes, Object.keys(viewportWidths).length),
 			recommendationContext: image.data.img === item ? '<code>&lt;img srcset=&quot;…&quot;&gt;</code>' : 'the ' + humanReadableIndex(itemIndex) + ' <code>&lt;source srcset=&quot;…&quot;&gt;</code>',
 		});
 
@@ -170,9 +175,9 @@ function humanReadableIndex(index) {
 	return index + suffixes[ordinal];
 }
 
-function buildRecommendation(dimensions, size, viewportsCount) {
-	const ratio = (size.width && size.height) ? size.height / size.width : 1;
-	return computeSrcsetWidths(dimensions, ratio, viewportsCount, {
+function buildRecommendation(dimensions, sizes, viewportsCount) {
+	const ratio = (sizes[0] && sizes[0].width && sizes[0].height) ? sizes[0].height / sizes[0].width : 1;
+	return computeSrcsetWidths(dimensions, ratio, viewportsCount, sizes.map(size => size.width).filter(Boolean), {
 		recommendedMinWidth,
 		recommendedMaxWidth,
 		megapixelThreshold,

diff --git a/src/util/computeSrcsetWidths.js b/src/util/computeSrcsetWidths.js
@@ -1,6 +1,6 @@
 import roundWidth from './roundWidth';
 
-export default function computeSrcsetWidths(dimensions, ratio, viewportsCount, {
+export default function computeSrcsetWidths(dimensions, ratio, viewportsCount, existingWidths, {
 	recommendedMinWidth = 0,
 	recommendedMaxWidth = 16384,
 	megapixelThreshold = 0.25,
@@ -31,12 +31,11 @@ export default function computeSrcsetWidths(dimensions, ratio, viewportsCount, {
 		}
 	});
 
+	fixedWidths.push(...existingWidths.filter(width => width >= minWidth && width <= maxWidth * 2));
+
 	fixedWidths.sort((a, b) => a < b ? -1 : 1);
 
 	if (!fixedWidths[0] || getMegapixels(minWidth) < getMegapixels(fixedWidths[0]) - megapixelThreshold) {
-		if (fixedWidths[0] && getMegapixels(minWidth) + megapixelThreshold < getMegapixels(fixedWidths[0])) {
-			fixedWidths.unshift(getWidth(getMegapixels(minWidth) + megapixelThreshold));
-		}
 		fixedWidths.unshift(roundWidth(minWidth));
 	}
 

diff --git a/tests/util/computeSrcsetWidths.js b/tests/util/computeSrcsetWidths.js
@@ -9,7 +9,7 @@ var config = {
 };
 
 test('Basic srcset with single viewport', () => {
-	expect(computeSrcsetWidths({'800x600': 400}, 1, 1)).toStrictEqual([400, 800]);
+	expect(computeSrcsetWidths({'800x600': 400}, 1, 1, [])).toStrictEqual([400, 800]);
 });
 
 test('Fluid 100vw square image', () => {
@@ -18,18 +18,29 @@ test('Fluid 100vw square image', () => {
 		dimensions[viewport+'x'+viewport] = viewport;
 	}
 
-	const widths = computeSrcsetWidths(dimensions, 1, Object.keys(dimensions).length, config);
+	const widths = computeSrcsetWidths(dimensions, 1, Object.keys(dimensions).length, [], config);
 
 	expect(widths).toStrictEqual([300, 880, 1210, 1470, 1680, 1870, 2048]);
 });
 
+test('Fluid 100vw square image with existing widths', () => {
+	const dimensions = {};
+	for (let viewport = 300; viewport < 3000; viewport+=10) {
+		dimensions[viewport+'x'+viewport] = viewport;
+	}
+
+	const widths = computeSrcsetWidths(dimensions, 1, Object.keys(dimensions).length, [10, 1200, 1999, 9999], config);
+
+	expect(widths).toStrictEqual([300, 880, 1200, 1440, 1650, 1830, 1999]);
+});
+
 test('Fluid 50vw square image', () => {
 	const dimensions = {};
 	for (let viewport = 300; viewport < 3000; viewport+=10) {
 		dimensions[viewport+'x'+viewport] = viewport / 2;
 	}
 
-	const widths = computeSrcsetWidths(dimensions, 1, Object.keys(dimensions).length, config);
+	const widths = computeSrcsetWidths(dimensions, 1, Object.keys(dimensions).length, [], config);
 
 	expect(widths).toStrictEqual([256, 890, 1230, 1500, 1700, 1880, 2048]);
 });
@@ -40,7 +51,7 @@ test('Fluid 100vw 4/1 image', () => {
 		dimensions[viewport+'x'+viewport] = viewport;
 	}
 
-	const widths = computeSrcsetWidths(dimensions, 1 / 4, Object.keys(dimensions).length, config);
+	const widths = computeSrcsetWidths(dimensions, 1 / 4, Object.keys(dimensions).length, [], config);
 
 	expect(widths).toStrictEqual([300, 1460, 2048]);
 });
@@ -51,7 +62,7 @@ test('Fluid 50vw 4/1 image', () => {
 		dimensions[viewport+'x'+viewport] = viewport / 2;
 	}
 
-	const widths = computeSrcsetWidths(dimensions, 1 / 4, Object.keys(dimensions).length, config);
+	const widths = computeSrcsetWidths(dimensions, 1 / 4, Object.keys(dimensions).length, [], config);
 
 	expect(widths).toStrictEqual([256, 1500, 2048]);
 });
@@ -62,7 +73,7 @@ test('Fluid 100vw square image max-width 600', () => {
 		dimensions[viewport+'x'+viewport] = Math.min(600, viewport);
 	}
 
-	const widths = computeSrcsetWidths(dimensions, 1, Object.keys(dimensions).length, config);
+	const widths = computeSrcsetWidths(dimensions, 1, Object.keys(dimensions).length, [], config);
 
 	expect(widths).toStrictEqual([300, 600, 950, 1200]);
 });
@@ -73,7 +84,7 @@ test('Static 500/1000/1500 square image', () => {
 		dimensions[viewport+'x'+viewport] = Math.max(500, Math.min(1500, Math.floor(viewport / 500) * 500));
 	}
 
-	const widths = computeSrcsetWidths(dimensions, 1, Object.keys(dimensions).length, config);
+	const widths = computeSrcsetWidths(dimensions, 1, Object.keys(dimensions).length, [], config);
 
 	// TODO: 1280, 1700 and 1880 should be removed for this case
 	expect(widths).toStrictEqual([500, 1000, 1280, 1500, 1700, 1880, 2048]);