feat: add `<PrismicImage>` #140

angeloashmore · 2022-03-31T02:17:54Z

Types of changes

Chore (a non-breaking change which is related to package maintenance)
Bug fix (a non-breaking change which fixes an issue)
New feature (a non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Description

This PR adds a <PrismicImage> component. It renders a Prismic Image field.

<PrismicImage field={imageField} />

This renders the following HTML (using placeholder values for the full image URLs).

<img
  src="https://images.prismic.io/repo/image.png"
  srcset="
    https://images.prismic.io/repo/image.png?width=400   400w,
    https://images.prismic.io/repo/image.png?width=800   800w,
    https://images.prismic.io/repo/image.png?width=1600 1600w
  "
  alt="The field's alt value."
/>

Note that a viewport-width-based srcset is automatically generated. This takes advantage of Imgix's Image URL API which is available to all Prismic users by default.

Note: Prefer using framework-specific image integrations over this component. For example, use Next.js's next/image component or Gatsby's gatsby-plugin-image component if you are using those frameworks. Only use <PrismicImage> if your environment does not have a built-in optimized image component.

Editing images with Imgix

Images can be edited through the image's URL parameters. This includes transformations like saturation, cropping, and blurring.

Arbitrary Imgix parameters can be provided using the imgixParams prop. See Imgix's Image URL API Reference for a full list of what's available.

// Renders grayscale images
<PrismicImage field={imageField} imgixParams={{ sat: -100 }} />

The src URL and each URL in srcset will contain the sat: -100 URL parameter as a result:

https://images.prismic.io/repo/image.png?width=400&sat=-100

Customizing the `srcset` widths

The widths used in the produced srcset can be controlled using the widths prop. It accepts the following values:

An array of widths in pixels (e.g. [100, 200, 300])
"thumbnails" - This will use the Image field's thumbnails (also called responsive views) for each srcset value and its widths.
"defaults" - Use a good set of defaults (currently [640, 750, 828, 1080, 1200, 1920, 2048, 3840])

// Use the good defaults
<PrismicImage field={imageField} />
<PrismicImage field={imageField} widths="defaults" />

// Use explicit widths
<PrismicImage field={imageField} widths={[100, 200, 300]} />

// Use the Image field's thumbnails for each srcset value
<PrismicImage field={imageField} widths="thumbnails" />

Using pixel density-based `srcset`s

Pixel density srcsets can also be generated in place of width-based srcsets by using the pixelDensities prop.

<PrismicImage field={imageField} pixelDensities={[1, 2, 3]} />

This renders the following HTML (using placeholder values for the full image URLs).

<img
  src="https://images.prismic.io/repo/image.png"
  srcset="
    https://images.prismic.io/repo/image.png?dpr=1 1x,
    https://images.prismic.io/repo/image.png?dpr=2 2x,
    https://images.prismic.io/repo/image.png?dpr=3 3x
  "
  alt="The field's alt value."
/>

The pixelDensities prop accepts the following values:

An array of pixel densities (e.g. [2, 4])
"defaults" - Use a good set of defaults (currently [1, 2, 3])

// Use the good defaults
<PrismicImage field={imageField} pixelDensities="defaults" />

// Use explicit pixel densities
<PrismicImage field={imageField} widths={[2, 4]} />

Checklist:

My change requires an update to the official documentation.
All TSDoc comments are up-to-date and new ones have been added where necessary.
All new and existing tests are passing.

🖼️🦙

lihbr

LGTM, left my comments on the @prismicio/helpers for opt-in/naming :)

lihbr · 2022-03-31T09:08:09Z

test/PrismicImage.test.tsx

