From 8c3f8c74dbdf76b1badc882475641ed7ef798339 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joakim=20J=C3=A4derberg?= Date: Tue, 2 Sep 2025 17:52:31 +0000 Subject: [PATCH] Merged in feature/SW-3365-blurry-images (pull request #2746) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Feature/SW-3365 reduce upscaling of images (fix blurry images) * fix: handle when images are wider than 3:2 but rendered in a 3:2 container * use dimensions everywhere applicable * fall back to using if possible * imageLoader: never nest * remove empty test file Approved-by: Anton Gunnarsson Approved-by: Matilda Landström --- .../(protected)/my-pages/[...path]/page.tsx | 1 + .../components/ContentCard/index.tsx | 1 + .../ContentType/CampaignPage/Hero/index.tsx | 1 + .../DestinationPage/TopImages/index.tsx | 1 + .../Facilities/CardGrid/CardImage/index.tsx | 1 + .../ContentType/StartPage/InfoCard/index.tsx | 1 + .../ContentType/StartPage/index.tsx | 1 + .../ContentType/StaticPages/index.tsx | 1 + .../DeprecatedJsonToHtml/renderOptions.tsx | 1 + apps/scandic-web/components/Hero/hero.ts | 3 +- apps/scandic-web/components/Hero/index.tsx | 8 ++- .../RoomListItem/RoomImage/index.tsx | 1 - .../TempDesignSystem/Card/index.tsx | 1 + .../TempDesignSystem/LoyaltyCard/index.tsx | 1 + .../TempDesignSystem/TeaserCard/index.tsx | 1 + .../design-system/lib/components/Image.tsx | 51 -------------- .../lib/components/Image/imageLoader.test.ts | 69 +++++++++++++++++++ .../lib/components/Image/imageLoader.ts | 57 +++++++++++++++ .../lib/components/Image/index.tsx | 46 +++++++++++++ .../lib/components/ImageContainer/index.tsx | 8 ++- .../lib/components/ImageGallery/index.tsx | 8 ++- .../lib/components/JsonToHtml/JsonToHtml.tsx | 4 ++ .../components/JsonToHtml/renderOptions.tsx | 1 + packages/design-system/package.json | 2 +- packages/design-system/vitest.config.ts | 8 ++- 25 files changed, 219 insertions(+), 59 deletions(-) delete mode 100644 packages/design-system/lib/components/Image.tsx create mode 100644 packages/design-system/lib/components/Image/imageLoader.test.ts create mode 100644 packages/design-system/lib/components/Image/imageLoader.ts create mode 100644 packages/design-system/lib/components/Image/index.tsx diff --git a/apps/scandic-web/app/[lang]/(live)/(protected)/my-pages/[...path]/page.tsx b/apps/scandic-web/app/[lang]/(live)/(protected)/my-pages/[...path]/page.tsx index 3b55c2288..3ac801791 100644 --- a/apps/scandic-web/app/[lang]/(live)/(protected)/my-pages/[...path]/page.tsx +++ b/apps/scandic-web/app/[lang]/(live)/(protected)/my-pages/[...path]/page.tsx @@ -43,6 +43,7 @@ export default async function MyPages({}: PageArgs< alt={hero_image.meta.alt || hero_image.meta.caption || ""} src={hero_image.url} focalPoint={hero_image.focalPoint} + dimensions={hero_image.dimensions} sizes="100vw" fill priority diff --git a/apps/scandic-web/components/ContentCard/index.tsx b/apps/scandic-web/components/ContentCard/index.tsx index 8c6dd5ec0..587864591 100644 --- a/apps/scandic-web/components/ContentCard/index.tsx +++ b/apps/scandic-web/components/ContentCard/index.tsx @@ -39,6 +39,7 @@ export default function ContentCard({ fill sizes="(min-width: 768px) 413px, 100vw" focalPoint={image.focalPoint} + dimensions={image.dimensions} /> {promoText ? ( {promoText} diff --git a/apps/scandic-web/components/ContentType/CampaignPage/Hero/index.tsx b/apps/scandic-web/components/ContentType/CampaignPage/Hero/index.tsx index 54e23883c..e4be06efe 100644 --- a/apps/scandic-web/components/ContentType/CampaignPage/Hero/index.tsx +++ b/apps/scandic-web/components/ContentType/CampaignPage/Hero/index.tsx @@ -48,6 +48,7 @@ export default async function CampaignHero({ fill sizes="(min-width: 768px) 800px, 100vw" focalPoint={image.focalPoint} + dimensions={image.dimensions} /> ) : null} diff --git a/apps/scandic-web/components/ContentType/DestinationPage/TopImages/index.tsx b/apps/scandic-web/components/ContentType/DestinationPage/TopImages/index.tsx index e59a8c60a..aa071d5f6 100644 --- a/apps/scandic-web/components/ContentType/DestinationPage/TopImages/index.tsx +++ b/apps/scandic-web/components/ContentType/DestinationPage/TopImages/index.tsx @@ -46,6 +46,7 @@ export default function TopImages({ images, destinationName }: TopImageProps) { width={width} height={height} focalPoint={image.focalPoint} + dimensions={image.dimensions} className={`${styles.image} ${images.length > 1 ? styles.clickable : ""}`} onClick={() => images.length diff --git a/apps/scandic-web/components/ContentType/HotelPage/Facilities/CardGrid/CardImage/index.tsx b/apps/scandic-web/components/ContentType/HotelPage/Facilities/CardGrid/CardImage/index.tsx index fb80892e3..ff5c83ad4 100644 --- a/apps/scandic-web/components/ContentType/HotelPage/Facilities/CardGrid/CardImage/index.tsx +++ b/apps/scandic-web/components/ContentType/HotelPage/Facilities/CardGrid/CardImage/index.tsx @@ -25,6 +25,7 @@ export default function CardImage({ fill sizes="(min-width: 768px) 900px, 100vw" focalPoint={backgroundImage.focalPoint} + dimensions={backgroundImage.dimensions} /> ) diff --git a/apps/scandic-web/components/ContentType/StartPage/InfoCard/index.tsx b/apps/scandic-web/components/ContentType/StartPage/InfoCard/index.tsx index d64e78589..aec9d9d4d 100644 --- a/apps/scandic-web/components/ContentType/StartPage/InfoCard/index.tsx +++ b/apps/scandic-web/components/ContentType/StartPage/InfoCard/index.tsx @@ -29,6 +29,7 @@ export default function InfoCard({ width={358} height={179} focalPoint={image.focalPoint} + dimensions={image.dimensions} className={styles.image} /> diff --git a/apps/scandic-web/components/ContentType/StartPage/index.tsx b/apps/scandic-web/components/ContentType/StartPage/index.tsx index 24fbbefb4..b56bad005 100644 --- a/apps/scandic-web/components/ContentType/StartPage/index.tsx +++ b/apps/scandic-web/components/ContentType/StartPage/index.tsx @@ -46,6 +46,7 @@ export default async function StartPage({ } src={header.hero_image.url} focalPoint={header.hero_image.focalPoint} + dimensions={header.hero_image.dimensions} sizes="100vw" fill priority diff --git a/apps/scandic-web/components/ContentType/StaticPages/index.tsx b/apps/scandic-web/components/ContentType/StaticPages/index.tsx index 485f6f560..66eaf6596 100644 --- a/apps/scandic-web/components/ContentType/StaticPages/index.tsx +++ b/apps/scandic-web/components/ContentType/StaticPages/index.tsx @@ -69,6 +69,7 @@ export default async function StaticPage({ alt={hero_image.meta.alt || hero_image.meta.caption || ""} src={hero_image.url} focalPoint={hero_image.focalPoint} + dimensions={hero_image.dimensions} /> ) : null} diff --git a/apps/scandic-web/components/DeprecatedJsonToHtml/renderOptions.tsx b/apps/scandic-web/components/DeprecatedJsonToHtml/renderOptions.tsx index 58f70599b..d4ca74731 100644 --- a/apps/scandic-web/components/DeprecatedJsonToHtml/renderOptions.tsx +++ b/apps/scandic-web/components/DeprecatedJsonToHtml/renderOptions.tsx @@ -397,6 +397,7 @@ export const renderOptions: RenderOptions = { src={image.url} width={width} focalPoint={image.focalPoint} + dimensions={image.dimensions} {...props} /> {image.meta.caption} diff --git a/apps/scandic-web/components/Hero/hero.ts b/apps/scandic-web/components/Hero/hero.ts index 59ab7e8ac..73f1b0aba 100644 --- a/apps/scandic-web/components/Hero/hero.ts +++ b/apps/scandic-web/components/Hero/hero.ts @@ -1,7 +1,8 @@ -import type { FocalPoint } from "@scandic-hotels/trpc/types/image" +import type { FocalPoint, Image } from "@scandic-hotels/trpc/types/image" export interface HeroProps { alt: string src: string focalPoint?: FocalPoint + dimensions?: Image["dimension"] } diff --git a/apps/scandic-web/components/Hero/index.tsx b/apps/scandic-web/components/Hero/index.tsx index 542690fae..bd856ba5b 100644 --- a/apps/scandic-web/components/Hero/index.tsx +++ b/apps/scandic-web/components/Hero/index.tsx @@ -4,7 +4,12 @@ import styles from "./hero.module.css" import type { HeroProps } from "./hero" -export default async function Hero({ alt, src, focalPoint }: HeroProps) { +export default async function Hero({ + alt, + src, + focalPoint, + dimensions, +}: HeroProps) { return ( ) diff --git a/apps/scandic-web/components/HotelReservation/SelectRate/RoomsContainer/Rooms/RoomsList/RoomListItem/RoomImage/index.tsx b/apps/scandic-web/components/HotelReservation/SelectRate/RoomsContainer/Rooms/RoomsList/RoomListItem/RoomImage/index.tsx index 05be00088..e245077a1 100644 --- a/apps/scandic-web/components/HotelReservation/SelectRate/RoomsContainer/Rooms/RoomsList/RoomListItem/RoomImage/index.tsx +++ b/apps/scandic-web/components/HotelReservation/SelectRate/RoomsContainer/Rooms/RoomsList/RoomListItem/RoomImage/index.tsx @@ -54,7 +54,6 @@ const RoomImage = memo(function RoomImage({ title={roomType} fill imageCountPosition="top" - sizes="(max-width: 768px) 768px, 420px" />
{roomTypeCode && room && ( diff --git a/apps/scandic-web/components/TempDesignSystem/Card/index.tsx b/apps/scandic-web/components/TempDesignSystem/Card/index.tsx index ee6128fb7..c96ce9b1c 100644 --- a/apps/scandic-web/components/TempDesignSystem/Card/index.tsx +++ b/apps/scandic-web/components/TempDesignSystem/Card/index.tsx @@ -60,6 +60,7 @@ export default function Card({ fill sizes="(min-width: 1367px) 700px, 900px" focalPoint={backgroundImage.focalPoint} + dimensions={backgroundImage.dimensions} />
)} diff --git a/apps/scandic-web/components/TempDesignSystem/LoyaltyCard/index.tsx b/apps/scandic-web/components/TempDesignSystem/LoyaltyCard/index.tsx index ea3634d98..567704437 100644 --- a/apps/scandic-web/components/TempDesignSystem/LoyaltyCard/index.tsx +++ b/apps/scandic-web/components/TempDesignSystem/LoyaltyCard/index.tsx @@ -33,6 +33,7 @@ export default function LoyaltyCard({ className={styles.image} alt={image.meta.alt || image.title} focalPoint={image.focalPoint} + dimensions={image.dimensions} /> ) : null} diff --git a/apps/scandic-web/components/TempDesignSystem/TeaserCard/index.tsx b/apps/scandic-web/components/TempDesignSystem/TeaserCard/index.tsx index 6314eaf7e..34ff94547 100644 --- a/apps/scandic-web/components/TempDesignSystem/TeaserCard/index.tsx +++ b/apps/scandic-web/components/TempDesignSystem/TeaserCard/index.tsx @@ -33,6 +33,7 @@ export default function TeaserCard({ alt={image.meta?.alt || ""} className={styles.image} focalPoint={image.focalPoint} + dimensions={image.dimensions} fill /> </div> diff --git a/packages/design-system/lib/components/Image.tsx b/packages/design-system/lib/components/Image.tsx deleted file mode 100644 index b4728a40f..000000000 --- a/packages/design-system/lib/components/Image.tsx +++ /dev/null @@ -1,51 +0,0 @@ -'use client' - -import NextImage, { - type ImageLoaderProps, - ImageProps as NextImageProps, -} from 'next/image' - -import ImageFallback from './ImageFallback' - -import type { CSSProperties } from 'react' - -type FocalPoint = { - x: number - y: number -} - -export interface ImageProps extends NextImageProps { - focalPoint?: FocalPoint -} - -function imageLoader({ quality, src, width }: ImageLoaderProps) { - const isAbsoluteUrl = src.startsWith('https://') || src.startsWith('http://') - const hasQS = src.indexOf('?') !== -1 - - if (width < 500) { - width += 150 // HACK! Slightly increase width for better quality - } - - if (isAbsoluteUrl) { - return `https://img.scandichotels.com/.netlify/images?url=${src}&w=${width}${quality ? '&q=' + quality : ''}` - } - - return `${src}${hasQS ? '&' : '?'}w=${width}${quality ? '&q=' + quality : ''}` -} - -// Next/Image adds & instead of ? before the params -export default function Image({ focalPoint, style, ...props }: ImageProps) { - const styles: CSSProperties = focalPoint - ? { - objectFit: 'cover', - objectPosition: `${focalPoint.x}% ${focalPoint.y}%`, - ...style, - } - : { ...style } - - if (!props.src) { - return <ImageFallback /> - } - - return <NextImage {...props} style={styles} loader={imageLoader} /> -} diff --git a/packages/design-system/lib/components/Image/imageLoader.test.ts b/packages/design-system/lib/components/Image/imageLoader.test.ts new file mode 100644 index 000000000..ec5ef2dd3 --- /dev/null +++ b/packages/design-system/lib/components/Image/imageLoader.test.ts @@ -0,0 +1,69 @@ +import { describe, it, expect } from 'vitest' +import { imageLoader } from './imageLoader' +describe('imageLoader', () => { + it('should generate the correct image URL for absolute URLs', () => { + const loader = imageLoader({ dimensions: { width: 800, height: 600 } }) + const url = loader({ + quality: 80, + src: 'https://example.com/image.jpg', + width: 800, + }) + + expect(url).toBe( + 'https://img.scandichotels.com/.netlify/images?url=https://example.com/image.jpg&w=800&q=80' + ) + }) + + it('should generate the correct image URL for relative URLs', () => { + const loader = imageLoader({ dimensions: { width: 800, height: 600 } }) + const url = loader({ + quality: 80, + src: '/image.jpg', + width: 800, + }) + + expect(url).toBe('/image.jpg?w=800&q=80') + }) + + it('should compensate for landscape 3:2 images', () => { + const loader = imageLoader({ dimensions: { width: 6000, height: 4000 } }) + const url = loader({ + src: '/image.jpg', + width: 400, + }) + + expect(url).toBe('/image.jpg?w=600') + }) + + it('should compensate for landscape ~3:2 images', () => { + const loader = imageLoader({ dimensions: { width: 7952, height: 5304 } }) + const url = loader({ + src: '/image.jpg', + width: 400, + }) + + expect(url).toBe('/image.jpg?w=600') + }) + + it('should compensate for standing 2:3 images', () => { + const loader = imageLoader({ dimensions: { width: 4000, height: 6000 } }) + const url = loader({ + src: '/image.jpg', + width: 800, + }) + + expect(url).toBe('/image.jpg?w=800') + }) + + it('should compensate for landscape 2:1 images', () => { + const loader = imageLoader({ dimensions: { width: 2000, height: 1000 } }) + const url = loader({ + src: '/image.jpg', + width: 800, + }) + + // used to fetch an image 800x400 image but we, probably, render it with a height of 533 + + expect(url).toBe('/image.jpg?w=1200') + }) +}) diff --git a/packages/design-system/lib/components/Image/imageLoader.ts b/packages/design-system/lib/components/Image/imageLoader.ts new file mode 100644 index 000000000..c4bb441f8 --- /dev/null +++ b/packages/design-system/lib/components/Image/imageLoader.ts @@ -0,0 +1,57 @@ +import { ImageLoaderProps } from 'next/image' + +export const imageLoader = + ({ + dimensions, + }: { + focalPoint?: { x: number; y: number } + dimensions?: { width: number; height: number } + }) => + ({ quality, src, width }: ImageLoaderProps) => { + const isAbsoluteUrl = + src.startsWith('https://') || src.startsWith('http://') + const hasQS = src.indexOf('?') !== -1 + + if ( + dimensions && + isLargerThanAspectRatio(dimensions, '3:2') && + width < dimensions.width + ) { + // If image is wider than 3:2, compensate for low height when rendering in a 3:2 container + const scale = width / dimensions.width + const minWidthFor32Aspect = + dimensions.height * scale * aspectRatios['3:2'] * 2 + + width = Math.max(minWidthFor32Aspect, width) + } + + if (width < 500) { + // Compensate for bad resizing library used by netlify + width += width / 2 + } + + width = roundToNearest(width, 10) + + if (isAbsoluteUrl) { + return `https://img.scandichotels.com/.netlify/images?url=${src}&w=${width}${quality ? '&q=' + quality : ''}` + } + + return `${src}${hasQS ? '&' : '?'}w=${width}${quality ? '&q=' + quality : ''}` + } + +const aspectRatios = { + '3:2': 3 / 2, +} +function isLargerThanAspectRatio( + dimensions: { + width: number + height: number + }, + aspectRatio: keyof typeof aspectRatios +) { + return dimensions.width / dimensions.height > aspectRatios[aspectRatio] +} + +function roundToNearest(value: number, nearest: number) { + return Math.ceil(value / nearest) * nearest +} diff --git a/packages/design-system/lib/components/Image/index.tsx b/packages/design-system/lib/components/Image/index.tsx new file mode 100644 index 000000000..853ddf378 --- /dev/null +++ b/packages/design-system/lib/components/Image/index.tsx @@ -0,0 +1,46 @@ +'use client' + +import NextImage, { ImageProps as NextImageProps } from 'next/image' + +import ImageFallback from '../ImageFallback' + +import type { CSSProperties } from 'react' +import { imageLoader } from './imageLoader' + +type FocalPoint = { + x: number + y: number +} + +export type ImageProps = NextImageProps & { + focalPoint?: FocalPoint + dimensions?: { width: number; height: number } +} + +// Next/Image adds & instead of ? before the params +export default function Image({ + focalPoint, + dimensions, + style, + ...props +}: ImageProps) { + const styles: CSSProperties = focalPoint + ? { + objectFit: 'cover', + objectPosition: `${focalPoint.x}% ${focalPoint.y}%`, + ...style, + } + : { ...style } + + if (!props.src) { + return <ImageFallback /> + } + + return ( + <NextImage + {...props} + style={styles} + loader={imageLoader({ dimensions, focalPoint })} + /> + ) +} diff --git a/packages/design-system/lib/components/ImageContainer/index.tsx b/packages/design-system/lib/components/ImageContainer/index.tsx index 9bf230c78..90a859e23 100644 --- a/packages/design-system/lib/components/ImageContainer/index.tsx +++ b/packages/design-system/lib/components/ImageContainer/index.tsx @@ -11,6 +11,10 @@ type Image = { x: number y: number } + dimensions?: { + width: number + height: number + } meta: { alt?: string | null caption?: string | null @@ -36,6 +40,7 @@ export default function ImageContainer({ width={600} alt={leftImage.meta.alt || leftImage.title} focalPoint={leftImage.focalPoint} + dimensions={leftImage.dimensions} /> <Caption>{leftImage.meta.caption}</Caption> </article> @@ -47,8 +52,9 @@ export default function ImageContainer({ width={600} alt={rightImage.meta.alt || rightImage.title} focalPoint={rightImage.focalPoint} + dimensions={rightImage.dimensions} /> - <Caption>{leftImage.meta.caption}</Caption> + <Caption>{rightImage.meta.caption}</Caption> </article> </section> ) diff --git a/packages/design-system/lib/components/ImageGallery/index.tsx b/packages/design-system/lib/components/ImageGallery/index.tsx index 86c111ead..52dc4ef3a 100644 --- a/packages/design-system/lib/components/ImageGallery/index.tsx +++ b/packages/design-system/lib/components/ImageGallery/index.tsx @@ -42,7 +42,13 @@ function ImageGallery({ const intl = useIntl() const [isOpen, setIsOpen] = useState(false) const [imageError, setImageError] = useState(false) - const imageProps = fill ? { fill, sizes } : { height, width: height * 1.5 } + const imageProps = fill + ? { + fill, + sizes: + sizes ?? 'auto, (max-width: 400px) 100vw, (min-width: 401px) 500px', + } + : { height, width: height * 1.5 } if (!images || images.length === 0 || imageError) { return <ImageFallback /> diff --git a/packages/design-system/lib/components/JsonToHtml/JsonToHtml.tsx b/packages/design-system/lib/components/JsonToHtml/JsonToHtml.tsx index 83b93d24d..8a0ce3ff8 100644 --- a/packages/design-system/lib/components/JsonToHtml/JsonToHtml.tsx +++ b/packages/design-system/lib/components/JsonToHtml/JsonToHtml.tsx @@ -20,6 +20,10 @@ type Image = { x: number y: number } + dimensions?: { + width: number + height: number + } meta: { alt?: string | null caption?: string | null diff --git a/packages/design-system/lib/components/JsonToHtml/renderOptions.tsx b/packages/design-system/lib/components/JsonToHtml/renderOptions.tsx index 9fb0408fa..094b73b99 100644 --- a/packages/design-system/lib/components/JsonToHtml/renderOptions.tsx +++ b/packages/design-system/lib/components/JsonToHtml/renderOptions.tsx @@ -495,6 +495,7 @@ export const renderOptions: RenderOptions = { fill sizes="(min-width: 1367px) 800px, (max-width: 1366px) and (min-width: 1200px) 1200px, 100vw" focalPoint={image.focalPoint} + dimensions={image.dimensions} {...props} /> </div> diff --git a/packages/design-system/package.json b/packages/design-system/package.json index 3926f00a7..a9d67b764 100644 --- a/packages/design-system/package.json +++ b/packages/design-system/package.json @@ -124,7 +124,7 @@ "./Icons/WardIcon": "./lib/components/Icons/Customised/Amenities_Facilities/Ward.tsx", "./Icons/WindowNotAvailableIcon": "./lib/components/Icons/Customised/Amenities_Facilities/WindowNotAvailable.tsx", "./Icons/WoodFloorIcon": "./lib/components/Icons/Customised/Amenities_Facilities/WoodFloor.tsx", - "./Image": "./lib/components/Image.tsx", + "./Image": "./lib/components/Image/index.tsx", "./ImageContainer": "./lib/components/ImageContainer/index.tsx", "./ImageFallback": "./lib/components/ImageFallback/index.tsx", "./ImageGallery": "./lib/components/ImageGallery/index.tsx", diff --git a/packages/design-system/vitest.config.ts b/packages/design-system/vitest.config.ts index b91703f25..327180b66 100644 --- a/packages/design-system/vitest.config.ts +++ b/packages/design-system/vitest.config.ts @@ -19,9 +19,14 @@ const browserInstances = isCI export default mergeConfig( viteConfig, defineConfig({ - // !isCI ? test: { projects: [ + { + test: { + name: 'unit', + environment: 'jsdom', + }, + }, { plugins: [ storybookTest({ @@ -47,6 +52,5 @@ export default mergeConfig( }, ], }, - //: {}, // Netlify CI fails to run playwright tests. Only supported locally for now }) )