From e5749c8bdf92e2672260c65d044cd4c5ea38fe27 Mon Sep 17 00:00:00 2001 From: Arnau Casau Date: Tue, 19 Nov 2024 18:19:39 +0100 Subject: [PATCH 1/4] Add scripts and tests --- .github/workflows/main.yml | 2 + package.json | 3 +- scripts/js/commands/checkAltText.ts | 43 ++++++++++++++++++++ scripts/js/lib/extractMarkdownImages.test.ts | 38 +++++++++++++++++ scripts/js/lib/extractMarkdownImages.ts | 42 +++++++++++++++++++ 5 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 scripts/js/commands/checkAltText.ts create mode 100644 scripts/js/lib/extractMarkdownImages.test.ts create mode 100644 scripts/js/lib/extractMarkdownImages.ts diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ed9fc9b2c45..e47d7bb617f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -28,6 +28,8 @@ jobs: - name: File metadata run: npm run check:metadata + - name: Check image alt text + run: npm run check:alt-text - name: Spellcheck run: npm run check:spelling - name: Check Qiskit bot config diff --git a/package.json b/package.json index f67d649c9f7..a4dec020db7 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,8 @@ "fmt": "prettier --write .", "test": "playwright test", "typecheck": "tsc", - "check": "npm run check:qiskit-bot && npm run check:patterns-index && npm run check:metadata && npm run check:spelling && npm run check:internal-links && npm run check:orphan-pages && npm run check:fmt", + "check": "npm run check:qiskit-bot && npm run check:patterns-index && npm run check:alt-text && npm run check:metadata && npm run check:spelling && npm run check:internal-links && npm run check:orphan-pages && npm run check:fmt", + "check:alt-text": "tsx scripts/js/commands/checkAltText.ts", "check:metadata": "tsx scripts/js/commands/checkMetadata.ts", "check:spelling": "tsx scripts/js/commands/checkSpelling.ts", "check:fmt": "prettier --check .", diff --git a/scripts/js/commands/checkAltText.ts b/scripts/js/commands/checkAltText.ts new file mode 100644 index 00000000000..55fed63bf74 --- /dev/null +++ b/scripts/js/commands/checkAltText.ts @@ -0,0 +1,43 @@ +// This code is a Qiskit project. +// +// (C) Copyright IBM 2024. +// +// This code is licensed under the Apache License, Version 2.0. You may +// obtain a copy of this license in the LICENSE file in the root directory +// of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. +// +// Any modifications or derivative works of this code must retain this +// copyright notice, and modified files need to carry a notice indicating +// that they have been altered from the originals. + +import { globby } from "globby"; +import { Image, extractMarkdownImages } from "../lib/extractMarkdownImages.js"; +import { readMarkdown } from "../lib/markdownReader.js"; + +async function main() { + const files = await globby(["docs/**/*.{ipynb,mdx}", "!docs/api/**/*.mdx"]); + const errors: string[] = []; + + for (const file of files) { + const markdown = await readMarkdown(file); + const images: Image[] = await extractMarkdownImages(markdown); + const fileErrors = images.flatMap((image: Image) => + image.altText + ? [] + : `The image '${image.imageName}' does not have an alt text defined.`, + ); + + if (fileErrors.length > 0) { + errors.push(`Error in file '${file}':\n\t- ${fileErrors.join("\n\t- ")}`); + } + } + + if (errors.length > 0) { + errors.forEach((error) => console.log(error)); + console.error("\nSome images are missing alt text 💔\n"); + process.exit(1); + } + console.log("\nAll images have alt text ✅\n"); +} + +main().then(() => process.exit()); diff --git a/scripts/js/lib/extractMarkdownImages.test.ts b/scripts/js/lib/extractMarkdownImages.test.ts new file mode 100644 index 00000000000..eb263091861 --- /dev/null +++ b/scripts/js/lib/extractMarkdownImages.test.ts @@ -0,0 +1,38 @@ +// This code is a Qiskit project. +// +// (C) Copyright IBM 2023. +// +// This code is licensed under the Apache License, Version 2.0. You may +// obtain a copy of this license in the LICENSE file in the root directory +// of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. +// +// Any modifications or derivative works of this code must retain this +// copyright notice, and modified files need to carry a notice indicating +// that they have been altered from the originals. + +import { expect, test } from "@playwright/test"; + +import { Image, extractMarkdownImages } from "./extractMarkdownImages.js"; + +test("Test the extraction of the images", async () => { + const markdown = ` +# A header + +![Our first image with alt text](/images/img1.png) + +![](/images/invalid_img1.png) + +![Here's another valid image](/images/img2.png) + +![And now, our last link](https://ibm.com) + `; + const images = await extractMarkdownImages(markdown); + const correct_images = [ + { imageName: "/images/img1.png", altText: "Our first image with alt text" }, + { imageName: "/images/invalid_img1.png", altText: "" }, + { imageName: "/images/img2.png", altText: "Here's another valid image" }, + { imageName: "https://ibm.com", altText: "And now, our last link" }, + ]; + + expect(images).toEqual(correct_images); +}); diff --git a/scripts/js/lib/extractMarkdownImages.ts b/scripts/js/lib/extractMarkdownImages.ts new file mode 100644 index 00000000000..87b7abf61f7 --- /dev/null +++ b/scripts/js/lib/extractMarkdownImages.ts @@ -0,0 +1,42 @@ +// This code is a Qiskit project. +// +// (C) Copyright IBM 2024. +// +// This code is licensed under the Apache License, Version 2.0. You may +// obtain a copy of this license in the LICENSE file in the root directory +// of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. +// +// Any modifications or derivative works of this code must retain this +// copyright notice, and modified files need to carry a notice indicating +// that they have been altered from the originals. + +import { unified } from "unified"; +import { Root } from "remark-mdx"; +import { visit } from "unist-util-visit"; +import remarkParse from "remark-parse"; +import remarkGfm from "remark-gfm"; +import remarkStringify from "remark-stringify"; + +export interface Image { + imageName: string; + altText: string | null; +} + +export async function extractMarkdownImages( + markdown: string, +): Promise { + const images: Image[] = []; + + await unified() + .use(remarkParse) + .use(remarkGfm) + .use(() => (tree: Root) => { + visit(tree, "image", (node) => + images.push({ imageName: node.url, altText: node.alt } as Image), + ); + }) + .use(remarkStringify) + .process(markdown); + + return images; +} From 5df19a40f7395561ce5a71e6e9c167ca2063f871 Mon Sep 17 00:00:00 2001 From: Arnau Casau Date: Tue, 19 Nov 2024 19:22:27 +0100 Subject: [PATCH 2/4] deduplicate errors --- scripts/js/commands/checkAltText.ts | 15 +++++++++------ scripts/js/lib/extractMarkdownImages.test.ts | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/scripts/js/commands/checkAltText.ts b/scripts/js/commands/checkAltText.ts index 55fed63bf74..e6ef108f9b5 100644 --- a/scripts/js/commands/checkAltText.ts +++ b/scripts/js/commands/checkAltText.ts @@ -16,24 +16,27 @@ import { readMarkdown } from "../lib/markdownReader.js"; async function main() { const files = await globby(["docs/**/*.{ipynb,mdx}", "!docs/api/**/*.mdx"]); - const errors: string[] = []; + const fileErrors: string[] = []; for (const file of files) { const markdown = await readMarkdown(file); const images: Image[] = await extractMarkdownImages(markdown); - const fileErrors = images.flatMap((image: Image) => + const imageErrors = images.flatMap((image: Image) => image.altText ? [] : `The image '${image.imageName}' does not have an alt text defined.`, ); + const imageErrorsDeduplicated = new Set(imageErrors); - if (fileErrors.length > 0) { - errors.push(`Error in file '${file}':\n\t- ${fileErrors.join("\n\t- ")}`); + if (imageErrorsDeduplicated.size > 0) { + fileErrors.push( + `Error in file '${file}':\n\t- ${[...imageErrorsDeduplicated].join("\n\t- ")}`, + ); } } - if (errors.length > 0) { - errors.forEach((error) => console.log(error)); + if (fileErrors.length > 0) { + fileErrors.forEach((error) => console.log(error)); console.error("\nSome images are missing alt text 💔\n"); process.exit(1); } diff --git a/scripts/js/lib/extractMarkdownImages.test.ts b/scripts/js/lib/extractMarkdownImages.test.ts index eb263091861..b4a57eaa9d4 100644 --- a/scripts/js/lib/extractMarkdownImages.test.ts +++ b/scripts/js/lib/extractMarkdownImages.test.ts @@ -12,7 +12,7 @@ import { expect, test } from "@playwright/test"; -import { Image, extractMarkdownImages } from "./extractMarkdownImages.js"; +import { extractMarkdownImages } from "./extractMarkdownImages.js"; test("Test the extraction of the images", async () => { const markdown = ` From d281e8bc2a4962f37589e73a6dd2ae75d4d5bfc8 Mon Sep 17 00:00:00 2001 From: Arnau Casau Date: Wed, 20 Nov 2024 11:18:00 +0100 Subject: [PATCH 3/4] Incorporate #1800 feedback and simplify the code. Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Co-authored-by: Shraddha Aangiras <63237790+shraddha-aangiras@users.noreply.github.com> --- README.md | 14 ++++++++++++++ scripts/js/commands/checkAltText.ts | 17 +++++++---------- scripts/js/lib/extractMarkdownImages.test.ts | 18 ++++++++++-------- scripts/js/lib/extractMarkdownImages.ts | 19 ++++++++----------- 4 files changed, 39 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index a1b5a5dfed6..3d616bf52ee 100644 --- a/README.md +++ b/README.md @@ -289,6 +289,20 @@ npm run check:metadata -- --apis npm run check ``` +## Check image alt text + +Every image needs to have alt text for accessibility. The `lint` job in CI will fail if images do not have alt text defined. + +You can check it locally by running: + +```bash +# Only check alt text +npm run check:alt-text + +# Or, run all the checks +npm run check +``` + ## Spellcheck We use [cSpell](https://cspell.org) to check for spelling. The `lint` job in CI will fail if there are spelling issues. diff --git a/scripts/js/commands/checkAltText.ts b/scripts/js/commands/checkAltText.ts index e6ef108f9b5..c1ddff96650 100644 --- a/scripts/js/commands/checkAltText.ts +++ b/scripts/js/commands/checkAltText.ts @@ -11,7 +11,7 @@ // that they have been altered from the originals. import { globby } from "globby"; -import { Image, extractMarkdownImages } from "../lib/extractMarkdownImages.js"; +import { findImagesWithoutAltText } from "../lib/extractMarkdownImages.js"; import { readMarkdown } from "../lib/markdownReader.js"; async function main() { @@ -20,22 +20,19 @@ async function main() { for (const file of files) { const markdown = await readMarkdown(file); - const images: Image[] = await extractMarkdownImages(markdown); - const imageErrors = images.flatMap((image: Image) => - image.altText - ? [] - : `The image '${image.imageName}' does not have an alt text defined.`, + const images = await findImagesWithoutAltText(markdown); + const imageErrors = [...images].map( + (image) => `The image '${image}' does not have an alt text defined.`, ); - const imageErrorsDeduplicated = new Set(imageErrors); - if (imageErrorsDeduplicated.size > 0) { + if (imageErrors.length) { fileErrors.push( - `Error in file '${file}':\n\t- ${[...imageErrorsDeduplicated].join("\n\t- ")}`, + `Error in file '${file}':\n\t- ${imageErrors.join("\n\t- ")}`, ); } } - if (fileErrors.length > 0) { + if (fileErrors.length) { fileErrors.forEach((error) => console.log(error)); console.error("\nSome images are missing alt text 💔\n"); process.exit(1); diff --git a/scripts/js/lib/extractMarkdownImages.test.ts b/scripts/js/lib/extractMarkdownImages.test.ts index b4a57eaa9d4..86c094ce044 100644 --- a/scripts/js/lib/extractMarkdownImages.test.ts +++ b/scripts/js/lib/extractMarkdownImages.test.ts @@ -12,7 +12,7 @@ import { expect, test } from "@playwright/test"; -import { extractMarkdownImages } from "./extractMarkdownImages.js"; +import { findImagesWithoutAltText } from "./extractMarkdownImages.js"; test("Test the extraction of the images", async () => { const markdown = ` @@ -24,15 +24,17 @@ test("Test the extraction of the images", async () => { ![Here's another valid image](/images/img2.png) +![](/images/invalid_img2.png) + +![](/images/invalid_img2.png) + ![And now, our last link](https://ibm.com) `; - const images = await extractMarkdownImages(markdown); - const correct_images = [ - { imageName: "/images/img1.png", altText: "Our first image with alt text" }, - { imageName: "/images/invalid_img1.png", altText: "" }, - { imageName: "/images/img2.png", altText: "Here's another valid image" }, - { imageName: "https://ibm.com", altText: "And now, our last link" }, - ]; + const images = await findImagesWithoutAltText(markdown); + const correct_images = new Set([ + "/images/invalid_img1.png", + "/images/invalid_img2.png", + ]); expect(images).toEqual(correct_images); }); diff --git a/scripts/js/lib/extractMarkdownImages.ts b/scripts/js/lib/extractMarkdownImages.ts index 87b7abf61f7..766714823b0 100644 --- a/scripts/js/lib/extractMarkdownImages.ts +++ b/scripts/js/lib/extractMarkdownImages.ts @@ -17,23 +17,20 @@ import remarkParse from "remark-parse"; import remarkGfm from "remark-gfm"; import remarkStringify from "remark-stringify"; -export interface Image { - imageName: string; - altText: string | null; -} - -export async function extractMarkdownImages( +export async function findImagesWithoutAltText( markdown: string, -): Promise { - const images: Image[] = []; +): Promise> { + const images = new Set(); await unified() .use(remarkParse) .use(remarkGfm) .use(() => (tree: Root) => { - visit(tree, "image", (node) => - images.push({ imageName: node.url, altText: node.alt } as Image), - ); + visit(tree, "image", (node) => { + if (!node.alt) { + images.add(node.url); + } + }); }) .use(remarkStringify) .process(markdown); From bec5706a80e101a33962ffbf6263f3c072a89023 Mon Sep 17 00:00:00 2001 From: Arnau Casau Date: Wed, 20 Nov 2024 11:47:38 +0100 Subject: [PATCH 4/4] Fix image without alt text --- docs/guides/custom-transpiler-pass.ipynb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/guides/custom-transpiler-pass.ipynb b/docs/guides/custom-transpiler-pass.ipynb index abfa3536099..ed4fb3fb5ee 100644 --- a/docs/guides/custom-transpiler-pass.ipynb +++ b/docs/guides/custom-transpiler-pass.ipynb @@ -72,7 +72,7 @@ " qc.draw(output='mpl')\n", "\n", "```\n", - "![The circuit's DAG consists of nodes that are connected by directional edges. It is a visual way to represent qubits or classical bits, the operations, and the way that data flows. ](/images/guides/custom-transpiler-pass/DAG_circ.png \"DAG\")\n", + "![Circuit preparing a Bell state and applying an $R_Z$ rotation depending on the measurement outcome.](/images/guides/custom-transpiler-pass/DAG_circ.png \"Circuit\")\n", "\n", "Use the `qiskit.tools.visualization.dag_drawer()` function to view this circuit's DAG. There are three kinds of graph nodes: qubit/clbit nodes (green), operation nodes (blue), and output nodes (red). Each edge indicates data flow (or dependency) between two nodes.\n", "\n", @@ -83,7 +83,7 @@ "dag = circuit_to_dag(qc)\n", "dag_drawer(dag)\n", "```\n", - "![](/images/guides/custom-transpiler-pass/DAG.png)\n", + "![The circuit's DAG consists of nodes that are connected by directional edges. It is a visual way to represent qubits or classical bits, the operations, and the way that data flows.](/images/guides/custom-transpiler-pass/DAG.png \"DAG\")\n", "" ] },