From dec0305b7577b431637a129e19fbbe6a28469587 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Wed, 4 Dec 2024 18:26:17 +0000 Subject: [PATCH] fix: don't apply duplicate class to images (#12631) * fix: don't apply duplicate class to images * Fromat --- .changeset/silver-hats-agree.md | 5 + packages/astro/components/Image.astro | 4 +- packages/astro/components/Picture.astro | 4 +- packages/astro/test/core-image-layout.test.js | 121 ++++++++++++++++++ 4 files changed, 130 insertions(+), 4 deletions(-) create mode 100644 .changeset/silver-hats-agree.md diff --git a/.changeset/silver-hats-agree.md b/.changeset/silver-hats-agree.md new file mode 100644 index 000000000000..dec17873ea5f --- /dev/null +++ b/.changeset/silver-hats-agree.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes a bug where the class attribute was rendered twice on the image component diff --git a/packages/astro/components/Image.astro b/packages/astro/components/Image.astro index 6e7a83751689..318fb42fcd9a 100644 --- a/packages/astro/components/Image.astro +++ b/packages/astro/components/Image.astro @@ -47,7 +47,7 @@ if (import.meta.env.DEV) { additionalAttributes['data-image-component'] = 'true'; } -const attributes = useResponsive +const { class: className, ...attributes } = useResponsive ? applyResponsiveAttributes({ layout, image, @@ -58,4 +58,4 @@ const attributes = useResponsive --- {/* Applying class outside of the spread prevents it from applying unnecessary astro-* classes */} - + diff --git a/packages/astro/components/Picture.astro b/packages/astro/components/Picture.astro index 5d03823790d3..7f3ceeee630c 100644 --- a/packages/astro/components/Picture.astro +++ b/packages/astro/components/Picture.astro @@ -102,7 +102,7 @@ if (fallbackImage.srcSet.values.length > 0) { imgAdditionalAttributes.srcset = fallbackImage.srcSet.attribute; } -const attributes = useResponsive +const { class: className, ...attributes } = useResponsive ? applyResponsiveAttributes({ layout, image: fallbackImage, @@ -133,5 +133,5 @@ if (import.meta.env.DEV) { }) } {/* Applying class outside of the spread prevents it from applying unnecessary astro-* classes */} - + diff --git a/packages/astro/test/core-image-layout.test.js b/packages/astro/test/core-image-layout.test.js index 8cbcb8b20fda..bb0e03c481a6 100644 --- a/packages/astro/test/core-image-layout.test.js +++ b/packages/astro/test/core-image-layout.test.js @@ -112,6 +112,7 @@ describe('astro:image:layout', () => { it('passes in a parent class', () => { let $img = $('#local-class img'); + assert.match($img.attr('class'), /green/); }); @@ -576,4 +577,124 @@ describe('astro:image:layout', () => { }); }); }); + + describe('build', () => { + before(async () => { + fixture = await loadFixture({ + root: './fixtures/core-image-layout/', + image: { + service: testImageService({ foo: 'bar' }), + domains: ['avatars.githubusercontent.com'], + }, + }); + + await fixture.build(); + }); + + describe('basics', () => { + let $; + let html; + before(async () => { + html = await fixture.readFile('/index.html'); + $ = cheerio.load(html); + }); + + it('Adds the tag', () => { + let $img = $('#local img'); + assert.equal($img.length, 1); + assert.ok($img.attr('src').startsWith('/_astro')); + }); + + it('includes lazy loading attributes', () => { + let $img = $('#local img'); + assert.equal($img.attr('loading'), 'lazy'); + assert.equal($img.attr('decoding'), 'async'); + assert.equal($img.attr('fetchpriority'), 'auto'); + }); + + it('includes priority loading attributes', () => { + let $img = $('#local-priority img'); + assert.equal($img.attr('loading'), 'eager'); + assert.equal($img.attr('decoding'), 'sync'); + assert.equal($img.attr('fetchpriority'), 'high'); + }); + + it('has width and height - no dimensions set', () => { + let $img = $('#local img'); + assert.equal($img.attr('width'), '2316'); + assert.equal($img.attr('height'), '1544'); + }); + + it('has proper width and height - only width', () => { + let $img = $('#local-width img'); + assert.equal($img.attr('width'), '350'); + assert.equal($img.attr('height'), '233'); + }); + + it('has proper width and height - only height', () => { + let $img = $('#local-height img'); + assert.equal($img.attr('width'), '300'); + assert.equal($img.attr('height'), '200'); + }); + + it('has proper width and height - has both width and height', () => { + let $img = $('#local-both img'); + assert.equal($img.attr('width'), '300'); + assert.equal($img.attr('height'), '400'); + }); + + it('sets the style', () => { + let $img = $('#local-both img'); + assert.match($img.attr('style'), /--w: 300/); + assert.match($img.attr('style'), /--h: 400/); + assert.equal($img.data('astro-image'), 'responsive'); + }); + + it('sets the style when no dimensions set', () => { + let $img = $('#local img'); + assert.match($img.attr('style'), /--w: 2316/); + assert.match($img.attr('style'), /--h: 1544/); + assert.equal($img.data('astro-image'), 'responsive'); + }); + + it('sets style for fixed image', () => { + let $img = $('#local-fixed img'); + assert.match($img.attr('style'), /--w: 800/); + assert.match($img.attr('style'), /--h: 600/); + assert.equal($img.data('astro-image'), 'fixed'); + }); + + it('sets style for full-width image', () => { + let $img = $('#local-full-width img'); + assert.equal($img.data('astro-image'), 'full-width'); + }); + + it('passes in a parent class', () => { + let $img = $('#local-class img'); + assert.equal($img.prop('class'), 'green'); + }); + + it('only passes the class in once', () => { + // Check that class="green" only appears once + // We can't use cheerio because it normalises the DOM, so we have to use a regex + const matches = html.match(/class="green"/g); + assert.equal(matches.length, 1); + }); + + it('passes in a parent style', () => { + let $img = $('#local-style img'); + assert.match($img.attr('style'), /border: 2px red solid/); + }); + + it('passes in a parent style as an object', () => { + let $img = $('#local-style-object img'); + assert.match($img.attr('style'), /border:2px red solid/); + }); + + it('injects a style tag', () => { + const style = $('style').text(); + assert.match(style, /\[data-astro-image\]/); + }); + }); + }); });