From 221a1895ee2e7fe8be232021a43626d58bdedb94 Mon Sep 17 00:00:00 2001 From: Ruben Sibon Date: Fri, 1 Nov 2024 16:23:19 +0100 Subject: [PATCH 1/8] feat(image)!: alt prop is required --- packages/react/src/Image/Image.test.tsx | 10 +++++----- packages/react/src/Image/Image.tsx | 2 ++ storybook/src/docs/components/AppIcons.tsx | 16 ++++++++-------- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/react/src/Image/Image.test.tsx b/packages/react/src/Image/Image.test.tsx index dbb18575a1..47272ef775 100644 --- a/packages/react/src/Image/Image.test.tsx +++ b/packages/react/src/Image/Image.test.tsx @@ -5,7 +5,7 @@ import '@testing-library/jest-dom' describe('Image', () => { it('renders', () => { - const { container } = render() + const { container } = render({'Image'}) const component = container.querySelector(':only-child') @@ -14,7 +14,7 @@ describe('Image', () => { }) it('renders a design system BEM class name', () => { - const { container } = render() + const { container } = render({'Image'}) const component = container.querySelector(':only-child') @@ -22,7 +22,7 @@ describe('Image', () => { }) it('renders an additional class name', () => { - const { container } = render() + const { container } = render({'Image'}) const component = container.querySelector(':only-child') @@ -31,7 +31,7 @@ describe('Image', () => { }) it('renders a class name to display the image as large as its container', () => { - const { container } = render() + const { container } = render({'Image'}) const component = container.querySelector(':only-child') @@ -41,7 +41,7 @@ describe('Image', () => { it('supports ForwardRef in React', () => { const ref = createRef() - const { container } = render() + const { container } = render({'Image'}) const component = container.querySelector(':only-child') diff --git a/packages/react/src/Image/Image.tsx b/packages/react/src/Image/Image.tsx index 38a857e0dd..394064563a 100644 --- a/packages/react/src/Image/Image.tsx +++ b/packages/react/src/Image/Image.tsx @@ -8,6 +8,8 @@ import { forwardRef } from 'react' import type { ForwardedRef, ImgHTMLAttributes } from 'react' export type ImageProps = { + /** Descriptive text for the image. */ + alt: string /** Whether to display the image exactly as large as its container. This will clip the image if necessary. */ cover?: boolean } & ImgHTMLAttributes diff --git a/storybook/src/docs/components/AppIcons.tsx b/storybook/src/docs/components/AppIcons.tsx index b417b8e6d8..80eaf3ec2a 100644 --- a/storybook/src/docs/components/AppIcons.tsx +++ b/storybook/src/docs/components/AppIcons.tsx @@ -3,7 +3,7 @@ import { Image } from '@amsterdam/design-system-react' export const AppleTouchIcon = () => (
- + apple-touch-icon.png (180px)
apple-touch-icon.png (180px)
@@ -12,19 +12,19 @@ export const AppleTouchIcon = () => ( export const Favicon = () => (
- + favicon.ico (16px)
favicon.ico (16px)
- + favicon.ico (32px)
favicon.ico (32px)
- + favicon.ico (48px)
favicon.ico (48px)
- + favicon.ico (64px)
favicon.ico (64px)
@@ -33,7 +33,7 @@ export const Favicon = () => ( export const SvgIcon = () => (
- + icon.svg (64px)
icon.svg (64px)
@@ -42,11 +42,11 @@ export const SvgIcon = () => ( export const WebAppIcons = () => (
- + icon-192.png (192px)
icon-192.png (192px)
- + icon-512.png (512px)
icon-512.png (512px)
From 27a86c4ea72347115d23234d13725b7438be5e30 Mon Sep 17 00:00:00 2001 From: Ruben Sibon Date: Fri, 1 Nov 2024 16:27:18 +0100 Subject: [PATCH 2/8] =?UTF-8?q?docs(image):=20=F0=9F=93=84=20adjust=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/css/src/components/image/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/css/src/components/image/README.md b/packages/css/src/components/image/README.md index da8fbbcfff..314fefab18 100644 --- a/packages/css/src/components/image/README.md +++ b/packages/css/src/components/image/README.md @@ -6,7 +6,7 @@ Displays an image. ## Guidelines -- Do not forget to include a description of the image in the `alt` attribute. +- Always add a description of the image in the `alt` attribute. This ensures that screen reader users can also access this information. Additionally, it can aid in search engine optimization. - A description is unnecessary for decorative images; use `alt=""` for these. From d9879aa6d781ab74fc9faf7b2723617cfc281dc2 Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Fri, 8 Nov 2024 11:58:24 +0100 Subject: [PATCH 3/8] Update guidelines for alternate text --- packages/css/src/components/image/README.md | 14 ++++++++++---- packages/react/src/Image/Image.tsx | 3 +-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/css/src/components/image/README.md b/packages/css/src/components/image/README.md index 5efef254f9..7cea8ec07a 100644 --- a/packages/css/src/components/image/README.md +++ b/packages/css/src/components/image/README.md @@ -12,11 +12,17 @@ If the intrinsic dimensions of the source do not match an aspect ratio, the imag ## Guidelines -- Always add a description of the image in the `alt` attribute. - This ensures that screen reader users can also access this information. - Additionally, it can aid in search engine optimization. +- The `alt` attribute is required, but can be empty. + This helps to consider providing an alternate text for every image. - A description is unnecessary for decorative images; use `alt=""` for these. - Examples are images that add little to the nearby text or pictures that purely contribute to the design or atmosphere of the page (source: [W3C Web Accessibility Initiative](https://www.w3.org/WAI/tutorials/images/decorative/)). + Non-visual browsers will then hide the image from the user. + Examples are images that add little to the nearby text, and pictures that purely contribute to the design or atmosphere of the page (source: [W3C Web Accessibility Initiative](https://www.w3.org/WAI/tutorials/images/decorative/)). +- Do describe the content of the image if the image isn’t only decorative. + When choosing alt strings for your images, imagine what you would say when reading the page to someone over the phone without mentioning that there's an image on the page (source: [MDN](https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt)). +- Every image’s alternate text should be able to replace the image without altering the meaning of the page. + You should never use alt for text that could be construed as a caption or title (source: [MDN](https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt)). +- The alternate text will also display if the image is not (yet) loaded. + Additionally, it can be helpful for search engine optimization. - Optionally specify multiple candidates for the image through the `srcSet` attribute. For example, provide small, medium, and large variants for various screen sizes. This prevents unnecessary downloading of large files. diff --git a/packages/react/src/Image/Image.tsx b/packages/react/src/Image/Image.tsx index 778f77cea7..ec4efcd0f4 100644 --- a/packages/react/src/Image/Image.tsx +++ b/packages/react/src/Image/Image.tsx @@ -10,8 +10,7 @@ import { AspectRatioProps } from '../common/types' export type ImageProps = { /** - * Describes the image to blind or visually impaired users. - * Also displayed if the image is not (yet) loaded. + * A textual description of the content of the image. */ alt: string } & AspectRatioProps & From f4dc33fa0076097ab0c425dba372f37939ccbcdc Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Fri, 8 Nov 2024 11:58:52 +0100 Subject: [PATCH 4/8] Simplify alt prop value in tests --- packages/react/src/Image/Image.test.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/react/src/Image/Image.test.tsx b/packages/react/src/Image/Image.test.tsx index 9e6b335da4..3c1c62daa3 100644 --- a/packages/react/src/Image/Image.test.tsx +++ b/packages/react/src/Image/Image.test.tsx @@ -6,7 +6,7 @@ import '@testing-library/jest-dom' describe('Image', () => { it('renders', () => { - const { container } = render({'Image'}) + const { container } = render() const component = container.querySelector(':only-child') @@ -15,7 +15,7 @@ describe('Image', () => { }) it('renders a design system BEM class name', () => { - const { container } = render({'Image'}) + const { container } = render() const component = container.querySelector(':only-child') @@ -23,7 +23,7 @@ describe('Image', () => { }) it('renders an additional class name', () => { - const { container } = render({'Image'}) + const { container } = render() const component = container.querySelector(':only-child') @@ -32,7 +32,7 @@ describe('Image', () => { aspectRatioOptions.forEach((aspectRatio) => { it(`renders class names to display the image in the ${aspectRatio} aspect ratio`, () => { - const { container } = render() + const { container } = render() const component = container.querySelector(':only-child') @@ -43,7 +43,7 @@ describe('Image', () => { it('supports ForwardRef in React', () => { const ref = createRef() - const { container } = render({'Image'}) + const { container } = render() const component = container.querySelector(':only-child') From 3967d8befca76749761f1ed5744ee42b463c2d42 Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Fri, 8 Nov 2024 11:59:52 +0100 Subject: [PATCH 5/8] Remove alternate text for decorative images --- storybook/src/docs/components/AppIcons.tsx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/storybook/src/docs/components/AppIcons.tsx b/storybook/src/docs/components/AppIcons.tsx index 80eaf3ec2a..45281bdf4b 100644 --- a/storybook/src/docs/components/AppIcons.tsx +++ b/storybook/src/docs/components/AppIcons.tsx @@ -3,7 +3,7 @@ import { Image } from '@amsterdam/design-system-react' export const AppleTouchIcon = () => (
- apple-touch-icon.png (180px) +
apple-touch-icon.png (180px)
@@ -12,19 +12,19 @@ export const AppleTouchIcon = () => ( export const Favicon = () => (
- favicon.ico (16px) +
favicon.ico (16px)
- favicon.ico (32px) +
favicon.ico (32px)
- favicon.ico (48px) +
favicon.ico (48px)
- favicon.ico (64px) +
favicon.ico (64px)
@@ -33,7 +33,7 @@ export const Favicon = () => ( export const SvgIcon = () => (
- icon.svg (64px) +
icon.svg (64px)
@@ -42,11 +42,11 @@ export const SvgIcon = () => ( export const WebAppIcons = () => (
- icon-192.png (192px) +
icon-192.png (192px)
- icon-512.png (512px) +
icon-512.png (512px)
From 1bd7bdc2985974bc4799cbce64f473b63d8840dc Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Fri, 8 Nov 2024 12:06:51 +0100 Subject: [PATCH 6/8] Sort alt prop and make apostrophe curly --- packages/css/src/components/image/README.md | 2 +- packages/react/src/Avatar/Avatar.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/css/src/components/image/README.md b/packages/css/src/components/image/README.md index 7cea8ec07a..44ac11c09d 100644 --- a/packages/css/src/components/image/README.md +++ b/packages/css/src/components/image/README.md @@ -18,7 +18,7 @@ If the intrinsic dimensions of the source do not match an aspect ratio, the imag Non-visual browsers will then hide the image from the user. Examples are images that add little to the nearby text, and pictures that purely contribute to the design or atmosphere of the page (source: [W3C Web Accessibility Initiative](https://www.w3.org/WAI/tutorials/images/decorative/)). - Do describe the content of the image if the image isn’t only decorative. - When choosing alt strings for your images, imagine what you would say when reading the page to someone over the phone without mentioning that there's an image on the page (source: [MDN](https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt)). + When choosing alt strings for your images, imagine what you would say when reading the page to someone over the phone without mentioning that there’s an image on the page (source: [MDN](https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt)). - Every image’s alternate text should be able to replace the image without altering the meaning of the page. You should never use alt for text that could be construed as a caption or title (source: [MDN](https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt)). - The alternate text will also display if the image is not (yet) loaded. diff --git a/packages/react/src/Avatar/Avatar.tsx b/packages/react/src/Avatar/Avatar.tsx index b32cb9dfe5..a103565e3d 100644 --- a/packages/react/src/Avatar/Avatar.tsx +++ b/packages/react/src/Avatar/Avatar.tsx @@ -36,7 +36,7 @@ type AvatarContentProps = { const AvatarContent = ({ imageSrc, initials }: AvatarContentProps) => { if (imageSrc) { - return + return } if (initials.length) { From cf329ef1ea5e31c9fe152acc69d81709bb16a00e Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Fri, 8 Nov 2024 13:09:10 +0100 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: Aram <37216945+alimpens@users.noreply.github.com> --- packages/css/src/components/image/README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/css/src/components/image/README.md b/packages/css/src/components/image/README.md index 44ac11c09d..e8d7be066d 100644 --- a/packages/css/src/components/image/README.md +++ b/packages/css/src/components/image/README.md @@ -13,14 +13,13 @@ If the intrinsic dimensions of the source do not match an aspect ratio, the imag ## Guidelines - The `alt` attribute is required, but can be empty. - This helps to consider providing an alternate text for every image. - A description is unnecessary for decorative images; use `alt=""` for these. Non-visual browsers will then hide the image from the user. Examples are images that add little to the nearby text, and pictures that purely contribute to the design or atmosphere of the page (source: [W3C Web Accessibility Initiative](https://www.w3.org/WAI/tutorials/images/decorative/)). - Do describe the content of the image if the image isn’t only decorative. When choosing alt strings for your images, imagine what you would say when reading the page to someone over the phone without mentioning that there’s an image on the page (source: [MDN](https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt)). - Every image’s alternate text should be able to replace the image without altering the meaning of the page. - You should never use alt for text that could be construed as a caption or title (source: [MDN](https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt)). + You should never use `alt` for text that could be construed as a caption or title (source: [MDN](https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt)). - The alternate text will also display if the image is not (yet) loaded. Additionally, it can be helpful for search engine optimization. - Optionally specify multiple candidates for the image through the `srcSet` attribute. From 007b2df0fd1653f2adaad34c7c37cbb046699b1f Mon Sep 17 00:00:00 2001 From: Vincent Smedinga Date: Fri, 8 Nov 2024 13:09:25 +0100 Subject: [PATCH 8/8] Apply suggestions from code review --- packages/css/src/components/image/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/css/src/components/image/README.md b/packages/css/src/components/image/README.md index e8d7be066d..2c120ac8cc 100644 --- a/packages/css/src/components/image/README.md +++ b/packages/css/src/components/image/README.md @@ -17,7 +17,7 @@ If the intrinsic dimensions of the source do not match an aspect ratio, the imag Non-visual browsers will then hide the image from the user. Examples are images that add little to the nearby text, and pictures that purely contribute to the design or atmosphere of the page (source: [W3C Web Accessibility Initiative](https://www.w3.org/WAI/tutorials/images/decorative/)). - Do describe the content of the image if the image isn’t only decorative. - When choosing alt strings for your images, imagine what you would say when reading the page to someone over the phone without mentioning that there’s an image on the page (source: [MDN](https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt)). + When choosing a description for your images, imagine what you would say when reading the page to someone over the phone without mentioning that there’s an image on the page (source: [MDN](https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt)). - Every image’s alternate text should be able to replace the image without altering the meaning of the page. You should never use `alt` for text that could be construed as a caption or title (source: [MDN](https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt)). - The alternate text will also display if the image is not (yet) loaded.