+
+	const actual = renderJSON(<PrismicImage field={field} />);
+	const expected = renderJSON(
+		<img src={src} srcSet={srcset} alt={undefined} />,


Does this renders to an empty string alt? (<img src="..." srcset="..." alt="" />)

From: prismicio/prismic-vue#55, this is a better accessibility trade-off.

It does not; it will render <img> without an alt attribute by default if the Image field's alt text is null.

Using alt="" explicitly states that the image is decorative and should be ignored by screen readers. A null alt text in Prismic, however, does not always mean the image is decorative. In most cases, this happens because the content writer forgot to or just didn't write alt text.

Omitting alt is a signal to the developer that the Image field needs alt text to be added in Prismic, or that it needs to be explicitly opted-out of the screen reader's flow. Automated checkers could also flag these alt-less <img>s and provide reports, where an <img> with alt="" may not be flagged.

I think declaring an image as decorative should be explicit, requiring the developer to state it with alt="" or fallbackAlt="".

I know there are nuances to this, and I might not have a correct understanding of <img> and alt. If this is incorrect, please let me know. 😀

Ok got it, I like this approach with the default alt.

Personally, I think I still side for the approach that was taken with Vue, but this way is great also!

lihbr · 2022-03-31T09:11:48Z

test/PrismicImage.test.tsx

+test("supports overriding alt explicitly when field has an alt value", (t) => {
+	const field = prismicM.value.image({
+		seed: t.title,
+		model: prismicM.model.image({ seed: t.title }),
+	});
+	field.alt = "provided alt";
+
+	const { src, srcset } = prismicH.asImageWidthSrcSet(field);
+
+	const actual = renderJSON(<PrismicImage field={field} alt="explicit alt" />);
+	const expected = renderJSON(
+		<img src={src} srcSet={srcset} alt="explicit alt" />,
+	);
+
+	t.deepEqual(actual, expected);
+});


I think this is really an anti-pattern to overwrite content writers' work with explicit alt. Are there use-cases where this is really needed?

To me, we should not provide that possibility and let it be a manual process (users would have to override the field's object so that it feels like something wrong to do).

The only legitimate use cases I can think of is providing alt="" or populating alt with content from another field (e.g. a distinct "Alt Text" Key Text field).

I see what you mean though. There isn't really a good reason for overwriting an Image field's alt text with a static string, at least not that I can think of.

Disallowing alt flexibility seems overly restrictive, even if it is not recommended. Perhaps there's a different API that could handle this without requiring users to hack their API responses.

I'm by far not an accessibility expert, but the following is pretty much what I know

e.g. a distinct "Alt Text" Key Text field

I don't think this is a real use case (but maybe I'm wrong), every image you append in Prismic gets a free alt field that you can use to overwrite the default image's alt for that purpose in the editor (and you cannot disable it, maybe there should be a way to mark an image field as decorative within the content type definition).

Using a distinct "Alt Text" Key Text field makes for a confusing UI because you end up with 2 alt text fields (the image one, and the additional one) and there's little way to let the content writer know which one has priority.

Computing the alt text value after another element of the design leads to duplication of information for screenreader users. For example, naming the alt of a video thumbnail after the video title next to it is wrong as you end up duplicating information (screenreaders will read the video title, then the video thumbnail alt, which would be the same). Such alt value should either be omitted (mark image as decorative with role="presentation" or alt="") or left empty (alt="") when the content writer didn't bother to describe the thumbnail.

Maybe we could think about a decorative prop to hide images from screenreaders properly. Although role="presentation" is not the only/always preferred way to hide images from screenreaders (I think alt="" tends to be preferred) so it would make the component more opinionated which I'm not really a fan of.

Yep, I agree with all of that. Using a separate field for alt text is an anti-pattern.

Adding alt="" is in itself declaring the image as decorative, so I don't think we need to have a specialize API over that to declare it as such. With this approach, the JSX more closely matches the HTML output.

Compared to role="presentation", alt="" seems to be the more standard approach for images.

It seems the only valid use case for a configurable alt prop is to declare it as decorative. As such, what if we do this:

Only allow an empty string alt (i.e. alt=""). This is less opinionated than a decorative prop and represents exactly what the HTML output will look like. Declaring alt="" overrides the field's alt text in all cases.

Warn the user via console.warn() if a non-empty string alt is given only when process.env.NODE_ENV === "development".

Type alt to only accept "".

Apply the same behavior to fallbackAlt (i.e. only allow fallbackAlt="").

I'm going to move forward with this approach, but I can write a patch if you think we should adjust it.

codecov-commenter · 2022-04-15T03:11:56Z

Codecov Report

Merging #140 (e070946) into master (192bdf6) will increase coverage by 0.16%.
The diff coverage is 93.93%.

@@            Coverage Diff             @@
##           master     #140      +/-   ##
==========================================
+ Coverage   92.41%   92.58%   +0.16%     
==========================================
  Files          16       18       +2     
  Lines         277      310      +33     
  Branches       62       72      +10     
==========================================
+ Hits          256      287      +31     
  Misses          5        5              
- Partials       16       18       +2

Impacted Files	Coverage Δ
src/PrismicImage.tsx	`93.10% <93.10%> (ø)`
src/index.ts	`100.00% <100.00%> (ø)`
src/lib/devMsg.ts	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 192bdf6...e070946. Read the comment docs.

github-actions · 2022-04-15T03:12:29Z

size-limit report 📦

Path	Size
dist/index.js	5.23 KB (+18.91% 🔺)
dist/index.cjs	6.75 KB (+7.13% 🔺)

feat: add <PrismicImage>

51d368d

angeloashmore marked this pull request as draft March 31, 2022 02:35

angeloashmore mentioned this pull request Mar 31, 2022

Add <PrismicImage> #124

Closed

angeloashmore added 3 commits March 30, 2022 16:40

test: check that Imgix parameters are supported in <PrismicImage>

288ebee

Merge branch 'master' into aa/prismic-image

437747b

Merge branch 'master' into aa/prismic-image

68f6d02

lihbr approved these changes Mar 31, 2022

View reviewed changes

angeloashmore added 2 commits April 14, 2022 17:07

fix: only allow alt and fallbackAlt to declare an image as decorative

036c9d7

fix: incorrect import

e070946

angeloashmore added 3 commits April 14, 2022 17:27

docs: add TSDoc

c0a140c

docs: document default alt behavior

5603291

docs: add note about empty image field alt behavior

049fadf

angeloashmore marked this pull request as ready for review April 15, 2022 03:38

angeloashmore merged commit 7a93f69 into master Apr 15, 2022

angeloashmore deleted the aa/prismic-image branch April 15, 2022 03:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add `<PrismicImage>` #140

feat: add `<PrismicImage>` #140

angeloashmore commented Mar 31, 2022 •

edited

Loading

lihbr left a comment

lihbr Mar 31, 2022

angeloashmore Apr 1, 2022

lihbr Apr 6, 2022

lihbr Mar 31, 2022

angeloashmore Apr 1, 2022

lihbr Apr 6, 2022 •

edited

Loading

angeloashmore Apr 15, 2022 •

edited

Loading

codecov-commenter commented Apr 15, 2022

github-actions bot commented Apr 15, 2022 •

edited

Loading

feat: add <PrismicImage> #140

feat: add <PrismicImage> #140

Conversation

angeloashmore commented Mar 31, 2022 • edited Loading

Types of changes

Description

Editing images with Imgix

Customizing the srcset widths

Using pixel density-based srcsets

Checklist:

lihbr left a comment

Choose a reason for hiding this comment

lihbr Mar 31, 2022

Choose a reason for hiding this comment

angeloashmore Apr 1, 2022

Choose a reason for hiding this comment

lihbr Apr 6, 2022

Choose a reason for hiding this comment

lihbr Mar 31, 2022

Choose a reason for hiding this comment

angeloashmore Apr 1, 2022

Choose a reason for hiding this comment

lihbr Apr 6, 2022 • edited Loading

Choose a reason for hiding this comment

angeloashmore Apr 15, 2022 • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Apr 15, 2022

Codecov Report

github-actions bot commented Apr 15, 2022 • edited Loading

size-limit report 📦

feat: add `<PrismicImage>` #140

feat: add `<PrismicImage>` #140

angeloashmore commented Mar 31, 2022 •

edited

Loading

Customizing the `srcset` widths

Using pixel density-based `srcset`s

lihbr Apr 6, 2022 •

edited

Loading

angeloashmore Apr 15, 2022 •

edited

Loading

github-actions bot commented Apr 15, 2022 •

edited

Loading