Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Require an alt prop for every Image #1739

Merged
merged 11 commits into from
Nov 8, 2024
14 changes: 10 additions & 4 deletions packages/css/src/components/image/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,17 @@ If the intrinsic dimensions of the source do not match an aspect ratio, the imag

## Guidelines

- Do not forget to include 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.
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved
- 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)).
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved
- 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.
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/Avatar/Avatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type AvatarContentProps = {

const AvatarContent = ({ imageSrc, initials }: AvatarContentProps) => {
if (imageSrc) {
return <Image src={imageSrc} alt="" />
return <Image alt="" src={imageSrc} />
}

if (initials.length) {
Expand Down
10 changes: 5 additions & 5 deletions packages/react/src/Image/Image.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import '@testing-library/jest-dom'

describe('Image', () => {
it('renders', () => {
const { container } = render(<Image />)
const { container } = render(<Image alt="" />)

const component = container.querySelector(':only-child')

Expand All @@ -15,15 +15,15 @@ describe('Image', () => {
})

it('renders a design system BEM class name', () => {
const { container } = render(<Image />)
const { container } = render(<Image alt="" />)

const component = container.querySelector(':only-child')

expect(component).toHaveClass('ams-image')
})

it('renders an additional class name', () => {
const { container } = render(<Image className="extra" />)
const { container } = render(<Image alt="" className="extra" />)

const component = container.querySelector(':only-child')

Expand All @@ -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(<Image aspectRatio={aspectRatio} />)
const { container } = render(<Image alt="" aspectRatio={aspectRatio} />)

const component = container.querySelector(':only-child')

Expand All @@ -43,7 +43,7 @@ describe('Image', () => {
it('supports ForwardRef in React', () => {
const ref = createRef<HTMLImageElement>()

const { container } = render(<Image ref={ref} />)
const { container } = render(<Image alt="" ref={ref} />)

const component = container.querySelector(':only-child')

Expand Down
8 changes: 7 additions & 1 deletion packages/react/src/Image/Image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@ import { forwardRef } from 'react'
import type { ForwardedRef, ImgHTMLAttributes } from 'react'
import { AspectRatioProps } from '../common/types'

export type ImageProps = AspectRatioProps & ImgHTMLAttributes<HTMLImageElement>
export type ImageProps = {
/**
* A textual description of the content of the image.
*/
alt: string
} & AspectRatioProps &
ImgHTMLAttributes<HTMLImageElement>

export const Image = forwardRef(
({ aspectRatio, className, ...restProps }: ImageProps, ref: ForwardedRef<HTMLImageElement>) => (
Expand Down
16 changes: 8 additions & 8 deletions storybook/src/docs/components/AppIcons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Image } from '@amsterdam/design-system-react'
export const AppleTouchIcon = () => (
<div className="ams-docs-gallery">
<figure className="ams-docs-figure">
<Image src="favicon/apple-touch-icon.png" width={180} height={180} />
<Image alt="" src="favicon/apple-touch-icon.png" width={180} height={180} />
<figcaption>apple-touch-icon.png (180px)</figcaption>
</figure>
</div>
Expand All @@ -12,19 +12,19 @@ export const AppleTouchIcon = () => (
export const Favicon = () => (
<div className="ams-docs-gallery">
<figure className="ams-docs-figure">
<Image src="favicon/favicon.ico" width={16} height={16} />
<Image alt="" src="favicon/favicon.ico" width={16} height={16} />
<figcaption>favicon.ico (16px)</figcaption>
</figure>
<figure className="ams-docs-figure">
<Image src="favicon/favicon.ico" width={32} height={32} />
<Image alt="" src="favicon/favicon.ico" width={32} height={32} />
<figcaption>favicon.ico (32px)</figcaption>
</figure>
<figure className="ams-docs-figure">
<Image src="favicon/favicon.ico" width={48} height={48} />
<Image alt="" src="favicon/favicon.ico" width={48} height={48} />
<figcaption>favicon.ico (48px)</figcaption>
</figure>
<figure className="ams-docs-figure">
<Image src="favicon/favicon.ico" width={64} height={64} />
<Image alt="" src="favicon/favicon.ico" width={64} height={64} />
<figcaption>favicon.ico (64px)</figcaption>
</figure>
</div>
Expand All @@ -33,7 +33,7 @@ export const Favicon = () => (
export const SvgIcon = () => (
<div className="ams-docs-gallery">
<figure className="ams-docs-figure">
<Image src="favicon/icon.svg" width={64} height={64} />
<Image alt="" src="favicon/icon.svg" width={64} height={64} />
<figcaption>icon.svg (64px)</figcaption>
</figure>
</div>
Expand All @@ -42,11 +42,11 @@ export const SvgIcon = () => (
export const WebAppIcons = () => (
<div className="ams-docs-gallery">
<figure className="ams-docs-figure">
<Image src="app-icons/icon-192.png" width={192} height={192} />
<Image alt="" src="app-icons/icon-192.png" width={192} height={192} />
<figcaption>icon-192.png (192px)</figcaption>
</figure>
<figure className="ams-docs-figure">
<Image src="app-icons/icon-512.png" width={512} height={512} />
<Image alt="" src="app-icons/icon-512.png" width={512} height={512} />
<figcaption>icon-512.png (512px)</figcaption>
</figure>
</div>
Expand Down