feat: experimental responsive images by ascorbic · Pull Request #12377 · withastro/astro

ascorbic · 2024-11-05T11:00:21Z

Changes

Implements responsive images RFC withastro/roadmap#1051

Testing

Docs

changeset-bot · 2024-11-05T11:00:27Z

🦋 Changeset detected

Latest commit: cc1db75

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

* feat: add experimental responsive images config option * Apply suggestions from code review Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> * Update config types * Move config into `images` * Move jsdocs --------- Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

* feat: add experimental responsive images config option * Apply suggestions from code review Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> * Update config types * Move config into `images` * Move jsdocs * wip: responsive image component * Improve srcset logic * Improve fixture * Lock * Update styling * Fix style prop handling * Update test (there's an extra style for images now) * Safely access the src props * Remove unused export * Handle priority images * Consolidate styles * Update tests * Comment * Refactor srcset * Avoid dupes of original image * Calculate missing dimensions * Bugfixes * Add tests * Fix test * Correct order * Lint * Fix fspath * Update test * Fix test * Conditional component per flag * Fix class concatenation * Remove logger * Rename helper * Add comments * Format * Fix markdoc tests * Add test for style tag --------- Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

* wip: add crop support to image services * Add tests * Strip crop attributes * Don't upscale * Format * Get build working properly * Changes from review

* feat: add responsive support to picture component * Add picture tests * Fix test * Implement own define vars * Share logic * Prevent extra astro-* class * Use dataset scoping * Revert unit test * Revert "Fix test" This reverts commit f9ec6e2. * Changes from review

* docs: add responsive images flag docs * docs: adds jsdoc for image components * chore: updates from review * Fix jsdoc * Changes from review

Princesseuh · 2024-11-15T12:34:38Z

+			width: resolvedOptions.width,
+			layout,
+			originalWidth,
+			breakpoints: isLocalService(service) ? LIMITED_RESOLUTIONS : DEFAULT_RESOLUTIONS,


Should people / image service be able to customise this? I'm notably talking about Vercel here, where the breakpoints needs to somewhat matches the allowed widths (hahaha)

I did consider adding a breakpoints option to the config, though people can still set widths manually. Maybe it would be a good idea.

OK, config now supports image.experimentalBreakpoints

Princesseuh · 2024-11-15T12:43:01Z


 	let imageURL = await service.getURL(validatedOptions, imageConfig);
+
+	const matchesOriginal = (transform: ImageTransform) =>


In theory, couldn't the quality be different here? Or any other parameters that affects the image, for image services who support more.

I think in this scenario it can't, as this is just checking if it's the srcset entry which is the same as the src. I don't think we expose a way to set this separately.

Can't an image service set a different quality for a srcset transform?

I guess in theory. I'm comfortable with not supporting that though. It's a terrible idea.

Princesseuh · 2024-11-15T12:43:56Z

+	750, // iPhone 6-8
+	828, // iPhone XR/11
+	1080, // iPhone 6-8 Plus


Apple really is something

Princesseuh · 2024-11-15T12:44:35Z

+ * - For fixed layout we return 1x and 2x the requested width, unless the original image is smaller than that.
+ * - For responsive layout we return all breakpoints smaller than 2x the requested width, unless the original image is smaller than that.
+ */
+export const getWidths = ({


Appreciate the comments in this one, great work.

Princesseuh · 2024-11-15T12:48:33Z

-	getSrcSet(options) {
-		const srcSet: UnresolvedSrcSetValue[] = [];
-		const { targetWidth } = getTargetDimensions(options);
+	getSrcSet(options): Array<UnresolvedSrcSetValue> {


I don't personally care all that much, but UnresolvedSrcSetValue[] is more common in our codebase

Princesseuh · 2024-11-15T13:24:18Z

 					export { inferRemoteSize } from "astro/assets/utils/inferRemoteSize.js";

-					export const imageConfig = ${JSON.stringify(settings.config.image)};
+					export const imageConfig = ${JSON.stringify({ ...settings.config.image, experimentalResponsiveImages: settings.config.experimental.responsiveImages })};


might want to add the new breakpoint option here as well?

That's part of settings.config.image.

Princesseuh · 2024-11-15T13:25:14Z

 					}),
 				)
 				.default([]),
+			experimentalLayout: z.enum(['responsive', 'fixed', 'full-width', 'none']).optional(),


A single source of truth for this would be great, you can add it in const, derive the type from it and also use it here

cries in single source of truth for the config types ahem it would, yes

Princesseuh

Left a bunch of comments, but nothing blocking. Awesome work!

chore: changeset

5f328e0

github-actions Bot added pkg: astro Related to the core `astro` package (scope) docs pr labels Nov 6, 2024

ascorbic and others added 2 commits November 8, 2024 13:58

Merge branch 'next' into responsive-images

ffe063c

github-actions Bot added the pkg: integration Related to any renderer integration (scope) label Nov 11, 2024

ascorbic added 7 commits November 11, 2024 12:00

Merge branch 'next' into responsive-images

814b873

feat: add crop support to image services (#12414)

46055a6

* wip: add crop support to image services * Add tests * Strip crop attributes * Don't upscale * Format * Get build working properly * Changes from review

Merge branch 'next' into responsive-images

fd68ea9

Merge branch 'next' into responsive-images

b500b10

Fix jsdoc

582f997

Merge branch 'next' into responsive-images

d28ba03

ascorbic added the pr preview Apply this label to a PR to generate a preview release label Nov 14, 2024

withastro deleted a comment from github-actions Bot Nov 14, 2024

ascorbic removed the pr preview Apply this label to a PR to generate a preview release label Nov 14, 2024

docs: add docs for responsive images (#12429)

a6b283a

* docs: add responsive images flag docs * docs: adds jsdoc for image components * chore: updates from review * Fix jsdoc * Changes from review

ascorbic changed the title ~~wip: experimental responsive images~~ feat: experimental responsive images Nov 15, 2024

Merge branch 'next' into responsive-images

b746c15

ascorbic marked this pull request as ready for review November 15, 2024 12:16

Princesseuh reviewed Nov 15, 2024

View reviewed changes

ascorbic added 2 commits November 15, 2024 13:07

Add breakpoints option

4889154

typo

cc1db75

Princesseuh reviewed Nov 15, 2024

View reviewed changes

Princesseuh approved these changes Nov 15, 2024

View reviewed changes

ascorbic merged commit af867f3 into next Nov 15, 2024

ascorbic deleted the responsive-images branch November 15, 2024 13:29

astrobot-houston mentioned this pull request Nov 15, 2024

[ci] release (beta) #12430

Merged

joshmedeski mentioned this pull request Nov 21, 2024

Adds experimental support for automatic responsive images joshmedeski/joshmedeski.com#43

Open

ArmandPhilippot mentioned this pull request Dec 12, 2025

fix(astro:assets): add docs for missing imports and types withastro/docs#12867

Merged


		let imageURL = await service.getURL(validatedOptions, imageConfig);

		const matchesOriginal = (transform: ImageTransform) =>

Uh oh!

Conversation

ascorbic commented Nov 5, 2024

Changes

Testing

Docs

Uh oh!

changeset-bot Bot commented Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Princesseuh left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot Bot commented Nov 5, 2024 •

edited

Loading