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

Add storybook a11y testing #142 #1450

Merged
merged 22 commits into from
Jul 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
50600b6
Add storybook storyshots puppeteer addon
akegan Jun 30, 2021
35297ca
Get yarn:a11y test up and running locally
akegan Jul 2, 2021
7951469
Fix alt text-related issues
akegan Jul 2, 2021
484835a
Try running a11y tests on circleCI
akegan Jul 3, 2021
5b7a9a3
Make sure static storybook build is gitignored
akegan Jul 3, 2021
0a9136a
Actually add a11y tests to circleCI pipeline
akegan Jul 3, 2021
4ce6730
Disable nested interactive rule for Tab-like Nav
akegan Jul 7, 2021
5413fe4
Move storyshots-puppeteer and puppeteer into dev dependencies
akegan Jul 7, 2021
73502d2
Add unmet peer dependency of preact
akegan Jul 7, 2021
061eaf2
Fix failing jest tests
akegan Jul 7, 2021
2c7cb5e
Try node docker image with browsers
akegan Jul 8, 2021
e083707
Use non-legacy docker image
akegan Jul 8, 2021
3aefec2
Fix code style issues with Prettier
lint-action Jul 8, 2021
53c3f81
Fix a11y issue with dropzone story
akegan Jul 8, 2021
df701ea
Merge branch 'add-storybook-a11y-testing-#142' of https://github.com/…
akegan Jul 8, 2021
b64be61
Add changes to changelog
akegan Jul 8, 2021
90268eb
Fix code style issues with Prettier
lint-action Jul 8, 2021
e5da44d
Merge branch 'dev' into add-storybook-a11y-testing-#142
akegan Jul 14, 2021
6a756c4
Merge remote-tracking branch 'remotes/upstream/dev' into add-storyboo…
seanmalbert Jul 14, 2021
579a10c
adds description prop to ImageCard for alt text
seanmalbert Jul 14, 2021
e9e1ab9
updates ImageCard tests
seanmalbert Jul 14, 2021
378ceb9
Fix code style issues with Prettier
lint-action Jul 14, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ executors:
REDIS_USE_TLS: "0"
CLOUDINARY_SECRET: "fake_secret"
CLOUDINARY_KEY: "fake_key"
puppeteer-node:
docker:
- image: "cimg/node:12.18.4-browsers"

jobs:
setup:
Expand Down Expand Up @@ -58,6 +61,12 @@ jobs:
- restore_cache:
key: build-cache-{{ .Environment.CIRCLE_SHA1 }}
- run: yarn test:shared:ui
jest-ui-components-a11y:
executor: puppeteer-node
steps:
- restore_cache:
key: build-cache-{{ .Environment.CIRCLE_SHA1 }}
- run: yarn test:shared:ui:a11y
jest-backend:
executor: standard-node
steps:
Expand Down Expand Up @@ -104,6 +113,9 @@ workflows:
- jest-ui-components:
requires:
- setup
- jest-ui-components-a11y:
requires:
- setup
- jest-backend:
requires:
- setup
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ All notable changes to this project will be documented in this file. The format
### UI Components

- Added:
- Automated a11y testing for ui-components ([#1450](https://github.com/bloom-housing/bloom/pull/1450))
- Add ActionBlock component ([#1404](https://github.com/bloom-housing/bloom/pull/1459)) (Marcin Jedras)

## 1.0.4 / 2021-07-07
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"dev:partners": "concurrently \"yarn dev:backend\" \"yarn dev:app:partners\"",
"dev:public": "concurrently \"yarn dev:backend\" \"yarn dev:app:public\"",
"test:shared:ui": "cd ui-components && yarn && yarn test",
"test:shared:ui:a11y": "cd ui-components && yarn && yarn test:a11y",
"test:backend:core:dbsetup": "cd backend/core && yarn db:migration:run && yarn db:seed",
"test:backend:core": "cd backend/core && yarn test",
"test:e2e:backend:core": "cd backend/core && yarn test:e2e",
Expand Down
11 changes: 11 additions & 0 deletions ui-components/.storybook/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,15 @@ export const parameters = {
],
},
},
a11y: {
config: {
rules: [
// TODO: Enable color-contrast after resolving #1488
{
id: "color-contrast",
enabled: false,
},
],
},
},
}
15 changes: 11 additions & 4 deletions ui-components/__tests__/blocks/ImageCard.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,18 @@ const listing = Object.assign({}, ArcherListing) as Listing
afterEach(cleanup)

describe("<ImageCard>", () => {
it("renders title, subtitle, and image", () => {
it("renders title, subtitle, image and alt text", () => {
const { getByText, getByAltText } = render(
<ImageCard imageUrl={"/images/listing.jpg"} title={"My Building"} subtitle={"The Address"} />
<ImageCard
imageUrl={"/images/listing.jpg"}
title={"My Building"}
subtitle={"The Address"}
description={"A description of the image"}
/>
)
expect(getByText("My Building")).not.toBeNull()
expect(getByText("The Address")).not.toBeNull()
expect(getByAltText("My Building")).not.toBeNull()
expect(getByAltText("A description of the image")).not.toBeNull()
})
it("renders with a link", () => {
const { getByAltText } = render(
Expand All @@ -25,7 +30,9 @@ describe("<ImageCard>", () => {
href="/listings"
/>
)
expect(getByAltText("My Building").closest("a")?.getAttribute("href")).toBe("/listings")
expect(getByAltText("A picture of the building").closest("a")?.getAttribute("href")).toBe(
"/listings"
)
})
it("renders with an application status bar", () => {
const { getByText } = render(
Expand Down
12 changes: 12 additions & 0 deletions ui-components/__tests__/storyshots.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// From https://www.npmjs.com/package/@storybook/addon-storyshots-puppeteer

import initStoryshots from "@storybook/addon-storyshots"
import { axeTest } from "@storybook/addon-storyshots-puppeteer"
import * as path from "path"

initStoryshots({
suite: "a11y checks",
test: axeTest({
storybookUrl: `file://${path.resolve(__dirname, "../storybook-static")}`,
}),
})
7 changes: 6 additions & 1 deletion ui-components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"start": "start-storybook -s ./public",
"test": "jest -w 1",
"test:coverage": "jest -w 1 --coverage --watchAll=false",
"prettier": "prettier --write \"**/*.tsx\""
"prettier": "prettier --write \"**/*.tsx\"",
"test:a11y": "build-storybook -o ./storybook-static && jest storyshot --testRegex=storyshots.spec.ts"
},
"devDependencies": {
"@babel/core": "^7.11.6",
Expand All @@ -21,6 +22,8 @@
"@storybook/addon-actions": "^6.1.17",
"@storybook/addon-docs": "^6.0.26",
"@storybook/addon-knobs": "^6.1.1",
"@storybook/addon-storyshots": "^6.3.2",
"@storybook/addon-storyshots-puppeteer": "^6.0.26",
"@storybook/addon-viewport": "^6.0.26",
"@storybook/react": "^6.0.26",
"@testing-library/jest-dom": "^5.11.9",
Expand All @@ -44,6 +47,8 @@
"mockdate": "^3.0.2",
"node-sass": "^4.14.1",
"postcss-loader": "^3.0.0",
"preact": "^10.5.14",
"puppeteer": "^10.1.0",
"react": "^16.13.1",
"react-dom": "^16.13.1",
"react-is": "^16.13.1",
Expand Down
14 changes: 14 additions & 0 deletions ui-components/src/actions/Button.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,17 @@ export const loading = () => (
export const inaccessible = () => (
<button style={{ backgroundColor: "red", color: "darkRed" }}>Inaccessible button</button>
)

// Example of how you can override axe a11y checks
inaccessible.parameters = {
a11y: {
config: {
rules: [
{
id: "color-contrast",
enabled: false,
},
],
},
},
}
8 changes: 8 additions & 0 deletions ui-components/src/blocks/ImageCard.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,11 @@ export const withListing = () => (
listing={listing}
/>
)

export const withDescriptionAsAlt = () => (
<ImageCard
imageUrl="/images/listing.jpg"
title="Hello World"
description="An image of the building"
/>
)
5 changes: 4 additions & 1 deletion ui-components/src/blocks/ImageCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export interface ImageCardProps {
title: string
href?: string
listing?: Listing
description?: string
}

const ImageCard = (props: ImageCardProps) => {
Expand All @@ -26,7 +27,9 @@ const ImageCard = (props: ImageCardProps) => {
const image = (
<div className="image-card__wrapper">
<figure className="image-card">
{props.imageUrl && <img src={props.imageUrl} alt={props.title} />}
{props.imageUrl && (
<img src={props.imageUrl} alt={props.description || "A picture of the building"} />
)}

<figcaption className="image-card__figcaption">
<h2 className="image-card__title">{props.title}</h2>
Expand Down
6 changes: 3 additions & 3 deletions ui-components/src/blocks/InfoCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ const InfoCard = (props: InfoCardProps) => {
return (
<div className={wrapperClasses.join(" ")}>
{props.externalHref ? (
<h4 className="info-card__title">
<h3 className="info-card__title">
Copy link
Contributor Author

@akegan akegan Jul 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this because of the rule "Ensures the order of headings is semantically correct". We had a jump from an h2 at the InfoCard grid level to this h3. It doesn't affect the styling, just the semantics

More info

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Longterm plan, should have props for tags so that you set it at the appropriate level depending on the context. Add a TODO to add props, or create an issue about it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added #1516 to address this.

<a href={props.externalHref} target="_blank">
{props.title}
</a>
</h4>
</h3>
) : (
<h4 className="info-card__title">{props.title}</h4>
<h3 className="info-card__title">{props.title}</h3>
)}
{typeof props.children == "string" ? (
<div className="markdown">
Expand Down
7 changes: 6 additions & 1 deletion ui-components/src/forms/Dropzone.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,12 @@ export const defaultDropzone = () => {
{progressValue == 0 && (
<p className="mt-16">(Provide Cloudinary credentials via the Knobs below.)</p>
)}
<img src={cloudinaryImage} style={{ width: "200px" }} />
<img
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ARIA role is not valid.

src={cloudinaryImage}
style={{ width: "200px" }}
role={cloudinaryImage ? undefined : "none"}
alt={cloudinaryImage && "uploaded file"}
/>
</>
)
}
2 changes: 1 addition & 1 deletion ui-components/src/headers/SiteHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const SiteHeader = (props: SiteHeaderProps) => {
<img
className={`logo__image ${props.imageOnly && "navbar-image-only"}`}
src={props.logoSrc}
alt={props.title}
alt={"Site logo"}
/>
{!props.imageOnly && <div className="logo__title">{props.title}</div>}
</div>
Expand Down
35 changes: 35 additions & 0 deletions ui-components/src/navigation/TabNav.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,23 @@ export const Default = () => {
</TabNav>
)
}
// TODO: Fix tab roles for this component in #1489
Default.parameters = {
a11y: {
config: {
rules: [
{
id: "no-focusable-content",
enabled: false,
},
{
id: "nested-interactive",
enabled: false,
},
],
},
},
}

export const Other = () => {
return (
Expand All @@ -31,3 +48,21 @@ export const Other = () => {
</TabNav>
)
}

// TODO: Fix tab roles for this component in #1489
Other.parameters = {
a11y: {
config: {
rules: [
{
id: "no-focusable-content",
enabled: false,
},
{
id: "nested-interactive",
enabled: false,
},
],
},
},
}
2 changes: 1 addition & 1 deletion ui-components/src/sections/InfoCardGrid.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ More content
className="is-normal-primary-lighter"
>
{`
### Header 3
#### Header 4
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as this comment


* A list
* of items
Expand Down
4 changes: 2 additions & 2 deletions ui-components/src/tables/MinimalTable.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ const headersWithImage = { image: "Image", ...headers }
const dataWithImage = [...data] as any
dataWithImage[0].image = (
<TableThumbnail>
<img src="/images/listing.jpg" />
<img src="/images/listing.jpg" alt="listing picture" />
</TableThumbnail>
)
dataWithImage[1].image = (
<TableThumbnail>
<img src="/images/logo_glyph.svg" />
<img src="/images/logo_glyph.svg" alt="site logo" />
</TableThumbnail>
)

Expand Down
4 changes: 2 additions & 2 deletions ui-components/src/tables/StandardTable.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ const dataWithImage = [...data] as any
dataWithImage[0].image = (
<TableThumbnail>
<a href="#">
<img src="/images/listing.jpg" />
<img src="/images/listing.jpg" alt="listing image" />
</a>
</TableThumbnail>
)
dataWithImage[1].image = (
<TableThumbnail>
<img src="/images/logo_glyph.svg" />
<img src="/images/logo_glyph.svg" alt="logo" />
</TableThumbnail>
)

Expand Down
Loading